diff --git a/docs/workspace-panel-todo.md b/docs/workspace-panel-todo.md index 78413cf3..3b16a2b4 100644 --- a/docs/workspace-panel-todo.md +++ b/docs/workspace-panel-todo.md @@ -18,12 +18,12 @@ before deciding what to pick up next. Its conclusions in one paragraph: 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. - **Done as of 2026-08-07:** §1's `.catch(() => {})` (`70c2f08`), all of §2 and §3's missing `userId` - 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 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 @@ -74,10 +74,14 @@ Both are two-line fixes, and without them you cannot tell whether any later fix 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. - **Still swallowed, and not through this hook:** the three direct `client.patch('/dashboards', …)` + **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. Same defect, different call site; they bypass `useDashboardState` - entirely. A failed delete there leaves a dashboard that reappears on reload. + 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. --- @@ -355,20 +359,36 @@ does not just resurrect a deleted panel, it **re-writes an older `config`**, so 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` holds a - 500 ms timer in a ref with **no `useEffect`, therefore no cleanup**, and its callback closes over +- [x] **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 - 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. + 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. +- [x] **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. +- [x] **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. - [ ] **`tpl-N` panel ids collide across dashboards.** `DashboardPreview.tsx:25-26` — @@ -551,8 +571,8 @@ _(move items here with the commit and a one-line resolution)_ 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 is the three direct `client.patch('/dashboards', …)` calls in - `apps/Dashboards/` — see §1. + The remaining member of that family — the three direct `client.patch('/dashboards', …)` calls in + `apps/Dashboards/` — followed in `81ad3ef`. 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 @@ -562,6 +582,20 @@ _(move items here with the commit and a one-line resolution)_ 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. +- [x] **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. + --- ## 8. Dead code sweep