diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index f016cb0a..05c1d704 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -394,6 +394,39 @@ const FALLBACK_RUNTIME_OPTIONS: RuntimeOption[] = [ { value: "hermes", label: "Hermes", models: [], providers: [], registryBacked: false, registryProviders: [], registryModels: [] }, ]; +// modelsForRuntime — the set of model slugs the backend will accept for a +// runtime, sourced from the SAME server-served data the selector dropdown +// renders. Registry-backed runtimes carry the registry's native model list +// (registry_gen.go `Runtimes` map, surfaced as registry_models on GET +// /templates); older / non-registry runtimes fall back to the template's +// models[]. This is the canvas-side view of the (runtime, model) pairing the +// workspace-server validates atomically on Save — keeping the two in sync is +// what makes the on-runtime-change reset pick a model that won't 422. +export function modelIdsForRuntime(opt: RuntimeOption | null | undefined): string[] { + if (!opt) return []; + const registryBacked = opt.registryBacked && opt.registryModels.length > 0; + const src = registryBacked + ? opt.registryModels.map((m) => m.id) + : opt.models.map((m) => m.id); + // Wildcard model ids (e.g. "openrouter/*") aren't concrete defaults — a + // wildcard runtime has no single safe auto-pick, so exclude them from the + // default-selection candidate set (the user types the id by hand). + return src.filter((id) => !!id && !id.includes("*")); +} + +// defaultModelForRuntime — the model the form should reset to when the user +// switches INTO this runtime. The first concrete registered model is a safe, +// always-valid pick for the new (runtime, model) pair, eliminating the 422 + +// silent-rollback the backend would otherwise return when the prior runtime's +// model isn't registered for the newly-selected one. Empty string when the +// runtime serves no concrete models (e.g. a template that ships none, or a +// wildcard-only runtime) — the form then leaves the model blank and the +// existing modelUnresolved / empty-model guards keep Save from sending an +// invalid pair. +export function defaultModelForRuntime(opt: RuntimeOption | null | undefined): string { + return modelIdsForRuntime(opt)[0] ?? ""; +} + export function ConfigTab({ workspaceId }: Props) { const [config, setConfig] = useState({ ...DEFAULT_CONFIG }); const [originalYaml, setOriginalYaml] = useState(""); @@ -449,6 +482,11 @@ export function ConfigTab({ workspaceId }: Props) { // value to display. Used to show a "derived from environment" hint and to // block Save from appearing to wipe env-derived routing. const [modelSource, setModelSource] = useState<"workspace_secrets" | "unresolved" | null>(null); + // When a runtime change forces a model reset (the prior model isn't + // registered for the new runtime), record the {from,to} so the UI can show + // the user the swap happened instead of it being silent. Cleared on the next + // model edit or a runtime change that keeps the model valid. + const [modelResetNote, setModelResetNote] = useState<{ from: string; to: string } | null>(null); const successTimerRef = useRef>(undefined); useEffect(() => { @@ -752,6 +790,48 @@ export function ConfigTab({ workspaceId }: Props) { setConfig((prev) => ({ ...prev, [key]: value })); }; + // When the runtime changes, the previously-selected model is almost never + // registered for the NEW runtime — the backend validates the (runtime, + // model) pair atomically on Save and returns 422 (`model "X" is not a + // registered model for runtime "Y"`), after which the runtime silently + // rolls back and the user thinks nothing changed. We pre-empt that here by + // resetting the model to a valid default for the new runtime (the first + // concrete registered model), so the form can never submit an invalid pair. + // The model dropdown is already constrained to the new runtime's models + // because it derives its options from selectedRuntime. `provider` is also + // cleared so the back-derived provider re-resolves from the new model. + // modelResetNote surfaces the reset so it isn't silent. + const handleRuntimeChange = (nextRuntime: string) => { + const nextOption = runtimeOptions.find((o) => o.value === nextRuntime) ?? null; + setConfig((prev) => { + if ((prev.runtime || "") === nextRuntime) return prev; + const prevModelId = prev.runtime_config?.model || prev.model || ""; + const nextModel = defaultModelForRuntime(nextOption); + // Only announce a reset when the model actually had to change (the old + // model isn't valid for the new runtime). If the old model happens to be + // registered for the new runtime too, keep it and stay quiet. + const stillValid = + !!prevModelId && modelIdsForRuntime(nextOption).includes(prevModelId); + const resolvedModel = stillValid ? prevModelId : nextModel; + if (!stillValid && prevModelId && resolvedModel !== prevModelId) { + setModelResetNote({ from: prevModelId, to: resolvedModel }); + } else { + setModelResetNote(null); + } + return { + ...prev, + runtime: nextRuntime, + // Mirror into both top-level + nested so currentModelId (which reads + // runtime_config.model first) and the YAML Save path stay consistent. + model: resolvedModel, + runtime_config: { ...(prev.runtime_config ?? {}), model: resolvedModel }, + }; + }); + // The provider override is derived from (runtime, model); clear the local + // hint so it re-derives from the freshly-reset model. + setProvider(""); + }; + const updateNested = (key: K, subKey: string, value: unknown) => { setConfig((prev) => ({ ...prev, @@ -940,7 +1020,47 @@ export function ConfigTab({ workspaceId }: Props) { // an empty /model PUT), but it is confusing and can stall the user on other // fields without surfacing why. Block save until a model is picked. const modelUnresolved = modelSource === "unresolved" && !currentModelId; - const canSave = isDirty && !modelUnresolved; + // Hard guard against submitting an invalid (runtime, model) pair. The + // workspace-server validates the pair atomically on Save and 422s + + // silently rolls back the runtime when the model isn't registered for it. + // The runtime-change reset normally keeps the pair valid; this is the + // belt-and-suspenders for the raw-YAML edit path, where a user can hand- + // type a runtime/model mismatch the selector would never produce. + // + // Only enforced for REGISTRY-BACKED runtimes, whose served model set is the + // exhaustive registered list (registry_gen.go `Runtimes`) — there, a model + // outside the set is GUARANTEED to 422. Non-registry runtimes (hermes free- + // text, custom-escape) keep the existing permissive behavior: their served + // list isn't authoritative, so a hand-entered slug may still be valid and + // blocking it would break the legitimate power-user path. + // + // In raw-YAML mode the pair is read from the draft (the user can hand-type a + // runtime/model mismatch there); in form mode it's the live config. Either + // way the runtime must be re-resolved against its option so the registered + // set matches what the user actually typed, not the form's prior runtime. + const effectivePair = useMemo(() => { + if (rawMode) { + const parsed = parseYaml(rawDraft) as { + runtime?: string; + model?: string; + runtime_config?: { model?: string }; + }; + return { + runtime: (parsed.runtime || "").trim(), + model: (parsed.runtime_config?.model || parsed.model || "").trim(), + }; + } + return { runtime: config.runtime || "", model: currentModelId }; + }, [rawMode, rawDraft, config.runtime, currentModelId]); + const effectiveRuntimeOption = + runtimeOptions.find((o) => o.value === effectivePair.runtime) ?? null; + const registeredModelIds = modelIdsForRuntime(effectiveRuntimeOption); + const modelPairInvalid = + (effectiveRuntimeOption?.registryBacked ?? false) && + registeredModelIds.length > 0 && + !!effectivePair.model && + !registeredModelIds.includes(effectivePair.model); + const canSave = isDirty && !modelUnresolved && !modelPairInvalid; if (loading) { return
Loading config...
; @@ -1019,7 +1139,7 @@ export function ConfigTab({ workspaceId }: Props) { + {/* Make the runtime-change model reset VISIBLE. The backend + validates the (runtime, model) pair atomically — switching + runtime would otherwise 422 on the stale model and silently + roll the runtime back. We reset the model to a registered + default here and tell the user so the change isn't mysterious. */} + {modelResetNote && ( +
+ Model reset to{" "} + {modelResetNote.to}{" "} + because {modelResetNote.from}{" "} + isn't available for this runtime. +
+ )} {/* core#2594: env-resolved workspaces have no stored model/provider. Surface that clearly so users don't see empty required dropdowns on a healthy workspace, and can't hit Save expecting it to stay @@ -1051,6 +1189,9 @@ export function ConfigTab({ workspaceId }: Props) { value={selectorValue} onChange={(next) => { setSelectorValue(next); + // The user is explicitly choosing a model now — dismiss the + // runtime-change reset notice so it doesn't linger. + setModelResetNote(null); // Platform-managed providers (CP LLM proxy) do NOT // require tenant-supplied credentials. Skip injecting // their auth_env (e.g. MOLECULE_LLM_USAGE_TOKEN) into diff --git a/canvas/src/components/tabs/__tests__/ConfigTab.runtimeModelReset.test.tsx b/canvas/src/components/tabs/__tests__/ConfigTab.runtimeModelReset.test.tsx new file mode 100644 index 00000000..ea380a9d --- /dev/null +++ b/canvas/src/components/tabs/__tests__/ConfigTab.runtimeModelReset.test.tsx @@ -0,0 +1,296 @@ +// @vitest-environment jsdom +// +// Regression: ux/configtab-runtime-resets-model. +// +// Bug: in the Config tab, changing the Runtime did NOT reset the Model. The +// backend validates the (runtime, model) pair atomically on Save — if the +// stale model isn't registered for the newly-selected runtime it returns 422 +// (`model "X" is not a registered model for runtime "Y"`) and the runtime +// SILENTLY rolls back. Repro: switch to `google-adk` while the model is a +// claude-code-only model (e.g. moonshot/kimi-k2.6) → 422 → runtime stays +// claude-code. The user thinks they switched but nothing changed. +// +// Fix: on runtime change the form resets the model to a valid default for the +// new runtime, constrains the model dropdown to that runtime's registered +// models, makes the reset visible, and blocks Save on an invalid pair for +// registry-backed runtimes. This suite pins: +// (a) changing runtime resets the model to one valid for the new runtime, +// (b) an invalid (runtime, model) pair can't be submitted (Save disabled), +// both for google-adk. + +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react"; +import React from "react"; + +afterEach(cleanup); + +const apiGet = vi.fn(); +const apiPatch = vi.fn(); +const apiPut = vi.fn(); +vi.mock("@/lib/api", () => ({ + api: { + get: (path: string) => apiGet(path), + patch: (path: string, body: unknown) => apiPatch(path, body), + put: (path: string, body: unknown) => apiPut(path, body), + post: vi.fn(), + del: vi.fn(), + }, +})); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: Object.assign( + (selector: (s: unknown) => unknown) => + selector({ restartWorkspace: vi.fn(), updateNodeData: vi.fn(), nodes: [] }), + { + getState: () => ({ + restartWorkspace: vi.fn().mockResolvedValue(undefined), + updateNodeData: vi.fn(), + nodes: [], + }), + }, + ), +})); + +vi.mock("../AgentCardSection", () => ({ + AgentCardSection: () =>
, +})); + +import { ConfigTab, modelIdsForRuntime, defaultModelForRuntime } from "../ConfigTab"; + +// /templates payload mirroring registry_gen.go `Runtimes`: +// - claude-code (registry-backed): includes moonshot/kimi-k2.6 +// - google-adk (registry-backed): platform:gemini-2.5-pro / -flash only +const TEMPLATES = [ + { + id: "t-claude-code", + name: "Claude Code", + runtime: "claude-code", + registry_backed: true, + registry_providers: [ + { name: "platform", display_name: "Platform", billing_mode: "platform_managed" }, + ], + registry_models: [ + { id: "moonshot/kimi-k2.6", name: "Kimi K2.6", provider: "platform" }, + { id: "anthropic/claude-opus-4-8", name: "Claude Opus 4.8", provider: "platform" }, + ], + }, + { + id: "t-google-adk", + name: "Google ADK", + runtime: "google-adk", + registry_backed: true, + registry_providers: [ + { name: "platform", display_name: "Platform", billing_mode: "platform_managed" }, + ], + registry_models: [ + { id: "platform:gemini-2.5-pro", name: "Gemini 2.5 Pro", provider: "platform" }, + { id: "platform:gemini-2.5-flash", name: "Gemini 2.5 Flash", provider: "platform" }, + ], + }, +]; + +// A claude-code workspace running moonshot/kimi-k2.6 — the exact repro state. +function wireClaudeCodeWorkspace() { + apiGet.mockImplementation((path: string) => { + if (path === "/workspaces/ws-cc") return Promise.resolve({ runtime: "claude-code", tier: 2 }); + if (path === "/workspaces/ws-cc/model") + return Promise.resolve({ model: "moonshot/kimi-k2.6", source: "workspace_secrets" }); + if (path === "/workspaces/ws-cc/files/config.yaml") + return Promise.resolve({ + content: "name: cc\nruntime: claude-code\nruntime_config:\n model: moonshot/kimi-k2.6\n", + }); + if (path === "/templates") return Promise.resolve(TEMPLATES); + return Promise.reject(new Error(`unmocked api.get: ${path}`)); + }); + apiPut.mockResolvedValue({}); + apiPatch.mockResolvedValue({}); +} + +beforeEach(() => { + apiGet.mockReset(); + apiPatch.mockReset(); + apiPut.mockReset(); +}); + +describe("modelIdsForRuntime / defaultModelForRuntime helpers", () => { + it("returns the registry model ids for a registry-backed runtime", () => { + const opt = { + value: "google-adk", + label: "Google ADK", + models: [], + providers: [], + registryBacked: true, + registryProviders: [], + registryModels: [ + { id: "platform:gemini-2.5-pro" }, + { id: "platform:gemini-2.5-flash" }, + ], + }; + expect(modelIdsForRuntime(opt)).toEqual([ + "platform:gemini-2.5-pro", + "platform:gemini-2.5-flash", + ]); + expect(defaultModelForRuntime(opt)).toBe("platform:gemini-2.5-pro"); + }); + + it("falls back to template models[] for a non-registry runtime and skips wildcards", () => { + const opt = { + value: "hermes", + label: "Hermes", + models: [{ id: "openrouter/*" }, { id: "nousresearch/hermes-4-70b" }], + providers: [], + registryBacked: false, + registryProviders: [], + registryModels: [], + }; + expect(modelIdsForRuntime(opt)).toEqual(["nousresearch/hermes-4-70b"]); + expect(defaultModelForRuntime(opt)).toBe("nousresearch/hermes-4-70b"); + }); + + it("returns empty for a null / model-less runtime", () => { + expect(modelIdsForRuntime(null)).toEqual([]); + expect(defaultModelForRuntime(undefined)).toBe(""); + }); +}); + +describe("ConfigTab — runtime change resets model (google-adk)", () => { + it("(a) switching claude-code → google-adk resets the model to a valid google-adk model", async () => { + wireClaudeCodeWorkspace(); + render(); + + const runtimeSelect = (await waitFor(() => + screen.getByRole("combobox", { name: /runtime/i }), + )) as HTMLSelectElement; + // Wait for /templates to populate so registry_models are available. + await waitFor(() => + expect( + Array.from(runtimeSelect.options).map((o) => o.value), + ).toContain("google-adk"), + ); + + // Sanity: the model dropdown currently shows the claude-code model. + const modelSelectBefore = (await waitFor(() => + screen.getByTestId("model-select"), + )) as HTMLSelectElement; + expect(modelSelectBefore.value).toBe("moonshot/kimi-k2.6"); + + // Switch runtime to google-adk. + fireEvent.change(runtimeSelect, { target: { value: "google-adk" } }); + + // The model must reset to a google-adk-registered model, and the dropdown + // must only offer google-adk models (no kimi). + const modelSelectAfter = (await waitFor(() => + screen.getByTestId("model-select"), + )) as HTMLSelectElement; + await waitFor(() => + expect(modelSelectAfter.value).toBe("platform:gemini-2.5-pro"), + ); + const optsAfter = Array.from(modelSelectAfter.options).map((o) => o.value); + expect(optsAfter).toContain("platform:gemini-2.5-pro"); + expect(optsAfter).not.toContain("moonshot/kimi-k2.6"); + + // The reset is VISIBLE. + const note = await waitFor(() => screen.getByTestId("model-reset-note")); + expect(note.textContent).toMatch(/platform:gemini-2\.5-pro/); + expect(note.textContent).toMatch(/moonshot\/kimi-k2\.6/); + }); + + it("(a') Save after the runtime switch PUTs a valid google-adk model — never the stale kimi model", async () => { + wireClaudeCodeWorkspace(); + render(); + + const runtimeSelect = (await waitFor(() => + screen.getByRole("combobox", { name: /runtime/i }), + )) as HTMLSelectElement; + await waitFor(() => + expect( + Array.from(runtimeSelect.options).map((o) => o.value), + ).toContain("google-adk"), + ); + + fireEvent.change(runtimeSelect, { target: { value: "google-adk" } }); + await waitFor(() => + expect( + (screen.getByTestId("model-select") as HTMLSelectElement).value, + ).toBe("platform:gemini-2.5-pro"), + ); + + fireEvent.click(screen.getByRole("button", { name: /save & restart/i })); + + // runtime PATCH carries google-adk; model PUT carries a google-adk model. + await waitFor(() => + expect(apiPatch).toHaveBeenCalledWith( + "/workspaces/ws-cc", + expect.objectContaining({ runtime: "google-adk" }), + ), + ); + await waitFor(() => + expect(apiPut).toHaveBeenCalledWith("/workspaces/ws-cc/model", { + model: "platform:gemini-2.5-pro", + }), + ); + // The stale claude-code model must NEVER be PUT for the new runtime. + const modelPuts = apiPut.mock.calls.filter( + ([p]) => p === "/workspaces/ws-cc/model", + ); + for (const [, body] of modelPuts) { + expect((body as { model: string }).model).not.toBe("moonshot/kimi-k2.6"); + } + }); + + it("(b) an invalid (runtime, model) pair can't be submitted — Save is disabled", async () => { + // Drive the form into a raw-YAML invalid pair: runtime=google-adk with a + // claude-code-only model. This is the path the selector won't produce but + // a raw edit can. Save must be blocked (modelPairInvalid) so the 422 + + // silent rollback can't happen. + apiGet.mockImplementation((path: string) => { + if (path === "/workspaces/ws-bad") return Promise.resolve({ runtime: "google-adk", tier: 2 }); + if (path === "/workspaces/ws-bad/model") + return Promise.resolve({ model: "platform:gemini-2.5-pro", source: "workspace_secrets" }); + if (path === "/workspaces/ws-bad/files/config.yaml") + return Promise.resolve({ + content: + "name: bad\nruntime: google-adk\nruntime_config:\n model: platform:gemini-2.5-pro\n", + }); + if (path === "/templates") return Promise.resolve(TEMPLATES); + return Promise.reject(new Error(`unmocked api.get: ${path}`)); + }); + apiPut.mockResolvedValue({}); + apiPatch.mockResolvedValue({}); + + render(); + + const runtimeSelect = (await waitFor(() => + screen.getByRole("combobox", { name: /runtime/i }), + )) as HTMLSelectElement; + await waitFor(() => + expect( + Array.from(runtimeSelect.options).map((o) => o.value), + ).toContain("google-adk"), + ); + + // Flip into raw-YAML mode and inject the invalid pair. + fireEvent.click(screen.getByLabelText(/raw yaml/i)); + const rawEditor = (await waitFor(() => + screen.getByLabelText(/raw yaml editor/i), + )) as HTMLTextAreaElement; + fireEvent.change(rawEditor, { + target: { + value: + "name: bad\nruntime: google-adk\nruntime_config:\n model: moonshot/kimi-k2.6\n", + }, + }); + + // Save & Restart must be disabled — the (google-adk, moonshot/kimi-k2.6) + // pair is invalid for this registry-backed runtime. + const saveBtn = screen.getByRole("button", { name: /save & restart/i }); + await waitFor(() => expect((saveBtn as HTMLButtonElement).disabled).toBe(true)); + + // And no model PUT can fire while it stays invalid. + fireEvent.click(saveBtn); + const modelPuts = apiPut.mock.calls.filter( + ([p]) => p === "/workspaces/ws-bad/model", + ); + expect(modelPuts).toHaveLength(0); + }); +});