From 8a86b6615992cce9a532e0f3e6928d4fd5fe871c Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sat, 2 May 2026 22:10:51 -0700 Subject: [PATCH] fix(workspace-server): set universal MODEL env on every templated provision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug B fix, server-side complement to molecule-runtime PR #2538. The runtime PR taught `workspace/config.py` to honour `MODEL_PROVIDER` over `runtime_config.model` from the template's verbatim YAML. This PR is the upstream half: workspace-server's `applyRuntimeModelEnv` now sets `MODEL=` for **every** runtime, not just hermes (which got `HERMES_DEFAULT_MODEL` already). Pre-fix: applyRuntimeModelEnv's per-runtime switch only emitted HERMES_DEFAULT_MODEL for hermes; every other runtime got nothing, so the adapter read its template's default model from /configs/config.yaml. Surfaced 2026-05-02 — picking MiniMax-M2 in canvas → workspace booted with model=sonnet (claude-code template default) and demanded CLAUDE_CODE_OAUTH_TOKEN. Post-fix: MODEL is set unconditionally before the per-runtime switch. HERMES_DEFAULT_MODEL stays for backwards compat. Adapters opt in by reading os.environ["MODEL"] in their executor (claude-code adapter already does this since the same Bug B fix; see workspace-configs-templates/claude-code-default/adapter.py). Tests ===== - `TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes`: table-driven across claude-code/hermes/langgraph/crewai + empty-model fallback + MODEL_PROVIDER-secret-fallback path. Adding a new runtime = adding a row, not writing a new test. - All 6 sub-cases pass + existing `TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider` pin still green. Why now ======= This was authored alongside the runtime PR but stashed (not committed) during a session-handoff cleanup. The molecule-runtime side shipped at SHA 16ac895a and is live on PyPI as molecule-ai-workspace-runtime 0.1.84, but until the workspace-server side ships, the canvas-picked MODEL env never reaches non-hermes adapters. Caught by the systematic stash audit triggered by the user's discovery that ProviderModelSelector had been similarly stashed. Closes the workspace-server side of #246. Builds on merged #2538. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/workspace_provision.go | 13 +++ .../workspace_provision_shared_test.go | 82 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index b28b2225..561860f9 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -700,6 +700,19 @@ func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { if model == "" { 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. + envVars["MODEL"] = model + switch runtime { case "hermes": // template-hermes install.sh reads this into ~/.hermes/config.yaml's diff --git a/workspace-server/internal/handlers/workspace_provision_shared_test.go b/workspace-server/internal/handlers/workspace_provision_shared_test.go index 77149f13..2166cb23 100644 --- a/workspace-server/internal/handlers/workspace_provision_shared_test.go +++ b/workspace-server/internal/handlers/workspace_provision_shared_test.go @@ -623,3 +623,85 @@ func TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider(t *testi t.Errorf("sqlmock expectations not met — unknown-prefix model should mint MODEL_PROVIDER but skip LLM_PROVIDER: %v", err) } } + +// TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes pins the +// fix for Bug B (2026-05-02): canvas-selected model was silently dropped +// for templated workspaces because the per-runtime switch only set +// HERMES_DEFAULT_MODEL for hermes — every other runtime got nothing. +// The adapter then read its template's default model from /configs/config.yaml +// and demanded the wrong env var (e.g. claude-code/sonnet → CLAUDE_CODE_OAUTH_TOKEN +// even though the user had picked MiniMax-M2 with MINIMAX_API_KEY set). +// +// Post-fix: applyRuntimeModelEnv unconditionally sets MODEL= for +// every runtime, in addition to any vendor-specific name (HERMES_DEFAULT_MODEL +// stays for backwards compat). Adapters opt in to honouring MODEL by reading +// os.environ["MODEL"] in their executor (claude-code adapter does this since +// the same Bug B fix; see workspace-configs-templates/claude-code-default/adapter.py). +// +// Table-driven so adding a new runtime means adding a row, not writing a +// new test function. +func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) { + cases := []struct { + name string + runtime string + model string + modelProviderEnv string + wantMODEL string + wantHermesDefault string // empty string = must be unset + }{ + { + name: "claude-code: picked model populates MODEL", + runtime: "claude-code", + model: "MiniMax-M2", + wantMODEL: "MiniMax-M2", + }, + { + name: "hermes: picked model populates BOTH MODEL and 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)", + runtime: "langgraph", + model: "anthropic:claude-opus-4-7", + wantMODEL: "anthropic:claude-opus-4-7", + }, + { + name: "crewai: picked model populates MODEL (no vendor-specific name)", + runtime: "crewai", + model: "openai:gpt-4o", + wantMODEL: "openai:gpt-4o", + }, + { + name: "empty model + empty MODEL_PROVIDER fallback: nothing set", + runtime: "claude-code", + model: "", + }, + { + name: "empty model + MODEL_PROVIDER fallback hits: MODEL set from secret", + runtime: "claude-code", + model: "", + modelProviderEnv: "MiniMax-M2", + wantMODEL: "MiniMax-M2", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + envVars := map[string]string{} + if tc.modelProviderEnv != "" { + envVars["MODEL_PROVIDER"] = tc.modelProviderEnv + } + applyRuntimeModelEnv(envVars, tc.runtime, tc.model) + + if got := envVars["MODEL"]; got != tc.wantMODEL { + t.Errorf("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) + } + }) + } +}