molecule-ai/molecule-core
35
0
Fork 2
Code Issues 16 Pull Requests 2 Actions 49 Packages Projects Releases Wiki Activity

fix(canvas): ConfigTab is single source of truth for tier/provider/model

Browse Source 
Three drift bugs in ConfigTab + ProviderModelSelector. Same root pattern:
the form's display, the diff baseline, and the canvas store all read or
write from different copies of the same data, so what the user sees and
what the runtime actually uses can diverge silently.

(1) currentModelId read runtime_config.model first; loadConfig overrode
only top-level config.model. With template YAML `runtime_config.model:
sonnet` and live MODEL_PROVIDER=`MiniMax-M2`, the form rendered
"Claude Code subscription / Claude Sonnet (OAuth)" while the container
env (and chat) used MiniMax-M2. Fix: loadConfig propagates
wsMetadataModel into BOTH places.

(2) handleSave's nextModel-vs-oldModel diff compared the form value to
the YAML default. After (1) mirrors wsMetadataModel into the form's
runtime_config.model for display, that diff was always non-zero on
no-op saves and would fire /model PUT — which auto-restarts. New
originalModel state tracks the loaded MODEL_PROVIDER and is the diff
baseline.

(3) handleSave PATCHed the workspace row but never pushed the same
fields into useCanvasStore.updateNodeData. User picked T3, hit Save &
Restart, DB updated to tier=3, header pill kept showing T2 until full
hydrate. Fix: mirror dbPatch into the store.

Bonus: ProviderModelSelector.handleProviderChange used to auto-default
the model to next.models[0] (alphabetically first) when switching
providers. User picked the MiniMax provider intending MiniMax-M2.7;
the form silently set MiniMax-M2 (first in the bucket) and the
workspace deployed with the wrong model. Now empty-default for
multi-model providers, force explicit pick — Save/Deploy already gate
on model.trim() === "".

Three new tests in ConfigTab.provider.test.tsx pin (1)/(2)/(3); two
existing ProviderModelSelector tests updated to reflect the no-silent-
default behaviour, with a new single-model-auto-pick test for the
0-vs-many boundary. 1212/1212 canvas tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-05-02 23:31:02 -07:00
parent 5259ce3ea1
commit 7f0c58d563
4 changed files with 246 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
canvas/src/components/ProviderModelSelector.tsx
View File

