fix(canvas): subtree-aware layout + org-import reliability + UX polish - #1994
Merged
Merged
Conversation
Five tightly-related fixes surfaced while stress-testing org-template
imports (Legal Team, Molecule Company, etc.) on a running control plane:
1) Org import was silently failing — INSERT wrote `collapsed` into the
`workspaces` table but that column lives on `canvas_layouts`
(005_canvas_layouts.sql). Every import returned 207 with 0 rows
created, which `api.post` treated as success → green "Imported"
toast + empty canvas. Moved the write to canvas_layouts; updated
the workspace_crud PATCH path to UPSERT there too; refreshed the
test mock. Added a client-side assertion that throws on
2xx-with-`error`-body so future partial-failures surface a red
toast rather than lying about success.
2) Multi-level nested layout was collision-prone: children that were
themselves parents (CTO → Dev Lead → 6 engineers) got the same
leaf-sized grid slot as leaf siblings and clipped into each other.
Added post-order `sizeOfSubtree` + sibling-size-aware
`childSlotInGrid` on both the Go server and the TS client (kept in
sync). `buildNodesAndEdges` now uses subtree sizes for both parent
dimensions and the rescue heuristic. `setCollapsed` on expand now
reads each child's actual rendered width/height instead of the
leaf-count formula — a regression test covers the CTO/Dev Lead
scenario.
3) Provisioning-timeout banner was unusable during large imports: a
30-workspace tree triggered 27 simultaneous "stuck" warnings 2
minutes in (server paces + provision concurrency = 3 guarantee tail
items legitimately wait longer). Scaled threshold with concurrent
count (base + 45s per queue slot beyond concurrency) and added a
Dismiss (×) button per banner.
4) Auto pan-and-zoom on org ready: after the last workspace flips out
of `provisioning`, canvas now fitView's with a 1.2s animation,
0.25 padding, `maxZoom: 0.8` and `minZoom: 0.25`. Without the zoom
caps fitView was hitting the component's maxZoom=2 on small trees
and zooming in instead of out.
5) Toolbar was visually busy: `+ N sub` count wrapped onto a second
row on narrow viewports; status dot and workspace total were in
separate border-delimited cells. Merged into one segment with
`whitespace-nowrap`; A2A / Audit / Search / Help collapsed to
icon-only 28px buttons with tooltip + aria-label (Figma/Linear
pattern). Stop All / Restart Pending keep text — they're urgent.
Also:
- `api.{get,post,...}` accept an optional `{ timeoutMs }` so callers
that hit intentionally-slow endpoints (org import paces 2s between
siblings) don't trip the 15s default and report false aborts.
- `WorkspaceNode` clamps role text to 2 lines so verbose descriptions
don't unboundedly grow card height and break the grid.
- `PARENT_HEADER_PADDING` bumped 44→130 to clear name + runtime +
2-line role + the currentTask banner that appears during the
initial-prompt phase.
Tests: 930 canvas tests + full Go handler suite pass. Added
regressions for (i) 207 partial-success surfacing as throw, and
(ii) setCollapsed sizing with nested-parent children.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
… A2A gate (#1994 follow-on) The A2A e2e historically asserted only response SHAPE (test_a2a_e2e.sh checked '"kind":"text"' only). A broken agent returns its error AS a text part -- {"kind":"text","text":"Agent error (Exception) ..."} -- which STILL matches the shape check, so it PASSED on a fully broken agent. That is why the 2026-05-2x drained-key / byok-misroute failures (agents-team PM + reno marketing erroring on every LLM call) sailed through CI. "Channel returns text shape" is not "agent completed an LLM round-trip." Adds, ADDITIVELY (no existing assertion weakened or removed): - tests/e2e/lib/completion_assert.sh -- reusable gates: * a2a_assert_real_completion: deterministic known-answer round-trip; asserts CONTAINS the expected token AND NOT an error-as-text marker (Agent error / Exception / error result / MISSING_BYOK_CREDENTIAL). * provider_liveness_matrix + offered_platform_models_for_runtime: per-offered-provider cheap (max_tokens:4) probe; the offered set is read from the providers.yaml SSOT (runtimes.<rt>.providers[platform] .models) -- not a hardcoded list -- so the matrix tracks the SSOT. * assert_byok_not_platform_proxy: #1994 regression guard -- a byok-resolving workspace must NOT resolve platform_managed (reads the same derived resolver GET /admin/workspaces/:id/llm-billing-mode the provision strip gate uses). - tests/e2e/test_staging_full_saas.sh (the live-agent lane, MiniMax primary): new stanzas 8b (PINEAPPLE known-answer, the core gate), 8c (byok-routing guard), 8d (SSOT-driven per-provider liveness matrix). - tests/e2e/test_a2a_e2e.sh: added check_no_error_as_text on Echo + SEO replies so the brief's literal shape-only example now FAILS on an error-as-text payload. - tests/e2e/test_completion_assert_unit.sh: offline fail-direction proof (16 cases) that the negative gates are load-bearing -- error-as-text MUST fail, platform_managed MUST trip the #1994 guard. Wired into ci.yml "Run E2E bash unit tests (no live infra)" (required, per-PR + main). e2e-staging-saas.yml paths filter extended to re-trigger the live lane on lib changes. No #1994 fix code touched -- tests/e2e + workflow wiring only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
… cred; stop stripping it (molecule-core#1994) Corrected-model credential fix (CTO-confirmed). `global_secrets` is the TENANT's own secret store (shared across that tenant's workspaces), NOT the platform's. The platform's own LLM credential is the CP proxy usage token, injected separately on the platform_managed path; it is never stored in a tenant's global_secrets. The internal#711 provider-aware strip rested on the inverted premise that a global-scope LLM credential was "the platform's own". On the byok/disabled branch it stripped the tenant's OWN oauth when that oauth lived at global scope, leaving the workspace credential-less -> MISSING_BYOK_CREDENTIAL -> dead (Reno Stars Marketing/SEO byok agents, live-confirmed 2026-05-28). Changes: - workspace_provision.go: remove the stripGlobalOriginLLMCreds call on the byok/disabled branch; delete the now-dead function; drop the unused globalKeys parameter from applyPlatformManagedLLMEnv. - secrets.go: remove the symmetric byok strip on the remote-pull path (GET /workspaces/:id/secrets/values) + its now-unused globalKeys tracking; the bundle is the tenant's merged secrets served verbatim. - platform_managed path UNCHANGED: still strips direct oauth + forces the CP proxy usage token (metered). Only byok/disabled stop being stripped. - Fail-closed UNCHANGED in spirit: a byok workspace with no LLM credential at ANY scope still aborts MISSING_BYOK_CREDENTIAL; the trigger narrowed from "no workspace-scoped cred" to "no cred at any scope". Guard (co-mingling prevention at the write boundary): - SetGlobal still rejects bypass-list keys for a platform_managed tenant (keeps a platform-shaped credential out of global_secrets going forward); added a regression test pinning it. Tests: inverted the strip-asserting unit + e2e tests to the corrected model (global-scope oauth survives, byok runs direct, no proxy); added genuinely- credential-less byok fail-closed coverage; all three behavior changes are mutation-load-bearing (re-adding either strip / dropping the SetGlobal guard turns the respective test RED). build + vet + golangci-lint + the full integration-tagged handlers suite green. The #1994 model-passthrough fix and the MiniMax A2A e2e on this branch are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…trip stray non-matching global-origin LLM creds (internal#728 Bug 1) [BEHAVIOR-AFFECTING — CTO merge-go] #1995 removed the blanket global-LLM-cred strip on the byok branch (correct for the platform-key co-mingling it targeted), but left EVERY claude-code workspace inheriting the tenant-global CLAUDE_CODE_OAUTH_TOKEN. The claude-code runtime greedily prefers that oauth (llm-auth: detected oauth -> api.anthropic.com), so a workspace whose RESOLVED provider is NOT anthropic-oauth (minimax, kimi-byok) routes its non-Anthropic model to Anthropic -> "Claude Code returned an error result" (agents-team Dev Engineer B, MiniMax-M2.7; live-confirmed 2026-05-28 via SSM container logs, internal#728 comment 52493). Fix: provider-AWARE replacement for the over-removed strip. On the byok/disabled branch, keep ONLY the global-origin LLM bypass creds whose env-var name is in the RESOLVED provider's auth_env; strip the rest. - minimax auth_env MINIMAX_API_KEY/ANTHROPIC_AUTH_TOKEN/ANTHROPIC_API_KEY -> stray global CLAUDE_CODE_OAUTH_TOKEN is non-matching -> stripped (fixes DevB). - anthropic-oauth auth_env CLAUDE_CODE_OAUTH_TOKEN -> matches -> kept (PM opus + reno opus-byok NOT regressed; #1994 ByokGlobalScopeOAuthSurvives guard holds). NOT a return to the blanket strip (which would re-break the byok-anthropic-oauth case #1994 fixed) — keyed off DeriveProvider's resolved provider. Provenance-scoped: only operator-store (global_secrets) origin keys are provider-gated. User-authored workspace_secrets (provenance flag cleared by loadWorkspaceSecrets) are NEVER stripped — JRS kimi workspace-key, reno's own oauth are exempt. Fail-OPEN: an underivable provider / unavailable registry strips nothing (keep-first; worst case is a kept stray, never removing the only usable cred -> never fail-closes a legitimate byok workspace). Threads loadWorkspaceSecrets's globalKeys provenance side-channel into applyPlatformManagedLLMEnv (signature +map[string]struct{}); caller prepareProvisionContext already has it. Tests (llm_billing_mode_provision_parity_test.go): - MinimaxStripsStrayGlobalOAuth — DevB repro: minimax-resolving ws strips the stray global oauth + keeps MINIMAX_API_KEY routing. - WorkspaceOriginCredExemptFromStrip — user-authored ws_secrets cred survives even when non-matching. - ByokGlobalScopeOAuthSurvives (strengthened) — global-origin oauth on opus SURVIVES via provider match (PM/reno regression guard). Mutation-load-bearing (verified RED): (1) remove strip -> blanket-keep regresses DevB; (2) empty keep set (provider-unaware) -> minimax routing + reno oauth stripped; (3) iterate all bypass keys (provenance-unaware) -> user-authored cred stripped. build ok; build -tags=integration ok; go test ./internal/handlers/ ok; golangci-lint ./internal/handlers/ -> 0 issues. Refs internal#728. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…roxy env absent (#2162) applyPlatformManagedLLMEnv falsely reported HasUsableLLMCred:true when MOLECULE_LLM_BASE_URL + MOLECULE_LLM_USAGE_TOKEN were empty, causing claude-code workspaces to boot credential-less and hit the 600s provision-timeout sweep (adk-demo dark-wedge class). Fix: - Empty-proxy-env path returns HasUsableLLMCred:false (was true). - Caller aborts with MISSING_PLATFORM_PROXY, symmetric to the BYOK MISSING_BYOK_CREDENTIAL hard-fail. - User-facing error message explains the boot-race and retry path. Regression tests: - TestApplyPlatformManagedLLMEnv_MissingProxyEnvFailClosed: asserts HasUsableLLMCred=false when proxy env absent. - TestApplyPlatformManagedLLMEnv_ProxyEnvPresentInjectsCredential: asserts ANTHROPIC_API_KEY + ANTHROPIC_BASE_URL injected when proxy env present. Refs: #2162, #711 (BYOK fail-closed pattern), #1994 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…2263) The staging SaaS E2E provisioned its claude-code canary with the BARE id `MiniMax-M2`. The deployed staging tenant ws-server's compiled model registry lags source, so validateRegisteredModelForRuntime returns HTTP 400 on the bare id at workspace-create. The sibling Platform Boot job, on the SAME image, succeeds with the NAMESPACED `moonshot/kimi-k2.6` — only the id form differs (deploy-skew, internal#718; NOT flaky). Harness-side fix: switch the claude-code MiniMax default from bare `MiniMax-M2` to the COLON-namespaced `minimax:MiniMax-M2.7`. Crucially this is the colon (BYOK) form, NOT the slash/platform form `minimax/MiniMax-M2.7` the issue floated: the canary injects E2E_MINIMAX_API_KEY (BYOK), so the #1994 byok-not-platform guard asserts provider_selection=minimax. The colon form stays in the BYOK `minimax` arm (providers.yaml:851 → provider=minimax, passes the guard); the slash form resolves to provider=platform and would trip it. Mirrors how the proven-working kimi BYOK colon-form is registered. Changed both the operator-override default in e2e-staging-saas.yml (which sets E2E_MODEL_SLUG and wins over pick_model_slug) and the pick_model_slug fallback in lib/model_slug.sh, plus the pinned unit-test expectations. Also: widen the known-answer A2A POST retry grep to include the Cloudflare-shaped literal `error code: 502/504` token, matching the cold-start PONG probe and delegation loops. A single un-retried edge 502 right after a healthy round-trip (Platform Boot, task 268859) fell through to break and failed the gate on the first attempt. Bounded by the existing 6-attempt/sleep-10 loop — no new sleep-as-fix. NOTE: harness-side only. The durable fix is promoting the staging tenant ws-server runtime image to a build whose compiled registry includes the bare id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…2.7 not colon The e2e-staging-saas regression guard 422s at parent workspace-create with UNREGISTERED_MODEL_FOR_RUNTIME for model "minimax:MiniMax-M2.7" on runtime "claude-code" (internal#718; real failure job 295233, main 4b3590e). PR #2311 fixed the bare-vs-colon slug in tests/e2e/lib/model_slug.sh, but the workflow env var E2E_MODEL_SLUG OVERRIDES the pick_model_slug lib (it returns $E2E_MODEL_SLUG verbatim when set), so the saas run kept sending the colon form. The claude-code adapter can't strip the `minimax:` prefix, so the colon id is UNREGISTERED (derive_provider_matrix_test.go:288). The bare registered id `MiniMax-M2.7` is the BYOK-minimax form (registry_gen.go:88, MINIMAX_API_KEY), which keeps the #1994 byok-not-platform guard passing. Swap the default fallback to the bare form and correct the stale comment. Per-runtime overrides (hermes/codex/google-adk) are unchanged. Test-infra-only: workflow file + comment, zero production/registry/test-script changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…efore vendor-key write The staging full-SaaS E2E (test_staging_full_saas.sh) provisions its parent (and child) workspace by POSTing /workspaces with the customer's OWN LLM key in `secrets` (MINIMAX_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY / OPENAI_API_KEY+HERMES_CUSTOM_*). After #2311/#2312 made bare `MiniMax-M2.7` registry-valid, a real staging run (job 295385, main f1558b5) now PASSES model validation but FAILS at parent-create: {"error":"direct vendor key writes are blocked for platform-managed workspaces; ... or set this workspace's billing mode to 'byok' via /admin/workspaces/:id/llm-billing-mode","key":"MINIMAX_API_KEY"} This 400 is INTENDED product behavior, not a product bug. workspace-server's secret-write gate (rejectPlatformManagedDirectLLMBypassForWorkspace in workspace-server/internal/handlers/secrets.go) blocks direct writes of any strip-listed vendor key while a workspace resolves to platform_managed (the org/CTO default). A bare vendor key in the create payload does NOT auto-derive byok — at create time no auth-env is present yet, so the resolver derives platform_managed. The resolver's org rung was retired (internal#718 P2-B), so ResolveLLMBillingMode ignores the org default entirely; the ONLY explicit byok opt-in is a per-workspace override via PUT /admin/workspaces/:id/llm-billing-mode {"mode":"byok"}. Mechanism — per-workspace override (NOT org-default): the org rung is retired, so an org-create billing field could not satisfy this gate even if /cp/admin/orgs accepted one. For any arm whose secrets contain strip-listed keys we now: (1) create the workspace WITHOUT those keys (create succeeds platform_managed), (2) PUT billing-mode=byok (per-tenant admin token already fetched at step 3), (3) write the deferred keys (now allowed). This mirrors the real BYOK user flow. Touches ONLY tests/e2e/test_staging_full_saas.sh — zero production .go changes, no workflow/gating/trigger changes. The strip-list mirrors secrets.go platformManagedDirectLLMBypassKeys. Untouched: - The platform path (E2E_LLM_PATH=platform) produces SECRETS_JSON='{}', carries no strip-listed key, so no opt-in fires — it stays platform_managed (the moonshot/kimi NOT_CONFIGURED regression guard). - The #1994 byok-routing guard (8c) runs AFTER the opt-in, so it sees a legitimately-byok workspace (explicit override) and still validates real routing (resolved_mode=byok) — not masked/weakened. bash -n + shellcheck -x clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…ary + SSOT model-discovery endpoint (core#2608) CTO 2026-06-11: 'teach the agent to look up what is available, and if it doesn't work it should hard fail — why does it even pass and create a not-working workspace.' 1. validateBYOKCredentialSatisfiable at Create (after the registered-model and derived-provider gates): a model deriving to a NON-platform provider with no accepted credential at workspace (payload secrets) OR org (global_secrets keys — the #711-revert/Reno-Stars contract) scope is rejected 422 MISSING_BYOK_CREDENTIAL before any row exists, with the same actionable message provisioning would emit on the dead node, plus a pointer to the SSOT platform default. Platform slash-form ids stay query-free; derive failures and registry/global-scan errors fail open (#1994 contract; the provision preflight remains the backstop). 2. GET /admin/llm/offered-models?runtime= — discovery derived per-id through the SAME DeriveProvider the gate enforces: model, owning provider, platform_billed, auth_env. The concierge looks up instead of guessing. Live trigger: enter-os first-run — concierge passed a BYOK-form claude id, create passed, provisioning failed MISSING_BYOK_CREDENTIAL seconds later, user saw dead red nodes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five tightly-related fixes surfaced while stress-testing org-template imports (Legal Team, Molecule Company) on a running control plane.
1. Org import was silently failing
INSERTwrotecollapsedinto theworkspacestable but that column lives oncanvas_layouts(005_canvas_layouts.sql). Every import returned 207 with 0 rows created, whichapi.posttreated as success → green "Imported" toast + empty canvas.canvas_layouts;workspace_crudPATCH path now UPSERTs there tooimportOrgTemplatethrows on2xx+errorbody so partial failures surface a red toast2. Multi-level nested layout was collision-prone
Children that were themselves parents (CTO → Dev Lead → 6 engineers) got the same leaf-sized grid slot as leaf siblings and clipped into each other.
sizeOfSubtree+ sibling-size-awarechildSlotInGridon both Go server and TS client (kept in sync via mirrored constants)buildNodesAndEdgesnow uses subtree sizes for parent dimensions AND the rescue heuristicsetCollapsedon expand reads each child's actual width/height instead of the leaf-count formula3. Provisioning-timeout banner was unusable during large imports
30-workspace tree triggered 27 simultaneous "stuck" warnings 2 minutes in (server paces + provision concurrency = 3 guarantee tail items legitimately wait longer).
base + 45s per queue slot beyond concurrency4. Auto pan-and-zoom on org ready
After the last workspace flips out of
provisioning, canvas fitView's with a 1.2s animation,0.25padding,maxZoom: 0.8,minZoom: 0.25. Without the zoom capsfitViewwas hitting the component'smaxZoom=2on small trees and zooming in instead of out.5. Toolbar compaction
+ N subcount was wrapping onto a second row on narrow viewports — addedwhitespace-nowrapand merged status dot + workspace total into one segmentaria-label(Figma/Linear pattern); Stop All / Restart Pending keep text because they're urgentAlso
api.{get,post,...}accept{ timeoutMs }so callers hitting intentionally-slow endpoints (org import paces 2s/sibling) don't trip the 15s defaultWorkspaceNodeclamps role text to 2 lines so verbose descriptions don't unboundedly grow card heightPARENT_HEADER_PADDINGbumped 44→130 to clear name + runtime + 2-line role + thecurrentTaskbanner that appears during initial-prompt phaseTest plan
pnpm vitest run src/)go test ./internal/handlers/)2xx + errorresponse from/org/importsurfaces as thrown errorsetCollapsedsizes expanded parent to fit nested-parent children🤖 Generated with Claude Code