Files
platform/docs/workspace-panel-todo.md
T

67 KiB
Raw Blame History

Workspaces & Panels — working TODO

Living list. Add items as they are found, tick them as they land, and write the resolution into the item rather than deleting it — the reason a thing was done is worth more later than a clean list. Move anything fully settled to §7.

How the framework actually works is documented separately, in workspace-panels.md — read that first if you are new to it. This file is only the defect list and the work queue.

Why any of it matters is in agent-coordination.md — the north star.

This list stays ordered by defect severity. The re-rank against the objective lives in agent-coordination.md §6 (done 2026-08-07), and the two orderings deliberately disagree — read §6 before deciding what to pick up next. Its conclusions in one paragraph:

  • Tier A, on the critical path: the swallowed persist failure (§1), the PATCH dispatcher's missing else (§2), validate-on-read + the '[]' layout default + an error boundary (§4), the two resize-debounce lost-update items (§5.5), the never-invalidated cache (§5.5), and tests for layout-utils.ts (§9). Five of the seven are one defect — a write that silently did not land — and they matter because panel identity now lives in the layout jsonb. Tier A is empty as of 2026-08-07. §1's .catch(() => {}) (70c2f08), all of §2 and §3's missing userId predicate (f4ed740), §9 (85452d1), all three Tier A items in §4 — the error boundary (64961d4), validate-on-read and the '[]' layout default (ef036df) — and §5.5's three: the two resize-debounce lost updates (b0a32ae) and the never-invalidated cache (81ad3ef). Everything the re-rank put on the critical path for "the human opens the dashboard in the morning and sees the result" has landed. Pick up from Tier B.
  • Tier B: dashboardId-as-a-bag (§5.9) — the agent address book is keyed on it; panel lifecycle (§5.1), but inverted: the requirement is that closing a chat panel must not destroy the agent; normalizeLayout as framework (§5.4); and the mobile-collapse decision (§6), which swings on one unanswered question. §5.9 is closed (dbe585f, 717580f, 585f234, d3922bd, ca046a3) — the bag is a parsed identity, the app-config fields are deleted or moved onto the app, and the inert context is built once. §5.1's first two items are closed too (04371a9, 198dc71, c92b51c) — panels have a close signal that fires on a real close and on nothing else, and the terminals use it; the chat panel deliberately does not, which is the inversion above. §5.4's first item is closed (1de1d92) — the allow-list is a WorkspaceView prop and the fourteen copies are gone. Tier B is now the pty backstop (the third item of §5.1) and the mobile decision (§6) — both of which are blocked on a question only the owner can answer, not on work. §5.4's second item was inventoried and dropped as compression rather than deduplication; see the item for the reasoning and the three defects the inventory turned up instead. Tier B has no unblocked work left.
  • Tier C, orthogonal: everything else — including §5.2 (remounts) and §5.3 (drag-to-move), both large downgrades. A panel is now a pointer to a server-side session, so a remount costs a replay, and the drag hazard is disarmed by resolving identity by name (e588524 + bc82086).
  • And the result that matters: no item in this file blocked building the MVP. It was built and ran unattended on the framework as it stands. Stop treating this list as the prerequisite queue for the objective.

Findings and full reasoning: COMMS/workspace-panel-framework-analysis-2026-08-07.md. Every file:line below was opened; DB claims were run against live officer_dev. Paths are relative to platform/src/workspaces/officerdev/src/ unless they start with servers/, databases/ or sidecars/.

Framework core: components/Workspace/ — 12 files, 1,533 lines. No tests exist for any of it.

Two open decisions gate real work — see §6. Don't start item 4.1 or 5.3 before those are answered.


1. Observability first — do these before anything else

Both are two-line fixes, and without them you cannot tell whether any later fix worked. Both are done as of 2026-08-07 — they went in with the terminal close wiring (§5.1), which is what they were blocking: there was no way to see whether a shell had actually been killed. §1's third item (the swallowed persist failure) was already closed. §1 is empty.

  • Fix the Running Shells paths — the panel has 404'd since 2026-07-31. — done c92b51c. apps/Terminal/RunningShells.tsx:43,49 call /terminal/sessions and DELETE /terminal/sessions/:id. useClient prefixes /api; servers/sidecar/create-proxy.ts:69 strips the mount prefix, so the sidecar receives /sessions. The pty sidecar serves only /_officer/sessions (sidecars/pty/server.mjs:30) and /_officer/sessions/:id (:34) — everything else 404s at :40. Correct paths: /terminal/_officer/sessions and /terminal/_officer/sessions/:id. The panel (fccf212) predates the proxy move (7129cd8) that deleted the old servers/api/terminal/router.ts and was never updated. Symptom today: permanently reads "No shells running", kill button is a silent no-op. Confirmed against the running sidecar before fixing: GET /sessions{"error":"not found"}, GET /_officer/sessions → the list. Both paths corrected.

  • Surface the clients count in Running Shells. — done alongside the paths. RunningShells.tsx dropped the clients field the sidecar returns (sidecars/pty/sessions.mjs:169) — the one field that distinguishes an orphan (clients: 0) from a live shell. Now on the type, rendered as "N attached", and clients === 0 also earns the row an orphan badge, since the count is only useful if you do not have to read it to notice.

  • Stop swallowing persist failures. state/src/useDashboardState.ts:46 is .catch(() => { }). Every 500 in this document is invisible because of it — the optimistic cache keeps the UI correct until reload. At minimum log; better, surface a toast and roll the cache back. Tier A1 — the single highest-value item in this file against the objective (agent-coordination.md §6.2). A panel's config.agentName is written through this path, so a swallowed 500 leaves a panel that shows its name, answers to its name, and forgets it on reload: invisible for exactly as long as nobody is looking, which is the window the whole project serves. Resolved 70c2f08 — both catches now console.error and raise a toast naming the key, and roll the cache back. The rollback is a compare-and-swap: it only reverts if the cache still holds exactly what that call wrote. Writes to one key overlap freely (a window resize fires one per group), and a blind rollback over a later successful write would turn one failure into two. The other three, not through this hook, resolved 81ad3ef: the direct client.patch('/dashboards', …) calls in apps/Dashboards/DashboardListApp.tsx:93 and DashboardPreview.tsx:323,326,356 — dashboard create, rename and delete, which build multi-key patches and so bypass useDashboardState. They now call persistDashboardState, which shares the hook's in-flight bookkeeping and, on failure, invalidates rather than reverts: once the roster has been rewritten and keys dropped from the blob there is no single previous value to swap back, and a refetch is the only thing that makes the list agree with the server. A failed delete used to leave the dashboard gone from the list and alive on the server, reappearing at the next reload with no hint why.


2. Live data loss