@ -310,12 +310,26 @@ export function ProviderModelSelector({
onChange({ providerId: "", model: "", envVars: [] });
return;
}
// When switching providers, default the model to the first concrete
// entry in that provider (or empty if wildcard). Avoids showing a
// stale model id from the previous provider.
// When switching providers:
// - wildcard provider → empty (free-text input takes over)
// - exactly 1 concrete model → auto-pick (no choice to make)
// - 2+ concrete models → leave empty so the operator MUST pick
//
// Background: previously this defaulted to `next.models[0]` for any
// non-wildcard provider, which silently set the alphabetically-first
// model in the bucket. Bit a real user on 2026-05-03 — they picked
// the MiniMax provider intending `MiniMax-M2.7` but the form silently
// set `MiniMax-M2` (first in the list). They never saw the model
// dropdown change because the provider+model widgets are visually
// distinct, and the workspace deployed with the wrong model. Caller
// already disables Deploy/Save while `model.trim() === ""`, so the
// empty default forces an explicit pick without loosening any other
// gate.
const defaultModel = next.wildcard
? ""
: next.models[0]?.id ?? "";
: next.models.length === 1
? next.models[0]?.id ?? ""
: "";
onChange({
providerId: next.id,
model: defaultModel,

33
canvas/src/components/__tests__/ProviderModelSelector.test.tsx
View File

@ -182,15 +182,38 @@ describe("<ProviderModelSelector>", () => {
expect(modelSelect.disabled).toBe(true);
});
it("picking provider emits onChange with default model + envVars", () => {
it("picking a multi-model provider emits onChange with empty model (forces explicit pick)", () => {
const { onChange } = setup();
const providerSelect = screen.getByTestId("provider-select");
const catalog = buildProviderCatalog(CLAUDE_CODE_MODELS);
const minimax = catalog.find((p) => p.vendor === "minimax")!;
// MiniMax bucket holds 2 models (MiniMax-M2 + MiniMax-M2.7). Auto-
// picking the first one used to bite a real user (2026-05-03):
// they wanted M2.7 but the silent default put M2 in the deploy
// payload. Now the model field must come back empty so the next
// dropdown is required-empty and Save/Deploy stay disabled until
// the user picks.
fireEvent.change(providerSelect, { target: { value: minimax.id } });
expect(onChange).toHaveBeenCalledWith({
providerId: minimax.id,
model: "MiniMax-M2",
model: "",
envVars: ["ANTHROPIC_AUTH_TOKEN"],
});
});
it("picking a single-model provider auto-fills the model (no choice to make)", () => {
const { onChange } = setup();
const providerSelect = screen.getByTestId("provider-select");
const catalog = buildProviderCatalog(CLAUDE_CODE_MODELS);
// GLM-4.6 is the only model under the zai vendor in the fixture —
// a "0 vs many" boundary check. With only one option, forcing the
// user to re-pick adds friction without preventing any error.
const zai = catalog.find((p) => p.vendor === "zai")!;
expect(zai.models.length).toBe(1);
fireEvent.change(providerSelect, { target: { value: zai.id } });
expect(onChange).toHaveBeenCalledWith({
providerId: zai.id,
model: "GLM-4.6",
envVars: ["ANTHROPIC_AUTH_TOKEN"],
});
});
@ -250,7 +273,7 @@ describe("<ProviderModelSelector>", () => {
expect(screen.getByText(/requires:/).textContent).toMatch(/CLAUDE_CODE_OAUTH_TOKEN/);
});
it("switching provider resets model to first concrete option", () => {
it("switching to a multi-model provider clears the stale model id", () => {
const catalog = buildProviderCatalog(CLAUDE_CODE_MODELS);
const oauth = catalog.find((p) => p.vendor === "anthropic-oauth")!;
const minimax = catalog.find((p) => p.vendor === "minimax")!;
@ -260,9 +283,11 @@ describe("<ProviderModelSelector>", () => {
onChange,
});
fireEvent.change(screen.getByTestId("provider-select"), { target: { value: minimax.id } });
// Empty rather than auto-picked — see "picking a multi-model
// provider …" test above for the user-facing rationale.
expect(onChange).toHaveBeenCalledWith({
providerId: minimax.id,
model: "MiniMax-M2",
model: "",
envVars: ["ANTHROPIC_AUTH_TOKEN"],
});
});

53
canvas/src/components/tabs/ConfigTab.tsx
View File

@ -191,6 +191,16 @@ export function ConfigTab({ workspaceId }: Props) {
// data, written into /configs/config.yaml on next provision too).
const [provider, setProvider] = useState("");
const [originalProvider, setOriginalProvider] = useState("");
// Track the model that loaded from the DB (workspace_secrets.MODEL_PROVIDER
// via /workspaces/:id/model) separately from the YAML's runtime_config.model.
// handleSave's diff used to compare nextModel against the YAML's value;
// after the loadConfig fix mirrors wsMetadataModel into runtime_config.model
// for display, that diff would always be non-zero (YAML default vs.
// overridden value) and trigger a /model PUT — which auto-restarts —
// on every Save. Comparing against the loaded MODEL_PROVIDER instead
// keeps unrelated saves (tier change, skill edit) from rebooting the
// workspace just because the template's YAML default differs.
const [originalModel, setOriginalModel] = useState("");
const successTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
useEffect(() => {
@ -220,6 +230,7 @@ export function ConfigTab({ workspaceId }: Props) {
const m = await api.get<{ model?: string }>(`/workspaces/${workspaceId}/model`);
wsMetadataModel = (m.model || "").trim();
} catch { /* non-fatal */ }
setOriginalModel(wsMetadataModel);
// Load explicit provider override (Option B PR-5). Endpoint returns
// {provider: "", source: "default"} when no override is set, so the
@ -249,7 +260,17 @@ export function ConfigTab({ workspaceId }: Props) {
// form doesn't contradict the node badge (issue: badge=T3, form=T2).
const merged = { ...DEFAULT_CONFIG, ...parsed } as ConfigData;
if (wsMetadataRuntime) merged.runtime = wsMetadataRuntime;
if (wsMetadataModel) merged.model = wsMetadataModel;
if (wsMetadataModel) {
// Single source of truth: MODEL_PROVIDER (DB) is the live runtime
// value. Override BOTH top-level + nested runtime_config.model so
// currentModelId (which reads runtime_config.model first) doesn't
// silently fall through to the template default. Without the
// nested override, a workspace deployed with `MiniMax-M2` shows
// the template's `runtime_config.model: sonnet` in the UI even
// though the container env (and chat) actually use MiniMax-M2.
merged.model = wsMetadataModel;
merged.runtime_config = { ...(merged.runtime_config ?? {}), model: wsMetadataModel };
}
if (wsMetadataTier !== null) merged.tier = wsMetadataTier;
setConfig(merged);
} catch {
@ -265,6 +286,10 @@ export function ConfigTab({ workspaceId }: Props) {
...DEFAULT_CONFIG,
runtime: wsMetadataRuntime,
model: wsMetadataModel,
// Mirror the merged-path fix above — keep top-level + nested in
// sync so currentModelId picks up wsMetadataModel even when the
// form falls into the no-config.yaml branch (hermes/external).
...(wsMetadataModel ? { runtime_config: { model: wsMetadataModel } } : {}),
...(wsMetadataTier !== null ? { tier: wsMetadataTier } : {}),
} as ConfigData);
} finally {
@ -415,6 +440,15 @@ export function ConfigTab({ workspaceId }: Props) {
}
if (Object.keys(dbPatch).length > 0) {
await api.patch(`/workspaces/${workspaceId}`, dbPatch);
// Mirror the DB write into the canvas store node data so the
// header pill (TIER T2/T3, RUNTIME claude-code/hermes) and the
// node card update immediately. Without this push, the workspace
// row reflects the new tier but every UI surface that reads from
// useCanvasStore.nodes (header badge, ContextMenu, etc.) keeps
// showing the stale value until the next full hydrate. Bug
// surfaced 2026-05-03 — user picked T3, hit Save & Restart,
// database said tier=3, badge still said T2.
useCanvasStore.getState().updateNodeData(workspaceId, dbPatch);
}
// Model has its own endpoint (separate from the general workspace
@ -436,21 +470,26 @@ export function ConfigTab({ workspaceId }: Props) {
// configured" error in the chat. Caught 2026-04-30 on hongmingwang
// hermes workspace 32993ee7-…cb9d75d112a5.
const nextModelRaw = (nextSource.runtime_config as Record<string, unknown> | undefined)?.model;
const oldModelRaw = (oldParsed.runtime_config as Record<string, unknown> | undefined)?.model;
const nextModel =
typeof nextModelRaw === "string" && nextModelRaw
? nextModelRaw
: typeof nextSource.model === "string"
? nextSource.model
: "";
const oldModel =
typeof oldModelRaw === "string" && oldModelRaw
? oldModelRaw
: (oldParsed.model as string) || "";
// Diff against the loaded MODEL_PROVIDER (the runtime source of
// truth), not the YAML's runtime_config.model. After loadConfig
// mirrors wsMetadataModel into runtime_config.model for display,
// nextModel always equals the loaded value on a no-op save —
// diffing against oldModelRaw (the unmirrored YAML default) would
// make every Save fire a /model PUT and trigger an auto-restart,
// even when the user only changed an unrelated field. Comparing
// against `originalModel` keeps the PUT scoped to actual user
// intent.
let modelSaveError: string | null = null;
if (nextModel && nextModel !== oldModel) {
if (nextModel && nextModel !== originalModel) {
try {
await api.put(`/workspaces/${workspaceId}/model`, { model: nextModel });
setOriginalModel(nextModel);
} catch (e) {
modelSaveError = e instanceof Error ? e.message : "Model update was rejected";
}

155
canvas/src/components/tabs/__tests__/ConfigTab.provider.test.tsx
View File

@ -38,10 +38,15 @@ vi.mock("@/lib/api", () => ({
},
}));
// Shared store stub — `updateNodeData` is exposed so a test can assert the
// node-data flush happens after a successful PATCH (regression: previously
// the DB updated but the canvas badge stayed stale until full hydrate).
const storeUpdateNodeData = vi.fn();
const storeRestartWorkspace = vi.fn();
vi.mock("@/store/canvas", () => ({
useCanvasStore: Object.assign(
(selector: (s: unknown) => unknown) => selector({ restartWorkspace: vi.fn(), updateNodeData: vi.fn() }),
{ getState: () => ({ restartWorkspace: vi.fn(), updateNodeData: vi.fn() }) },
(selector: (s: unknown) => unknown) => selector({ restartWorkspace: storeRestartWorkspace, updateNodeData: storeUpdateNodeData }),
{ getState: () => ({ restartWorkspace: storeRestartWorkspace, updateNodeData: storeUpdateNodeData }) },
),
}));
@ -90,6 +95,8 @@ beforeEach(() => {
apiGet.mockReset();
apiPatch.mockReset();
apiPut.mockReset();
storeUpdateNodeData.mockReset();
storeRestartWorkspace.mockReset();
});
describe("ConfigTab — Provider override (Option B PR-5)", () => {
@ -333,4 +340,148 @@ describe("ConfigTab — Provider override (Option B PR-5)", () => {
expect(providerCalls[0][1]).toEqual({ provider: "" });
});
});
// Display-vs-storage drift regression (2026-05-03 incident, workspace
// e13aebd8…). User deployed claude-code with MiniMax-M2 stored in
// MODEL_PROVIDER. The container env (MODEL=MiniMax-M2) and chat
// worked correctly, but the Config tab showed "Claude Code
// subscription / Claude Sonnet (OAuth)" — i.e. the template's
// runtime_config.model: sonnet default — because currentModelId
// reads runtime_config.model first and loadConfig was overriding
// only the top-level config.model field. The merged shape was:
// { model: "MiniMax-M2", runtime_config: { model: "sonnet" } }
// and currentModelId picked "sonnet". Fix: loadConfig propagates
// wsMetadataModel into BOTH places so the form is a single source
// of truth (DB-backed MODEL_PROVIDER). Pinning the merged-path
// branch with the exact reproducing shape: claude-code template
// YAML has runtime_config.model: sonnet; live workspace's
// MODEL_PROVIDER is MiniMax-M2; tab must show the latter.
it("prefers MODEL_PROVIDER over the template's runtime_config.model on load", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [
{ id: "sonnet", name: "Claude Sonnet (OAuth)", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
{ id: "MiniMax-M2", name: "MiniMax M2", required_env: ["MINIMAX_API_KEY"] },
{ id: "MiniMax-M2.7", name: "MiniMax M2.7", required_env: ["MINIMAX_API_KEY"] },
],
},
],
});
render(<ConfigTab workspaceId="ws-test" />);
const modelSelect = (await screen.findByTestId("model-select")) as HTMLSelectElement;
await waitFor(() => expect(modelSelect.value).toBe("MiniMax-M2"));
// Provider dropdown should also reflect MiniMax (back-derived from
// the model slug since LLM_PROVIDER is unset). Without the fix,
// the selector falls back to the first catalog entry whose first
// model matches "sonnet" → anthropic-oauth bucket → "Claude Code
// subscription".
const providerSelect = screen.getByTestId("provider-select") as HTMLSelectElement;
const selectedOption = providerSelect.options[providerSelect.selectedIndex];
expect(selectedOption.textContent ?? "").toMatch(/MiniMax/);
});
// Sibling pin to the display-fix above. The display fix mirrors
// wsMetadataModel into runtime_config.model so the selector renders
// the live value; that mirror means handleSave's old YAML-vs-form
// diff would always be non-zero on a no-op save (YAML default
// "sonnet" vs. mirrored "MiniMax-M2") and PUT /model — which
// server-side SetModel chains into an auto-restart. handleSave now
// diffs against the loaded MODEL_PROVIDER instead. Pin: an
// unrelated edit (tier change) must NOT touch /model when the
// model itself didn't change.
it("does not PUT /model on a no-op save when only an unrelated field changed", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\ntier: 2\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [
{ id: "sonnet", name: "Claude Sonnet", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
{ id: "MiniMax-M2", name: "MiniMax M2", required_env: ["MINIMAX_API_KEY"] },
],
},
],
});
apiPut.mockResolvedValue({});
apiPatch.mockResolvedValue({});
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
await waitFor(() => {
const tierPatches = apiPatch.mock.calls.filter(([path, body]) =>
path === "/workspaces/ws-test" && (body as { tier?: number }).tier === 3,
);
expect(tierPatches.length).toBe(1);
});
// Spurious /model PUT would fire here without the originalModel
// diff baseline. The model itself didn't change, so /model must
// stay untouched (otherwise SetModel auto-restarts).
const modelPuts = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/model");
expect(modelPuts.length).toBe(0);
});
// Save-then-stale-badge regression (2026-05-03 incident). User
// selected T3 in the Tier dropdown, hit Save & Restart, the workspace
// PATCH succeeded (`tier: 3` in DB), but the canvas header pill kept
// showing "TIER T2" until a full hydrate. Root cause: handleSave
// sent the PATCH to workspace-server but never pushed the same
// change into useCanvasStore.updateNodeData, so every UI surface
// reading from the store kept its stale value. Pin: a successful
// tier PATCH must mirror into the store so the badge updates
// synchronously with the response.
it("flushes the dbPatch into useCanvasStore.updateNodeData after a successful PATCH", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\ntier: 2\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [{ id: "sonnet", name: "Sonnet", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] }],
},
],
});
apiPatch.mockResolvedValue({ status: "updated" });
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
await waitFor(() => {
expect(apiPatch.mock.calls.some(([p]) => p === "/workspaces/ws-test")).toBe(true);
});
// Without the store flush, the badge would keep reading tier=2
// from useCanvasStore.nodes until a full hydrate. Pin: handleSave
// pushes the same fields it PATCHed.
expect(storeUpdateNodeData).toHaveBeenCalledWith(
"ws-test",
expect.objectContaining({ tier: 3 }),
);
});
});