fix(provisioner): export MOLECULE_MODEL canonical env + read it first; drop stray brace in delegation_test.go #286

hongming-pc2 · 2026-05-10T10:11:58Z

hongming-pc2 commented

2026-05-10 10:11:58 +00:00

internal#226 follow-up #1 — match the provisioner side of the model-env boundary to the runtime side (#280: MOLECULE_MODEL > MODEL > legacy MODEL_PROVIDER).

Change

applyRuntimeModelEnv (workspace_provision.go): reads MOLECULE_MODEL ahead of MODEL / MODEL_PROVIDER; exports both MOLECULE_MODEL and MODEL (the latter kept for back-compat with everything that already reads os.environ["MODEL"], e.g. the claude-code adapter). So a workspace whose secrets carry the unambiguous MOLECULE_MODEL is honoured, and the misnamed MODEL_PROVIDER (which the post-2026-05-08 persona-env convention (mis)set to provider slugs like minimax and even runtime names like claude-code) is the lowest-priority fallback. Resolution-order comment updated.
Also drops a stray trailing } in delegation_test.go (committed in 97768272) that made internal/handlers fail to parse — one of the blockers keeping the package from compiling for tests.

Tests

TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes now also asserts MOLECULE_MODEL mirrors MODEL on every case, + 2 new cases (MOLECULE_MODEL env fallback; MOLECULE_MODEL beats MODEL_PROVIDER).

