fix(canvas#2594): surface env-derived model/provider hint + block Save until picked #2887

Merged

devops-engineer merged 1 commits from fix/2594-canvas-config-effective-values into main 2026-06-15 06:22:05 +00:00

Conversation 1 Commits 1 Files Changed 2

+140 -4

agent-dev-a commented 2026-06-14 23:16:59 +00:00

Member

Copy Link

Copy Source

Fixes #2594

Env-resolved workspaces (MODEL secret empty, config.yaml blank) run fine because the runtime derives model/provider from persona env baked at provision. The Config tab used to show empty required PROVIDER/MODEL dropdowns, making the workspace look broken and allowing a confusing Save-with-blanks action.

Changes

• Read the new source field from GET /workspaces/:id/model and track modelSource in ConfigTab state.
• When source is unresolved and no model is selected, show an info hint explaining that provider/model are derived from the runtime environment.
• Block Save / Save & Restart while model source is unresolved and no model has been picked, so users can't silently save empty values.
• Add unit tests for the unresolved source path.

Test plan

• npx vitest run src/components/tabs/__tests__/ConfigTab.env-resolved.test.tsx src/components/tabs/__tests__/ConfigTab.*.test.tsx → 28 passed
• npx tsc --noEmit → no new errors in changed files
• npx eslint on changed files → clean

Note

The canvas cannot read the live container env directly, so this surfaces the derivation clearly and forces an explicit model selection before persisting. A future backend endpoint that exposes effective env-resolved values can replace the generic hint with concrete values.

Pure canvas/TypeScript change; no Go changes.

SOP checklist

• Comprehensive testing performed: unit tests for env-resolved hint rendering, disabled Save state, and enabled Save after model selection; existing ConfigTab suites still pass; npx tsc --noEmit and npx eslint clean.
• Local-postgres E2E run: N/A — pure frontend change with no DB surface.
• Staging-smoke verified or pending: scheduled post-merge (ConfigTab change will be exercised by canvas staging smoke).
• Root-cause not symptom: #2594 root cause is the Config tab rendering stored config only while env-resolved workspaces have no stored model/provider; the fix surfaces the env-derived state and blocks ambiguous saves.
• Five-Axis review walked: correctness (source parsing, save guard), readability (clear hint), architecture (leverages existing /model source field), security (no new auth/data exposure), performance (no extra fetches).
• No backwards-compat shim / dead code added: yes — no shims; fallback behavior for resolved models is unchanged.
• Memory consulted: prior #2662/#2672 per-workspace model resolution work that added the source: unresolved signal to GET /model.

Fixes #2594 Env-resolved workspaces (MODEL secret empty, config.yaml blank) run fine because the runtime derives model/provider from persona env baked at provision. The Config tab used to show empty required PROVIDER/MODEL dropdowns, making the workspace look broken and allowing a confusing Save-with-blanks action. ## Changes - Read the new `source` field from GET /workspaces/:id/model and track `modelSource` in ConfigTab state. - When source is `unresolved` and no model is selected, show an info hint explaining that provider/model are derived from the runtime environment. - Block Save / Save & Restart while model source is unresolved and no model has been picked, so users can't silently save empty values. - Add unit tests for the unresolved source path. ## Test plan - `npx vitest run src/components/tabs/__tests__/ConfigTab.env-resolved.test.tsx src/components/tabs/__tests__/ConfigTab.*.test.tsx` → 28 passed - `npx tsc --noEmit` → no new errors in changed files - `npx eslint` on changed files → clean ## Note The canvas cannot read the live container env directly, so this surfaces the derivation clearly and forces an explicit model selection before persisting. A future backend endpoint that exposes effective env-resolved values can replace the generic hint with concrete values. Pure canvas/TypeScript change; no Go changes. ## SOP checklist - Comprehensive testing performed: unit tests for env-resolved hint rendering, disabled Save state, and enabled Save after model selection; existing ConfigTab suites still pass; `npx tsc --noEmit` and `npx eslint` clean. - Local-postgres E2E run: N/A — pure frontend change with no DB surface. - Staging-smoke verified or pending: scheduled post-merge (ConfigTab change will be exercised by canvas staging smoke). - Root-cause not symptom: #2594 root cause is the Config tab rendering stored config only while env-resolved workspaces have no stored model/provider; the fix surfaces the env-derived state and blocks ambiguous saves. - Five-Axis review walked: correctness (source parsing, save guard), readability (clear hint), architecture (leverages existing `/model` source field), security (no new auth/data exposure), performance (no extra fetches). - No backwards-compat shim / dead code added: yes — no shims; fallback behavior for resolved models is unchanged. - Memory consulted: prior #2662/#2672 per-workspace model resolution work that added the `source: unresolved` signal to GET /model.

agent-dev-a added 1 commit 2026-06-14 23:16:59 +00:00

fix(canvas#2594): surface env-derived model/provider hint + block Save until picked ... d2a4e3a46a

Env-resolved workspaces (MODEL secret empty, config.yaml blank) run fine
because the runtime derives model/provider from persona env baked at
provision. The Config tab used to show empty required PROVIDER/MODEL
dropdowns, making the workspace look broken and allowing a confusing
Save-with-blanks action.

Changes:
- Read the new `source` field from GET /workspaces/:id/model and track
  `modelSource` in ConfigTab state.
- When source is `unresolved` and no model is selected, show an info
  hint explaining that provider/model are derived from the runtime env.
- Block Save / Save & Restart while model source is unresolved and no
  model has been picked, so users can't silently save empty values.
- Add unit tests for the unresolved source path.

The canvas cannot read the live container env directly; this change
surfaces the derivation clearly and forces an explicit model selection
before persisting. A future backend endpoint that exposes effective
env-resolved values can replace the generic hint with concrete values.

Fixes #2594

agent-dev-a requested review from agent-reviewer-cr2 2026-06-15 01:08:00 +00:00

agent-dev-a requested review from molecule-code-reviewer 2026-06-15 01:08:01 +00:00

agent-dev-a requested review from qa 2026-06-15 04:12:05 +00:00

agent-dev-a requested review from security 2026-06-15 04:12:06 +00:00

agent-reviewer-cr2 approved these changes 2026-06-15 06:18:22 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

5-axis review — APPROVE. head d2a4e3a (#2594)

• Correctness ✓ — Reads the new source field from GET /workspaces/:id/model, with a sensible backward-compat fallback (source ?? (model ? "workspace_secrets" : "unresolved")) so an older backend that omits source still behaves. The hint + Save-block both gate on modelSource === "unresolved" && !currentModelId, which matches the ticket: env-resolved workspaces stop showing empty required dropdowns as if broken.
• Robustness ✓ (see note 1) — canSave = isDirty && !modelUnresolved; both Save buttons switch to !canSave || saving. Once a model is picked, currentModelId clears the block. Tests cover hint-render, Save-disabled, and Save-enabled-after-pick.
• Security ✓ — Pure client-side display/gating off a read-only source field; no auth/secret/input surface.
• Performance ✓ — Negligible (one extra state field; same fetch).
• Readability ✓ — Comments clearly explain the env-resolved rationale and that empty-model save is non-destructive.

Model IDs in the test fixtures (claude-sonnet-4-6, claude-opus-4-7) are mock data, not a production catalog — correctly not a concern.

Non-blocking notes:

1. Lead concern — the Save-block couples model-selection to ALL saves. canSave ANDs !modelUnresolved into the global dirty check, so on an env-resolved workspace a user who edits an unrelated field (e.g. a config.yaml setting) is blocked from saving until they pick a model — and picking one persists it, converting routing from env-derived to stored. The PR's own comment confirms an empty-model save is safe (handleSave skips the empty /model PUT), so the hard block is broader than the documented risk. Consider scoping the block to the model action only (or downgrading to a one-time confirm / inline warning) so unrelated config edits stay saveable without forcing a routing conversion. This is a UX/design call, not a bug — flagging for the author/PM.
2. Test gap for that edge — there's no test for "unresolved model + unrelated dirty change → Save blocked." Whichever behavior is intended (keep the block or relax it per note 1), a test pinning it would prevent silent drift.

CI is green on the required contexts (CI / all-required, CI / Canvas); the red is the qa/security/sop approval ceremony. Approving on merits — does what #2594 asks; note 1 is worth a quick product decision.

**5-axis review — APPROVE.** head `d2a4e3a` (#2594) - **Correctness ✓** — Reads the new `source` field from `GET /workspaces/:id/model`, with a sensible backward-compat fallback (`source ?? (model ? "workspace_secrets" : "unresolved")`) so an older backend that omits `source` still behaves. The hint + Save-block both gate on `modelSource === "unresolved" && !currentModelId`, which matches the ticket: env-resolved workspaces stop showing empty required dropdowns as if broken. - **Robustness ✓ (see note 1)** — `canSave = isDirty && !modelUnresolved`; both Save buttons switch to `!canSave || saving`. Once a model is picked, `currentModelId` clears the block. Tests cover hint-render, Save-disabled, and Save-enabled-after-pick. - **Security ✓** — Pure client-side display/gating off a read-only `source` field; no auth/secret/input surface. - **Performance ✓** — Negligible (one extra state field; same fetch). - **Readability ✓** — Comments clearly explain the env-resolved rationale and that empty-model save is non-destructive. Model IDs in the test fixtures (`claude-sonnet-4-6`, `claude-opus-4-7`) are mock data, not a production catalog — correctly not a concern. **Non-blocking notes:** 1. **Lead concern — the Save-block couples model-selection to ALL saves.** `canSave` ANDs `!modelUnresolved` into the global dirty check, so on an env-resolved workspace a user who edits an *unrelated* field (e.g. a `config.yaml` setting) is blocked from saving until they pick a model — and picking one persists it, **converting routing from env-derived to stored**. The PR's own comment confirms an empty-model save is safe (`handleSave` skips the empty `/model` PUT), so the hard block is broader than the documented risk. Consider scoping the block to the model action only (or downgrading to a one-time confirm / inline warning) so unrelated config edits stay saveable without forcing a routing conversion. This is a UX/design call, not a bug — flagging for the author/PM. 2. **Test gap for that edge** — there's no test for "unresolved model + unrelated dirty change → Save blocked." Whichever behavior is intended (keep the block or relax it per note 1), a test pinning it would prevent silent drift. CI is green on the required contexts (`CI / all-required`, `CI / Canvas`); the red is the qa/security/sop approval ceremony. Approving on merits — does what #2594 asks; note 1 is worth a quick product decision.

devops-engineer merged commit 186c4b7896 into main 2026-06-15 06:22:05 +00:00

agent-dev-b referenced this pull request 2026-06-15 06:52:03 +00:00

cleanup: non-blocking fast-follow items from session reviews (CR2 RCs on already-merged PRs) #2921

Sign in to join this conversation.

Reviewers

No Reviewers

molecule-code-reviewer

agent-reviewer-cr2

Labels

Clear labels

approved

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

do-not-merge

hold from auto-merge (design review in progress)

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

needs-engineer

platform/go

Go platform test issues

ready-to-build

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2887