diff --git a/workspace/config.py b/workspace/config.py index 4e199c57..6a256579 100644 --- a/workspace/config.py +++ b/workspace/config.py @@ -100,6 +100,16 @@ class RuntimeConfig: # "minimax"). Falls back to the top-level resolved # provider when empty. Adapters (hermes, claude-code, # codex) prefer this over slug-parsing the model name. + # Per-model entries surfaced in the canvas Model dropdown. Each entry is a + # raw dict with at least ``id``; ``required_env`` is the per-model auth + # list (e.g. ``{"id": "MiniMax-M2.7", "required_env": ["MINIMAX_API_KEY"]}``). + # Preflight prefers an entry's ``required_env`` over the top-level + # ``required_env`` when the picked ``model`` matches an entry's ``id`` + # (case-insensitive). The top-level list remains the fallback so single- + # model templates need not migrate. Surfaced 2026-05-02 after a user + # picked MiniMax in canvas, set MINIMAX_API_KEY, and still got booted + # into a CLAUDE_CODE_OAUTH_TOKEN preflight failure. + models: list[dict] = field(default_factory=list) # Deprecated — use required_env + secrets API instead. Kept for backward compat. auth_token_env: str = "" auth_token_file: str = "" @@ -426,25 +436,36 @@ def load_config(config_path: Optional[str] = None) -> WorkspaceConfig: args=runtime_raw.get("args", []), required_env=runtime_raw.get("required_env", []), timeout=runtime_raw.get("timeout", 0), - # Fall back to top-level resolved `model` (which already honors - # MODEL_PROVIDER env override, line 277) when YAML doesn't carry - # runtime_config.model. Without this fallback, SaaS workspaces - # silently boot with the adapter's hard-coded default — - # claude-code-default reads `runtime_config.model or "sonnet"`, - # so a user who picks Opus in the canvas Config tab gets Sonnet - # on the next CP-driven restart. Root cause: the CP user-data - # script regenerates /configs/config.yaml at every boot with - # only `name`, `runtime`, `a2a` keys (intentionally minimal so - # it doesn't carry stale state), losing runtime_config.model. - # MODEL_PROVIDER is plumbed as an env var, so picking it up via - # the top-level resolved model keeps the selection sticky. - model=runtime_raw.get("model") or model, + # Picked-model precedence (priority order): + # 1. MODEL_PROVIDER env var — canvas-picked model, plumbed via + # workspace-server's secret-mint path or the universal + # MODEL/MODEL_PROVIDER env from applyRuntimeModelEnv. The + # operator's canvas selection MUST win over the template's + # baked-in default; previously the template's + # `runtime_config.model: sonnet` always won and the picked + # MiniMax/GLM/etc model was silently dropped (Bug B, + # surfaced 2026-05-02 during E2E). + # 2. runtime_raw.model — explicit YAML override in the + # template's runtime_config. + # 3. top-level `model` — already honors MODEL_PROVIDER (line + # 359) but only when YAML lacks a top-level `model:`. This + # is the SaaS restart case (CP regenerates a minimal + # config.yaml on every boot, dropping runtime_config.model). + # Centralising here means EVERY adapter gets the override for + # free — no per-adapter env-reading code required. + model=os.environ.get("MODEL_PROVIDER") or runtime_raw.get("model") or model, # Same fallback shape as ``model`` above: an explicit # ``runtime_config.provider`` wins; otherwise inherit the # top-level resolved provider so adapters see a single # consistent choice without each one re-implementing # env/YAML/slug-prefix resolution. provider=runtime_raw.get("provider") or provider, + # Per-model entries (canvas Model dropdown source). Pass through + # raw dicts so the schema can grow without a parser change. Only + # entries that are dicts are kept — a malformed YAML element + # (string, list, None) is silently dropped rather than raising, + # matching the rest of this parser's lenient defaults. + models=[m for m in (runtime_raw.get("models") or []) if isinstance(m, dict)], # Deprecated fields — kept for backward compat auth_token_env=runtime_raw.get("auth_token_env", ""), auth_token_file=runtime_raw.get("auth_token_file", ""), diff --git a/workspace/preflight.py b/workspace/preflight.py index 1e6aaad2..d6a5f0a3 100644 --- a/workspace/preflight.py +++ b/workspace/preflight.py @@ -140,6 +140,44 @@ def run_preflight(config: WorkspaceConfig, config_path: str) -> PreflightReport: # Check required environment variables (e.g. CLAUDE_CODE_OAUTH_TOKEN, OPENAI_API_KEY). # These are declared per-runtime in config.yaml and injected via the secrets API. required_env = getattr(config.runtime_config, "required_env", []) or [] + + # Per-model override path. When the template's runtime_config declares + # `models[]` (canvas Model dropdown), prefer the picked model's own + # `required_env` over the top-level fallback. The picked model is + # `runtime_config.model` (which already honors the MODEL_PROVIDER env + # override at parse time — see config.py:RuntimeConfig.model resolution). + # Match on `entry["id"]` case-insensitively because canvas-side ids + # ("MiniMax-M2.7") and adapter-side normalization ("minimax-m2.7") drift + # by case across registries. + # + # Bug surfaced 2026-05-02: claude-code-default top-level required_env + # demands CLAUDE_CODE_OAUTH_TOKEN, but the user picked MiniMax and only + # set MINIMAX_API_KEY. Without this lookup, preflight failed and the + # workspace crash-looped despite the user having satisfied the picked + # model's actual auth requirement. + models = getattr(config.runtime_config, "models", None) or [] + picked_model = (getattr(config.runtime_config, "model", "") or "").strip() + if models and picked_model: + picked_lower = picked_model.lower() + for entry in models: + if not isinstance(entry, dict): + continue + entry_id = str(entry.get("id", "")).strip() + if not entry_id: + continue + if entry_id.lower() != picked_lower: + continue + per_model_env = entry.get("required_env") + if per_model_env: + # Per-model required_env wins outright — do NOT union with the + # top-level list. Templates use per-model entries precisely + # to express that different models have *different* auth + # paths (OAuth token vs API key vs third-party provider key); + # unioning would re-introduce the very crash-loop this fix + # closes. + required_env = list(per_model_env) + break + for env_var in required_env: if not os.environ.get(env_var): report.failures.append( diff --git a/workspace/tests/test_preflight.py b/workspace/tests/test_preflight.py index 3bb4a793..d56e02db 100644 --- a/workspace/tests/test_preflight.py +++ b/workspace/tests/test_preflight.py @@ -286,6 +286,140 @@ def test_required_env_empty_list_passes(tmp_path): assert report.ok is True +# ---------- Per-model required_env (models[] override) ---------- + + +def test_per_model_required_env_wins_over_top_level(tmp_path, monkeypatch): + """When `runtime_config.models[]` declares per-model `required_env` and + the picked `model` matches an entry id, the entry's required_env wins + over the top-level fallback. The 2026-05-02 MiniMax-on-claude-code bug: + user picks MiniMax + sets MINIMAX_API_KEY, top-level demands + CLAUDE_CODE_OAUTH_TOKEN — without this override path the workspace + crash-loops on a stale top-level requirement.""" + monkeypatch.setenv("MINIMAX_API_KEY", "mx-test") + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + + config = make_config( + runtime="claude-code", + runtime_config=RuntimeConfig( + model="MiniMax-M2.7", + required_env=["CLAUDE_CODE_OAUTH_TOKEN"], # top-level fallback + models=[ + {"id": "sonnet", "required_env": ["CLAUDE_CODE_OAUTH_TOKEN"]}, + {"id": "MiniMax-M2.7", "required_env": ["MINIMAX_API_KEY"]}, + ], + ), + ) + + report = run_preflight(config, str(tmp_path)) + + assert report.ok is True + assert not any(issue.title == "Required env" for issue in report.failures) + + +def test_top_level_required_env_used_when_no_models_declared(tmp_path, monkeypatch): + """No `models[]` field → preserve the existing top-level behavior. This + is the single-model template path — claude-code-default before it grew + a Model dropdown, codex-default today, etc.""" + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + + config = make_config( + runtime="claude-code", + runtime_config=RuntimeConfig( + model="sonnet", + required_env=["CLAUDE_CODE_OAUTH_TOKEN"], + models=[], + ), + ) + + report = run_preflight(config, str(tmp_path)) + + assert report.ok is False + assert any( + issue.title == "Required env" and "CLAUDE_CODE_OAUTH_TOKEN" in issue.detail + for issue in report.failures + ) + + +def test_top_level_used_when_picked_model_not_in_models_list(tmp_path, monkeypatch): + """`models[]` declared but the picked `model` isn't listed → fall back + to the top-level required_env. Defensive: protects against typos / + template drift / a CP override that names a model the template doesn't + enumerate. Never silently accept zero-auth in that case.""" + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + + config = make_config( + runtime="claude-code", + runtime_config=RuntimeConfig( + model="some-unknown-model", + required_env=["CLAUDE_CODE_OAUTH_TOKEN"], + models=[ + {"id": "sonnet", "required_env": ["CLAUDE_CODE_OAUTH_TOKEN"]}, + {"id": "MiniMax-M2.7", "required_env": ["MINIMAX_API_KEY"]}, + ], + ), + ) + + report = run_preflight(config, str(tmp_path)) + + assert report.ok is False + assert any( + issue.title == "Required env" and "CLAUDE_CODE_OAUTH_TOKEN" in issue.detail + for issue in report.failures + ) + + +def test_per_model_match_is_case_insensitive(tmp_path, monkeypatch): + """Match `entry["id"]` against `runtime_config.model` case-insensitively + — canvas surfaces `MiniMax-M2.7`, registries normalise to lowercase + `minimax-m2.7`, MODEL_PROVIDER env may carry either. The match must + not be brittle to that drift or templates ship preflight failures + on a working auth setup.""" + monkeypatch.setenv("MINIMAX_API_KEY", "mx-test") + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + + config = make_config( + runtime="claude-code", + runtime_config=RuntimeConfig( + model="minimax-m2.7", # lowercase + required_env=["CLAUDE_CODE_OAUTH_TOKEN"], + models=[ + {"id": "MiniMax-M2.7", "required_env": ["MINIMAX_API_KEY"]}, # mixed case + ], + ), + ) + + report = run_preflight(config, str(tmp_path)) + + assert report.ok is True + assert not any(issue.title == "Required env" for issue in report.failures) + + +def test_per_model_match_with_no_required_env_falls_back_to_top_level(tmp_path, monkeypatch): + """An entry that matches the picked model but has no `required_env` + (or an empty one) falls back to the top-level list. This protects + against partially-specified template entries — many templates list + a `name`/`description` per model without enumerating env vars when + the auth is identical across the family.""" + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "sk-test") + + config = make_config( + runtime="claude-code", + runtime_config=RuntimeConfig( + model="sonnet", + required_env=["CLAUDE_CODE_OAUTH_TOKEN"], + models=[ + {"id": "sonnet", "name": "Claude Sonnet"}, # no required_env + ], + ), + ) + + report = run_preflight(config, str(tmp_path)) + + assert report.ok is True + assert not any(issue.title == "Required env" for issue in report.failures) + + # ---------- Legacy auth_token_file backward compat ----------