From 36d4e9a29636a948811a5e2623b194550af4aa3c Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sun, 26 Jul 2026 21:42:03 +0200 Subject: [PATCH] docs(relay): record worktree-autodetect leg 3 and relay completion --- .pi-web/relays/worktree-autodetect/log.md | 86 ++++++++++ .pi-web/relays/worktree-autodetect/status.md | 157 ++++++------------- 2 files changed, 133 insertions(+), 110 deletions(-) diff --git a/.pi-web/relays/worktree-autodetect/log.md b/.pi-web/relays/worktree-autodetect/log.md index 0e629ce..19893c4 100644 --- a/.pi-web/relays/worktree-autodetect/log.md +++ b/.pi-web/relays/worktree-autodetect/log.md @@ -359,3 +359,89 @@ session daemon restart is required**. ### Handing off? **Yes.** Work committed, packet updated, leg 3 dispatched via `spawn_session`. + +--- + +## Leg 3 — wire the refresh to browser resume, document, changeset, full verify (final) + +### What I did + +Wired the leg-2 refresh into the two existing refresh paths, documented the behavior, +added the changeset, and ran the full `npm run verify` the charter requires of the last leg. + +1. **Wiring (`src/client/src/components/PiWebApp.ts`, 2 lines).** Added + `this.workspaces.refreshSelectedProjectTopology()` to the `Promise.all` in + `refreshAfterBrowserResume` (~438) and in `refreshAppData` (~496). Nothing else in that + 2300-line file was read or touched, per the charter's reading discipline. +2. **Wiring test (`src/client/src/components/PiWebApp.workspaceTopology.test.ts`, new, + 3 tests).** Verifies the refresh is invoked by the exact callback + `BrowserResumeController` calls (`browserResume.callbacks.refreshAfterResume`, not a + hand-picked method reference), by `refreshAppData`, and that it still runs when a sibling + refresh in the same batch rejects. Sibling refreshes are stubbed so the assertions observe + only the topology call. +3. **Docs (`docs/faq.html`).** New FAQ entry `#worktree-list-out-of-date` plus its TOC link: + worktrees are listed on demand and never registered; the list is re-read on tab + focus/visibility rather than continuously, so refocusing updates it; selection/session/ + scroll are preserved; worktrees with a missing checkout directory are hidden and git still + tracks them until `git worktree prune`. Placed in the FAQ per + `.agents/skills/documentation-guide/SKILL.md` (troubleshooting/edge-case content); + `README.md` deliberately untouched. +4. **Changeset (`.changeset/worktree-autodetect.md`).** `patch` for `@jmfederico/pi-web` + (CalVer: patch is correct for a non-breaking user-facing capability), written as user + behavior rather than an implementation log. + +### Decisions and why + +- **Did not add the `connectRealtime` `onReconnect` call.** `plan.md` marked it optional + "only if it costs nothing". It is not free: `onReconnect` captures the machine id at + connect time, while `refreshSelectedProjectTopology` reads the selected machine at call + time, so wiring them together would either need a machine-scoped variant or would fire a + refresh for a machine the user has since left. Adding a second concept for no user-visible + gain over the resume path failed the cost test. Socket reconnect on resume is already + covered by the resume path itself. +- **Did not pass a message-prefixing `onBackgroundError` sink from `PiWebApp`.** Leg 2 made + it optional with a `console.warn` default that already includes project and machine in the + message. `status.md` explicitly recorded this as not-required work; adding it would be + scope creep for identical output. +- **Verified the wiring test is not vacuous.** Removed both call sites with `perl`, re-ran the + file: 3/3 failed. Restored the file (verified 2 occurrences back) and re-ran: 3/3 passed. +- **No charter boundary crossed.** No timer, watcher, process, endpoint, or push channel; + no change inside `refreshSelectedProjectTopology`; leg 2's tests were not weakened; the + resume path re-derives no selection from the refreshed list. + +### Checks run + +- `npx vitest --run src/client/src/components/PiWebApp.workspaceTopology.test.ts` → + **3 passed** (first run had 1 failure from a stub-shape mistake in my own test helper, + fixed by failing one named sibling refresh instead of all of them) +- mutation check (call sites removed) → **3 failed**, as intended; restored → **3 passed** +- `git diff --check` → clean +- `npx eslint` on the new test and `PiWebApp.ts` → **clean** +- **`npm run verify` → green**: typecheck, lint, knip, and 228 test files / + 1840 passed, 2 skipped +- pre-commit `npm run verify:staged` → typecheck (cached), knip, eslint, related vitest + (6 files / 20 tests) all **passed** + +### Artifacts changed + +- `src/client/src/components/PiWebApp.ts` (2 lines added) +- `src/client/src/components/PiWebApp.workspaceTopology.test.ts` (new, 3 tests) +- `docs/faq.html` (new FAQ entry + TOC link) +- `.changeset/worktree-autodetect.md` (new) +- committed as `84545fb feat(workspaces): refresh worktrees on browser resume` +- `status.md` rewritten as a finished-relay baton: finish-line conditions checked off one by + one with their commits, leg tracking set to last completed 3 / next none, shipped behavior + described in user terms, and the never-route-through-`selectWorkspace` invariant recorded + for whoever edits this code next + +### Blockers + +None. No intervention signal fired in this leg or any earlier one. No sessiond code touched, +so **no manual session daemon restart is required**; the change lands on the autoreloading +web/UI service path only. + +### Handing off? + +**No — this was the final leg.** All five charter finish-line conditions are met and +`npm run verify` is green, so per the charter this runner stops instead of spawning. The +branch `feat/worktree-autodetect` is ready for human review and merge. diff --git a/.pi-web/relays/worktree-autodetect/status.md b/.pi-web/relays/worktree-autodetect/status.md index ced8027..874a5ac 100644 --- a/.pi-web/relays/worktree-autodetect/status.md +++ b/.pi-web/relays/worktree-autodetect/status.md @@ -1,127 +1,64 @@ # Status — relay "worktree-autodetect" -## ✅ APPROVED — relay is live +## 🏁 FINISHED — the relay reached its finish line -The human approved the reduced scope in leg 0 and answered every open question. There are -no outstanding decisions. Run leg 3 — the final leg. +All three legs are complete and committed on `feat/worktree-autodetect`. `npm run verify` is +green. No further leg was spawned; leg 3 was the last one by design. + +Remaining human action: review the branch and merge it. Nothing is blocked. ## Current position -Legs 1 and 2 are complete and committed (`266f941`, `d0f8f9f`). The server hides prunable -worktrees, and the client now has a proven-inert refresh method. **Nothing calls it yet**, -so newly created worktrees still only appear on a full page load. Leg 3 wires the trigger -and is the last leg. +Every charter finish-line condition is satisfied: -The design remains reduced scope: detection piggybacked on browser resume. No watchers, -no timers, no new processes, no new push channel. Breakdown is in `plan.md`. +1. ✅ **Prunable worktrees hidden** — `266f941`. `discoverGitWorktrees` parses `prunable`; + `WorkspaceService` filters those worktrees out, while always keeping the project's own + worktree so a project is never empty. +2. ✅ **Non-disruptive client refresh** — `d0f8f9f`. + `WorkspaceController.refreshSelectedProjectTopology()` applies results through + `applyProjectWorkspaces` only, never `selectWorkspace`, so selection, session, file tree, + git status, and terminal selection survive. A vanished selected workspace is left alone + for the existing deletion path to handle. +3. ✅ **Wired to the existing resume path** — `84545fb`. Called from + `PiWebApp.refreshAfterBrowserResume` and `refreshAppData`. No new timer, watcher, + process, endpoint, or push channel. Remote machines work through the existing + `machinePrefix` + `FEDERATED_HTTP_ROUTES` plumbing. +4. ✅ **Tests** — prunable parsing/filtering (`gitWorktreeDiscovery.test.ts`, + `workspaceService.test.ts`), refresh-preserves-selection and + refresh-when-selected-workspace-disappeared (`workspaceController.test.ts`, 7 tests), + resume + app-data wiring (`PiWebApp.workspaceTopology.test.ts`, 3 tests). +5. ✅ **`npm run verify` green** (228 files / 1840 passed, 2 skipped) and + `.changeset/worktree-autodetect.md` exists. + +Nothing in this work touched `src/server/sessiond.ts`, session runtime ownership, or the +daemon protocol. **No manual session daemon restart is required.** ## Leg tracking -- **Last completed leg:** 2 (client: non-disruptive topology refresh method) -- **Next leg to run:** 3 (final) +- **Last completed leg:** 3 (final — wiring, docs, changeset, full verify) +- **Next leg to run:** none. Relay complete; do not spawn another runner. -## Next task — leg 3 +## Shipped behavior, as a user sees it -Wire the refresh to the existing resume path, document it, add the changeset. +A worktree created or deleted outside PI WEB shows up in (or disappears from) the workspace +list the next time the browser tab regains focus or becomes visible. Detection is +resume-scoped, not instant, by explicit human decision. A worktree whose checkout directory +was `rm -rf`ed no longer appears as a selectable workspace. Documented in `docs/faq.html` +under `#worktree-list-out-of-date`. -See `plan.md` → "Leg 3". Summary: call `this.workspaces.refreshSelectedProjectTopology()` -from `PiWebApp.refreshAfterBrowserResume` (~line 432) and `refreshAppData` (~line 485) — -touch only those two methods in that 2300-line file. Optionally also on -`connectRealtime`'s `onReconnect`, only if it costs nothing. Then one short honest doc -paragraph under `docs/` (detection is resume-scoped, not instant; do not grow `README.md`), -and the `.changeset/*.md` fragment — **still missing, and leg 3 owns it**. +## Relevant context if anyone picks this branch up -Finish with `npm run verify` (charter requires green) — this is the final leg. - -Signature detail leg 3 needs: `refreshSelectedProjectTopology()` takes no arguments, -returns `Promise`, never rejects (failures go to the injected background error sink, -defaulting to `console.warn`), and no-ops when no project is selected. So it can be dropped -directly into the existing `Promise.all` without a `.catch`. - -## Relevant context for the next runner - -Facts established in leg 0 — trust these, they were verified against the running code and -real git; do not re-derive them: - -- **Worktrees are already derived, never registered.** `WorkspaceService.list()` shells out - to `git worktree list --porcelain` on **every** `GET /projects/:projectId/workspaces` - request. There is no server-side cache and no invalidation to design. A worktree created - outside PI WEB is *already* discovered — the gap is purely that the browser never re-asks. -- **`projects.json` holds projects only, not workspaces.** So "auto-adoption" is a - non-problem: nothing needs to be written to a registry, and there is no adopt-vs-visible - distinction to design. This collapsed most of the feature's apparent complexity. -- **Cost of discovery is ~2ms** (measured: 20 sequential `git worktree list --porcelain` - runs in 42ms on this repo). -- **`prunable` is real and load-bearing.** After `rm -rf`ing a worktree directory without - `git worktree remove`, `git worktree list --porcelain` still lists it, with an added - `prunable gitdir file points to non-existent location` line. PI WEB currently shows this - as a normal selectable workspace. `locked` appears as a bare valueless line. -- **The resume path already exists and is already debounced.** - `src/client/src/appShell/browserResumeController.ts` listens to window `focus` and - document `visibilitychange`, batches per animation frame, and collapses concurrent - requests via `TrailingRefreshCoordinator`. It calls - `PiWebApp.refreshAfterBrowserResume()` (~line 432), which already refreshes the selected - session, machine activities, and workspace-deletion runs. Workspace topology is the one - thing missing from that list. -- **Remote machines come for free.** `workspacesApi.workspaces(projectId, machineId)` - routes through `machinePrefix`, and `GET /projects/:projectId/workspaces` is already in - `FEDERATED_HTTP_ROUTES` in `src/shared/federatedRoutes.ts`. No transport work needed. -- **Leg 2's method is inert by construction and covered.** `refreshSelectedProjectTopology()` - applies results via `applyProjectWorkspaces` only, so it writes at most `workspaces` and - `workspacesByProjectId`. Seven tests in - `src/client/src/controllers/workspaceController.test.ts` cover: new worktree appears, - selection + session + file tree + terminal state preserved and `clearActiveSession` not - called, selected workspace disappeared (selection deliberately left alone), stale project - response discarded, stale machine response discarded, rejection reported to the error sink - without touching `state.error`, and no-project no-op. Leg 2 mutation-tested this: injecting - the plausible `selectPreferredWorkspace` + `selectWorkspace` version made 2 tests fail, so - the guard is real and not vacuous. Leg 3 must not weaken these tests to fit its wiring. -- **`WorkspaceControllerDependencies` gained `onBackgroundError`** (optional, defaults to - `console.warn`). `PiWebApp` constructs `WorkspaceController`; leg 3 may pass a message- - prefixing sink to match sibling controllers, but the default is already correct — this is - not required work. -- **Leg 1 shipped a seam leg 2 does not need but should know about.** `WorkspaceService` - now takes an optional `WorkspaceGitPort` (`{ isGitRepository, discoverGitWorktrees }`) - in its constructor, defaulting to the real git implementation, so workspace policy is - testable without a repo. `parseGitWorktreeList(stdout)` is exported from - `gitWorktreeDiscovery.ts` for pure parser tests. Server-side `Workspace` shape is - unchanged — `prunable`/`locked` live on `GitWorktreeInfo` only and never reach the API, - so the client needs no type changes. -- **The danger is `selectWorkspace`.** It calls `clearActiveSession()` and - `resetWorkspaceScopedState()`. A refresh must apply the new list via - `applyProjectWorkspaces` **only**, and must not route through `selectWorkspace` when the - selection is still valid. `ProjectActivityOwnershipCoordinator` is the existing precedent - for background topology hydration that does not disturb selection — read it if leg 2 - needs a model. -- **No session daemon involvement.** Nothing in this design touches `src/server/sessiond.ts`, - session runtime ownership, or the daemon protocol. **No manual sessiond restart needed.** - -## Progress documentation expected of each runner - -- Commit the slice (Conventional Commit message). -- Update this file: current position, leg tracking, next task, blockers. -- Append to `log.md`: what, why, artifacts, exact checks run and results. -- Add the `.changeset/*.md` fragment no later than leg 3. - -## Decisions settled by the human (do not re-open) - -1. **Latency: resume-scoped is acceptable.** Detection on tab refocus/visibility is the - agreed behavior. Do not add a timer or a watcher to shorten it. -2. **Removed worktrees: hide them.** Filtering `prunable` worktrees out of the workspace - list is wanted and approved. -3. **Sibling overlap: assume the other session does nothing.** This relay owns the - workspace-topology refresh seam outright. Build it here, do not design for sharing, - and do not read `/srv/dev/pi-web-worktrees/worktree-create-ui`. If that branch later - merges something overlapping, resolving it is that branch's problem, not this one's. +- Commits: `266f941` (server filter), `d0f8f9f` (client refresh method), + `84545fb` (wiring + docs + changeset), plus three `docs(relay)` packet commits. +- The invariant to protect on any future edit: **never route a background topology refresh + through `selectWorkspace`.** It calls `clearActiveSession()` and + `resetWorkspaceScopedState()` with no already-selected guard, which would close the session + socket and blank the chat on every browser resume. `workspaceController.test.ts` was + mutation-checked against exactly that regression in leg 2. +- The relay packet lives under `.pi-web/`, which is gitignored; packet commits used + `git add -f`. ## Blockers -None. Leg 2's main known risk (UI churn on refresh) did **not** materialize: applying the -list through `applyProjectWorkspaces` alone required no workaround, and -`handleWorkspaceChange` early-returns on an unchanged selected workspace id, so a -fresh-but-equal list causes no downstream churn. No intervention signal fired. - -Leg 3's own watch item: after wiring, confirm nothing in the resume path re-derives -selection from the refreshed list. If wiring turns out to need a change *inside* -`refreshSelectedProjectTopology` to stay non-disruptive, that is the intervention signal — -stop rather than relaxing the invariant. +None. No intervention signal fired in any leg. The one serious known risk — visible UI churn +on refresh — did not materialize and needed no workaround.