From 575b4a596670bbf25b4c0ce8a9affcac9ff3de42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 02:58:16 +0000 Subject: [PATCH] add a living todo for the workspace/panel framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- TODO.md | 11 ++ docs/workspace-panel-todo.md | 370 +++++++++++++++++++++++++++++++++++ 2 files changed, 381 insertions(+) create mode 100644 docs/workspace-panel-todo.md diff --git a/TODO.md b/TODO.md index 7f7bed3c..b93ef04a 100644 --- a/TODO.md +++ b/TODO.md @@ -135,6 +135,17 @@ day. Kept here for what the policy work turned up, which is not obvious from the behind nginx, `tailnet` is not a headscale concept and has to be recorded on the invite, and `/api/v1/enroll/*` collides with headscale's own namespace. +## Workspaces & Panels + +Its own list — **`docs/workspace-panel-todo.md`** — because it is long and actively worked. Full +analysis behind it: `COMMS/workspace-panel-framework-analysis-2026-08-07.md`. + +Headlines: the framework has **no panel lifecycle** (a panel can be destroyed but never told, which is +the real cause of the terminal orphan leak); three state-key families are **silently dropped on every +write**, orphaning a pty per reload; Running Shells has **404'd since 2026-07-31**; there are **zero +error boundaries** anywhere in the repo; and `dashboards.id` is a **global** primary key fed by +`slugify(name)`, so two members naming a dashboard the same thing collide. + ## Known bugs - [ ] **`bootstrap.ts` runs `npm install -g` for Pi on every boot.** `findPiPackageDir` checks stale diff --git a/docs/workspace-panel-todo.md b/docs/workspace-panel-todo.md new file mode 100644 index 00000000..85a822f2 --- /dev/null +++ b/docs/workspace-panel-todo.md @@ -0,0 +1,370 @@ +# 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,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. + +- [ ] **Surface the `clients` count in Running Shells.** `RunningShells.tsx:12-20` drops the `clients` + field 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: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. + +--- + +## 2. Live data loss + +- [ ] **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-`; the shell running + `tmux attach` does not.) + Two parts: add the three families, **and** add a fallback `else` that 400s on an unknown key. + +- [ ] **`ws-terminals-{id}: null` on a live dashboard is a 500.** Same file, `:61-66` — the + `ws-layout-*` branch has a `value === null` → `deleteDashboard` 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. + +- [ ] **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. + +--- + +## 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:73` — + `db.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. + +--- + +## 4. Resilience — one bad row is currently a white screen + +- [ ] **Add an error boundary.** `grep -rln "componentDidCatch\|getDerivedStateFromError\|ErrorBoundary\|errorElement" src` returns **nothing** + across the whole repo. Wrap `WorkspaceRenderer` at 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:36` is `key in state ? state[key] as T : defaultValue` + — an unchecked cast over a value that arrived as `unknown` from jsonb. Add an `isLayoutNode()` + guard that falls back to `defaultValue`. + This also neutralises the next item without a migration. + +- [ ] **`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, `dashboards.ts:33`) 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. 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` → cast `as never` at + `:114` to satisfy drizzle → jsonb. No zod, no CHECK on any of the three layout columns. 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. + +- [ ] **A bare number is squatting in a framework namespace.** *(verified in the live DB)* + `apps/Soulseek/shared.ts:12` builds `screens/soulseek-zoom/${panelId}` and passes it to + `useDashboardState`. The server routes `^screens/(.+)$` into `screens.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 §2 `else` will otherwise start rejecting). + +--- + +## 5. The framework work proper + +### 5.1 Give panels a lifecycle — the highest-value change here + +- [ ] **`onClose` on `AppRegistryEntry`, invoked by the mutators, not by unmount.** + `components/Workspace/types.ts:38-47` has no close hook; the only `onClose` is + `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`) + — are pure tree rewrites that notify nobody. + **Invoke it from `WorkspaceView`'s `handleRemove`/`handleSetApp`, never from a `PanelSlot` + unmount** — a `PanelSlot` unmount 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` / `HostTerminalWrapper` can `DELETE /terminal/_officer/sessions/:id` and + 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-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 + +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` currently mints a *brand-new* panel id (`:151-154`) and carries + only `appType`, so it destroys all panel-keyed state and **silently drops `fitContent`**. Fix both + before re-enabling. + +### 5.4 Make `normalizeLayout` framework, not convention + +- [ ] **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**: per-screen `defaultLayout.ts` (×20), + `createDefaultLayout()` in the core, and the 6-entry template array at `DashboardPreview.tsx:33-142`. + +### 5.5 Persistence hygiene + +- [ ] **The resize debounce can resurrect a deleted panel.** `WorkspaceRenderer.tsx:108-124` holds a + 500 ms timer in a ref with **no `useEffect`, 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 make `setValue` accept an updater so it composes against current + state instead of a captured one. +- [ ] **Concurrent same-key writes clobber.** A window resize fires `onLayout` on 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: Infinity` and no `invalidateQueries` anywhere 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`/`insertPanel` redistribute evenly + (`100 / newChildren.length`), so one split discards carefully tuned proportions. +- [ ] **`tpl-N` panel 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 keys `terminal-conn-${panelId}` and `file-viewer:${panelId}`. + Use the core's `uid()` (`layout-utils.ts:4`), which stamps `p-${Date.now()}-${n}`. One line. + +### 5.6 Registry + +- [ ] **No duplicate-key guard.** `metasToRegistry` is `Object.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 `` sits at `frontend.tsx:32`, + above `` 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-149` — + `return () => { 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-348` — `DefaultHeader` 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`. + +--- + +## 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)* + +--- + +## 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.