Merge pull request #2538 from Molecule-AI/fix/canvas-picked-model-universal
fix(runtime): canvas-picked model wins universally + per-model required_env
This commit is contained in:
Hongming Wang
GitHub
commit
16ac895a39
@ -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", ""),
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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 ----------
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user