the framework has no tests, no error boundaries and a handful of known defects that keep resurfacing mid-feature. write them down once, ranked, so they can be picked off in the context of whatever is being built. notable: the dashboards PATCH dispatcher silently drops any key family it has no branch for, and three in use today (tmux, nvim, claude-code, all from Terminal's statePrefix) match nothing — so that state never persists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
25 KiB
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.
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.
-
Fix the Running Shells paths — the panel has 404'd since 2026-07-31.
apps/Terminal/RunningShells.tsx:43,49call/terminal/sessionsandDELETE /terminal/sessions/:id.useClientprefixes/api;servers/sidecar/create-proxy.ts:69strips 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/sessionsand/terminal/_officer/sessions/:id. The panel (fccf212) predates the proxy move (7129cd8) that deleted the oldservers/api/terminal/router.tsand was never updated. Symptom today: permanently reads "No shells running", kill button is a silent no-op. -
Surface the
clientscount in Running Shells.RunningShells.tsx:12-20drops theclientsfield the sidecar returns (sidecars/pty/sessions.mjs:169) — the one field that distinguishes an orphan (clients: 0) from a live shell. Add it to the type and render it. -
Stop swallowing persist failures.
state/src/useDashboardState.ts:46is.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.
2. Live data loss
-
The PATCH dispatcher silently drops three key families in active use.
servers/api/dashboards/dashboards.ts:26-88is a chain ofif (match) { …; continue; }that ends with noelse— 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:18writesws-${statePrefix}-${dashboardId}, and the registered prefixes (apps/Terminal/index.tsx:22,27,34) aretmux,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 vianew-session -A -s off-<panelId>; the shell runningtmux attachdoes not.) Two parts: add the three families, and add a fallbackelsethat 400s on an unknown key. -
ws-terminals-{id}: nullon a live dashboard is a 500. Same file,:61-66— thews-layout-*branch has avalue === null→deleteDashboardcase (:42); the terminals branches do not. A null falls to the UPDATE branch and sets aNOT NULLcolumn (databases/officer_db/src/queries/dashboards.ts:70) → 23502. -
Renaming a dashboard resurrects it as a zombie row.
apps/Dashboards/DashboardPreview.tsx:316-318PATCHesws-layout-old: nullandws-terminals-old: nulltogether. The first deletes the row; the second then callsupsertDashboard, finds nothing, and re-INSERTs it withname = id. The old slug reappears inworkspaceson the next GET as a duplicate. Fixed by the item above, but verify this specific sequence after fixing.
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.idis a global primary key but ids areslugify(name).databases/officer_db/src/schema/dashboards.tsdeclaresid: 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 fromDashboardPreview.tsx:300(slugify(trimmed) || generateSlug()) and the uniqueness loop at:340checks only the caller's own dashboards. Two users both naming a dashboard "Work" → both getid = '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 (seedatabases/CLAUDE.md→ "Composite keys") — harmless churn, but read the plan. -
upsertDashboard's UPDATE has nouserIdpredicate.databases/officer_db/src/queries/dashboards.ts:73—db.update(dashboards).set(set).where(eq(dashboards.id, id)). Theexistinglookup 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.
4. Resilience — one bad row is currently a white screen
-
Add an error boundary.
grep -rln "componentDidCatch\|getDerivedStateFromError\|ErrorBoundary\|errorElement" srcreturns nothing across the whole repo. WrapWorkspaceRendererat minimum, with a reset that offers "restore default layout". Today the only recovery from a malformed stored layout is SQL. -
Validate on read.
useDashboardState.ts:36iskey in state ? state[key] as T : defaultValue— an unchecked cast over a value that arrived asunknownfrom jsonb. Add anisLayoutNode()guard that falls back todefaultValue. This also neutralises the next item without a migration. -
layoutcolumns default to'[]', which is not a validLayoutNode.schema/dashboards.ts:13,31— an empty array for a column holding an object. Any path that upserts without a layout (e.g. theworkspacesbranch,dashboards.ts:33) writes it;key in stateis then true, so the stored[]wins over the caller's default andnormalizeLayoutcalls.children.mapon it and throws. Change the default, or drop it and make the column nullable. -
Validate on write. The whole path is
unknown:useDashboardState.ts:46→servers/api/dashboards/dashboards.ts:45,83→queries/dashboards.ts:51,97→ castas neverat:114to satisfy drizzle → jsonb. No zod, no CHECK on any of the three layout columns. Givendatabases/CLAUDE.md's stance that the schema is the source of truth for contents, a CHECK onjsonb_typeof(layout) = 'object'is the cheap half. -
A bare number is squatting in a framework namespace. (verified in the live DB)
apps/Soulseek/shared.ts:12buildsscreens/soulseek-zoom/${panelId}and passes it touseDashboardState<number>. The server routes^screens/(.+)$intoscreens.layout, so:user_id 1 | soulseek-zoom/soulseek-view | jsonb_typeof = number. That namespace belongs to layouts. Give per-panel scalar prefs their own key family (and a matching dispatcher branch, which the §2elsewill otherwise start rejecting).
5. The framework work proper
5.1 Give panels a lifecycle — the highest-value change here
-
onCloseonAppRegistryEntry, invoked by the mutators, not by unmount.components/Workspace/types.ts:38-47has no close hook; the onlyonCloseisPanelComponentEntry.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)andsetApp(root, id, null)(the red traffic light,PanelSlot.tsx:295) — are pure tree rewrites that notify nobody. Invoke it fromWorkspaceView'shandleRemove/handleSetApp, never from aPanelSlotunmount — aPanelSlotunmount is precisely the ambiguous signal this exists to replace. Implementation: diff removed panel ids old-tree ∖ new-tree, or have the mutators return them.This is the thing `apps/Terminal/TerminalWrapper.tsx:31-38` asks 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. -
Then: kill the pty on real close. Once the hook exists,
TerminalWrapper/CommandTerminalWrapper/HostTerminalWrappercanDELETE /terminal/_officer/sessions/:idand drop the map entry — the thing they each explain they cannot currently do. -
Then: reap orphans in the sidecar as a backstop.
sidecars/pty/sessions.mjs:127-130detach()never checksclients.size === 0; there is no idle timeout and no session cap.lastActivityAtis written (:110) and displayed (:167) but never read by a timer. Each orphan costs aSHELL -iwith the owner's full env plus up toBUFFER_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
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.
dragSourceIdcan never become non-null: the only code that would set it is commented out atPanelSlot.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:movePanelcurrently mints a brand-new panel id (:151-154) and carries onlyappType, so it destroys all panel-keyed state and silently dropsfitContent. Fix both before re-enabling.
5.4 Make normalizeLayout framework, not convention
- One
makeLayoutNormalizer(allowed, fallback)applied insideWorkspaceView. The guard currently exists 14 times, character-identical except for the allow-list and the fallback, plus a matchinguseMemo/persist-backuseEffectpair 13 times — ~150 duplicated lines. Consequences of it being convention: - 10WorkspaceViewconsumers have no guard at all, includingscreens/HomeScreen.tsx:11, which also omitslocked— 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 reachesPanelSlot.tsx:311-317, which on alockedscreen renders an empty teal-bordered box with no picker and no way for the user to recover. -screens/QrTransferScreen.tsx:19-39has 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: per-screen
defaultLayout.ts(×20),createDefaultLayout()in the core, and the 6-entry template array atDashboardPreview.tsx:33-142.
5.5 Persistence hygiene
- The resize debounce can resurrect a deleted panel.
WorkspaceRenderer.tsx:108-124holds a 500 ms timer in a ref with nouseEffect, therefore no cleanup, and its callback closes over the layout as it was when the drag began. Drag a splitter, remove a panel within 500 ms → the timer PATCHes the pre-deletion tree and the panel comes back. Same shape for a split. Fix: clean up on unmount, and makesetValueaccept an updater so it composes against current state instead of a captured one. - Concurrent same-key writes clobber. A window resize fires
onLayouton every group at once; each schedules its own timer against the same base tree and the last wins (WorkspaceView.tsx:76-81). Same fix as above. - The cache is never invalidated.
staleTime: Infinityand noinvalidateQueriesanywhere in the repo. Meanwhile every PATCH already computes and returns a full fresh state blob (servers/api/dashboards/dashboards.ts:90) which the client discards — 3 SELECTs per splitter release, thrown away. Consume the response, or stop computing it. Two tabs currently diverge permanently and neither is told. - Preserve sibling sizes on split.
splitInner/insertPanelredistribute evenly (100 / newChildren.length), so one split discards carefully tuned proportions. tpl-Npanel ids collide across dashboards.DashboardPreview.tsx:25-26—let 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}. Use the core'suid()(layout-utils.ts:4), which stampsp-${Date.now()}-${n}`. One line.
5.6 Registry
- No duplicate-key guard.
metasToRegistryisObject.fromEntries(state/src/useAppRegistry.ts:19) — a collision silently last-wins and one app just disappears. All 44 keys are unique today. Throw in dev. - Seeding depends on undocumented mount ordering. Three call sites call
useAppRegistry()with no argument, defaulting to[]. It works only because<AppRegistry />sits atfrontend.tsx:32, above<App />at:34. Mount aWorkspaceViewabove that and every panel renders empty. officerdev/file-vieweris a dead registration. Its provider readsusePanelChannel('file-viewer:' + panelId)and nothing in the repo writes that channel. AlsoavailableOnPanel: false, so it can't be picked. If it ever appeared in a layout it would say "No file selected" forever.dashboard-listis pickable in every panel on every dashboard.apps/Dashboards/index.ts:12-16omitsavailableOnPanelwhile its siblingdashboard-previewsetsfalse. 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 —
useClientidentity /DesktopViewdeps. Revisit later, not now.useClientreturns 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 unchangedsetStatus, so it settles rather than looping) andhooks/useChat.ts:124(auseCallback, harmless). Owner has used this pattern for years without issue; my first write-up called it a live bug and that was overstated. DictateDialognever releases the microphone.apps/FileBrowser/FileBrowserApp/components/DictateDialog.tsx:142-149—return () => { if (!showDictate) cleanup(); }. The cleanup that runs ontrue → falseis the one registered by thetruerender, whereshowDictateistrue, so it never fires. Mic stream,AudioContextand the rAF loop leak.apps/QrTransfer/Receiver.tsx:120is the correct version.useAudioRecordinghas no unmount cleanup at all (apps/Chat/useAudioRecording.ts— nouseEffect). Unmounting Chat mid-recording leaves the mic open for the life of the tab.HostTerminalWrapperstill has the cleanup the other two deliberately removed.apps/Terminal/HostTerminalWrapper.tsx:28-35deletes the panel→session mapping from persisted state on any unmount — exactly whatTerminalWrapper.tsx:30-38andCommandTerminalWrapper.tsx:31-32document removing, and why. Latent only becauseofficerdev/terminal-hostis in no default layout and hidden from the picker (apps/Terminal/index.tsx:51).useTaskRunnerabandons the running task.:60-63is a barews.close(). Astopmessage exists at:76-79and is never sent, and there is no re-attach path — unlikeusePipelineRunner, which re-attaches byjobId(:281-283).VideoPlayerkills the transcode on incidental unmount.apps/Jellyfin/VideoPlayer.tsx:217-223POSTsstopped, killing server-side ffmpeg, then renegotiates. Fires on every "yes" row in 5.2. Also: the comment at:215-216says 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 auseCallbackthat changes identity — keeps the first-render copy forever) andapps/FileBrowser/AudioStreamPlayer.tsx:70-143(safe today, latent). PanelSlotdefines a component inside render.:341-348—DefaultHeaderis 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— everyuseWorkspace()consumer in every panel re-renders on everyWorkspaceViewrender, including each maximize animation frame. Multiplies the cost of theDesktopViewbug.
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 calluseParamstoday. Nothing in the framework needs to change. Symptom:/soulseekreopens in your last section and/musicat your last album while the URL says the bare route.usePanelChannelis a slot on one process-global QueryClient (frontend.tsx:15-21) with no per-route scoping, andresetis called nowhere. Authority:docs/navigation-audit.md.
6. Decisions needed — not defects, don't guess
-
Mobile: thread
mobilePanelIdeverywhere, or delete the collapse?WorkspaceRenderer.tsx:143-165collapses any horizontal group with >1 child to a single child, choosingmobilePanelIdor falling back tochildren[0]. Only 4 of 23 consumers pass it. So/wallet,/music,/calendar,/contacts,/soulseek,/system-monitor,/headscale,/gitea,/photos,/invoices,/jellyfin,/transmissionrender 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 whileselected !== null. Commit4970e7ecould not have fixed it — its change is a no-op on mobile. Recorded as deliberately-unfixed inCLAUDE.mdpending 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)
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'sisMobile/mobilePanelId/onMobileBackprops — no caller passes them.fixedHeightonAppRegistryEntry— declared, read atWorkspaceRenderer.tsx:76, set by zero metas.registerApp(useAppRegistry) — zero callers.getDefaults(queries/dashboards.ts:136) — zero callers.screens.terminals/screens.hostTerminalscolumns — never written, never read.SELECTED_DASHBOARDconstant — zero consumers.preview:refreshandchat:active-sessionchannels — written byChatPanelWrapper.tsx:58-59, read by nobody. Notepreview:refreshis the exemplardocs/navigation-audit.md:126cites as the canonical good channel; fix the doc too.components/ui/hooks/use-mobile.tsx+ui/sidebar.tsx(~720 lines) — a seconduseIsMobileimplementation, and a sidebar imported by nothing.WorkspaceRenderer.tsx:84— unreachable duplicate condition insidefindChildById. 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 anyWorkspace*or*Screen*path.layout-utils.tsis 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.