From b92b4a27685a427585cf516e6a0e6df9f504cb0e Mon Sep 17 00:00:00 2001 From: core-devops Date: Wed, 3 Jun 2026 18:48:36 -0700 Subject: [PATCH] =?UTF-8?q?fix(canvas):=20CreateWorkspaceDialog=20uses=20r?= =?UTF-8?q?egistry=20provider=20catalog=20=E2=80=94=20platform=20bucket=20?= =?UTF-8?q?+=20correct=20llm=5Fprovider=20for=20moonshot/kimi-k2.6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: the Create Workspace dialog built its provider→model dropdown catalog with the LEGACY buildProviderCatalog(llmModels), whose inferVendor heuristic slash-splits a platform model id like `moonshot/kimi-k2.6` into vendor `moonshot`. There was therefore no `Platform` bucket and the create payload sent `llm_provider: "moonshot"` for a platform-managed model. ConfigTab was migrated to the registry-backed catalog (buildProviderCatalogFromRegistry) in internal#718 P3; CreateWorkspaceDialog was not. This mirrors that migration: - Thread the registry fields (registry_backed / registry_providers / registry_models) — already returned by GET /templates — through TemplateSpec. - When the selected runtime's /templates row is registry_backed, build the catalog from registry_providers/registry_models (each model carries its DERIVED provider, e.g. moonshot/kimi-k2.6 → "platform"), feed the selector the registry models, and pass the prebuilt catalog verbatim. Restores the `Platform` bucket and makes the payload send `llm_provider: platform`. - Non-registry runtimes / older backends keep the legacy buildProviderCatalog fallback unchanged. Tests: added a registry-backed claude-code fixture whose plain models[] is UN-annotated (so the legacy path would mis-bucket to "moonshot"), asserting the Platform bucket appears and selecting moonshot/kimi-k2.6 yields llm_provider: platform; plus a MiniMax derived-provider/BYOK case. Verified the 3 new tests FAIL on the pre-fix code and PASS after. Full canvas suite: 3334 passed / 3 skipped. tsc: 0 new errors (223→223, all pre-existing test Mock drift). eslint clean on touched files. Fix C of the RFC#340 convergence (cosmetic/UX, client-only, no serving-path risk). Fix A (workspace-server) is the boot fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/CreateWorkspaceDialog.tsx | 70 ++++++++-- .../__tests__/CreateWorkspaceDialog.test.tsx | 122 ++++++++++++++++++ 2 files changed, 183 insertions(+), 9 deletions(-) diff --git a/canvas/src/components/CreateWorkspaceDialog.tsx b/canvas/src/components/CreateWorkspaceDialog.tsx index c46c84160..f03e61924 100644 --- a/canvas/src/components/CreateWorkspaceDialog.tsx +++ b/canvas/src/components/CreateWorkspaceDialog.tsx @@ -8,9 +8,12 @@ import { ExternalConnectModal, type ExternalConnectionInfo } from "./ExternalCon import { ProviderModelSelector, buildProviderCatalog, + buildProviderCatalogFromRegistry, findProviderForModel, type SelectorModel, type SelectorValue, + type RegistryProvider, + type RegistryModel, } from "./ProviderModelSelector"; interface WorkspaceOption { @@ -32,6 +35,16 @@ interface TemplateSpec { model?: string; models?: SelectorModel[]; providers?: string[]; + // internal#718 P3 registry-served fields (additive; absent on older + // backends and for non-registry runtimes). When registry_backed is true the + // provider→model catalog is built from registry_providers/registry_models so + // each model's DERIVED provider (e.g. moonshot/kimi-k2.6 → "platform") drives + // the dropdown bucket and the create payload's llm_provider — instead of the + // legacy inferVendor heuristic that slash-splits the id into "moonshot". + // Mirrors ConfigTab's RuntimeOption loader (RFC#340 Fix C). + registry_backed?: boolean; + registry_providers?: RegistryProvider[]; + registry_models?: RegistryModel[]; } const DEFAULT_RUNTIME = "claude-code"; @@ -168,15 +181,53 @@ export function CreateWorkspaceButton() { }), [runtime, templateSpecs], ); - const llmModels = useMemo( - () => { - const sourceSpec = selectedTemplateSpec ?? selectedRuntimeTemplateSpec; - if (!sourceSpec?.models?.length) return []; - return sourceSpec.models; - }, + // The /templates row backing the LLM picker: an explicitly-selected + // workspace template wins, else the base runtime template row. + const llmSourceSpec = useMemo( + () => selectedTemplateSpec ?? selectedRuntimeTemplateSpec, [selectedRuntimeTemplateSpec, selectedTemplateSpec], ); - const llmCatalog = useMemo(() => buildProviderCatalog(llmModels), [llmModels]); + // internal#718 P3 / RFC#340 Fix C: a runtime is registry-backed when the + // /templates row says so AND it served a non-empty registry_models set. + // Mirrors ConfigTab's `registryBacked` derivation exactly. + const registryBacked = useMemo( + () => + llmSourceSpec?.registry_backed === true && + (llmSourceSpec.registry_models?.length ?? 0) > 0, + [llmSourceSpec], + ); + // Models fed to the selector dropdown. For a registry-backed runtime use the + // registry-served native set, carrying each model's DERIVED provider so the + // selector buckets it correctly (moonshot/kimi-k2.6 → "platform", not the + // inferVendor "moonshot"). Otherwise fall back to the template-served + // models[] + the legacy heuristic — same fallback ConfigTab keeps. + const llmModels = useMemo( + () => { + if (registryBacked) { + return (llmSourceSpec?.registry_models ?? []).map((m) => ({ + id: m.id, + name: m.name, + ...(m.provider ? { provider: m.provider } : {}), + })); + } + return llmSourceSpec?.models?.length ? llmSourceSpec.models : []; + }, + [registryBacked, llmSourceSpec], + ); + // Registry-backed path: build the catalog from registry_providers/ + // registry_models so dropdown labels + billing + the derived provider come + // from the provider-registry SSOT (restores the "Platform" bucket). Legacy + // path: re-infer from models[] via buildProviderCatalog (inferVendor). + const llmCatalog = useMemo( + () => + registryBacked + ? buildProviderCatalogFromRegistry( + llmSourceSpec?.registry_providers ?? [], + llmSourceSpec?.registry_models ?? [], + ) + : buildProviderCatalog(llmModels), + [registryBacked, llmSourceSpec, llmModels], + ); const selectedLLMProvider = useMemo( () => llmCatalog.find((p) => p.id === llmSelection.providerId) ?? llmCatalog[0], [llmCatalog, llmSelection.providerId], @@ -184,7 +235,7 @@ export function CreateWorkspaceButton() { useEffect(() => { if (llmCatalog.length === 0) return; - const sourceDefault = (selectedTemplateSpec ?? selectedRuntimeTemplateSpec)?.model?.trim(); + const sourceDefault = llmSourceSpec?.model?.trim(); const platformProvider = llmCatalog.find((p) => p.vendor === "platform"); const matched = sourceDefault ? findProviderForModel(llmCatalog, sourceDefault) : null; const next = platformProvider ?? matched ?? llmCatalog[0]; @@ -197,7 +248,7 @@ export function CreateWorkspaceButton() { envVars: next.envVars, }); setLLMSecret(""); - }, [llmCatalog, selectedRuntimeTemplateSpec, selectedTemplateSpec]); + }, [llmCatalog, llmSourceSpec]); // Reset form and load workspaces whenever dialog opens useEffect(() => { @@ -461,6 +512,7 @@ export function CreateWorkspaceButton() { { setLLMSelection(next); diff --git a/canvas/src/components/__tests__/CreateWorkspaceDialog.test.tsx b/canvas/src/components/__tests__/CreateWorkspaceDialog.test.tsx index 3b397d513..9eb4d18f8 100644 --- a/canvas/src/components/__tests__/CreateWorkspaceDialog.test.tsx +++ b/canvas/src/components/__tests__/CreateWorkspaceDialog.test.tsx @@ -454,6 +454,128 @@ describe("CreateWorkspaceDialog — dynamic runtime provider picker", () => { }); }); +// --------------------------------------------------------------------------- +// Registry-backed provider catalog (RFC#340 Fix C) +// +// Regression guard for the mis-bucketing bug: when a registry-backed +// claude-code template serves `moonshot/kimi-k2.6` whose DERIVED provider is +// `platform`, the dialog must build the dropdown from registry_providers/ +// registry_models (buildProviderCatalogFromRegistry) — NOT the legacy +// inferVendor heuristic which slash-splits the id into "moonshot". The +// distinguishing trait of this fixture: the plain `models[]` array does NOT +// carry an explicit `provider` field, so the LEGACY path would bucket the +// model under "moonshot" and send llm_provider:"moonshot". Only the +// registry-backed path yields the Platform bucket + llm_provider:"platform". +// --------------------------------------------------------------------------- + +// claude-code template whose plain models[] is UN-annotated (no explicit +// provider). The derived-provider annotation lives ONLY in registry_models. +const REGISTRY_TEMPLATE = { + id: "claude-code-default", + name: "Claude Code Agent", + runtime: "claude-code", + model: "moonshot/kimi-k2.6", + // Legacy fields — note: NO explicit provider on the platform model, so the + // legacy inferVendor path would slash-split it into "moonshot". + providers: ["platform", "minimax", "anthropic"], + models: [ + { id: "moonshot/kimi-k2.6", name: "Kimi K2.6", required_env: [] }, + { id: "MiniMax-M2.7", name: "MiniMax M2.7", required_env: ["MINIMAX_API_KEY"] }, + { id: "claude-sonnet-4-6", name: "Claude Sonnet 4.6", required_env: ["ANTHROPIC_API_KEY"] }, + ], + // Registry-served SSOT (internal#718 P3). DeriveProvider resolved + // moonshot/kimi-k2.6 → "platform"; MiniMax-M2.7 → "minimax". + registry_backed: true, + registry_providers: [ + { name: "platform", display_name: "Platform", auth_env: [], billing_mode: "platform_managed" }, + { name: "minimax", display_name: "MiniMax", auth_env: ["MINIMAX_API_KEY"], billing_mode: "byok" }, + { name: "anthropic", display_name: "Anthropic API", auth_env: ["ANTHROPIC_API_KEY"], billing_mode: "byok" }, + ], + registry_models: [ + { id: "moonshot/kimi-k2.6", name: "Kimi K2.6", provider: "platform", billing_mode: "platform_managed" }, + { id: "MiniMax-M2.7", name: "MiniMax M2.7", provider: "minimax", billing_mode: "byok" }, + { id: "claude-sonnet-4-6", name: "Claude Sonnet 4.6", provider: "anthropic", billing_mode: "byok" }, + ], +}; + +describe("CreateWorkspaceDialog — registry-backed provider catalog (RFC#340 Fix C)", () => { + beforeEach(() => { + mockGet.mockImplementation(async (url: string) => { + if (url === "/templates") { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return [REGISTRY_TEMPLATE] as any; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return SAMPLE_WORKSPACES as any; + }); + }); + + it("shows the Platform provider bucket for the registry-backed claude-code runtime", async () => { + await openDialog(); + const providerSelect = await waitFor(() => { + const sel = document.querySelector("[data-testid='provider-select']") as HTMLSelectElement; + expect(sel).toBeTruthy(); + return sel; + }); + const labels = Array.from(providerSelect.options).map((o) => o.text.trim()); + // Registry display_name "Platform" appears — NOT "moonshot" from the + // legacy slash-split heuristic. + expect(labels).toContain("Platform"); + expect(labels).not.toContain("moonshot"); + // Bucket id is the registry-keyed id, vendor is the bare provider name. + const values = Array.from(providerSelect.options).map((o) => o.value); + expect(values).toContain("registry|platform"); + }); + + it("sends llm_provider: platform (not moonshot) for moonshot/kimi-k2.6", async () => { + await openDialog(); + fireEvent.change(screen.getByPlaceholderText("e.g. SEO Agent"), { + target: { value: "Kimi Agent" }, + }); + // Wait for the registry default to settle on the Platform bucket + model. + await waitFor(() => { + const modelSelect = document.querySelector("[data-testid='model-select']") as HTMLSelectElement; + expect(modelSelect?.value).toBe("moonshot/kimi-k2.6"); + }); + + const createBtn = screen.getAllByRole("button").find((b) => b.textContent === "Create"); + fireEvent.click(createBtn!); + + await waitFor(() => expect(mockPost).toHaveBeenCalled()); + const body = mockPost.mock.calls[0][1] as Record; + expect(body.model).toBe("moonshot/kimi-k2.6"); + expect(body.llm_provider).toBe("platform"); + // Platform is auth-env-free → no BYOK secret. + expect(body.secrets).toBeUndefined(); + }); + + it("buckets MiniMax-M2.7 under its derived provider and sends llm_provider: minimax", async () => { + await openDialog(); + fireEvent.change(screen.getByPlaceholderText("e.g. SEO Agent"), { + target: { value: "MiniMax Agent" }, + }); + await waitFor(() => { + const sel = document.querySelector("[data-testid='provider-select']") as HTMLSelectElement; + expect(Array.from(sel.options).map((o) => o.value)).toContain("registry|minimax"); + }); + fireEvent.change(document.querySelector("[data-testid='provider-select']") as HTMLSelectElement, { + target: { value: "registry|minimax" }, + }); + fireEvent.change(document.getElementById("llm-secret-input") as HTMLInputElement, { + target: { value: "sk-minimax-test" }, + }); + + const createBtn = screen.getAllByRole("button").find((b) => b.textContent === "Create"); + fireEvent.click(createBtn!); + + await waitFor(() => expect(mockPost).toHaveBeenCalled()); + const body = mockPost.mock.calls[0][1] as Record; + expect(body.model).toBe("MiniMax-M2.7"); + expect(body.llm_provider).toBe("minimax"); + expect(body.secrets).toEqual({ MINIMAX_API_KEY: "sk-minimax-test" }); + }); +}); + // --------------------------------------------------------------------------- // budget_limit field tests (#541) // --------------------------------------------------------------------------- -- 2.52.0