diff --git a/workspace-server/internal/handlers/delegation_test.go b/workspace-server/internal/handlers/delegation_test.go index 427e71b2..38c63206 100644 --- a/workspace-server/internal/handlers/delegation_test.go +++ b/workspace-server/internal/handlers/delegation_test.go @@ -1262,4 +1262,3 @@ func TestExecuteDelegation_CleanProxyResponse_Unchanged(t *testing.T) { t.Errorf("unmet sqlmock expectations: %v", err) } } -} diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 57d6c5a6..bf910ff4 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -717,13 +717,16 @@ func deriveProviderFromModelSlug(model string) string { func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { // Resolution order (priority high → low): // 1. payload.Model (caller passed the canvas-picked model id verbatim) - // 2. envVars["MODEL"] (workspace_secret persisted by /org/import via + // 2. envVars["MOLECULE_MODEL"] (the canonical, unambiguous name) + // 3. envVars["MODEL"] (workspace_secret persisted by /org/import via // the persona env file — MODEL=MiniMax-M2.7-highspeed etc.) - // 3. envVars["MODEL_PROVIDER"] (legacy: this secret was historically a - // *model id* set by canvas Save+Restart's PUT /model; on the - // post-2026-05-08 persona-env convention it's a *provider slug* - // (e.g. "minimax") which is NOT a valid model id, so this fallback - // only fires when MODEL is absent.) + // 4. envVars["MODEL_PROVIDER"] (legacy + misleadingly named: it carries + // a *model id*, never the provider — that's LLM_PROVIDER. Historically + // set by canvas Save+Restart's PUT /model; the post-2026-05-08 + // persona-env convention sometimes (mis)set it to a provider slug + // ("minimax") or a runtime name ("claude-code"), neither a valid + // model id — see internal#226. Only fires when the better-named + // vars are absent.) // // Pre-fix bug: this function unconditionally OVERWROTE envVars["MODEL"] // with the MODEL_PROVIDER slug (when payload.Model was empty), wiping @@ -736,6 +739,9 @@ func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { // and the workspace template's adapter routed to providers[0] // (anthropic-oauth) and wedged at SDK initialize. Caught 2026-05-08 // during Phase 4 verification of template-claude-code PR #9. + if model == "" { + model = envVars["MOLECULE_MODEL"] + } if model == "" { model = envVars["MODEL"] } @@ -746,16 +752,18 @@ func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { return } - // Universal MODEL env var — every adapter that wants to honour the - // canvas-picked model (instead of its template's default) reads this. - // molecule-runtime's workspace/config.py already falls back to MODEL - // for runtime_config.model (#194). Without this line, the user's - // canvas selection is silently dropped on every templated provision — - // confirmed via crash-loop diagnosis on 2026-05-02 where MiniMax - // picks booted with model=sonnet (template default) and demanded - // CLAUDE_CODE_OAUTH_TOKEN. Set it FIRST so the per-runtime branches - // below can still layer on additional vendor-specific names without - // fighting over the canonical one. + // Canonical model env vars — molecule-runtime's workspace/config.py + // resolves the picked model as MOLECULE_MODEL > MODEL > (legacy) + // MODEL_PROVIDER (#280). Export both new names so adapters can read + // either; MODEL stays for backwards compat with everything that + // already reads os.environ["MODEL"] (the claude-code adapter does, + // since #194). Without this, the user's canvas selection is silently + // dropped on every templated provision — confirmed via crash-loop + // diagnosis on 2026-05-02 where MiniMax picks booted with model=sonnet + // (template default) and demanded CLAUDE_CODE_OAUTH_TOKEN. Set these + // FIRST so the per-runtime branches below can layer on additional + // vendor-specific names without fighting over the canonical one. + envVars["MOLECULE_MODEL"] = model envVars["MODEL"] = model switch runtime { diff --git a/workspace-server/internal/handlers/workspace_provision_shared_test.go b/workspace-server/internal/handlers/workspace_provision_shared_test.go index 7a85d118..01101ddf 100644 --- a/workspace-server/internal/handlers/workspace_provision_shared_test.go +++ b/workspace-server/internal/handlers/workspace_provision_shared_test.go @@ -665,46 +665,62 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) { runtime string model string modelProviderEnv string + moleculeModelEnv string wantMODEL string wantHermesDefault string // empty string = must be unset }{ { - name: "claude-code: picked model populates MODEL", + name: "claude-code: picked model populates MODEL + MOLECULE_MODEL", runtime: "claude-code", model: "MiniMax-M2", wantMODEL: "MiniMax-M2", }, { - name: "hermes: picked model populates BOTH MODEL and HERMES_DEFAULT_MODEL", + name: "hermes: picked model populates MODEL, MOLECULE_MODEL, HERMES_DEFAULT_MODEL", runtime: "hermes", model: "minimax/MiniMax-M2.7", wantMODEL: "minimax/MiniMax-M2.7", wantHermesDefault: "minimax/MiniMax-M2.7", }, { - name: "langgraph: picked model populates MODEL (no vendor-specific name)", + name: "langgraph: picked model populates MODEL + MOLECULE_MODEL (no vendor-specific name)", runtime: "langgraph", model: "anthropic:claude-opus-4-7", wantMODEL: "anthropic:claude-opus-4-7", }, { - name: "crewai: picked model populates MODEL (no vendor-specific name)", + name: "crewai: picked model populates MODEL + MOLECULE_MODEL (no vendor-specific name)", runtime: "crewai", model: "openai:gpt-4o", wantMODEL: "openai:gpt-4o", }, { - name: "empty model + empty MODEL_PROVIDER fallback: nothing set", + name: "empty model + no env fallback: nothing set", runtime: "claude-code", model: "", }, { - name: "empty model + MODEL_PROVIDER fallback hits: MODEL set from secret", + name: "empty model + MODEL_PROVIDER fallback hits: MODEL/MOLECULE_MODEL set from secret", runtime: "claude-code", model: "", modelProviderEnv: "MiniMax-M2", wantMODEL: "MiniMax-M2", }, + { + name: "empty model + MOLECULE_MODEL env fallback hits (canonical name)", + runtime: "claude-code", + model: "", + moleculeModelEnv: "opus", + wantMODEL: "opus", + }, + { + name: "MOLECULE_MODEL beats MODEL_PROVIDER when both set (misnomer guard, internal#226)", + runtime: "claude-code", + model: "", + moleculeModelEnv: "opus", + modelProviderEnv: "claude-code", + wantMODEL: "opus", + }, } for _, tc := range cases { @@ -713,11 +729,18 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) { if tc.modelProviderEnv != "" { envVars["MODEL_PROVIDER"] = tc.modelProviderEnv } + if tc.moleculeModelEnv != "" { + envVars["MOLECULE_MODEL"] = tc.moleculeModelEnv + } applyRuntimeModelEnv(envVars, tc.runtime, tc.model) if got := envVars["MODEL"]; got != tc.wantMODEL { t.Errorf("MODEL = %q, want %q", got, tc.wantMODEL) } + // MOLECULE_MODEL (the canonical name) must mirror MODEL exactly. + if got := envVars["MOLECULE_MODEL"]; got != tc.wantMODEL { + t.Errorf("MOLECULE_MODEL = %q, want %q", got, tc.wantMODEL) + } if got := envVars["HERMES_DEFAULT_MODEL"]; got != tc.wantHermesDefault { t.Errorf("HERMES_DEFAULT_MODEL = %q, want %q", got, tc.wantHermesDefault) }