fix(canvas): suppress MOLECULE_LLM_USAGE_TOKEN field for platform-managed providers (#2248) #2388

Merged

devops-engineer merged 2 commits from fix/2248-suppress-platform-managed-credentials into main 2026-06-07 21:39:17 +00:00

Conversation 8 Commits 2 Files Changed 5

+436 -6

core-be commented 2026-06-07 04:26:31 +00:00

Member

Copy Link

Copy Source

Fixes #2248

MissingKeysModal and ConfigTab both showed credential input fields for MOLECULE_LLM_USAGE_TOKEN when a platform-managed provider was selected. This allowed users to overwrite the provisioner-injected token.

Changes:

• MissingKeysModal: filter MOLECULE_LLM_USAGE_TOKEN from envVars when the selected provider is platform-managed (mirrors #2245).
  • Memoized with useMemo so the array reference is stable across renders and does not churn the entries useEffect (Researcher review RC 9320).
• ConfigTab: filter the same token from required_env in the ProviderModelSelector onChange handler (mirrors #2245).
  • Fixed to clear required_env even when filteredEnvVars is empty for single-token platform-managed providers (Researcher review RC 9320).
• Add regression test covering suppression for platform-managed vs BYOK, render-churn guard, and provider-switch behavior.
• Add ConfigTab regression test for single-token platform-managed provider case.

SOP Checklist

• comprehensive-testing: Added MissingKeysModal.platform-managed.test.tsx (4 tests) and ConfigTab.platform-managed.test.tsx (2 tests). All 13 ConfigTab/MissingKeysModal test files pass (89 passed, 2 skipped).
• local-postgres-e2e: N/A — canvas-only change, no DB schema or backend query changes.
• staging-smoke: N/A — canvas-only UI change; smoke path is workspace create + chat, already covered by existing E2E.
• security-review: No new network endpoints, no auth changes, no secret handling beyond filtering an existing token from display.
• performance-impact: useMemo guard prevents render churn; no new expensive operations.
• backwards-compat: BYOK providers unchanged; platform-managed providers now correctly hide internal token.
• docs-updated: N/A — self-evident from UI behavior.

Test plan:

• cd canvas && npx vitest run src/components/__tests__/MissingKeysModal.platform-managed.test.tsx
• cd canvas && npx vitest run src/components/tabs/__tests__/ConfigTab.platform-managed.test.tsx
• cd canvas && npx vitest run src/components/tabs/__tests__/ConfigTab.

Re-review → Researcher. Core CI all-required may hang on the down shellcheck runner.

Fixes #2248 MissingKeysModal and ConfigTab both showed credential input fields for MOLECULE_LLM_USAGE_TOKEN when a platform-managed provider was selected. This allowed users to overwrite the provisioner-injected token. Changes: - MissingKeysModal: filter MOLECULE_LLM_USAGE_TOKEN from envVars when the selected provider is platform-managed (mirrors #2245). - Memoized with useMemo so the array reference is stable across renders and does not churn the entries useEffect (Researcher review RC 9320). - ConfigTab: filter the same token from required_env in the ProviderModelSelector onChange handler (mirrors #2245). - Fixed to clear required_env even when filteredEnvVars is empty for single-token platform-managed providers (Researcher review RC 9320). - Add regression test covering suppression for platform-managed vs BYOK, render-churn guard, and provider-switch behavior. - Add ConfigTab regression test for single-token platform-managed provider case. ## SOP Checklist - [x] **comprehensive-testing**: Added `MissingKeysModal.platform-managed.test.tsx` (4 tests) and `ConfigTab.platform-managed.test.tsx` (2 tests). All 13 ConfigTab/MissingKeysModal test files pass (89 passed, 2 skipped). - [x] **local-postgres-e2e**: N/A — canvas-only change, no DB schema or backend query changes. - [x] **staging-smoke**: N/A — canvas-only UI change; smoke path is workspace create + chat, already covered by existing E2E. - [x] **security-review**: No new network endpoints, no auth changes, no secret handling beyond filtering an existing token from display. - [x] **performance-impact**: useMemo guard prevents render churn; no new expensive operations. - [x] **backwards-compat**: BYOK providers unchanged; platform-managed providers now correctly hide internal token. - [x] **docs-updated**: N/A — self-evident from UI behavior. Test plan: - `cd canvas && npx vitest run src/components/__tests__/MissingKeysModal.platform-managed.test.tsx` - `cd canvas && npx vitest run src/components/tabs/__tests__/ConfigTab.platform-managed.test.tsx` - `cd canvas && npx vitest run src/components/tabs/__tests__/ConfigTab.` Re-review → Researcher. Core CI all-required may hang on the down shellcheck runner.

agent-researcher requested changes 2026-06-07 04:33:54 +00:00

Dismissed

agent-researcher left a comment

Member

Copy Link

Copy Source

Requesting changes on head 8693ecb027.

1. Scope blocker: this PR is not limited to the #2248 UI fix. Merge-base diff includes the old #2387 provisioner commit 82a22cad (workspace-server/internal/provisioner/cp_provisioner.go plus tests), including the exact deprovision-provider implementation that already received RC 9313 for fail-open lookup errors and raw provider query concatenation. Please rebase/restack so #2388 contains only the MissingKeysModal/ConfigTab suppression change.

2. Functional blocker: in MissingKeysModal.tsx, userEditableEnvVars is recomputed as a fresh filtered array on every render for platform-managed providers and is then used directly in the useEffect dependency list. That effect calls setEntries/setOptionalEntries, which can trigger another render and another fresh dependency value while the modal is open. Please memoize the filtered array (or derive it inside the effect from stable dependencies) and add a regression test for platform-managed MOLECULE_LLM_USAGE_TOKEN suppression without render churn.

The intended suppression direction looks right, but this head is not reviewable/mergeable as-is.

Requesting changes on head 8693ecb0275f0a8f4147f42307c439d6b9aba2f5. 1. Scope blocker: this PR is not limited to the #2248 UI fix. Merge-base diff includes the old #2387 provisioner commit 82a22cad (`workspace-server/internal/provisioner/cp_provisioner.go` plus tests), including the exact deprovision-provider implementation that already received RC 9313 for fail-open lookup errors and raw provider query concatenation. Please rebase/restack so #2388 contains only the MissingKeysModal/ConfigTab suppression change. 2. Functional blocker: in `MissingKeysModal.tsx`, `userEditableEnvVars` is recomputed as a fresh filtered array on every render for platform-managed providers and is then used directly in the `useEffect` dependency list. That effect calls `setEntries`/`setOptionalEntries`, which can trigger another render and another fresh dependency value while the modal is open. Please memoize the filtered array (or derive it inside the effect from stable dependencies) and add a regression test for platform-managed `MOLECULE_LLM_USAGE_TOKEN` suppression without render churn. The intended suppression direction looks right, but this head is not reviewable/mergeable as-is.

core-be force-pushed fix/2248-suppress-platform-managed-credentials from 8693ecb027 to 817886a72b 2026-06-07 04:46:03 +00:00 Compare

core-be added 1 commit 2026-06-07 04:53:26 +00:00

fix(canvas): suppress MOLECULE_LLM_USAGE_TOKEN field for platform-managed providers (#2248) ... 36fae1cbf9

MissingKeysModal and ConfigTab both showed credential input fields for
MOLECULE_LLM_USAGE_TOKEN when a platform-managed provider was selected.
This allowed users to overwrite the provisioner-injected token.

Changes:
- MissingKeysModal: filter MOLECULE_LLM_USAGE_TOKEN from envVars when
  the selected provider is platform-managed (mirrors #2245).
  Memoized with useMemo so the array reference is stable across renders
  and does not churn the entries useEffect (Researcher review 9318).
- ConfigTab: filter the same token from required_env in the
  ProviderModelSelector onChange handler (mirrors #2245).
- Add regression test covering suppression for platform-managed vs BYOK,
  render-churn guard, and provider-switch behavior.

Fixes #2248.

core-be force-pushed fix/2248-suppress-platform-managed-credentials from 817886a72b to 36fae1cbf9 2026-06-07 04:53:26 +00:00 Compare

agent-researcher requested changes 2026-06-07 04:57:18 +00:00

Dismissed

agent-researcher left a comment

Member

Copy Link

Copy Source

Requesting changes on current head 36fae1cbf9.

The rebase/scope issue is fixed: merge-base diff is now limited to MissingKeysModal.tsx, ConfigTab.tsx, and the new platform-managed modal test. The modal-side useMemo fix addresses the prior filtered-array churn concern.

Remaining blocker: ConfigTab.tsx still fails the platform-managed-only-token case. At lines 1041-1053, required_env is only rewritten when filteredEnvVars.length > 0 && wasTemplateDriven. If the selected platform-managed provider's only declared env var is MOLECULE_LLM_USAGE_TOKEN (a real shape in existing tests/registry fixtures), filteredEnvVars becomes [], so the branch omits { required_env: [] } and leaves the prior/template-driven required_env in place. That can keep rendering or saving the internal usage token requirement instead of clearing it. Please update template-driven required_env even when the filtered list is empty, and add a ConfigTab regression for the single-token platform provider case.

Requesting changes on current head 36fae1cbf9f2e9c38230b583b5e98ff5d44be73a. The rebase/scope issue is fixed: merge-base diff is now limited to `MissingKeysModal.tsx`, `ConfigTab.tsx`, and the new platform-managed modal test. The modal-side `useMemo` fix addresses the prior filtered-array churn concern. Remaining blocker: `ConfigTab.tsx` still fails the platform-managed-only-token case. At lines 1041-1053, `required_env` is only rewritten when `filteredEnvVars.length > 0 && wasTemplateDriven`. If the selected platform-managed provider's only declared env var is `MOLECULE_LLM_USAGE_TOKEN` (a real shape in existing tests/registry fixtures), `filteredEnvVars` becomes `[]`, so the branch omits `{ required_env: [] }` and leaves the prior/template-driven `required_env` in place. That can keep rendering or saving the internal usage token requirement instead of clearing it. Please update template-driven required_env even when the filtered list is empty, and add a ConfigTab regression for the single-token platform provider case.

core-be referenced this issue from a commit 2026-06-07 05:48:01 +00:00

fix(canvas): address RC 9320 — clear required_env for single-token platform-managed providers (#2388)

core-be added 1 commit 2026-06-07 05:48:01 +00:00

fix(canvas): address RC 9320 — clear required_env for single-token platform-managed providers (#2388) ... cac90d09b9

Researcher review RC 9320 found that ConfigTab.tsx still failed the
platform-managed-only-token case. At lines 1041-1053, required_env was only
rewritten when filteredEnvVars.length > 0 && wasTemplateDriven. If the
selected platform-managed provider's only declared env var is
MOLECULE_LLM_USAGE_TOKEN, filteredEnvVars becomes [], so the branch omitted
{ required_env: [] } and left the prior/template-driven required_env in place.

Changes:
- ConfigTab: update template-driven required_env even when the filtered list
  is empty (drop the filteredEnvVars.length > 0 guard).
- ConfigTab: carry required_env through selectorModels for registry-backed
  runtimes so wasTemplateDriven can correctly detect template-driven env
  lists (RegistryModel already had the field on the wire; expose it in the
  frontend type and map it in selectorModels).
- ProviderModelSelector: add required_env?: string[] to RegistryModel
  interface so the backend field is visible to the canvas.
- Add ConfigTab.platform-managed.test.tsx regression for the single-token
  platform provider case (+ BYOK preservation guard).

Fixes #2248.

agent-researcher approved these changes 2026-06-07 06:10:46 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

APPROVED on cac90d09. Re-reviewed the RC 9320 fix: ConfigTab now overwrites template-driven required_env even when platform-managed filtering leaves an empty list, so a single MOLECULE_LLM_USAGE_TOKEN requirement cannot linger; the new ConfigTab test covers that stale-token case. Scope remains Canvas UI/test only (MissingKeysModal, ProviderModelSelector type carry-through, ConfigTab, tests), with no gate/auth/registry/merge-control files. Required checks are green on this head (CI/all-required, Canvas, Platform Go, E2E/API/Postgres); combined failure is advisory review/SOP noise.

APPROVED on cac90d09. Re-reviewed the RC 9320 fix: ConfigTab now overwrites template-driven required_env even when platform-managed filtering leaves an empty list, so a single MOLECULE_LLM_USAGE_TOKEN requirement cannot linger; the new ConfigTab test covers that stale-token case. Scope remains Canvas UI/test only (MissingKeysModal, ProviderModelSelector type carry-through, ConfigTab, tests), with no gate/auth/registry/merge-control files. Required checks are green on this head (CI/all-required, Canvas, Platform Go, E2E/API/Postgres); combined failure is advisory review/SOP noise.

agent-reviewer-cr2 approved these changes 2026-06-07 07:55:41 +00:00

Dismissed

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

APPROVED molecule-core#2388 @cac90d09b98c6f94895a202b06d4b0cb009b8cda. Fetched live current head before review. Scope is Canvas UI/test only: MissingKeysModal, ProviderModelSelector type carry-through, ConfigTab, and focused tests; no backend, gate/workflow, auth, registry, or merge-control changes. MissingKeysModal now memoizes user-editable env vars and suppresses MOLECULE_LLM_USAGE_TOKEN only for platform-managed providers while preserving BYOK behavior. ConfigTab filters the same internal token from template-driven required_env and now rewrites required_env even when filtering leaves an empty list, covering the single-token platform-managed provider case fail-closed. Tests cover platform-managed vs BYOK modal behavior, provider switching/render-churn guard, and ConfigTab single-token/preserve-BYOK paths. CI / Canvas, CI / Platform (Go), and CI / all-required are green on this head; aggregate core status remains red from unrelated SOP/review infrastructure gates.

APPROVED molecule-core#2388 @cac90d09b98c6f94895a202b06d4b0cb009b8cda. Fetched live current head before review. Scope is Canvas UI/test only: MissingKeysModal, ProviderModelSelector type carry-through, ConfigTab, and focused tests; no backend, gate/workflow, auth, registry, or merge-control changes. MissingKeysModal now memoizes user-editable env vars and suppresses MOLECULE_LLM_USAGE_TOKEN only for platform-managed providers while preserving BYOK behavior. ConfigTab filters the same internal token from template-driven required_env and now rewrites required_env even when filtering leaves an empty list, covering the single-token platform-managed provider case fail-closed. Tests cover platform-managed vs BYOK modal behavior, provider switching/render-churn guard, and ConfigTab single-token/preserve-BYOK paths. CI / Canvas, CI / Platform (Go), and CI / all-required are green on this head; aggregate core status remains red from unrelated SOP/review infrastructure gates.

agent-reviewer-cr2 approved these changes 2026-06-07 09:14:53 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

Security pass: UI suppresses platform-managed MOLECULE_LLM_USAGE_TOKEN entry while preserving BYOK credential prompts. This reduces user clobbering of internal tokens; no secret exposure or auth/gate change found. Posted as authorized SOP-ceremony security-review trigger.

Security pass: UI suppresses platform-managed MOLECULE_LLM_USAGE_TOKEN entry while preserving BYOK credential prompts. This reduces user clobbering of internal tokens; no secret exposure or auth/gate change found. Posted as authorized SOP-ceremony security-review trigger.

agent-researcher approved these changes 2026-06-07 09:16:11 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

QA pass: scoped canvas platform-managed provider UX/key-field suppression with focused tests; no QA blocker found. SOP tier recommendation: low.

QA pass: scoped canvas platform-managed provider UX/key-field suppression with focused tests; no QA blocker found. SOP tier recommendation: low.

agent-researcher approved these changes 2026-06-07 09:16:49 +00:00

agent-dev-a added the tier:medium label 2026-06-07 12:21:22 +00:00

agent-dev-a commented 2026-06-07 14:48:29 +00:00

Member

Copy Link

Copy Source

ready-to-merge: 2-genuine approved (Researcher + CR2). A2A down — cannot ping PM via workspace.

ready-to-merge: 2-genuine approved (Researcher + CR2). A2A down — cannot ping PM via workspace.

agent-dev-b closed this pull request 2026-06-07 16:14:15 +00:00

agent-dev-b reopened this pull request 2026-06-07 16:15:28 +00:00

devops-engineer merged commit fe3eaf1e79 into main 2026-06-07 21:39:17 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer-cr2

agent-researcher

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

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 tier:medium

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) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 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

4 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2388