⚠️ Could not run go test ./internal/handlers/ locally — after the delegation_test.go brace fix, the package is still blocked behind internal/plugins SourceResolver redeclared / resolver.Scheme undefined (the #248 plugin-router/resolver refactor — Core-BE's lane). CI will validate once #248 lands. The applyRuntimeModelEnv change is mechanical (mirrors the existing MODEL handling) — reviewer please eyeball.

Companions

molecule-core#280 — runtime config.py side (the MODEL_PROVIDER→MOLECULE_MODEL precedence + deprecation log).
molecule-ai-workspace-template-claude-code#14 — surface the real CLI stream error instead of the swallowed-stderr placeholder.

🤖 Generated with Claude Code

internal#226 follow-up #1 — match the provisioner side of the model-env boundary to the runtime side (#280: `MOLECULE_MODEL` > `MODEL` > legacy `MODEL_PROVIDER`). ### Change - `applyRuntimeModelEnv` (`workspace_provision.go`): reads `MOLECULE_MODEL` ahead of `MODEL` / `MODEL_PROVIDER`; exports **both** `MOLECULE_MODEL` and `MODEL` (the latter kept for back-compat with everything that already reads `os.environ["MODEL"]`, e.g. the claude-code adapter). So a workspace whose secrets carry the unambiguous `MOLECULE_MODEL` is honoured, and the misnamed `MODEL_PROVIDER` (which the post-2026-05-08 persona-env convention (mis)set to provider slugs like `minimax` and even runtime names like `claude-code`) is the lowest-priority fallback. Resolution-order comment updated. - Also drops a **stray trailing `}`** in `delegation_test.go` (committed in `97768272`) that made `internal/handlers` fail to parse — one of the blockers keeping the package from compiling for tests. ### Tests `TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes` now also asserts `MOLECULE_MODEL` mirrors `MODEL` on every case, + 2 new cases (`MOLECULE_MODEL` env fallback; `MOLECULE_MODEL` beats `MODEL_PROVIDER`). ⚠️ **Could not run `go test ./internal/handlers/` locally** — after the `delegation_test.go` brace fix, the package is still blocked behind `internal/plugins` `SourceResolver redeclared` / `resolver.Scheme undefined` (the #248 plugin-router/resolver refactor — Core-BE's lane). CI will validate once #248 lands. The `applyRuntimeModelEnv` change is mechanical (mirrors the existing `MODEL` handling) — reviewer please eyeball. ### Companions - molecule-core#280 — runtime `config.py` side (the `MODEL_PROVIDER`→`MOLECULE_MODEL` precedence + deprecation log). - molecule-ai-workspace-template-claude-code#14 — surface the real CLI stream error instead of the swallowed-stderr placeholder. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hongming-pc2 added 1 commit 2026-05-10 10:12:06 +00:00

fix(provisioner): export MOLECULE_MODEL (canonical model env) + read it first; drop stray brace in delegation_test.go

sop-tier-check / tier-check (pull_request) Failing after 17s

Details

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 20s

Details

audit-force-merge / audit (pull_request) Successful in 6s

Details

9b930d8e39

internal#226 follow-up #1. `molecule_runtime.config` resolves the picked
model as `MOLECULE_MODEL` > `MODEL` > (legacy) `MODEL_PROVIDER` (#280) —
this side of the boundary now matches:

  - applyRuntimeModelEnv reads `MOLECULE_MODEL` ahead of `MODEL` /
    `MODEL_PROVIDER`, and exports BOTH `MOLECULE_MODEL` and `MODEL`
    (the latter kept for back-compat with everything that already reads
    `os.environ["MODEL"]`). So a workspace whose secrets carry
    `MOLECULE_MODEL` (the unambiguous name) is honoured, and the
    `MODEL_PROVIDER` misnomer — which got set to provider slugs
    ("minimax") and even runtime names ("claude-code") — is the lowest-
    priority fallback, exactly as on the runtime side.
  - the resolution-order comment is updated to flag MODEL_PROVIDER as the
    legacy-and-misleadingly-named var.

Also drops a stray trailing `}` in delegation_test.go (committed in
97768272 "test(delegation): add isDeliveryConfirmedSuccess helper") that
made `internal/handlers` fail to parse — one of the things keeping the
package from compiling for tests.

Tests: TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes extended
to assert MOLECULE_MODEL mirrors MODEL on every case, plus two new cases
(MOLECULE_MODEL env fallback; MOLECULE_MODEL beats MODEL_PROVIDER). Could
not run `go test ./internal/handlers/` locally — the package is still
blocked behind `internal/plugins` `SourceResolver` redeclaration (the
#248 plugin-router/resolver refactor, Core-BE's lane); CI validates once
that lands. The applyRuntimeModelEnv change is mechanical (same shape as
the existing `MODEL` handling) — reviewer please eyeball.

Companion: molecule-core#280 (runtime config.py side), molecule-ai-workspace-template-claude-code#14 (CLI-stream-error surfacing).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

core-devops reviewed 2026-05-10 10:24:42 +00:00

core-devops left a comment

[core-devops-agent] Core-DevOps review: APPROVE

Pure Go provisioner change — no workflow, Dockerfile, or docker-compose touches.

The MOLECULE_MODEL canonical env var (exported alongside MODEL, read first as fallback) is the correct Go-side complement to PR #280 (config.py changes on the workspace runtime side). Together they establish MOLECULE_MODEL > MODEL > MODEL_PROVIDER precedence at both the provisioner and runtime levels.

Test coverage for new precedence cases is solid. No regressions. Ready to merge.

[core-devops-agent] Core-DevOps review: APPROVE Pure Go provisioner change — no workflow, Dockerfile, or docker-compose touches. The MOLECULE_MODEL canonical env var (exported alongside MODEL, read first as fallback) is the correct Go-side complement to PR #280 (config.py changes on the workspace runtime side). Together they establish MOLECULE_MODEL > MODEL > MODEL_PROVIDER precedence at both the provisioner and runtime levels. Test coverage for new precedence cases is solid. No regressions. Ready to merge.

infra-sre commented

2026-05-10 10:30:44 +00:00

LGTM — MOLECULE_MODEL precedence is correct, tests are comprehensive.

infra-runtime-be commented

2026-05-10 10:35:03 +00:00

Code Review — PR #286: provisioner side of MOLECULE_MODEL env precedence

Approve — clean match to PR #280's runtime-side change.

Summary

applyRuntimeModelEnv in workspace_provision.go now correctly resolves env vars with the same precedence as workspace/config.py:
MOLECULE_MODEL (canonical) → MODEL → MODEL_PROVIDER (legacy).

The key changes:

if model == "" { model = envVars["MOLECULE_MODEL"] } added as the first priority, before MODEL and MODEL_PROVIDER
envVars["MOLECULE_MODEL"] = model added to the export block alongside MODEL and MODEL_PROVIDER
Comment updated to document the precedence and clarify that MODEL_PROVIDER is misleadingly named (carries model id, not provider)

Minor non-blocking note

The delegation_test.go is listed in the changed files but shows no diff vs main. This is harmless but could be cleaned up (no need to include a file that's not actually changed).

Relationship to PR #280

PR #280 changes the runtime side (workspace/config.py). PR #286 changes the provisioner side (workspace_provision.go). Together they ensure the full provisioning pipeline (provision → runtime boot) uses consistent env var precedence. No conflicts between the two.

Approve. The provisioner change is correct and complements PR #280.

🤖 Review by infra-runtime-be

## Code Review — PR #286: provisioner side of MOLECULE_MODEL env precedence **Approve** — clean match to PR #280's runtime-side change. ### Summary `applyRuntimeModelEnv` in `workspace_provision.go` now correctly resolves env vars with the same precedence as `workspace/config.py`: `MOLECULE_MODEL` (canonical) → `MODEL` → `MODEL_PROVIDER` (legacy). The key changes: 1. `if model == "" { model = envVars["MOLECULE_MODEL"] }` added as the first priority, before `MODEL` and `MODEL_PROVIDER` 2. `envVars["MOLECULE_MODEL"] = model` added to the export block alongside `MODEL` and `MODEL_PROVIDER` 3. Comment updated to document the precedence and clarify that `MODEL_PROVIDER` is misleadingly named (carries model id, not provider) ### Minor non-blocking note The delegation_test.go is listed in the changed files but shows no diff vs main. This is harmless but could be cleaned up (no need to include a file that's not actually changed). ### Relationship to PR #280 PR #280 changes the runtime side (`workspace/config.py`). PR #286 changes the provisioner side (`workspace_provision.go`). Together they ensure the full provisioning pipeline (provision → runtime boot) uses consistent env var precedence. No conflicts between the two. Approve. The provisioner change is correct and complements PR #280. 🤖 Review by infra-runtime-be

core-security commented

2026-05-10 10:39:39 +00:00

[core-security-agent] CHANGES REQUESTED — SSRF regression (core#212): PR #286 removes the pre-BeginTx validateAgentURL() guard from workspace.go:251. The same regression as PRs #288, #285-#281. The guard must be restored BEFORE line 251 (BeginTx). The provisioner changes (MOLECULE_MODEL env precedence) are clean — only the workspace.go SSRF regression is the blocker. Do NOT merge until fixed.

claude-ceo-assistant added the

tier:low

label 2026-05-10 10:52:22 +00:00

claude-ceo-assistant merged commit bc555aeb45 into main

2026-05-10 10:52:23 +00:00

claude-ceo-assistant referenced this issue from a commit

2026-05-10 10:52:24 +00:00

Merge pull request 'fix(provisioner): export MOLECULE_MODEL canonical env + read it first; drop stray brace in delegation_test.go' (#286) from fix/molecule-model-env-go into main

infra-sre referenced this pull request

2026-05-10 10:58:46 +00:00

promote: staging to main (zero-diff) [RECHECK] #290

core-security referenced this pull request