Fix Split Canvas red-error flash and show background work in pane tabs - #1
Closed
ReganBell wants to merge 5 commits into
Closed
Fix Split Canvas red-error flash and show background work in pane tabs#1ReganBell wants to merge 5 commits into
ReganBell wants to merge 5 commits into
Conversation
The built-in magic-link broker cannot sign anyone in on a production deployment. Its call to POST /v1/auth/broker/claim is rejected with 401 "portal identity required", so every sign-in link fails to mint a code. The route is correct: it declares auth "source" and the gate reads that field on all three paths through the server. The failure comes from a second, independent check. Once REQUIRE_SIGNED_PORTAL_IDENTITY or production is on, the gate additionally demands a portal identity for any request that isUnclassifiedWrite reports, and that predicate treats a write as user-facing unless USER_SCOPED or SYSTEM names it. The broker's route is a service-to-service write that neither table listed, so it was misread as a user action and required an identity a plugin cannot hold. Add it to SYSTEM alongside the other plugin writes. The fix is deliberately not to let the broker mint a portal identity: that value asserts which human is acting, so a service able to forge it could impersonate any user, which would turn the sign-in service into an impersonation oracle. The gap survived review because the requirement is off by default. Unit suites, CI, and a local dev instance all run without it, so every test passed while the production path was broken. The regression test asserts the property rather than the single route, so a future broker endpoint that forgets the same classification fails immediately. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
A canvas refresh showed red errors in some panes, moving from pane to pane, clearing on their own after a few seconds. Nothing was failing. boot() fetches /api/runtime-config to fill the model menus, then mounts a chat — and the mount called refreshRuntimeSelection, which sets activeRuntimeConfig = null, redraws, and fetches the same URL again. For that whole round trip the composer rendered its pending state, and the pending state used .composer-error — color: var(--destructive). So a perfectly healthy mount painted red for one request. In a Split Canvas each pane is its own SPA boot, so the gap repeats per pane and lands staggered behind each pane's own /me -> runtime-config -> sessions chain. Hand boot's config to the composer and let the first mount in that scope render from it (consumed once; a later mount revalidates, and another scope still fetches its own). Render a load still in flight as .composer-note, keeping the error's red and its Retry for a real failure. A personal mount passes scopeId: null and the surface reads scopeId ?? personal:<user>, so both sides normalize through runtimeScopeKey — compared raw, the seed missed and a blank pane refetched the identical config. The same refresh also read as the page loading twice: the shell painted the single-chat empty state, then a session-list round trip later the canvas replaced it wholesale and only then did its iframes begin their own boots. boot() mounted the canvas after `await refreshSessions(...)`, but a pane's iframe src comes from the persisted layout, not the list — every pane was waiting on a request none of them reads. Mount on the bare-URL entry before the fetch instead, and make mountRestoredCanvas idempotent so the post-await call reports the live canvas rather than falling through to newChat(). Mounting the canvas before boot awaits the session list hands it to the user while that fetch is in flight — and a canvas is interactive. Closing down to one pane in that window runs reconcileAfterClose, which exits the canvas and mounts the survivor itself; mountRestoredCanvas() then correctly reports false and the unguarded newChat() replaced the conversation the user had just been handed. Gate the fallback on nothing being mounted: mountRestoredCanvas() alone cannot tell "no canvas to restore" from "the user just left the canvas on purpose"; chatState.threadRef can, and every user-driven mount sets it. Measured against a dev build with injected API latency: on a 12-pane refresh, twelve red banners spread over 5.4s become zero and /api/runtime-config drops from 25 requests to 13. On a 4-pane canvas, frames start at 0.9s instead of 1.3-4.0s and the canvas settles in 5.5s instead of 6.0-8.6s.
The Recents row and the Chats page badge a conversation that has detached work parked in it — running background jobs plus armed watches, folded into one chip by backgroundLabel. A pane's tab is that conversation's header once the pane is stacked behind a tab, and it showed only the working and awaiting dots, so work running in a tabbed-away conversation was invisible until you clicked the tab. The tab now renders the same chip through conversationBackground — the derivation the in-pane strip already uses — and computeHeaderSignature counts the background label, so a tab redraws when work starts or ends instead of holding a stale mark. The chip's pointer cursor and hover wash move to .bg-chip[role="button"]: only the sidebar's chip opens the inspector, while the tab's is a mark on a control that is itself clickable.
The mention-arming test seeded its name index with real people's first names, full names and a real surname. Test fixtures are the wrong place for them, and this repository is meant to read the same to everyone who clones it. Swap in placeholder identities. The cases are unchanged: an unambiguous handle still arms, a longest full-name match still beats the handle, a non-ASCII name still arms, a shorter candidate followed by a capitalized word still stays plain, and code spans, URLs, scoped package paths and link labels stay inert.
7 tasks
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
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.
Master spec: nothing. The spec no longer lives in this repo.
Two web-UI fixes and one test-fixture cleanup.
A Split Canvas refresh paints red errors in healthy panes
boot()fetches/api/runtime-configto fill the model menus, then mounts achat — and the mount called
refreshRuntimeSelection, which nullsactiveRuntimeConfig, redraws, and fetches the same URL again. For that wholeround trip the composer rendered its pending state, and the pending state used
.composer-error—color: var(--destructive). So a perfectly healthy mountpainted red for one request. In a Split Canvas each pane is its own SPA boot,
so the gap repeats per pane, staggered behind each pane's own
/me → runtime-config → sessionschain: some panes red at any instant, redmoving around, everything clean once the last one lands. Nothing was failing.
Hand boot's config to the composer and let the first mount in that scope render
from it. Render a load still in flight as
.composer-note, keeping the error'sred and its Retry for a real failure.
The same refresh also read as the page loading twice:
boot()mounted thecanvas after
await refreshSessions(...), but a pane's iframe src comes fromthe persisted layout, not the list — every pane was waiting on a request none
of them reads. Mount on the bare-URL entry before the fetch instead. That hands
the user an interactive canvas while the fetch is still in flight, so the
fallback is now gated on nothing being mounted: closing down to one pane in
that window runs
reconcileAfterClose, and the previously unguardednewChat()replaced the very conversation the user had just been handed.Measured against a dev build with injected API latency: on a 12-pane refresh,
twelve red banners spread over 5.4s become zero and
/api/runtime-configdropsfrom 25 requests to 13. On a 4-pane canvas, frames start at 0.9s instead of
1.3–4.0s and the canvas settles in 5.5s instead of 6.0–8.6s.
Show a conversation's background work in its pane tab
The Recents row and the Chats page badge a conversation that has detached work
parked in it. A pane's tab is that conversation's header once the pane is
stacked behind a tab, and it showed only the working and awaiting dots — so
work running in a tabbed-away conversation was invisible until you clicked the
tab. The tab now renders the same chip through
conversationBackground, andcomputeHeaderSignaturecounts the background label so a tab redraws when workstarts or ends instead of holding a stale mark.
Before / after, captured against synthetic data:
Neutral names in the mention-arming fixtures
test/slack-mrkdwn.test.tsseeded its name index with real people's firstnames, full names and a real surname. Placeholder identities instead; every
case is unchanged.
Verification
npm test(3553 pass, 0 fail),plugins/web-uitests (439 pass, 0 fail),typecheck,lintandformat:checkall clean.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.