record §4: the boundary, validate-on-read, and the layout default
Tier A now has only §5.5 left — the resize debounce and the stale cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+107
-78
@@ -1,6 +1,6 @@
|
||||
# Workspaces & Panels — working TODO
|
||||
|
||||
Living list. Add items as they are found, tick them as they land, and write the resolution *into* the
|
||||
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.
|
||||
|
||||
@@ -10,21 +10,22 @@ 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
|
||||
`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
|
||||
`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.
|
||||
**Done as of 2026-08-07:** §1's `.catch(() => {})` (`70c2f08`), all of §2 and §3's missing `userId`
|
||||
predicate (`f4ed740`), §9 (`85452d1`). **Left in Tier A: §4 entirely** — validate-on-read, the `'[]'`
|
||||
layout default, and the error boundary — **and §5.5's two debounce items plus the stale cache.** §4 is
|
||||
now the largest remaining piece and the one the objective actually ends on: work proceeds overnight and
|
||||
the human's only act is to open the dashboard and look at it.
|
||||
predicate (`f4ed740`), §9 (`85452d1`), and all three Tier A items in §4 — the error boundary
|
||||
(`64961d4`), validate-on-read and the `'[]'` layout default (`ef036df`).
|
||||
**Left in Tier A: §5.5's two debounce items and the never-invalidated cache.** Everything the re-rank
|
||||
put on the critical path for "the human opens the dashboard in the morning and sees the result" has
|
||||
landed; what remains is a write that can still be lost between a resize and its debounce.
|
||||
- **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;
|
||||
(§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.
|
||||
- **Tier C, orthogonal:** everything else — including **§5.2 (remounts) and §5.3 (drag-to-move), both
|
||||
@@ -98,7 +99,7 @@ and the rename sequence leaves `workspaces` with no zombie.
|
||||
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
|
||||
**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
|
||||
@@ -117,8 +118,8 @@ and the rename sequence leaves `workspaces` with no zombie.
|
||||
**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.
|
||||
|
||||
- [x] **`HostTerminalWrapper` never strips the prefix, so it mints phantom dashboards.** *(found
|
||||
2026-08-07, latent — `dashboards` is still 0 rows)*
|
||||
- [x] **`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
|
||||
@@ -136,14 +137,14 @@ and the rename sequence leaves `workspaces` with no zombie.
|
||||
fix; this closes the hole a mis-derived key could fall through.
|
||||
|
||||
- [x] **Renaming a dashboard resurrects it as a zombie row.**
|
||||
`apps/Dashboards/DashboardPreview.tsx:316-318` PATCHes `ws-layout-old: null` *and*
|
||||
`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
|
||||
ws-host-terminals-zombie-test: null}` in one body. `workspaces` comes back as
|
||||
`['agent-mvp', 'renamed']`.
|
||||
|
||||
---
|
||||
@@ -168,7 +169,7 @@ these.
|
||||
|
||||
- [x] **`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*
|
||||
`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.
|
||||
@@ -182,34 +183,57 @@ these.
|
||||
|
||||
## 4. Resilience — one bad row is currently a white screen
|
||||
|
||||
*The first three items are Tier A (`agent-coordination.md` §6.2, A3–A4). This is the last mile of the
|
||||
_The first three items are Tier A (`agent-coordination.md` §6.2, A3–A4). 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.*
|
||||
`chat_session_events`, which is why recovery must offer "restore default layout" rather than SQL._
|
||||
|
||||
- [ ] **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.
|
||||
**The first three are resolved — `64961d4` (the boundary) and `ef036df` (the other two).** §4 is the last
|
||||
Tier A item that was still whole; what is left of it is validate-on-write, below, which is the cheap half
|
||||
of a rule now enforced on both sides of the wire but nowhere in the database.
|
||||
|
||||
- [ ] **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.
|
||||
- [x] **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.
|
||||
|
||||
- [ ] **`layout` columns default to `'[]'`, which is not a valid `LayoutNode`.**
|
||||
- [x] **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 `ef036df`** — `readValue()` 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.
|
||||
|
||||
- [x] **`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.
|
||||
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 `ALTER`s plus the two known-harmless
|
||||
`pk_music_now_playing` lines.
|
||||
|
||||
- [ ] **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
|
||||
`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.
|
||||
|
||||
- [x] **A bare number is squatting in a framework namespace.** *(verified in the live DB)*
|
||||
- [x] **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
|
||||
@@ -228,7 +252,7 @@ malformed row turning that into a white screen loses the result at the one momen
|
||||
|
||||
- [ ] **`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
|
||||
`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.
|
||||
@@ -257,31 +281,31 @@ malformed row turning that into a white screen loses the result at the one momen
|
||||
`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.*
|
||||
_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
|
||||
_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.*
|
||||
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` |
|
||||
| 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.
|
||||
@@ -299,12 +323,12 @@ the whole table:
|
||||
`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
|
||||
**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
|
||||
_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.
|
||||
|
||||
@@ -313,26 +337,23 @@ the whole table:
|
||||
- [ ] **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.
|
||||
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
|
||||
|
||||
*Three Tier A items live here (`agent-coordination.md` §6.2, A5 and A7). The two lost-update bugs are
|
||||
_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.*
|
||||
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` holds a
|
||||
500 ms timer in a ref with **no `useEffect`, therefore no cleanup**, and its callback closes over
|
||||
@@ -353,7 +374,7 @@ work disagree permanently about the roster, with neither told — a direct contr
|
||||
- [ ] **`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}`.
|
||||
**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
|
||||
@@ -370,7 +391,7 @@ work disagree permanently about the roster, with neither told — a direct contr
|
||||
"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*.
|
||||
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
|
||||
|
||||
@@ -392,7 +413,7 @@ All the same bug: an app guessing "am I being closed?" from an unmount, or payin
|
||||
`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
|
||||
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`).
|
||||
@@ -407,7 +428,7 @@ All the same bug: an app guessing "am I being closed?" from an unmount, or payin
|
||||
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
|
||||
_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
|
||||
@@ -428,7 +449,7 @@ All the same bug: an app guessing "am I being closed?" from an unmount, or payin
|
||||
(`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)*
|
||||
### 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.
|
||||
@@ -444,7 +465,7 @@ touch the framework half, so the abstraction holds in one direction; the leak is
|
||||
`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.
|
||||
- [ ] **`dashboardId` is a bag whose *format* three apps parse.** It is literally `workspace.key`
|
||||
- [ ] **`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
|
||||
@@ -459,7 +480,7 @@ touch the framework half, so the abstraction holds in one direction; the leak is
|
||||
inside a `DashboardPreview` fall through to the `createContext` defaults. Whatever survives the
|
||||
three items above should be constructed in one place, not twice by hand.
|
||||
|
||||
### 5.10 Channel hygiene — *(found 2026-08-07)*
|
||||
### 5.10 Channel hygiene — _(found 2026-08-07)_
|
||||
|
||||
Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives to it.
|
||||
|
||||
@@ -478,7 +499,7 @@ Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives t
|
||||
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
|
||||
(`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
|
||||
@@ -496,7 +517,7 @@ Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives t
|
||||
`/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
|
||||
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.
|
||||
|
||||
@@ -512,19 +533,19 @@ Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives t
|
||||
|
||||
## 7. Landed
|
||||
|
||||
*(move items here with the commit and a one-line resolution)*
|
||||
_(move items here with the commit and a one-line resolution)_
|
||||
|
||||
- [x] **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
|
||||
- [x] **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
|
||||
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).
|
||||
|
||||
- [x] **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
|
||||
- [x] **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
|
||||
@@ -533,6 +554,14 @@ Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives t
|
||||
The remaining member of that family is the three direct `client.patch('/dashboards', …)` calls in
|
||||
`apps/Dashboards/` — see §1.
|
||||
|
||||
- [x] **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 does **not** do is validate on write — §4's last open item. The database will still accept
|
||||
any jsonb at all; the two new guards mean it can no longer reach a renderer.
|
||||
|
||||
---
|
||||
|
||||
## 8. Dead code sweep
|
||||
@@ -549,12 +578,12 @@ Low priority, but each line here is a line someone will read and believe.
|
||||
- [ ] `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.
|
||||
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
|
||||
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.
|
||||
|
||||
---
|
||||
@@ -566,7 +595,7 @@ Low priority, but each line here is a line someone will read and believe.
|
||||
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`,
|
||||
_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.
|
||||
@@ -576,7 +605,7 @@ Low priority, but each line here is a line someone will read and believe.
|
||||
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
|
||||
`onClearApp` → `null`), 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
|
||||
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`.
|
||||
|
||||
Reference in New Issue
Block a user