Merge pull request 'fix(workspace_provision): preserve MODEL secret over MODEL_PROVIDER slug on restart' (#136) from fix/preserve-model-secret-on-restart into main

workspace-server/internal/handlers/workspace_provision.go

@@ -715,14 +715,30 @@ func deriveProviderFromModelSlug(model string) string {
 // payload.Model at boot), this is a no-op — no harm in the switch
 // being empty for those cases.
 func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) {
-	// Fall back to the MODEL_PROVIDER workspace secret when the caller
-	// didn't pass one explicitly. This is the path that "Save+Restart"
-	// hits — Restart builds its payload from the workspaces row (no model
-	// column there) so payload.Model is always empty, but the user's
-	// canvas selection was stored as MODEL_PROVIDER via PUT /model and
-	// is already loaded into envVars here. Without this fallback hermes
-	// silently boots with the template default and errors "No LLM
-	// provider configured" even though the user picked a valid model.
+	// 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
+	//      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.)
+	//
+	// Pre-fix bug: this function unconditionally OVERWROTE envVars["MODEL"]
+	// with the MODEL_PROVIDER slug (when payload.Model was empty), wiping
+	// the operator's explicit per-persona MODEL secret on every restart.
+	// Symptom: a workspace whose persona env said
+	// MODEL=MiniMax-M2.7-highspeed booted fine on first /org/import (the
+	// envVars map was populated direct from the env file), then on the
+	// next Restart the workspace_secrets-derived MODEL got clobbered by
+	// MODEL_PROVIDER="minimax" — the literal slug, not a valid model id —
+	// 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["MODEL"]
+	}
 	if model == "" {
 		model = envVars["MODEL_PROVIDER"]
 	}

workspace-server/internal/handlers/workspace_provision_shared_test.go

@@ -724,3 +724,68 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) {
 		})
 	}
 }
+
+// TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved locks in the
+// 2026-05-08 fix that prevents the MODEL_PROVIDER-as-slug fallback from
+// silently overwriting a per-persona MODEL workspace_secret on restart.
+//
+// Pre-fix bug recurrence guard: when the persona env file (loaded into
+// workspace_secrets at /org/import time) declares both MODEL=<id> and
+// MODEL_PROVIDER=<slug>, the restart path used to overwrite envVars["MODEL"]
+// with the MODEL_PROVIDER slug because applyRuntimeModelEnv'\''s
+// payload.Model fallback consulted MODEL_PROVIDER first. Symptom: dev-tree
+// workspaces booted fine on first /org/import, then on next restart the
+// model id became literal "minimax" and the workspace template'\''s adapter
+// failed to match any registry prefix, fell through to anthropic-oauth,
+// and wedged at SDK initialize. Caught during Phase 4 verification of
+// template-claude-code PR #9.
+func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
+	cases := []struct {
+		name      string
+		envMODEL  string
+		envMP     string
+		wantMODEL string
+	}{
+		{
+			name:      "MODEL secret wins over MODEL_PROVIDER slug (persona-env shape on restart)",
+			envMODEL:  "MiniMax-M2.7-highspeed",
+			envMP:     "minimax",
+			wantMODEL: "MiniMax-M2.7-highspeed",
+		},
+		{
+			name:      "MODEL secret wins even when same as MODEL_PROVIDER",
+			envMODEL:  "opus",
+			envMP:     "claude-code",
+			wantMODEL: "opus",
+		},
+		{
+			name:      "MODEL absent → fall back to MODEL_PROVIDER (legacy canvas Save+Restart shape)",
+			envMODEL:  "",
+			envMP:     "MiniMax-M2.7",
+			wantMODEL: "MiniMax-M2.7",
+		},
+		{
+			name:      "Both absent → no MODEL set",
+			envMODEL:  "",
+			envMP:     "",
+			wantMODEL: "",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			envVars := map[string]string{}
+			if tc.envMODEL != "" {
+				envVars["MODEL"] = tc.envMODEL
+			}
+			if tc.envMP != "" {
+				envVars["MODEL_PROVIDER"] = tc.envMP
+			}
+			// payload.Model is empty (the restart case)
+			applyRuntimeModelEnv(envVars, "claude-code", "")
+			if got := envVars["MODEL"]; got != tc.wantMODEL {
+				t.Errorf("MODEL = %q, want %q (envMODEL=%q envMP=%q)",
+					got, tc.wantMODEL, tc.envMODEL, tc.envMP)
+			}
+		})
+	}
+}