Archived
docs(relay): record worktree-autodetect leg 3 and relay completion
This commit is contained in:
Federico Jaramillo Martinez
@@ -359,3 +359,89 @@ session daemon restart is required**.
|
|||||||
### Handing off?
|
### Handing off?
|
||||||
|
|
||||||
**Yes.** Work committed, packet updated, leg 3 dispatched via `spawn_session`.
|
**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.
|
||||||
|
|||||||
@@ -1,127 +1,64 @@
|
|||||||
# Status — relay "worktree-autodetect"
|
# 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
|
All three legs are complete and committed on `feat/worktree-autodetect`. `npm run verify` is
|
||||||
no outstanding decisions. Run leg 3 — the final leg.
|
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
|
## Current position
|
||||||
|
|
||||||
Legs 1 and 2 are complete and committed (`266f941`, `d0f8f9f`). The server hides prunable
|
Every charter finish-line condition is satisfied:
|
||||||
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.
|
|
||||||
|
|
||||||
The design remains reduced scope: detection piggybacked on browser resume. No watchers,
|
1. ✅ **Prunable worktrees hidden** — `266f941`. `discoverGitWorktrees` parses `prunable`;
|
||||||
no timers, no new processes, no new push channel. Breakdown is in `plan.md`.
|
`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
|
## Leg tracking
|
||||||
|
|
||||||
- **Last completed leg:** 2 (client: non-disruptive topology refresh method)
|
- **Last completed leg:** 3 (final — wiring, docs, changeset, full verify)
|
||||||
- **Next leg to run:** 3 (final)
|
- **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()`
|
## Relevant context if anyone picks this branch up
|
||||||
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**.
|
|
||||||
|
|
||||||
Finish with `npm run verify` (charter requires green) — this is the final leg.
|
- Commits: `266f941` (server filter), `d0f8f9f` (client refresh method),
|
||||||
|
`84545fb` (wiring + docs + changeset), plus three `docs(relay)` packet commits.
|
||||||
Signature detail leg 3 needs: `refreshSelectedProjectTopology()` takes no arguments,
|
- The invariant to protect on any future edit: **never route a background topology refresh
|
||||||
returns `Promise<void>`, never rejects (failures go to the injected background error sink,
|
through `selectWorkspace`.** It calls `clearActiveSession()` and
|
||||||
defaulting to `console.warn`), and no-ops when no project is selected. So it can be dropped
|
`resetWorkspaceScopedState()` with no already-selected guard, which would close the session
|
||||||
directly into the existing `Promise.all` without a `.catch`.
|
socket and blank the chat on every browser resume. `workspaceController.test.ts` was
|
||||||
|
mutation-checked against exactly that regression in leg 2.
|
||||||
## Relevant context for the next runner
|
- The relay packet lives under `.pi-web/`, which is gitignored; packet commits used
|
||||||
|
`git add -f`.
|
||||||
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.
|
|
||||||
|
|
||||||
## Blockers
|
## Blockers
|
||||||
|
|
||||||
None. Leg 2's main known risk (UI churn on refresh) did **not** materialize: applying the
|
None. No intervention signal fired in any leg. The one serious known risk — visible UI churn
|
||||||
list through `applyProjectWorkspaces` alone required no workaround, and
|
on refresh — did not materialize and needed no workaround.
|
||||||
`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.
|
|
||||||
|
|||||||
Reference in New Issue
Block a user