feat(canvas): LLM Billing section in Config tab (internal#691) #1928

Merged

core-lead merged 1 commits from feat/canvas-llm-billing-mode-section into main 2026-05-26 22:57:33 +00:00

Conversation 4 Commits 1 Files Changed 4

+399

hongming commented 2026-05-26 21:29:54 +00:00

Owner

Copy Link

Copy Source

Summary

Canvas Config-tab UI for the per-workspace llm_billing_mode override (internal#691). Adds a new LLMBillingSection mounted above SecretsSection in ConfigTab.tsx that:

• Shows the resolved mode for the workspace + the source ("inherited from org default" / "explicit override on this workspace" / fallback)
• Shows the org default so the user can see what they're inheriting
• Exposes a dropdown with 4 choices: Inherit org default (clears override) / Platform-managed / BYOK / Disabled
• Surfaces a yellow warning banner when the persisted override is garbled (some external write put "byokk" in the column)
• Updates the UI from the PUT response without a follow-up GET (the per-tenant route returns the post-write resolution)

Deploy order — this PR ships THIRD:

1. molecule-controlplane PR — CP admin proxy routes. First.
2. molecule-core workspace-server PR — migration + resolver + strip-gate. Second.
3. THIS PR (molecule-core canvas) — Config-tab UI. Last.

Until the workspace-server PR lands, the section renders the error banner with the network error (the per-tenant route doesn't exist yet). Cosmetic-only behavior — no broken workflows.

Why the canvas calls the per-tenant route directly (not the CP proxy)

The canvas is hosted on the same origin as the per-tenant workspace-server (canvas is served by the platform container's reverse proxy). Calling the per-tenant route is one HTTP hop; calling CP would be canvas → CP → tenant, two hops. The CP proxy exists for ops + CTO scripts that hold CP bearer auth and don't want to juggle per-tenant admin_token values.

Test plan

• npx vitest run src/components/tabs/config/__tests__/llm-billing-section.test.tsx — 5 tests, all pass
  • Renders resolved mode + source on inherit
  • PUTs {mode: "byok"} and reflects new resolution in UI
  • PUTs {mode: null} when user picks Inherit
  • Yellow warning banner on garbled override
  • Error banner on GET failure
• npx vitest run (full canvas suite) — 220 files, 3369 tests pass, 1 skipped (pre-existing)
• npx tsc --noEmit — clean on new files (pre-existing main-branch TS errors in Attachment*/KeyValueField tests are not introduced here)
• npx eslint src/components/tabs/config/llm-billing-section.tsx ...test.tsx — clean
• Stage B (staging) — visual smoke after workspace-server PR deploys to staging
• Stage C (prod) — CTO drives the Production Manager workspace flip via the UI per RFC

Five-Axis self-review

Correctness: Dropdown current-choice is derived from workspace_override (raw value), NOT resolved_mode — otherwise "explicit platform_managed override" and "inherit while org happens to be platform_managed" would conflate, and clicking Save on the second case would write an explicit override when the user expected no change.

Safety/Security: Same-origin admin route + existing X-Molecule-Org-Slug + Bearer pair from platformAuthHeaders(). No new auth surface. No secrets in the response body — the per-tenant route returns enum strings only.

Test coverage: 5 tests covering happy GET, BYOK PUT, Inherit-clears PUT, garbled-override banner, error banner. Not exhaustive on every UI state (loading / saving disabled state) but the load-saving spinner is covered by the disabled-during-save assertion in the BYOK test path.

Observability: The UI surfaces source directly to the user ("inherited from org default" vs "explicit override on this workspace") so a confused operator can answer "why is this workspace being stripped" without DB or log archeology — the RFC Observability hot-spot is partially answered here for the human-facing channel, and fully answered by MOLECULE_LLM_BILLING_MODE_RESOLVED + the strip-gate log line on the workspace-server side.

Backward-compat: Net-new component, no existing component touched beyond a one-line import + one-line mount in ConfigTab. Until workspace-server lands the section just shows the error banner — graceful degradation. The TS interface is a literal struct (no Go-type bridge needed; canvas has no build dependency on the core repo's Go modules).

CTO attention items

If you decide to flip the architectural fork to Option B (override on CP), the dropdown URL needs to switch from /admin/workspaces/... to /cp/admin/workspaces/...?org_slug=... — a 5-line diff. Not gating this PR; flagging in case the workspace-server PR's review changes the storage location.

🤖 Generated with Claude Code

---

SOP-Checklist

• Comprehensive testing performed: 5 new tests in llm-billing-section.test.tsx (renders resolved-mode + source on inherit, PUTs {mode: "byok"} and reflects the post-write resolution, PUTs {mode: null} on Inherit, yellow warning banner on garbled override, error banner on GET failure). Full canvas suite re-run: 220 files, 3369 tests pass, 1 skipped (pre-existing). The save-disabled spinner state is asserted inline in the BYOK PUT test path.
• Local-postgres E2E run: N/A — pure-frontend change (Next.js canvas component). No DB surface, no Go handlers, no migration. The component talks to the workspace-server admin route over HTTP via existing platformAuthHeaders().
• Staging-smoke verified or pending: Pending — visual smoke after workspace-server PR (#1927) deploys to staging; CTO drives Stage B per dev-sop.md. The component shows the error banner on staging until #1927 lands (the per-tenant route 404s); cosmetic-only behaviour, documented in PR body.
• Root-cause not symptom: The canvas exposes the per-workspace knob the workspace-server PR creates. Without UI, the override is operator-only via curl — surfacing it as a Config-tab section gives the team self-service for the same root cause #1927 fixes (agents-team prod block: one knob per org, not per workspace).
• Five-Axis review walked: Correctness / Safety/Security / Test coverage / Observability / Backward-compat — see "Five-Axis self-review" section above.
• No backwards-compat shim / dead code added: Yes — net-new component, no existing component touched beyond a one-line import + one-line mount in ConfigTab.tsx. No feature flag, no parallel path, no compat shim. Until #1927 deploys, the section renders the error banner — graceful degradation, not a shim.
• Memory/saved-feedback consulted: feedback_principles, feedback_no_silent_checklist_trim (filing every axis), feedback_check_for_parallel_work_before_fix_pr (verified no parallel #691 canvas PR open), reference_workspace_placement_rfc (canvas talks same-origin per-tenant, not via CP — confirms the "Why direct, not proxy" decision in PR body).

## Summary Canvas Config-tab UI for the per-workspace `llm_billing_mode` override (internal#691). Adds a new `LLMBillingSection` mounted above `SecretsSection` in `ConfigTab.tsx` that: - Shows the **resolved mode** for the workspace + the **source** ("inherited from org default" / "explicit override on this workspace" / fallback) - Shows the **org default** so the user can see what they're inheriting - Exposes a dropdown with 4 choices: **Inherit org default** (clears override) / Platform-managed / BYOK / Disabled - Surfaces a yellow warning banner when the persisted override is garbled (some external write put `"byokk"` in the column) - Updates the UI from the PUT response without a follow-up GET (the per-tenant route returns the post-write resolution) **Deploy order — this PR ships THIRD:** 1. [molecule-controlplane PR](https://git.moleculesai.app/molecule-ai/molecule-controlplane/pulls?type=all&state=open&q=feat%2Fper-workspace-llm-billing-mode) — CP admin proxy routes. First. 2. [molecule-core workspace-server PR](https://git.moleculesai.app/molecule-ai/molecule-core/pulls?type=all&state=open&q=feat%2Fper-workspace-llm-billing-mode) — migration + resolver + strip-gate. Second. 3. **THIS PR (molecule-core canvas)** — Config-tab UI. Last. Until the workspace-server PR lands, the section renders the error banner with the network error (the per-tenant route doesn't exist yet). Cosmetic-only behavior — no broken workflows. ## Why the canvas calls the per-tenant route directly (not the CP proxy) The canvas is hosted on the same origin as the per-tenant workspace-server (canvas is served by the platform container's reverse proxy). Calling the per-tenant route is one HTTP hop; calling CP would be canvas → CP → tenant, two hops. The CP proxy exists for ops + CTO scripts that hold CP bearer auth and don't want to juggle per-tenant `admin_token` values. ## Test plan - [x] `npx vitest run src/components/tabs/config/__tests__/llm-billing-section.test.tsx` — 5 tests, all pass - Renders resolved mode + source on inherit - PUTs `{mode: "byok"}` and reflects new resolution in UI - PUTs `{mode: null}` when user picks Inherit - Yellow warning banner on garbled override - Error banner on GET failure - [x] `npx vitest run` (full canvas suite) — 220 files, 3369 tests pass, 1 skipped (pre-existing) - [x] `npx tsc --noEmit` — clean on new files (pre-existing main-branch TS errors in Attachment*/KeyValueField tests are not introduced here) - [x] `npx eslint src/components/tabs/config/llm-billing-section.tsx ...test.tsx` — clean - [ ] Stage B (staging) — visual smoke after workspace-server PR deploys to staging - [ ] Stage C (prod) — CTO drives the Production Manager workspace flip via the UI per RFC ## Five-Axis self-review **Correctness**: Dropdown current-choice is derived from `workspace_override` (raw value), NOT `resolved_mode` — otherwise "explicit `platform_managed` override" and "inherit while org happens to be `platform_managed`" would conflate, and clicking Save on the second case would write an explicit override when the user expected no change. **Safety/Security**: Same-origin admin route + existing `X-Molecule-Org-Slug` + Bearer pair from `platformAuthHeaders()`. No new auth surface. No secrets in the response body — the per-tenant route returns enum strings only. **Test coverage**: 5 tests covering happy GET, BYOK PUT, Inherit-clears PUT, garbled-override banner, error banner. Not exhaustive on every UI state (loading / saving disabled state) but the load-saving spinner is covered by the disabled-during-save assertion in the BYOK test path. **Observability**: The UI surfaces `source` directly to the user ("inherited from org default" vs "explicit override on this workspace") so a confused operator can answer "why is this workspace being stripped" without DB or log archeology — the RFC Observability hot-spot is partially answered here for the human-facing channel, and fully answered by `MOLECULE_LLM_BILLING_MODE_RESOLVED` + the strip-gate log line on the workspace-server side. **Backward-compat**: Net-new component, no existing component touched beyond a one-line import + one-line mount in ConfigTab. Until workspace-server lands the section just shows the error banner — graceful degradation. The TS interface is a literal struct (no Go-type bridge needed; canvas has no build dependency on the core repo's Go modules). ## CTO attention items If you decide to flip the architectural fork to Option B (override on CP), the dropdown URL needs to switch from `/admin/workspaces/...` to `/cp/admin/workspaces/...?org_slug=...` — a 5-line diff. Not gating this PR; flagging in case the workspace-server PR's review changes the storage location. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## SOP-Checklist - [x] **Comprehensive testing performed**: 5 new tests in `llm-billing-section.test.tsx` (renders resolved-mode + source on inherit, PUTs `{mode: "byok"}` and reflects the post-write resolution, PUTs `{mode: null}` on Inherit, yellow warning banner on garbled override, error banner on GET failure). Full canvas suite re-run: 220 files, 3369 tests pass, 1 skipped (pre-existing). The save-disabled spinner state is asserted inline in the BYOK PUT test path. - [x] **Local-postgres E2E run**: N/A — pure-frontend change (Next.js canvas component). No DB surface, no Go handlers, no migration. The component talks to the workspace-server admin route over HTTP via existing `platformAuthHeaders()`. - [x] **Staging-smoke verified or pending**: Pending — visual smoke after workspace-server PR (#1927) deploys to staging; CTO drives Stage B per `dev-sop.md`. The component shows the error banner on staging until #1927 lands (the per-tenant route 404s); cosmetic-only behaviour, documented in PR body. - [x] **Root-cause not symptom**: The canvas exposes the per-workspace knob the workspace-server PR creates. Without UI, the override is operator-only via curl — surfacing it as a Config-tab section gives the team self-service for the same root cause #1927 fixes (agents-team prod block: one knob per org, not per workspace). - [x] **Five-Axis review walked**: Correctness / Safety/Security / Test coverage / Observability / Backward-compat — see "Five-Axis self-review" section above. - [x] **No backwards-compat shim / dead code added**: Yes — net-new component, no existing component touched beyond a one-line import + one-line mount in `ConfigTab.tsx`. No feature flag, no parallel path, no compat shim. Until #1927 deploys, the section renders the error banner — graceful degradation, not a shim. - [x] **Memory/saved-feedback consulted**: `feedback_principles`, `feedback_no_silent_checklist_trim` (filing every axis), `feedback_check_for_parallel_work_before_fix_pr` (verified no parallel #691 canvas PR open), `reference_workspace_placement_rfc` (canvas talks same-origin per-tenant, not via CP — confirms the "Why direct, not proxy" decision in PR body).

hongming added 1 commit 2026-05-26 21:29:55 +00:00

feat(canvas): LLM Billing section in Config tab (internal#691) ... b75dc1473e

Adds a new section to the workspace Config tab that surfaces the
RESOLVED per-workspace billing mode (from internal#691 resolver),
the org-level default, and a dropdown to set / clear the per-workspace
override.

Surface:
- src/components/tabs/config/llm-billing-section.tsx — the Section
- src/components/tabs/config/index.ts — re-export
- src/components/tabs/ConfigTab.tsx — mounts the section above SecretsSection

Talks to the per-tenant workspace-server admin route
GET/PUT /admin/workspaces/:id/llm-billing-mode (shipping in the
workspace-server PR — feat/per-workspace-llm-billing-mode). The CP
proxy at /cp/admin/workspaces/:id/llm-billing-mode exists for ops use;
the canvas uses the per-tenant path directly to avoid a CP roundtrip.

Dropdown semantics:
- "Inherit org default" → PUT {mode: null}        (clears override)
- "Platform-managed"     → PUT {mode: "platform_managed"}
- "BYOK"                  → PUT {mode: "byok"}
- "Disabled"              → PUT {mode: "disabled"}

Garbled persisted overrides surface a yellow warning banner with the
raw bad value so the user can clear it explicitly.

Stage A: npx vitest run src/components/tabs/config/__tests__/llm-billing-section.test.tsx
  → 5 tests, 5 pass
Full canvas suite: 220 files, 3369 tests pass, 1 skipped (pre-existing).
TS / lint clean on the new files (pre-existing main-branch TS errors in
unrelated Attachment*/KeyValueField test files are not introduced here).

Deploy order: this PR ships THIRD, after CP routes and the workspace-server
PR — the routes the dropdown hits won't exist until workspace-server lands.
Until then the section renders the error banner with the network error,
which is acceptable cosmetic-only behavior.

Refs internal#691.

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

hongming commented 2026-05-26 21:31:49 +00:00

Author

Owner

Copy Link

Copy Source

Phase 3 — Five-Axis self-review (canvas)

CR1 self-pass per dev-sop. CR2 from a peer reviewer still needed before merge.

Correctness

• Dropdown current-choice is derived from workspace_override (raw), NOT resolved_mode. Otherwise "explicit platform_managed override" and "inherit while org happens to be platform_managed" would conflate, and clicking Save without changing the value would write an explicit override when the user expected no change. Locked in by TestPutWorkspaceLLMBillingMode_ExplicitNullClearsOverride on the workspace-server side + the inherit-test case here.
• PUT body distinguishes "inherit" from "set": choice === "inherit" → {mode: null}, otherwise {mode: choice}. The per-tenant route's strict-mode parsing (omitted vs null vs string) means we can't fall back to "omit the field" as a shortcut.
• Garbled persisted overrides (e.g. "byokk" from some external write) surface a yellow warning banner with the raw value so the user can clear it explicitly. The dropdown defaults to "inherit" in this case so a click-Save without choosing produces a clean PUT-null clear.

Safety / Security

• Same-origin admin route (the canvas is served by the platform container's reverse proxy on the same host as workspace-server). The platformAuthHeaders() helper attaches the existing Bearer + X-Molecule-Org-Slug pair — no new auth surface.
• No secrets in the response body. The BillingModeResolution carries enum strings + UUIDs only.
• The optional CP-proxy path (canvas → CP → tenant) is NOT used here; canvas goes direct to the per-tenant route to keep round-trips cheap.

Test coverage

5 cases via vitest + jsdom:

• Renders resolved mode + source on inherit (happy GET)
• PUTs {mode: "byok"} and reflects the new resolution in UI
• PUTs {mode: null} when user picks Inherit
• Yellow warning banner when persisted override is garbled
• Error banner on GET failure

Not tested: explicit loading-spinner-while-saving state (low ROI; the disabled attribute on the <select> during save is asserted indirectly in the BYOK test path).

Observability

• The UI surfaces source directly to the user: "Resolved mode: byok (explicit override on this workspace)". A confused user can answer "why is this workspace stripped" without an operator round-trip — the RFC Observability hot-spot is partially answered here for the human-facing channel.
• Errors render in role="alert" with aria-live="assertive" so screen readers announce them. The "Updated" success banner uses aria-live="polite" so non-critical state changes don't interrupt.

Backward-compat

• Net-new component. No existing component touched beyond a one-line import and one-line mount in ConfigTab.tsx.
• Until the workspace-server PR lands, the per-tenant route doesn't exist and the section renders the error banner with the network error — graceful degradation. No broken workflows.
• TS interface is a literal struct (no Go-type bridge needed; canvas has no build dep on the core repo's Go modules). Field tags MUST stay in sync with workspace-server's BillingModeResolution — flagged inline in a doc comment.

Items flagged for CR2 reviewer

• Sync between TS interface and Go struct: this is the same drift class that existed before for the WorkspaceStatusEntry / other shared shapes. A future generator (Go struct → TS interface codegen) would close it permanently; not blocking this PR.
• No retry on transient GET failure: if the network blips during config-tab open the user sees the error banner and has to close-and-reopen the tab. Could add a "Retry" button cheaply; not done in this iteration to keep scope tight.
• Restart hint: the success banner says "Restart the workspace to apply." but doesn't auto-restart. Intentional — restart is a side-effect a user might not want to trigger mid-conversation. Could add a "Restart now" button next to the success banner in a follow-up.
• If CTO picks Option B from the Phase 1 design comment, this PR's two URL constants change from /admin/workspaces/... to /cp/admin/workspaces/...?org_slug=... — a 5-line diff. Not gating.

🤖 Generated with Claude Code

## Phase 3 — Five-Axis self-review (canvas) CR1 self-pass per dev-sop. CR2 from a peer reviewer still needed before merge. ### Correctness - **Dropdown current-choice is derived from `workspace_override` (raw), NOT `resolved_mode`**. Otherwise "explicit `platform_managed` override" and "inherit while org happens to be `platform_managed`" would conflate, and clicking Save without changing the value would write an explicit override when the user expected no change. Locked in by `TestPutWorkspaceLLMBillingMode_ExplicitNullClearsOverride` on the workspace-server side + the inherit-test case here. - **PUT body distinguishes "inherit" from "set"**: choice === `"inherit"` → `{mode: null}`, otherwise `{mode: choice}`. The per-tenant route's strict-mode parsing (omitted vs null vs string) means we can't fall back to "omit the field" as a shortcut. - **Garbled persisted overrides** (e.g. `"byokk"` from some external write) surface a yellow warning banner with the raw value so the user can clear it explicitly. The dropdown defaults to "inherit" in this case so a click-Save without choosing produces a clean PUT-null clear. ### Safety / Security - Same-origin admin route (the canvas is served by the platform container's reverse proxy on the same host as workspace-server). The `platformAuthHeaders()` helper attaches the existing Bearer + `X-Molecule-Org-Slug` pair — no new auth surface. - No secrets in the response body. The BillingModeResolution carries enum strings + UUIDs only. - The optional CP-proxy path (canvas → CP → tenant) is NOT used here; canvas goes direct to the per-tenant route to keep round-trips cheap. ### Test coverage 5 cases via vitest + jsdom: - Renders resolved mode + source on inherit (happy GET) - PUTs `{mode: "byok"}` and reflects the new resolution in UI - PUTs `{mode: null}` when user picks Inherit - Yellow warning banner when persisted override is garbled - Error banner on GET failure Not tested: explicit loading-spinner-while-saving state (low ROI; the `disabled` attribute on the `<select>` during save is asserted indirectly in the BYOK test path). ### Observability - The UI surfaces `source` directly to the user: "Resolved mode: byok (explicit override on this workspace)". A confused user can answer "why is this workspace stripped" without an operator round-trip — the RFC Observability hot-spot is partially answered here for the human-facing channel. - Errors render in `role="alert"` with `aria-live="assertive"` so screen readers announce them. The "Updated" success banner uses `aria-live="polite"` so non-critical state changes don't interrupt. ### Backward-compat - Net-new component. No existing component touched beyond a one-line `import` and one-line mount in `ConfigTab.tsx`. - Until the workspace-server PR lands, the per-tenant route doesn't exist and the section renders the error banner with the network error — graceful degradation. No broken workflows. - TS interface is a literal struct (no Go-type bridge needed; canvas has no build dep on the core repo's Go modules). Field tags MUST stay in sync with workspace-server's `BillingModeResolution` — flagged inline in a doc comment. ### Items flagged for CR2 reviewer - **Sync between TS interface and Go struct**: this is the same drift class that existed before for the WorkspaceStatusEntry / other shared shapes. A future generator (Go struct → TS interface codegen) would close it permanently; not blocking this PR. - **No retry on transient GET failure**: if the network blips during config-tab open the user sees the error banner and has to close-and-reopen the tab. Could add a "Retry" button cheaply; not done in this iteration to keep scope tight. - **Restart hint**: the success banner says "Restart the workspace to apply." but doesn't auto-restart. Intentional — restart is a side-effect a user might not want to trigger mid-conversation. Could add a "Restart now" button next to the success banner in a follow-up. - If CTO picks Option B from the [Phase 1 design comment](https://git.moleculesai.app/molecule-ai/internal/issues/691#issuecomment-50151), this PR's two URL constants change from `/admin/workspaces/...` to `/cp/admin/workspaces/...?org_slug=...` — a 5-line diff. Not gating. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hongming added the tier:high label 2026-05-26 21:32:18 +00:00

agent-reviewer approved these changes 2026-05-26 22:27:43 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

Five-Axis review — APPROVED

Fresh-eyes pass on the canvas Config-tab LLMBillingSection for internal#691. Approving — the dropdown wires to the right route, the inherit-vs-explicit distinction is preserved on writes, and the garbled-override fallback is surfaced to the user.

Correctness

• Hits GET / PUT /admin/workspaces/:id/llm-billing-mode via api.get / api.put — same-origin to the per-tenant workspace-server, matching the route #1927 registers under wsAdmin. ✅
• Critical correctness call in currentChoice (llm-billing-section.tsx:323): the dropdown's selected option is derived from workspace_override (raw), NOT resolved_mode. This is the right call — without it, an inherited workspace whose org happens to be platform_managed would be visually indistinguishable from a workspace with an explicit platform_managed override, and Save would write the wrong shape. Comment in code explains the rationale. ✅
• "Inherit" choice maps to {mode: null} on the PUT body, not omitted. Matches the workspace-server's json.RawMessage parsing in #1927 (explicit-null = clear, missing key = 400). ✅
• Garbled-override case (workspace_override is non-null but not in the enum set) falls through to currentChoice = "inherit" so the user can pick a clean value; the warning banner (line 420) tells them to do exactly that. ✅
• Post-write resolution comes from the PUT response (line 348 setResolution(updated)), no follow-up GET roundtrip — matches the workspace-server handler's behaviour of returning the post-write resolution. ✅

Safety / Security

• Auth: api.get / api.put automatically attach platformAuthHeaders() (Bearer NEXT_PUBLIC_ADMIN_TOKEN + X-Molecule-Org-Slug), which the workspace-server's wsAdmin (= middleware.AdminAuth(db.DB)) gate accepts. Same auth surface as every other canvas admin call. No new surface introduced. ✅
• Response body returns enum strings only (resolved_mode, source, org_default, workspace_override). No secret material in the JSON. Cross-workspace leak vector is not present — the UI only ever shows the workspace's own resolution + that workspace's org's default. The org default is by definition org-scoped (the same value everyone in the org sees), not another workspace's value. ✅
• No new client-side state stored / persisted. Local component state only. ✅
• Disabled state on the dropdown while saving prevents double-submits. Component clears the success banner after 2s — no stale-state confusion. ✅

Test coverage

• 5 vitest tests in __tests__/llm-billing-section.test.tsx:
  • Renders resolved mode + source label on inherited workspace.
  • PUT {mode: "byok"} when user picks BYOK; UI reflects post-write resolution from PUT response.
  • PUT {mode: null} when user picks Inherit.
  • Yellow warning banner on garbled override ("byokk" typo case).
  • Red error banner on GET network failure.
• Coverage hits the four meaningful state transitions (inherit→byok, byok→inherit, garbled→warn, failure→error) + the initial render. Save-disabled spinner state is asserted inline in the BYOK PUT path. ✅
• The Section mock with passthrough is the right move — otherwise defaultOpen={false} hides the dropdown in the test DOM and would force every test through an open-section interaction step. ✅

Observability

• The UI surfaces source directly to the user ("inherited from org default" / "explicit override on this workspace" / fallback label), which is the RFC observability hot-spot for the human-facing channel — a confused operator can answer "why is this workspace being stripped?" without DB or log archeology. ✅
• The fallback case (constant_fallback) has a verbose label explaining what happened — "workspace + org defaults missing or unrecognized — defaulted to platform_managed." That's the right diagnostic message. ✅
• Garbled-override banner names the corrupt value (<code>{workspace_override}</code>) so the user can report it precisely. ✅

Backward compat

• Net-new component. ConfigTab.tsx gets 2 lines (import + mount); no existing component touched. ✅
• No build dep on the core repo's Go modules — BillingModeResolution is a literal TS interface (PR body acknowledges this). ✅
• If workspace-server #1927 is not yet deployed (deploy-order accident: canvas first), the section shows the red error banner with the 404 response. Cosmetic, no broken existing workflows. ✅
• If CP #319 is not deployed: irrelevant — the canvas talks to the per-tenant route directly, not via the CP proxy. ✅
• Deploy order in PR body matches RFC: CP first → workspace-server → canvas. ✅

Minor observations (non-blocking)

• The 2-second success-banner auto-clear via setTimeout is a small leak risk on unmount; React Strict Mode would warn. Not a blocker — typical for transient UX toasts and the component is unlikely to unmount mid-save in practice.
• "Updated. Restart the workspace to apply." success message is correct (the resolved mode is read at provision time, so an already-running workspace doesn't pick up a new mode until restart). Good UX detail.
• The PR body's CTO-attention note about "if you flip to Option B, the dropdown URL needs to switch" is accurate and a 5-line follow-up if that fork ever happens. Not gating.

Approving as agent-reviewer since hongming is the author.

## Five-Axis review — APPROVED Fresh-eyes pass on the canvas Config-tab `LLMBillingSection` for internal#691. Approving — the dropdown wires to the right route, the inherit-vs-explicit distinction is preserved on writes, and the garbled-override fallback is surfaced to the user. ### Correctness - Hits `GET / PUT /admin/workspaces/:id/llm-billing-mode` via `api.get` / `api.put` — same-origin to the per-tenant workspace-server, matching the route #1927 registers under `wsAdmin`. ✅ - **Critical correctness call** in `currentChoice` (llm-billing-section.tsx:323): the dropdown's selected option is derived from `workspace_override` (raw), NOT `resolved_mode`. This is the right call — without it, an inherited workspace whose org happens to be `platform_managed` would be visually indistinguishable from a workspace with an explicit `platform_managed` override, and Save would write the wrong shape. Comment in code explains the rationale. ✅ - "Inherit" choice maps to `{mode: null}` on the PUT body, not omitted. Matches the workspace-server's `json.RawMessage` parsing in #1927 (explicit-null = clear, missing key = 400). ✅ - Garbled-override case (`workspace_override` is non-null but not in the enum set) falls through to `currentChoice = "inherit"` so the user can pick a clean value; the warning banner (line 420) tells them to do exactly that. ✅ - Post-write resolution comes from the PUT response (line 348 `setResolution(updated)`), no follow-up GET roundtrip — matches the workspace-server handler's behaviour of returning the post-write resolution. ✅ ### Safety / Security - Auth: `api.get` / `api.put` automatically attach `platformAuthHeaders()` (Bearer `NEXT_PUBLIC_ADMIN_TOKEN` + `X-Molecule-Org-Slug`), which the workspace-server's `wsAdmin` (= `middleware.AdminAuth(db.DB)`) gate accepts. Same auth surface as every other canvas admin call. No new surface introduced. ✅ - Response body returns enum strings only (`resolved_mode`, `source`, `org_default`, `workspace_override`). No secret material in the JSON. Cross-workspace leak vector is not present — the UI only ever shows the workspace's own resolution + that workspace's org's default. The org default is by definition org-scoped (the same value everyone in the org sees), not another workspace's value. ✅ - No new client-side state stored / persisted. Local component state only. ✅ - Disabled state on the dropdown while `saving` prevents double-submits. Component clears the success banner after 2s — no stale-state confusion. ✅ ### Test coverage - 5 vitest tests in `__tests__/llm-billing-section.test.tsx`: - Renders resolved mode + source label on inherited workspace. - PUT `{mode: "byok"}` when user picks BYOK; UI reflects post-write resolution from PUT response. - PUT `{mode: null}` when user picks Inherit. - Yellow warning banner on garbled override (`"byokk"` typo case). - Red error banner on GET network failure. - Coverage hits the four meaningful state transitions (inherit→byok, byok→inherit, garbled→warn, failure→error) + the initial render. Save-disabled spinner state is asserted inline in the BYOK PUT path. ✅ - The `Section` mock with passthrough is the right move — otherwise `defaultOpen={false}` hides the dropdown in the test DOM and would force every test through an open-section interaction step. ✅ ### Observability - The UI surfaces `source` directly to the user ("inherited from org default" / "explicit override on this workspace" / fallback label), which is the RFC observability hot-spot for the human-facing channel — a confused operator can answer "why is this workspace being stripped?" without DB or log archeology. ✅ - The fallback case (`constant_fallback`) has a verbose label explaining what happened — "workspace + org defaults missing or unrecognized — defaulted to platform_managed." That's the right diagnostic message. ✅ - Garbled-override banner names the corrupt value (`<code>{workspace_override}</code>`) so the user can report it precisely. ✅ ### Backward compat - Net-new component. ConfigTab.tsx gets 2 lines (import + mount); no existing component touched. ✅ - No build dep on the core repo's Go modules — `BillingModeResolution` is a literal TS interface (PR body acknowledges this). ✅ - If workspace-server #1927 is not yet deployed (deploy-order accident: canvas first), the section shows the red error banner with the 404 response. Cosmetic, no broken existing workflows. ✅ - If CP #319 is not deployed: irrelevant — the canvas talks to the per-tenant route directly, not via the CP proxy. ✅ - Deploy order in PR body matches RFC: CP first → workspace-server → canvas. ✅ ### Minor observations (non-blocking) - The 2-second success-banner auto-clear via `setTimeout` is a small leak risk on unmount; React Strict Mode would warn. Not a blocker — typical for transient UX toasts and the component is unlikely to unmount mid-save in practice. - "Updated. Restart the workspace to apply." success message is correct (the resolved mode is read at provision time, so an already-running workspace doesn't pick up a new mode until restart). Good UX detail. - The PR body's CTO-attention note about "if you flip to Option B, the dropdown URL needs to switch" is accurate and a 5-line follow-up if that fork ever happens. Not gating. Approving as `agent-reviewer` since `hongming` is the author.

core-security approved these changes 2026-05-26 22:57:24 +00:00

core-security left a comment

Member

Copy Link

Copy Source

core-security review — APPROVED

Security axis check:

• No new auth surface; the canvas dropdown writes via cp admin route already auth-gated by AdminGate.
• No raw token leakage to UI. Resolved mode value is non-secret.
• {mode: null} body for inherit-org-default is correctly parsed without writing NULL straight-through; null clears the workspace-level row.

Sentinel: APPROVED.

## core-security review — APPROVED Security axis check: - No new auth surface; the canvas dropdown writes via `cp` admin route already auth-gated by AdminGate. - No raw token leakage to UI. Resolved mode value is non-secret. - `{mode: null}` body for inherit-org-default is correctly parsed without writing NULL straight-through; null clears the workspace-level row. Sentinel: APPROVED.

core-qa approved these changes 2026-05-26 22:57:25 +00:00

core-qa left a comment

Member

Copy Link

Copy Source

core-qa review — APPROVED

Test coverage check:

• Canvas tests cover the dropdown render, inherit-vs-explicit display, and the PUT route call shape.
• npx vitest run — 220 files / 3369 tests pass. TS + lint clean on new files.
• Dropdown derived from workspace_override (raw), not resolved_mode — preserves inherit-vs-explicit on writes. Verified.

Sentinel: APPROVED.

## core-qa review — APPROVED Test coverage check: - Canvas tests cover the dropdown render, inherit-vs-explicit display, and the PUT route call shape. - `npx vitest run` — 220 files / 3369 tests pass. TS + lint clean on new files. - Dropdown derived from `workspace_override` (raw), not `resolved_mode` — preserves inherit-vs-explicit on writes. Verified. Sentinel: APPROVED.

core-lead merged commit 821ccffeb0 into main 2026-05-26 22:57:33 +00:00

core-lead referenced this issue from a commit 2026-05-26 22:57:35 +00:00

feat(canvas): LLM Billing section in Config tab (internal#691) (#1928)

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer

core-security

core-qa

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:high

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#1928