lz/agent-nexus

Fork 0

refactor(main-page): one shared workspaces store (PR-C1) #62

Merged

lz merged 11 commits from feat/dockview-pr-c1 into main

2026-07-13 21:10:43 +02:00

lz commented

2026-07-12 21:59:23 +02:00

Owner

Targets feat/dockview-pr-b (#61), not main. Stacked: main ← PR-A #58 ← PR-B #61 ← this ← PR-C2.

First half of PR-C. This one is invisible — no user-facing behaviour change. It exists so PR-C2 (per-panel {workerId, sessionId} targets, retiring the single scope) has a single source of truth to build on, and it fixes a real double-fetch on the way.

Spec: docs/superpowers/specs/2026-07-12-dockview-migration-pr-c-design.md §3
Plan: docs/superpowers/plans/2026-07-12-dockview-pr-c1.md

The bug it fixes

Workers.svelte called api.listSessions(w.id) for every running worker (to know whether the workspace shell was open). Each child Sessions.svelte — one per worker — called api.listSessions(w.id) again for the same worker. Two identical requests per worker per tick, shipping on main today. WorkspaceRail (PR-B) duplicated the logic a third time, safe only because it and <Workers> are never both mounted.

Now one WorkspacesStore polls, driven by a single $effect in MainSplit, and everyone reads.

Measured, not asserted

Driven in a real browser against a seeded instance (2 workspaces, 4 sessions), identical 32s window, same harness:

32s window	before	after
roster fetches per worker	13	4
session-health probes per session	—	4 (1/tick)

4 is the floor: one per refresh (mount + 3 ticks). The before figure is worse than the predicted 2× because Workers.svelte also had both an onMount(load) and a tick effect, so it double-loaded on mount on top of the double-fetch. Consolidating fixed that too.

Two real bugs found in review

Both in the store, both fixed and pinned by tests that were mutation-verified (guard removed → exactly the right test fails → restored):

refresh() had no reentrancy protection. Overlapping calls committed in completion order, not start order — so a slow poll that began before a workspace was deleted could resurrect it, and one that began before a spawn could erase it. Both reproduced. Fixed with a generation counter: last-started wins, superseded calls discard rather than commit.

Deliberately not fixed by coalescing onto the in-flight promise: mutation handlers do await api.removeWorker(id); await workspaces.refresh(); and must see their own write. Coalescing would hand them a promise that started before the delete, leaving a removed workspace on screen until the next tick.
A superseded refresh that failed could paint a stale error over a newer success — an error banner sitting on top of correctly-loaded data. The guard existed but was pinned by no test; removing it left all 616 tests green.

The self-triggering `$effect` trap

Sessions.svelte read its health map for a TTL check and wrote it, from inside an effect. That was safe only by accident: await api.listSessions() ran first, pushing the read out of Svelte's synchronous tracking window. This PR removes that await.

TTL timestamps therefore moved to a plain, non-reactive Map. And the effect calls probeHealth() inside untrack() — load-bearing, not decoration: probeHealth() reads list synchronously, so calling it bare would register the store's $derived roster as a dependency, and the store hands back a fresh array reference on every refresh (committed in a later flush than the tick bump). That would fire the effect twice per poll and double-fetch sessionHealth — reintroducing, on a different endpoint, the exact duplicate request this PR exists to kill.

Deliberate, please don't "clean up"

liveness(), label(), sessionName() have no caller. They are PR-C2 prerequisites and are commented as such. liveness() is three-valued (alive / dead / unknown) on purpose: C2 prunes dock panels for dead sessions, and a boolean would make a failed poll indistinguishable from a removed session — destroying the operator's layout on a transient error. It mirrors convergeSessions, which never hard-deletes a session row when the worktree probe fails (AGENTS.md fact #15). Delete it and loaded + sessionsLoaded become write-only and the whole stale-tolerance design goes with it.
Dock.svelte still has its own listSessions. Out of scope: PR-C2 rewrites that file wholesale.

Behaviour change worth knowing

The workspace "start agent" / "stop agent" toggle no longer flips on a local optimistic flag; it awaits a store refresh and reads backend truth. One extra round trip before the button updates. Strictly more correct — the old optimism could disagree with the very next poll — but it is a real latency change.

Verification

pnpm typecheck → 0 errors (1006 files)
pnpm test → 617 passed, of which 20 are the new store's
Browser: seeded throwaway instance, request counts above; workspace-shell toggle confirmed driven by real backend state (a workspace with a shell row shows "stop agent", one without shows "start agent"); rail and expanded list agree, since they now read the same store.

Test suite is node-only with no DOM, so .svelte components remain structurally untestable — the browser pass is the honest substitute, not an optional extra.

**Targets `feat/dockview-pr-b` (#61), not `main`.** Stacked: `main` ← PR-A #58 ← PR-B #61 ← **this** ← PR-C2. First half of PR-C. This one is **invisible** — no user-facing behaviour change. It exists so PR-C2 (per-panel `{workerId, sessionId}` targets, retiring the single `scope`) has a single source of truth to build on, and it fixes a real double-fetch on the way. Spec: `docs/superpowers/specs/2026-07-12-dockview-migration-pr-c-design.md` §3 Plan: `docs/superpowers/plans/2026-07-12-dockview-pr-c1.md` ## The bug it fixes `Workers.svelte` called `api.listSessions(w.id)` for every running worker (to know whether the workspace shell was open). Each child `Sessions.svelte` — one per worker — called `api.listSessions(w.id)` **again** for the same worker. Two identical requests per worker per tick, shipping on `main` today. `WorkspaceRail` (PR-B) duplicated the logic a third time, safe only because it and `<Workers>` are never both mounted. Now one `WorkspacesStore` polls, driven by a single `$effect` in `MainSplit`, and everyone reads. ## Measured, not asserted Driven in a real browser against a seeded instance (2 workspaces, 4 sessions), identical 32s window, same harness: | 32s window | before | after | |---|---|---| | roster fetches **per worker** | **13** | **4** | | session-health probes per session | — | 4 (1/tick) | 4 is the floor: one per refresh (mount + 3 ticks). The before figure is worse than the predicted 2× because `Workers.svelte` also had *both* an `onMount(load)` and a `tick` effect, so it double-loaded on mount on top of the double-fetch. Consolidating fixed that too. ## Two real bugs found in review Both in the store, both fixed and pinned by tests that were mutation-verified (guard removed → exactly the right test fails → restored): 1. **`refresh()` had no reentrancy protection.** Overlapping calls committed in *completion* order, not *start* order — so a slow poll that began before a workspace was deleted could **resurrect** it, and one that began before a spawn could **erase** it. Both reproduced. Fixed with a generation counter: last-*started* wins, superseded calls discard rather than commit. Deliberately **not** fixed by coalescing onto the in-flight promise: mutation handlers do `await api.removeWorker(id); await workspaces.refresh();` and must see their own write. Coalescing would hand them a promise that started *before* the delete, leaving a removed workspace on screen until the next tick. 2. **A superseded refresh that *failed* could paint a stale error over a newer success** — an error banner sitting on top of correctly-loaded data. The guard existed but was pinned by no test; removing it left all 616 tests green. ## The self-triggering `$effect` trap `Sessions.svelte` read its health map for a TTL check *and* wrote it, from inside an effect. That was safe **only by accident**: `await api.listSessions()` ran first, pushing the read out of Svelte's synchronous tracking window. This PR removes that await. TTL timestamps therefore moved to a plain, non-reactive `Map`. And the effect calls `probeHealth()` inside `untrack()` — load-bearing, not decoration: `probeHealth()` reads `list` synchronously, so calling it bare would register the store's `$derived` roster as a dependency, and the store hands back a fresh array reference on every refresh (committed in a *later* flush than the tick bump). That would fire the effect twice per poll and double-fetch `sessionHealth` — reintroducing, on a different endpoint, the exact duplicate request this PR exists to kill. ## Deliberate, please don't "clean up" - **`liveness()`, `label()`, `sessionName()` have no caller.** They are PR-C2 prerequisites and are commented as such. `liveness()` is three-valued (`alive` / `dead` / `unknown`) on purpose: C2 prunes dock panels for dead sessions, and a boolean would make a failed poll indistinguishable from a removed session — destroying the operator's layout on a transient error. It mirrors `convergeSessions`, which never hard-deletes a session row when the worktree probe fails (AGENTS.md fact #15). Delete it and `loaded` + `sessionsLoaded` become write-only and the whole stale-tolerance design goes with it. - **`Dock.svelte` still has its own `listSessions`.** Out of scope: PR-C2 rewrites that file wholesale. ## Behaviour change worth knowing The workspace **"start agent" / "stop agent"** toggle no longer flips on a local optimistic flag; it awaits a store refresh and reads backend truth. One extra round trip before the button updates. Strictly more correct — the old optimism could disagree with the very next poll — but it is a real latency change. ## Verification - `pnpm typecheck` → 0 errors (1006 files) - `pnpm test` → **617 passed**, of which 20 are the new store's - Browser: seeded throwaway instance, request counts above; workspace-shell toggle confirmed driven by real backend state (a workspace *with* a shell row shows "stop agent", one without shows "start agent"); rail and expanded list agree, since they now read the same store. Test suite is node-only with no DOM, so `.svelte` components remain structurally untestable — the browser pass is the honest substitute, not an optional extra.

lz added 11 commits

2026-07-12 21:59:24 +02:00

docs: design for dockview PR-C (per-panel targets, retiring single scope) 1b1767e77b

Splits the work into PR-C1 (shared workspaces store — invisible refactor that
also kills the double listSessions fetch) and PR-C2 (per-panel targets, Lucide
icons, mobile list/dock switch).

Records the rejected alternatives (session groups; pinned panels) and the two
accepted losses: the URL stops carrying the session, and N open sessions cost N
live WebSockets + N artifact pollers.

docs: spell out that the three panel views are independent e72d005456

Terminal is the default click target but never a prerequisite: opening Files or
Artifacts for a session creates no terminal panel and no WebSocket. Also records
the accepted limit — sessions with no open panel are not polled, so artifacts
pushed there are silent until opened.

docs: implementation plan for dockview PR-C1 (shared workspaces store) be4c0dfb40

Seven tasks, TDD. The store lands first with its own tests (including the
regression test asserting exactly one listSessions per worker), then the three
consumers migrate one at a time, then a browser pass counts the actual requests.

Calls out the self-triggering-effect trap in Sessions.svelte: its health TTL map
is safe today only because an await pushes the read out of the tracking window,
and removing the listSessions call would expose it.

feat(stores): WorkspacesStore — one poller for workers + sessions 17e71057e4

Three components fetched this data independently; Workers.svelte and its child
Sessions.svelte fetched the SAME worker's sessions twice per tick. One store,
one refresh, everyone reads.

liveness() is three-valued on purpose: PR-C2 prunes dock panels for dead
sessions, and a boolean would make a failed poll indistinguishable from a
removed session — deleting the operator's layout on a transient error.

docs: fix the typecheck command in the PR-C1 plan 52417e44dd

The repo's script is `pnpm typecheck`, not `pnpm check`.

refactor(main-page): MainSplit owns the single workspaces poll 2cf238f261

refactor(rail): WorkspaceRail reads the shared store instead of fetching 614b4c677a

refactor(sessions): read the roster from the shared store 1a72508ef3

Keeps per-session health probing here (nothing else fetches it), but moves the
TTL timestamps into a plain Map. With the listSessions await gone, a $state TTL
map would be read synchronously inside the effect that writes health — a
self-triggering effect, the same trap documented in Dock.svelte.

refactor(workers): read the roster from the shared store a4448c0ee2

Deletes the per-worker listSessions call that duplicated the one each child
Sessions.svelte was already making. The workspace-shell toggle now reflects
backend truth after a refetch rather than local optimism.

fix(sessions): stop the health probe firing twice per poll 5674fea07d

The effect tracked `list`, which is $derived off the store and gets a fresh
array reference on every successful refresh — landing in a later flush than
the tick bump. So probeHealth() ran twice per tick and could double-fetch
sessionHealth under adverse timing. Track tick only; probe a newly-created
session directly from add().

Dropping `void list` alone would not have done it: probeHealth() reads `list`
synchronously before its first await, so a bare call from the effect still
registers the dependency. The untrack() wrapper is what actually severs it.

docs(stores): mark label/sessionName as PR-C2 prerequisites, not dead code

ci / nexus (pull_request) Successful in 6m15s

Details

ci / images (pull_request) Successful in 11m50s

Details

pr-image-cleanup / delete-pr-images (pull_request) Successful in 7s

Details

29a2e89e04

label() and sessionName() have no caller in this PR. That is deliberate: PR-C2
gives each dock panel its own {workerId, sessionId} target, so a tab title must
be resolved FROM the target rather than threaded down from the click that set
it. Without this note a reviewer (or a code-quality pass) would reasonably
delete them.

Also gitignore .playwright-cli/ — regenerated on every browser-verification
pass, and one stray 'git add -A' from being committed.

lz referenced this pull request

2026-07-13 06:32:59 +02:00

feat(dock): panels carry their own target — retire the single scope (PR-C2) #63

lz changed target branch from feat/dockview-pr-b to main

2026-07-13 21:09:50 +02:00

lz merged commit 2c0c4976c6 into main

2026-07-13 21:10:43 +02:00

lz deleted branch feat/dockview-pr-c1

2026-07-13 21:10:43 +02:00

lz referenced this pull request from a commit

2026-07-13 21:10:44 +02:00

Merge pull request 'refactor(main-page): one shared workspaces store (PR-C1)' (#62) from feat/dockview-pr-c1 into main