All four resolved in f4ed740 — they were one defect with four faces, so they were fixed as one change. Each item keeps its diagnosis below and records what was done. Verified against the live server: an unknown key 400s, the three prefixes round-trip through a GET, a null on a live dashboard is a no-op, and the rename sequence leaves workspaces with no zombie.

  • The PATCH dispatcher silently drops three key families in active use. servers/api/dashboards/dashboards.ts:26-88 is a chain of if (match) { …; continue; } that ends with no else — unmatched keys are dropped and the request returns 200 with a fresh state blob. Matched: workspaces, ws-layout-*, ws-terminals-*, ws-host-terminals-*, screens/*. apps/Terminal/CommandTerminalWrapper.tsx:18 writes ws-${statePrefix}-${dashboardId}, and the registered prefixes (apps/Terminal/index.tsx:22,27,34) are tmux, nvim, claude-code. So the panel→session map for those three survives only in the React Query cache, for the life of the tab. Every reload mints a fresh uuid and abandons the previous pty — alive, unreachable, never killed. (Tmux's own state survives via new-session -A -s off-<panelId>; the shell running tmux attach does not.) Two parts: add the three families, and add a fallback else that 400s on an unknown key. Tier A2 — the server half of A1. ws-layout-* is matched, so panel config persists today (verified: the agent-mvp layout round-trips with config: {agentName: …} intact). The missing else means the next key family added for coordination is a silent no-op returning 200. Resolved. The three maps live in a new dashboards.panel_state jsonb bag keyed by prefix ({tmux: {panelId: sessionId}, …}), with dashboard_defaults.panel_state for panels on a screen. A bag rather than three columns because the fourth prefix should not need a schema change; an allow-list rather than a regex because ws-<prefix>-<id> cannot be split without one — both halves may contain dashes. PANEL_STATE_PREFIXES in servers/api/dashboards/dashboards.ts must stay in step with the statePrefix props in apps/Terminal/index.tsx, and the new fallback else — a 400 naming the key — is what tells you when it does not. Living on the dashboard row also means the maps are deleted with it.

  • ws-terminals-{id}: null on a live dashboard is a 500. Same file, :61-66 — the ws-layout-* branch has a value === nulldeleteDashboard case (:42); the terminals branches do not. A null falls to the UPDATE branch and sets a NOT NULL column (databases/officer_db/src/queries/dashboards.ts:70) → 23502. Resolved. A null on either terminals branch is now a no-op: it means "forget this key", and it only ever arrives paired with ws-layout-{id}: null on a rename, by which point the row is gone.

  • HostTerminalWrapper never strips the prefix, so it mints phantom dashboards. (found 2026-08-07, latent — dashboards is still 0 rows) apps/Terminal/HostTerminalWrapper.tsx:12 is `ws-host-terminals-${dashboardId}` with no regex, while its sibling TerminalWrapper.tsx:13-14 correctly matches ^ws-layout-(.+)$ first. So the key becomes ws-host-terminals-ws-layout-<id> or ws-host-terminals-screens/terminal, the dispatcher's ^ws-host-terminals-(.+)$ branch captures that whole string as an id, and upsertDashboard inserts a row when the id is unknown (queries/dashboards.ts:75-86, name: data.name ?? id). getAllDashboardState maps every dashboards row into workspaces, so the garbage id surfaces in the Dashboards list as a real dashboard. Two apps deriving a state key from the same string by two different rules is the actual defect; the id-shaped-string-as-a-bag problem behind it is §5.9. Latent only because officerdev/terminal-host is in no default layout and hidden from the picker. Resolved. All three wrappers now call terminalStateKey(prefix, dashboardId) (apps/Terminal/state-key.ts) — TerminalWrapper's rule, extracted, with the reasoning beside it. Belt and braces on the server: the terminals and panel-state branches use a new updateDashboard that will not INSERT, and 404 on an unknown id instead of creating one. §5.9 is still the real fix; this closes the hole a mis-derived key could fall through.

  • Renaming a dashboard resurrects it as a zombie row. apps/Dashboards/DashboardPreview.tsx:316-318 PATCHes ws-layout-old: null and ws-terminals-old: null together. The first deletes the row; the second then calls upsertDashboard, finds nothing, and re-INSERTs it with name = id. The old slug reappears in workspaces on the next GET as a duplicate. Fixed by the item above, but verify this specific sequence after fixing. Resolved, and verified as the specific sequence: create zombie-test, then PATCH {ws-layout-renamed: …, ws-layout-zombie-test: null, ws-terminals-zombie-test: null, ws-host-terminals-zombie-test: null} in one body. workspaces comes back as ['agent-mvp', 'renamed'].


3. Multi-user correctness — before another member creates a dashboard

dashboards is empty (0 rows) today, so none of this has fired yet. Members can now sign in (d8ee678), so it is a matter of time. Note TODO.md's preamble still says the platform is collapsing to single-user — that predates the capability permission model and should not be used to deprioritise these.

  • dashboards.id is a global primary key but ids are slugify(name). databases/officer_db/src/schema/dashboards.ts declares id: text('id').primaryKey(). Live: "dashboards_pkey" PRIMARY KEY, btree (id) plus a redundant "uq_dashboards_user_id" UNIQUE, btree (user_id, id) — evidence per-user ids were intended and half-built. Ids come from DashboardPreview.tsx:300 (slugify(trimmed) || generateSlug()) and the uniqueness loop at :340 checks only the caller's own dashboards. Two users both naming a dashboard "Work" → both get id = 'work' → the second violates the PK → 500 → swallowed → the dashboard shows, then vanishes on reload. Fix: composite PK (user_id, id), or uuid ids. Composite PKs have a known drizzle re-diff quirk (see databases/CLAUDE.md → "Composite keys") — harmless churn, but read the plan.

  • upsertDashboard's UPDATE has no userId predicate. databases/officer_db/src/queries/dashboards.ts:73db.update(dashboards).set(set).where(eq(dashboards.id, id)). The existing lookup above it is scoped, so it cannot reach another user's row today, but it is a non-transactional read-then-write. It becomes a live cross-user overwrite the moment the PK above is made composite. Do both in one change or the first fix opens the second. Resolved f4ed740, incidentally. upsertDashboard is now updateDashboard (scoped by (userId, id), RETURNING to say whether it matched) with an INSERT only when that returns nothing — so the read-then-write is gone as well as the missing predicate. The composite-PK item above is still open, and is still the one that matters; this just no longer opens a second hole when it lands.


4. Resilience — one bad row is currently a white screen

The first three items are Tier A (agent-coordination.md §6.2, A3A4). This is the last mile of the objective: work proceeds overnight, and the human's only act is to open the dashboard and look. A malformed row turning that into a white screen loses the result at the one moment it is consumed — and config added a new unvalidated field to an already-unvalidated tree. The work itself is safe in chat_session_events, which is why recovery must offer "restore default layout" rather than SQL.

§4 is closed64961d4 (the boundary), ef036df (validate-on-read and the '[]' default), 92b8905 (validate-on-write). "A layout is an object" is now stated in four places: the column, the route, the read, and the renderer's willingness to survive being handed something else anyway.

  • Add an error boundary. grep -rln "componentDidCatch\|getDerivedStateFromError\|ErrorBoundary\|errorElement" src returned nothing across the whole repo. Resolved 64961d4 — two of them, because "recover" means different things at different depths. components/ErrorBoundary.tsx is a generic render-prop class (resetKeys so a boundary cannot latch); Screens/Dashboard/Layout/DashboardLayout.tsx wraps the routed screen with the dock and header deliberately outside it, so navigating away is itself a way out, and its fallback offers a two-click reset of every screens/* layout — dashboards are left alone, being user-created and holding content. PanelSlot.tsx wraps each app so one bad panel leaves the rest of the workspace running, with "clear this panel" as its recovery, offered only when the layout is the user's to edit. Not a targeted "reset this screen": React unmounts the subtree before rendering a fallback, so an effect-based registry of "which layout key was I rendering" is empty at exactly the moment you need it.

  • Validate on read. useDashboardState.ts:36 was key in state ? state[key] as T : defaultValue — an unchecked cast over a value that arrived as unknown from jsonb. Resolved ef036dfreadValue() checks kind-compatibility before casting and falls back to the caller's default, warning once per key. Not isLayoutNode(): all 24 layout call sites would have had to pass a predicate, and the same wrong-container bug applies to the panelId → sessionId maps. So the rule is generic and only guards object-shaped defaults — a wrong primitive is a cosmetic surprise, a wrong container is a crash. It deliberately does not write the correction back; a read should not overwrite the server, and the next real setValue repairs the row.

  • layout columns default to '[]', which is not a valid LayoutNode. schema/dashboards.ts:13,31 — an empty array for a column holding an object. Any path that upserts without a layout (e.g. the workspaces branch) writes it; key in state is then true, so the stored [] wins over the caller's default and normalizeLayout calls .children.map on it and throws. Creating a dashboard from the dashboard list is exactly that path, so this was reproducible, not theoretical. Resolved ef036df — both columns are nullable with no default (NULL means "none stored", which is the truth), and getAllDashboardState omits the key when what is stored is not an object, so a row written before this is repaired by the next write instead of crashing the read. Verified live: creating a dashboard with no layout emits no ws-layout key, and a row hand-set back to '[]' is omitted too. bun db:push planned exactly the four expected ALTERs plus the two known-harmless pk_music_now_playing lines.

  • Validate on write. The whole path is unknown: useDashboardState.ts:46servers/api/dashboards/dashboards.ts:45,83queries/dashboards.ts:51,97 → cast as never at :114 to satisfy drizzle → jsonb. No zod, no CHECK on either layout column. Given databases/CLAUDE.md's stance that the schema is the source of truth for contents, a CHECK on jsonb_typeof(layout) = 'object' is the cheap half. Resolved 92b8905 — both halves, because they do different jobs. ck_dashboards_layout_object / ck_screens_layout_object are the backstop: nothing reaches those columns without passing them, including a psql session or a future writer nobody has written yet. NULL stays legal — it means "none stored", which is the honest state of a dashboard created without a layout. But a constraint violation surfaces as a 500 that tells the caller nothing, so the PATCH dispatcher checks the same rule first and answers 400 "ws-layout-x" must be a layout object. Deliberately not a full LayoutNode schema: this is the wrong-container class of bug — the one that crashes normalizeLayout — and a structural validator would need the client's zoom/config vocabulary in the database layer to say anything more. Verified live: db:push planned exactly the two ADD CONSTRAINTs plus the known pk_music_now_playing churn and was stable on a second run; update … set layout = '[]' and = '3' are both rejected by Postgres; array, number and string bodies to ws-layout-{id} and screens/{name} all 400, and an object still round-trips.

  • A bare number is squatting in a framework namespace. (verified in the live DB) apps/Soulseek/shared.ts:12 built screens/soulseek-zoom/${panelId} and passed it to useDashboardState<number>. The server routes ^screens/(.+)$ into screens.layout, so: user_id 1 | soulseek-zoom/soulseek-view | jsonb_typeof = number. That namespace belongs to layouts. Resolved ba664dc — Soulseek's private zoom became a framework feature (LayoutPanel.zoom), so the scalar now rides on the layout node inside screens/soulseek-v2 and needs no key of its own. The squatting row is gone: select name from screens returns 15 rows, none soulseek-zoom/*. This is also the general answer for per-panel scalar prefs — put them on the node, not in a key, because useDashboardState seeds a row per key on mount.


5. The framework work proper

5.1 Give panels a lifecycle — the highest-value change here

  • A close hook, invoked by the mutators, not by unmount. — done 198dc71. components/Workspace/types.ts:38-47 had no close hook; the only onClose was PanelComponentEntry.onClose (:55), which is a header-button handler for ephemeral panels and is suppressed on mobile (PanelSlot.tsx:382). So the two ways a panel dies — removePanel(root, id) and setApp(root, id, null) (the red traffic light, PanelSlot.tsx:295) — were pure tree rewrites that notified nobody.

    Shipped as `usePanelClose(panelId, handler)` rather than a registry field: what needs releasing is
    known to the component, not to the registry entry, and `CommandTerminalWrapper` shares one registry
    key across several panels with different sessions. `WorkspaceView` fires it from `handleRemove` and
    from `handleSetApp` when the app actually changes, and from nowhere else. Registration is never
    torn down — honouring an unmount would reintroduce the exact bug — and handlers are stamped with
    the workspace they were registered on so one dashboard's panel id cannot fire another's.
    
    **Strike the implementation this item used to suggest** ("diff removed panel ids old-tree 
    new-tree"). It does not work, and `layout-utils.test.ts` now pins why (`04371a9`): `movePanel`
    inserts through `newPanelFrom`, which mints a fresh `uid()`, so a dragged panel's id is absent from
    the new tree while its app is still on screen; and `swapPanels` exchanges `{appType, config}`
    between two ids that both stay put, so a swap reads as two closes. **A panel id is a position in
    the tree, not an app instance**, and a diff of positions cannot answer a question about instances.
    The second half of the suggestion — have the mutators report it — is what landed, in the form of
    firing at the call site that already knows the user asked for a close.
    
    This is the thing `apps/Terminal/TerminalWrapper.tsx:31-38` asked for in writing: *"Killing on
    unmount is not an option until the panel system can tell a real close from an incidental
    remount."* It closes the terminal orphan leak as a consequence rather than as a special case, and
    the same gap affects every panel holding a server-side resource.
    
    **The requirement inverts for a chat panel** *(2026-08-07)* — do not wire one into this hook
    without deciding first. `agent-coordination.md` §5 Q2: idle reaping deliberately leaves the
    `sessionKey → claudeSessionId` pointer intact, but the explicit `disconnect` path calls
    `clearClaudeSession`, which destroys it and orphans the transcript. A panel is a pointer to a
    server-side session; **closing the window must not delete what it points at.** What coordination
    wants from `onClose` is a guarantee that nothing rides the disconnect path, not an eager cleanup.
    `ChatPanelWrapper` therefore does **not** call `usePanelClose`, and that is a decision, not an
    omission — the hook's own doc comment says so, so the next person to add one reads it there.
    
  • Then: kill the pty on real close. — done c92b51c. The three wrappers now share useTerminalSession, which drops the map entry and DELETEs the session from usePanelClose. Two things fell out of doing it. HostTerminalWrapper still had the unmount cleanup the other two had removed, so it had been orphaning a host shell on every layout or route change. And RunningShells — the panel that exists to make orphans visible — was calling /terminal/sessions, while the proxy strips /api/terminal and the sidecar only answers under /_officer; verified live, /sessions returns {"error":"not found"}. That panel had always read "No shells running" and its kill button had always been a no-op.

  • Then: reap orphans in the sidecar as a backstop. sidecars/pty/sessions.mjs:127-130 detach() never checks clients.size === 0; there is no idle timeout and no session cap. lastActivityAt is written (:110) and displayed (:167) but never read by a timer. Each orphan costs a SHELL -i with the owner's full env plus up to BUFFER_MAX = 512 KiB (:10) — bounded per session, unbounded in aggregate. Note: sidecars/ is platform, so in scope — but confirm before touching the pty protocol.

5.2 Stop the avoidable remounts

Tier C — the largest downgrade in the re-rank (agent-coordination.md §6.4). A remount used to threaten whatever the panel was holding; a panel now holds nothing. A chat panel that remounts re-runs resume-cursor against the durable log and replays, costing latency, which §2 of the north star declares free. Fix these for the interaction quality they are genuinely about — scroll position, media playback, transcodes — not as a prerequisite for agent coordination.

A panel's React identity is its position plus key={child.node.id} on its nearest ancestor group slot (WorkspaceRenderer.tsx:178,204) — the panel's own id is a key nowhere. That's the root cause of the whole table:

operation remounts? why
split, direction matches parent no layout-utils.ts:34-44 splices children through by reference
split, direction differs / leaf yes :20-30 wraps in a new group with a fresh uid()
split the root when root is one panel yes, everything WorkspaceRenderer.tsx:40 renders one unkeyed child whose type changes
remove from a 2-child group yes — the survivor :68-70 collapses the group, re-keying the sibling
swap apps remount setApp twice; the component type at each slot changes
resize / maximize no ids preserved; maximize is a CSS toggle
mobile panel switch yes, by design only the active child renders
any ephemeral panel on mobile yes — every panel WorkspaceView.tsx:165 replaces the workspace instead of overlaying
viewport crossing 768px yes, full useIsMobile flips the branch at WorkspaceView.tsx:165
  • Reuse the panel's id when wrapping it in a new group, so the ancestor slot key doesn't flip. Kills rows 2 and 3.
  • Don't re-key the survivor when a group collapses (layout-utils.ts:68-70, 83-88). Kills row 4.
  • Overlay the mobile ephemeral panel instead of replacing the workspace (WorkspaceView.tsx:165). Affects /files, /email, /chat, /browser, /dashboards.
  • Reference for how it should feel: maximize (PanelSlot.tsx:430-457) is a CSS state toggle on the same element — no portal, no remount, scroll position and media playback preserved. Every mutator should be held to that.

5.3 Drag-to-move: finish it or delete it

  • Decide, then act — do not patch it. dragSourceId can never become non-null: the only code that would set it is commented out at PanelSlot.tsx:240-256. So the whole path is dead: DragOverlay.tsx (99 lines, imported by nothing), LayoutEditor.tsx (51, imported by nothing), movePanel + insertPanel + DropPosition (layout-utils.ts:142-169), and three context fields. ~180 lines. If finishing it: movePanel mints a brand-new panel id, so it destroys all panel-keyed state, and PanelContents (layout-utils.ts:229) carries only {appType, config} — so a move silently drops fitContent and zoom. Fix before re-enabling. Downgraded 2026-08-07 — this was on the critical path in agent-coordination.md §1.5 (P3: "dragging a panel would silently sever its session binding") and no longer is. Q3 made the agent's name the address and the panel id merely where it lives, and e588524 made move and swap carry config with the panel. useAgentPanel resolves by name and re-anchors the row afterwards. Still worth deciding — it is dead code — but it gates nothing. See agent-coordination.md §6.4.

5.4 Make normalizeLayout framework, not convention

  • One normaliser applied inside WorkspaceView. (1de1d92) — it landed as a prop rather than a factory: <WorkspaceView appTypes={{ allowed, fallback }} />, with normalizeLayout in layout-utils.ts and the persist-back inside WorkspaceView. All fourteen copies are deleted; QrTransfer gained the persist-back it never had. Two notes for whoever reads this next. The framework normaliser drops config when it replaces an app — the copies kept it, which left the old app's config on a panel the new app now owns, the opposite of setApp. And Headscale keeps a local useMemo, because its check is "is the server picker present at all", which is about a panel being missing and no allow-list can see that. Still open from this item: deriving the allow-lists from the AppRegistry (they are still hand-written literals, so a type still allow-listed but deleted from the registry passes and reaches the unrecoverable empty box at PanelSlot.tsx:311-317). The unguarded screens are done (653201f): Dashboards, Desktop, Terminal and Files now pass one, checked against what is persisted rather than against their defaults; Browser and Email deliberately do not, because their panels resolve through components, which PanelSlot keys on the panel id, so their appType is never read. Home and the two dashboard screens are not locked and stay open. The original text follows.

    **One `makeLayoutNormalizer(allowed, fallback)` applied inside `WorkspaceView`.**
    The guard currently exists **14 times, character-identical except for the allow-list and the
    fallback**, plus a matching `useMemo`/persist-back `useEffect` pair 13 times — ~150 duplicated
    lines. Consequences of it being convention: - **10 `WorkspaceView` consumers have no guard at all**, including `screens/HomeScreen.tsx:11`,
    which also omits `locked` — the one screen where a user can set any registry appType with
    nothing pinning it back. - Allow-lists are hand-written literals, never derived from the AppRegistry. They catch a
    _renamed_ appType; they do **not** catch one still allow-listed but deleted from the registry —
    that passes and reaches `PanelSlot.tsx:311-317`, which on a `locked` screen renders an empty
    teal-bordered box with no picker and no way for the user to recover. - `screens/QrTransferScreen.tsx:19-39` has the guard but no persist-back, so it re-normalises on
    every mount forever and never heals the row.
    
  • Then collapse the three default-layout mechanisms — inventoried and dropped. Per-screen defaultLayout.ts (21, not 20 — Home/defaultLayout.tsx is misnamed), createDefaultLayout() in the core, and the 6-entry template array at DashboardPreview.tsx:33-142.

    They are not three copies of one mechanism. `createDefaultLayout()` is a one-liner meaning "the
    empty state of a workspace" and the templates are the picker for a *user-created* dashboard,
    where the user chooses the apps afterwards. The 21 files are *built-in locked screens*, where
    the apps are fixed. Collapsing across that line would merge two different questions.
    
    Nor are the 21 duplication. Fourteen share a shape — flat horizontal split, nav on the left —
    but across seven different size pairs, with panel ids that other code hardcodes (`components`
    maps, `mobilePanelId`, `ChatPanelWrapper panelId=…`, and now the `appTypes` allow-lists). A
    `splitH(...)` helper would turn 11 lines into 1 without removing a single decision: still 14
    call sites, still 14 argument sets, plus a layer between "what does /music look like" and the
    answer. That is compression, not deduplication.
    
    What the inventory did turn up, none of which is an abstraction:
    - **`DashboardPreview`'s edit path reset the layout on every submit** — renaming a dashboard or
      editing its description threw away the panel arrangement, and dropped the terminal maps on a
      rename. Fixed and verified against the running server (`cd1f161`).
    - `Home/defaultLayout.tsx` is a `vertical` group with one child at `size: 100` — a bare panel
      with a wrapper that does nothing, unlike Files/Terminal/Desktop next to it. Cosmetic: the
      persisted `screens/home` row is what actually renders, so changing it changes nothing today.
    - Browser and Email are the one genuine copy-paste pair (same nesting, same 25/75 and 60/40,
      same trailing chat panel). One duplicate, and the two screens are diverging anyway.
    

5.5 Persistence hygiene

Three Tier A items live here (agent-coordination.md §6.2, A5 and A7). The two lost-update bugs are worse than described now that identity is in the layout: a debounce timer closing over a pre-drag tree does not just resurrect a deleted panel, it re-writes an older config, so a panel that was just named reverts to anonymous. And the never-invalidated cache means two windows onto the same server-side work disagree permanently about the roster, with neither told — a direct contradiction of §5 Q1.

  • The resize debounce can resurrect a deleted panel. WorkspaceRenderer.tsx:108-124 held a 500 ms timer in a ref with no useEffect, therefore no cleanup, and its callback closed over the layout as it was when the drag began. Drag a splitter, remove a panel within 500 ms → the timer PATCHed the pre-deletion tree and the panel came back. Same shape for a split. Resolved b0a32ae — the timer now lives behind a useEffect that flushes it on unmount rather than dropping it: with the write expressed as an updater the early write is correct, and dropping would lose a splitter drag made just before navigating away, which the no-cleanup version did at least persist.
  • Concurrent same-key writes clobber. A window resize fires onLayout on every group at once; each scheduled its own timer against the same base tree and the last won (WorkspaceView.tsx:76-81). Resolved b0a32ae — all eight mutations in WorkspaceView pass an updater to setValue (onLayoutChange((prev) => …)) instead of a tree computed from the layout their callback closed over, so each composes against the current cache. The no-op guards (countPanels, the === layout identity checks) still test the rendered layout: they only decide whether a write is worth making, and being one render stale there costs a redundant no-op write at worst. Worth keeping in mind for anything new that writes the layout: compute from prev, always. The cost of getting it wrong is no longer a size that snaps back — it is an older config, so a panel that was just given an agent's name reverts to anonymous and the agent stops being addressable.
  • The cache is never invalidated. staleTime: Infinity and no invalidateQueries anywhere in the repo. Meanwhile every PATCH computed and returned a full fresh state blob which the client discarded — 3 SELECTs per splitter release, thrown away. Resolved 81ad3ef — both halves. The PATCH returns {ok: true}; nothing had ever read that body, and a caller that did would be reading state assembled before whatever concurrent write it raced. The client refetches on focus, with three non-default guards, because this cache is optimistic: a refetch that started before an in-flight PATCH landed would overwrite the value already on screen — the same lost-update shape as the two items above, and self-healing only until the next mutation composes on top of the stale tree. So refetchOnMount: false (splitting a panel mounts a fresh consumer, which is exactly when a write is in flight), refetchOnReconnect: false, and refetchOnWindowFocus gated on a module-level in-flight count plus a 2 s quiet period.
  • Preserve sibling sizes on split. splitInner/insertPanel redistribute evenly (100 / newChildren.length), so one split discards carefully tuned proportions. Resolved abea7a3 — the new sibling takes half of the target's size and nothing else moves. One helper serves both call sites, because the drop path (movePanelinsertPanel) carried the identical bug. Two of the three tests were already in layout-utils.test.ts asserting the even split, written to the old behaviour deliberately; they now assert the new one. The third documents the interaction worth knowing: removePanel renormalises the group as the panel leaves, so a move reads as renormalise-then-halve, not halve alone.
  • tpl-N panel ids collide across dashboards. DashboardPreview.tsx:25-26let tplCounter = 0; const tplUid = () => \tpl-${++tplCounter}`— module-level, no entropy, resets every page load. Two dashboards created from templates after a reload hold panels with **identical ids**, and panel id keysterminal-conn-${panelId}andfile-viewer:${panelId}. **Resolved 6fd60e5** — the templates now call the core's uid(), which is exported from the Workspace barrel for the first time so that there is exactly one way to mint a panel id. The duplicate minter is deleted rather than fixed: a second implementation of "make me an id" is how this happened, and the collision was no longer only a settings mix-up — agent_panelsaddresses a panel by(dashboardId, panelId)`, so two dashboards built from templates in the same page load could hand two different agents the same address.

5.6 Registry

  • No duplicate-key guard. metasToRegistry is Object.fromEntries (AppRegistry/useAppRegistry/useAppRegistry.ts:19) — a collision silently last-wins and one app just disappears. Resolved 6fd60e5, but not by throwing, which is what this item asked for. A duplicate key is an authoring mistake made at edit time; throwing punishes the owner at runtime for it, by taking down every dashboard rather than one app. So it is a test over the real meta list (AppRegistry.test.ts) plus a console.error at runtime — the mistake is caught before it ships and named if it somehow does. Confirmed: all 44 keys are unique today, and the test says so. Getting the real list into a test needed one thing beyond exporting it: test-setup.ts was not providing localStorage, and MusicPlayer/useLyricsOpen.ts reads it at import time, so the whole app graph was unimportable from a test. That is now fixed, which unblocks testing anything else that pulls in a panel app.
  • Seeding depends on undocumented mount ordering. Three call sites call useAppRegistry() with no argument, defaulting to []. It works only because <AppRegistry /> sits at frontend.tsx:32, above <App /> at :34. Mount a WorkspaceView above that and every panel renders empty.
  • officerdev/file-viewer is a dead registration. Its provider reads usePanelChannel('file-viewer:' + panelId) and nothing in the repo writes that channel. Also availableOnPanel: false, so it can't be picked. If it ever appeared in a layout it would say "No file selected" forever.
  • dashboard-list is pickable in every panel on every dashboard. apps/Dashboards/index.ts:12-16 omits availableOnPanel while its sibling dashboard-preview sets false. Almost certainly unintended — the flag is opt-out, and 12 of 44 metas omit it. Consider making it opt-in.

5.7 Effect hygiene in panel apps

All the same bug: an app guessing "am I being closed?" from an unmount, or paying for a remount from 5.2.

  • PARKED — useClient identity / DesktopView deps. Revisit later, not now. useClient returns a fresh object every render (workspaces/hooks/src/useClient.ts:30-38), but it is consumed by calling verbs at call time, which is identity-agnostic. Do not memoize it. Only two dep-array sites exist: apps/Desktop/DesktopView.tsx:205 (one redundant connect at mount — React bails on the unchanged setStatus, so it settles rather than looping) and hooks/useChat.ts:124 (a useCallback, harmless). Owner has used this pattern for years without issue; my first write-up called it a live bug and that was overstated.
  • DictateDialog never releases the microphone. apps/FileBrowser/FileBrowserApp/components/DictateDialog.tsx:142-149return () => { if (!showDictate) cleanup(); }. The cleanup that runs on true → false is the one registered by the true render, where showDictate is true, so it never fires. Mic stream, AudioContext and the rAF loop leak. apps/QrTransfer/Receiver.tsx:120 is the correct version.
  • useAudioRecording has no unmount cleanup at all (apps/Chat/useAudioRecording.ts — no useEffect). Unmounting Chat mid-recording leaves the mic open for the life of the tab.
  • HostTerminalWrapper still has the cleanup the other two deliberately removed. apps/Terminal/HostTerminalWrapper.tsx:28-35 deletes the panel→session mapping from persisted state on any unmount — exactly what TerminalWrapper.tsx:30-38 and CommandTerminalWrapper.tsx:31-32 document removing, and why. Latent only because officerdev/terminal-host is in no default layout and hidden from the picker (apps/Terminal/index.tsx:51).
  • useTaskRunner abandons the running task. :60-63 is a bare ws.close(). A stop message exists at :76-79 and is never sent, and there is no re-attach path — unlike usePipelineRunner, which re-attaches by jobId (:281-283).
  • VideoPlayer kills the transcode on incidental unmount. apps/Jellyfin/VideoPlayer.tsx:217-223 POSTs stopped, killing server-side ffmpeg, then renegotiates. Fires on every "yes" row in 5.2. Also: the comment at :215-216 says the dep list "must stay empty" while the code passes [sendReport] — harmless today, misleading.
  • Two stale-closure sockets. usePipelineRunner.ts:270-301 (deps [] but calls a useCallback that changes identity — keeps the first-render copy forever) and apps/FileBrowser/AudioStreamPlayer.tsx:70-143 (safe today, latent).
  • PanelSlot defines a component inside render. :341-348DefaultHeader is a new component type every render, so the header subtree remounts constantly. Harmless while stateless; a trap the moment it isn't.
  • The context value is a fresh literal. WorkspaceView.tsx:145-163 — every useWorkspace() consumer in every panel re-renders on every WorkspaceView render, including each maximize animation frame. Multiplies the cost of the DesktopView bug.

5.8 Navigation — finish the refactor

  • Move the seven channel-driven apps' selection into the URL. Split today: 11 URL-driven (everything post-refactor), 7 channel-driven, 3 on bare useState. The channel-driven set: Music (music:cwd), Soulseek sections, SystemMonitor (monitor:scope), Chat detail (chat:selected-session), Email, Browser, and all five Settings pages (one fix point, SettingsPanel.tsx:56). This is chronological, not architectural — panels render as ordinary children of the screen's Route element (PanelSlot.tsx:333), so every one of them can call useParams today. Nothing in the framework needs to change. Symptom: /soulseek reopens in your last section and /music at your last album while the URL says the bare route. usePanelChannel is a slot on one process-global QueryClient (frontend.tsx:15-21) with no per-route scoping, and reset is called nowhere. Authority: docs/navigation-audit.md.

5.9 The context has grown an app-config section — (found 2026-08-07)

WorkspaceContext is 18 fields, of which the framework itself reads none of the first six. Apps never touch the framework half, so the abstraction holds in one direction; the leak is entirely outbound.

§5.9 is closed as of 2026-08-07. The context is 15 fields, and the outbound half is workspace, cwd, root — all three facts about where the panel is, which is the one thing a framework of this shape genuinely owes an app. Nothing left on it is an app's vocabulary: the file-browser pair is deleted, the chat's system prompt is a prop on the chat, and the key three apps used to parse is a parsed identity. The two hand-written copies of the inert half are one named constant.

  • Delete initialFilePath and defaultFileSort. Declared WorkspaceContext.ts:14-15, plumbed through WorkspaceView.tsx:19-20,30, read only by apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx:7,9,10 — and set by zero callers. DefaultFileSort ({field: 'name'|'size'|'type'|'date'}) is file-browser vocabulary living in the framework's type file, and it is re-exported from the barrel (Workspace/index.ts:27). Pure leak, no payoff, entirely deletable. Gone in 717580f, along with the two barrel re-exports. Deleting them orphaned the props they were feeding, so 585f234 took out the layer below: FileBrowserApp's initialPath/defaultSort, the duplicate DefaultSort type in two files, and useFileBrowserApp's isolated — which was !!initialPath and so had always been false. No behaviour changed at either level; both were already running on the defaults.
  • Move promptPrefix onto the component, not the context. WorkspaceContext.ts:16Chat/ChatPanelWrapper.tsx:78useEmbeddableChat.ts:107. Set by EmailScreen.tsx:53 and BrowserScreen.tsx:35, each a screen-local ~40-word system prompt. The framework is a courier for a string only one app understands, and the components prop already exists for exactly this — Email can supply a pre-configured chat by panel id. Done in d3922bd, exactly that way: both screens put their own ChatPanelWrapper in components under the chat panel's id and pass the prefix as a prop. PanelSlot prefers a components entry over the registry for the body only, so the panel keeps its registry header — the screens did not have to reproduce any chrome. ChatPanelWrapper is exported from the barrel for it. The same prop came off WorkspaceLayout, where it had no callers at all: every settings pane and job detail rendering through it had always been passing its chat panels undefined.
  • dashboardId is a bag whose format three apps parse. It is literally workspace.key (WorkspaceView.tsx:163). Consumers reverse-engineer meaning from its shape: Chat/ChatPanelWrapper.tsx:51-56 does dashboardId === 'email' || dashboardId === 'screens/email' → email context, and !startsWith('screens/') → dashboard context — so renaming a screen key silently changes the agent's system context. Worse since 2026-08-07: the agent address book (agent_panels.dashboard_id, via useAgentPanel) is keyed on this same string, so changing the derivation orphans every named agent on that dashboard — the rows and sessions survive and the panels can no longer find them. Promoted to Tier B in agent-coordination.md §6.3. TerminalWrapper.tsx:13-16 regexes it; HostTerminalWrapper.tsx:12 doesn't (§2). Give the context the parsed facts ({ kind: 'screen'|'dashboard', id }) instead of the raw key, and the three parsers collapse. Done in dbe585f. workspace: WorkspaceIdentity | null replaces dashboardId, parsed once in WorkspaceView by parseWorkspaceKey and memoised on the key — the identity lands in useAgentPanel's query key, and a fresh object per render there is the useClient() trap that disabled the Jellyfin playback reports. The identity carries key as well as kind/id, because the raw key is the address agent_panels.dashboard_id stores: parsed halves are for deciding, the key is for storing. Two behaviours tightened deliberately — an unrecognised key is no longer treated as a dashboard (the old !startsWith('screens/') would have let a panel register an agent against a workspace with no row to hang it on), and terminalStateKey now takes the identity, so it can no longer nest a key inside itself. Seven tests in workspace-identity.test.ts, and the two live agents on ws-layout-agent-mvp were confirmed still addressable after the restart.
  • WorkspaceLayout.tsx:36 silently omits root — apps inside a DashboardPreview fall through to the createContext defaults. (initialFilePath and defaultFileSort were on this list too and are now deleted; workspace is omitted there on purpose, since a preview is not a place an agent can be addressed.) Whatever survives the three items above should be constructed in one place, not twice by hand. Done in ca046a3. The eleven interaction fields — everything a panel uses to rearrange the workspace, plus the state those interactions run on — are one exported inertInteraction, spread by WorkspaceLayout and by the createContext default. root stays omitted, and that is now a stated decision rather than an oversight: it is only ever read when cwd is scoped, and no caller of WorkspaceLayout passes a cwd at all, so there is nothing for it to be the root of.

5.10 Channel hygiene — (found 2026-08-07)

Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives to it.

  • Four channels are bare string literals with no constant. 'files:refresh-signal' (repeated in 4 files), 'chat:selected-session' (3 files — plus a module-private CHANNEL const in a 4th place that only one of them uses), 'chat:active-session', 'preview:refresh'. A typo silently yields a fresh channel pinned to initialData; nothing errors. Export a constant per channel, next to its payload type.
  • Payload types are per-call-site, not per-channel. usePanelChannel<T> takes T from each caller, so a publisher and a subscriber can disagree and nothing checks. 'files:refresh-signal' is number in all four places by convention only. A defineChannel<T>(name) helper returning a typed hook would fix both this and the item above.
  • Two write idioms disagree on the same channel. files:refresh-signal is bumped with Date.now() at the Chat sites and setRefreshSignal((n) => n + 1) at the FileViewer sites — and useGlobal's functional form applies against the render-time snapshot (useGlobal.ts:18), so two increments in one render window collapse into one. Standardise on the nonce. (useLyricsOpen.ts:19-23 already documents avoiding the functional form for this reason.)
  • system-settings:run-command has no writer. run-command-channel.ts:6; the only two writes (SystemSettings.tsx:87,133) are both clears, and the sibling ServerSettings sections never import it. The panel it drives — a terminal that opens with a command pre-loaded — appears unreachable. Wire it or delete it; add to §8 either way.
  • PanelComponentEntry.component is typed with no props (types.ts:60-62) but PanelSlot.tsx:521 passes panelId at runtime. components-supplied panels get a prop they cannot see; registry apps get the honest { panelId: string }. One-line type fix.

6. Decisions needed — not defects, don't guess

  • Mobile: thread mobilePanelId everywhere, or delete the collapse? WorkspaceRenderer.tsx:143-165 collapses any horizontal group with >1 child to a single child, choosing mobilePanelId or falling back to children[0]. Only 4 of 23 consumers pass it. So /wallet, /music, /calendar, /contacts, /soulseek, /system-monitor, /headscale, /gitea, /photos, /invoices, /jellyfin, /transmission render the nav sidebar only — content permanently unreachable, no tab bar, no indicator, no affordance. User-created dashboards (screens/DashboardScreen.tsx:22) show only their left column, forever. Deleting the collapse repairs 16 screens (cramped but complete) at the cost of the 4 using it correctly. Threading it everywhere is 19 small edits and a design question about how you switch. Also note the collapse only applies to horizontal groups.

  • The known "invisible edit form on mobile" bug is a sub-case of the above. It is a mutual exclusion, not a layout glitch: the edit pencil is reachable only while selected === null, and the form renders only while selected !== null. Commit 4970e7e could not have fixed it — its change is a no-op on mobile. Recorded as deliberately-unfixed in CLAUDE.md pending the native app; worth re-deciding now that the cause is known.

  • Drag-to-move: finish or delete? See 5.3. ~180 lines of dead code with a known defect in it.


7. Landed

(move items here with the commit and a one-line resolution)

  • Editing a dashboard no longer resets it. (cd1f161, branch agent-coordination-mvp) — the edit form rebuilt the layout from the template on every submit, so a rename or a typo fix in the description silently discarded the panel arrangement; a rename additionally dropped ws-terminals-<id>/ws-host-terminals-<id> instead of carrying them, abandoning every shell the dashboard held. Found while inventorying §5.4's second item, which was itself dropped.

  • The appType allow-list is a prop, not a convention. (1de1d92, branch agent-coordination-mvp) — closes the first item of §5.4. Fourteen character-identical normalisers, each with its useMemo and persist-back useEffect, collapse to appTypes={{ allowed, fallback }}. 402 lines deleted for 215 added, most of the additions being the seven tests that pin the normaliser — including idempotence, because a normaliser that does not normalise to itself turns the persist-back into an infinite write loop.

  • Panels can carry per-panel settings, and keep them through a move. (e588524, branch agent-coordination-mvp)LayoutPanel.config, opaque to the framework, exposed as usePanelConfig(panelId). The substantive half is in layout-utils.ts: swapPanels and movePanel now carry {appType, config} as one unit via a PanelContents type, where before they carried only appType and therefore reset any per-panel state on a drag. This is the one framework change the agent-coordination MVP needed; it is what lets a panel remember which named agent it is. Follow-ons: PanelContents still drops fitContent and zoom (§5.3), and the mutators are still untested (§9).

  • A write that does not land no longer looks like one that did. (70c2f08 + f4ed740, branch agent-coordination-mvp) — Tier A1 and A2 of the re-rank, and between them the whole of §2 and the second half of §3. Client: useDashboardState rolls back and toasts instead of .catch(() => {}). Server: the PATCH dispatcher gained the three missing key families, a 400 on anything unmatched, a 404 rather than an INSERT for an unknown dashboard id, and a null that means "forget" rather than "write NULL into a NOT NULL column". Framework: one rule for deriving a terminal's state key (apps/Terminal/state-key.ts) where there had been three. The remaining member of that family — the three direct client.patch('/dashboards', …) calls in apps/Dashboards/ — followed in 81ad3ef. See §1.

  • A bad stored value is no longer a white screen. (64961d4 + ef036df, branch agent-coordination-mvp) — Tier A3 and A4, and with them the first three items of §4. Three layers, none of which was enforcing anything before: the columns stopped defaulting to a value that could not be a layout, the read stopped handing over one that is not, and the render stopped taking the whole app down when something else does. Per-item detail in §4. What this did not do is validate on write; that followed in 92b8905, closing §4.

  • A layout write can no longer be quietly undone by an older one. (b0a32ae + 81ad3ef, branch agent-coordination-mvp) — Tier A5 and A7, and with them the whole of §5.5, which empties Tier A. One defect in three places, all of it the same shape: a write computed from a snapshot that a later write had already superseded. WorkspaceView's eight mutations pass updaters rather than trees; WorkspaceRenderer's resize debounce gained the useEffect cleanup it never had, flushing on unmount rather than firing against a captured layout; and the query no longer sits on staleTime: Infinity with nothing to converge it — it refetches on focus, and deliberately not on mount or reconnect, because an optimistic cache makes a badly-timed refetch the same lost update again. The PATCH stopped assembling a full state blob nobody read. Worth remembering for anything new here: the cost of getting this wrong is no longer a panel size that snaps back. Panel identity lives in the layout jsonb, so a resurrected tree carries an older config — the panel that was just given an agent's name goes anonymous, and the agent stops being addressable through it.

  • A layout column cannot hold a non-layout. (92b8905, branch agent-coordination-mvp) — §4's last item, and the end of §4. A CHECK on each layout column (NULL or jsonb_typeof = 'object') plus the same rule in the PATCH dispatcher, so the caller gets a 400 naming the key rather than a 500 naming nothing. Detail in §4.


8. Dead code sweep

Low priority, but each line here is a line someone will read and believe.

  • DragOverlay.tsx (99) + LayoutEditor.tsx (51) — imported by nothing. Gated on 5.3.
  • WorkspaceLayout's isMobile/mobilePanelId/onMobileBack props — no caller passes them.
  • fixedHeight on AppRegistryEntry — declared, read at WorkspaceRenderer.tsx:76, set by zero metas.
  • registerApp (useAppRegistry) — zero callers.
  • getDefaults (queries/dashboards.ts:136) — zero callers.
  • screens.terminals / screens.hostTerminals columns — never written, never read.
  • SELECTED_DASHBOARD constant — zero consumers.
  • preview:refresh and chat:active-session channels — written by ChatPanelWrapper.tsx:58-59, read by nobody. Note preview:refresh is the exemplar docs/navigation-audit.md:126 cites as the canonical good channel; fix the doc too.
  • components/ui/hooks/use-mobile.tsx + ui/sidebar.tsx (~720 lines) — a second useIsMobile implementation, and a sidebar imported by nothing.
  • WorkspaceRenderer.tsx:84 — unreachable duplicate condition inside findChildById. While there: the doc comment says "find a panel node by id anywhere in the tree", but it returns the direct child whose subtree contains the id, which is what the mobile collapse needs. The behaviour is right and the description is wrong — fix the comment, not the code.

9. Tests

  • There are none. No *.test.* exists under any Workspace* or *Screen* path. layout-utils.ts is 214 lines of pure functions over a serialisable tree — the cheapest high-value test target in the codebase, and every item in 5.2 and 5.5 is a regression test waiting to be written. Start here before the mutator work, not after. Promoted to the critical path 2026-08-07 (agent-coordination.md §6.2, A6). e588524 put agent identity inside those functions — setContents, swapPanels, movePanel, setPanelConfig, collectPanelConfigs, newPanelFrom — and shipped them with no tests. A regression in swapPanels now silently swaps two agents' identities between panels, which is a worse failure than any layout glitch the same bug could previously cause. Resolved 85452d1layout-utils.test.ts, 47 tests, 99.56% lines / 100% functions, the first test under any Workspace* path. It paid for itself immediately: setApp was preserving config whenever the new appType was non-null, so switching a panel from chat to terminal handed the terminal the chat's {agentName} to read as its own settings — the exact leak the comment beside it claimed to prevent. Not reachable through the UI (the only route out of an app is onClearAppnull), but setApp is exported from the barrel. Fixed in the same commit. Two tests are deliberately written to the current behaviour and marked KNOWN GAP so they fail the day it changes: a move drops zoom and fitContent (§5.3). Still untested: the other eleven files of the framework — WorkspaceView, WorkspaceRenderer, PanelSlot, normalizeLayout. §5.5's debounce lost-updates live there, not in layout-utils.

  • A panel is told where it is, instead of being handed a key to reverse-engineer. (dbe585f + 717580f + 585f234, branch agent-coordination-mvp) — the first three items of §5.9. The context field dashboardId: string — which was never an id, always the whole workspace.key — is now workspace: WorkspaceIdentity | null, parsed once by parseWorkspaceKey and memoised on the key. Three apps had been parsing the format independently, each with its own idea of what a non-dashboard looks like; they now read kind and id. The raw key rides along because it is the address agent_panels.dashboard_id stores, and a stored address must not depend on which feature wrote it. In the same pass, the two file-browser fields the context had grown were deleted, and with them the prop chain underneath that had no other caller. Net: the context is 16 fields from 18, and four dead parameters and a permanently-false flag are gone from the file browser.

  • The framework stopped carrying things only one app understands. (d3922bd + ca046a3, branch agent-coordination-mvp) — the last two items of §5.9, and with them the section. A screen's ~40-word system prompt used to travel screen → WorkspaceView → context → ChatPanelWrapper, three of those four steps by a framework that has no idea what the string is. It is now a prop on the chat panel the screen mounts itself through components — the prop that already existed for supplying a panel's component, and which PanelSlot honours for the body while still taking header and provider from the registry entry, so nothing about the panel's chrome had to be reproduced. The second half is the inert context: WorkspaceLayout and the createContext default had each written out the same eleven switched-off fields by hand. inertInteraction names the idea once. Net across all of §5.9: the context is 15 fields from 18, and everything still on it answers "where is this panel" rather than "what should this panel do".

  • A panel can be told it was closed, and only when it was. (04371a9 + 198dc71 + c92b51c, branch agent-coordination-mvp) — the first two items of §5.1, and the terminal orphan leak with them. usePanelClose(panelId, handler), fired by WorkspaceView from handleRemove and from handleSetApp when the app actually changes, and from nowhere else. The interesting part is what it is not. This item used to propose diffing the layout before and after; two tests now stand in layout-utils.test.ts to stop anyone trying it, because movePanel mints a fresh panel id on the way and swapPanels exchanges contents between stationary ones — so a drag reads as a close and a swap reads as two. A panel id is a position in the tree, not an app instance. Nor is it an unmount hook: registration is deliberately never torn down, since "this component went away" is the ambiguous signal the whole thing exists to stop trusting. The terminals use it through a shared useTerminalSession, which turned up two more defects while being written: HostTerminalWrapper had kept the unmount cleanup the other two wrappers had removed, orphaning a host shell on every layout change; and RunningShells, the panel whose entire job is to make orphans visible, was asking the pty sidecar for a route it does not serve, so it had always shown an empty list and its kill button had never done anything. A chat panel deliberately does not use the hook — it is a pointer to a server-side session, and closing a window must not delete what it points at. That is recorded in the hook's own doc comment, where the next person to add one will read it.