Archived
docs(relay): record worktree-autodetect leg 1
This commit is contained in:
@@ -195,3 +195,81 @@ git status, open diff, terminal selection).
|
|||||||
|
|
||||||
**Yes.** Packet committed, `status.md` un-parked with the decisions recorded, leg 1
|
**Yes.** Packet committed, `status.md` un-parked with the decisions recorded, leg 1
|
||||||
dispatched via `spawn_session`.
|
dispatched via `spawn_session`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Leg 1 — Stop reporting removed worktrees (server truth)
|
||||||
|
|
||||||
|
**Commit:** `266f941` — `fix(workspaces): hide worktrees whose checkout directory is gone`
|
||||||
|
|
||||||
|
### What I did
|
||||||
|
|
||||||
|
1. **Parser reports facts.** `src/server/workspaces/gitWorktreeDiscovery.ts` now reads the
|
||||||
|
`prunable` and `locked` keys. Extracted the pure parsing step into an exported
|
||||||
|
`parseGitWorktreeList(stdout)`; `discoverGitWorktrees` is now just the `execFile`
|
||||||
|
boundary plus that call. `GitWorktreeInfo` gained optional `prunable` / `locked`.
|
||||||
|
2. **Service decides policy.** `src/server/workspaces/workspaceService.ts` filters prunable
|
||||||
|
linked worktrees in a small private `selectable()` step, keeping the entry whose path
|
||||||
|
equals `project.path` unconditionally. Existing "no worktrees" fallback then also covers
|
||||||
|
the case where every listed worktree was filtered away, so a project can never present
|
||||||
|
an empty workspace list.
|
||||||
|
3. **Injected the git boundary.** `WorkspaceService` now takes an optional
|
||||||
|
`WorkspaceGitPort` (`{ isGitRepository, discoverGitWorktrees }`) defaulting to the real
|
||||||
|
implementation. This is what made the policy testable without a real repo or a
|
||||||
|
subclass-override fake, and it left every existing `new WorkspaceService()` call site
|
||||||
|
(`app.ts`, `app.testSupport.ts`, `sessiond.ts`) untouched.
|
||||||
|
4. **Two new test files** (the plan predicted one; the policy assertions belong next to the
|
||||||
|
service that owns them, not next to the parser):
|
||||||
|
- `gitWorktreeDiscovery.test.ts` — parser only, fixtures captured verbatim from real git.
|
||||||
|
- `workspaceService.test.ts` — prunable hidden, locked kept, project path kept even when
|
||||||
|
marked prunable, fallback when everything is filtered, plus pre-existing labeling and
|
||||||
|
non-git behavior pinned so leg 2/3 have a regression net.
|
||||||
|
|
||||||
|
### Decisions and why
|
||||||
|
|
||||||
|
- **Facts in the parser, policy in the service**, as `plan.md` preferred. The words
|
||||||
|
`prunable` and `project.path` now appear in exactly one place each.
|
||||||
|
- **Constructor injection rather than subclass-override fakes.** `workspaceDeletionRoutes.test.ts`
|
||||||
|
fakes `WorkspaceService` by subclassing and overriding `list()`, which cannot test `list()`
|
||||||
|
itself. A narrow port is the smaller seam and keeps the production default unchanged.
|
||||||
|
- **Read-only, as mandated.** No `git worktree prune`, no repo metadata mutation anywhere.
|
||||||
|
- **`locked` is parsed but deliberately not acted on.** A locked worktree is a real checkout
|
||||||
|
and stays selectable. It is surfaced now because git emits it in the same records and
|
||||||
|
future UI may want it; adding it later would mean touching the parser again.
|
||||||
|
- **Re-verified the git behavior** rather than trusting it: created a throwaway repo,
|
||||||
|
`rm -rf`'d a linked worktree and locked another. Confirmed `prunable gitdir file points to
|
||||||
|
non-existent location` and `locked keep me`. Test fixtures are that exact output. Probe
|
||||||
|
repo deleted afterwards.
|
||||||
|
- **No API/type change.** `prunable`/`locked` never leave the server, so the shared
|
||||||
|
`Workspace` type and the client are untouched. Leg 2 needs no type work.
|
||||||
|
|
||||||
|
### Checks run
|
||||||
|
|
||||||
|
- `npm test -- --run src/server/workspaces/gitWorktreeDiscovery.test.ts src/server/workspaces/workspaceService.test.ts` → 10 passed
|
||||||
|
- `npm test -- --run src/server/workspaces/` → 12 files, 88 passed
|
||||||
|
- `npm test -- --run src/server/app.projects.test.ts` → 5 passed (workspace list route contract)
|
||||||
|
- `npm run typecheck` → clean
|
||||||
|
- `npx eslint` on all four changed files → clean
|
||||||
|
- pre-commit `verify:staged` → cached typecheck, knip, eslint, 14 related test files / 63 tests, all green
|
||||||
|
|
||||||
|
`npm run verify` was not run; per `plan.md` that is leg 3's gate.
|
||||||
|
|
||||||
|
### Artifacts changed
|
||||||
|
|
||||||
|
- `src/server/workspaces/gitWorktreeDiscovery.ts` (modified)
|
||||||
|
- `src/server/workspaces/workspaceService.ts` (modified)
|
||||||
|
- `src/server/workspaces/gitWorktreeDiscovery.test.ts` (new)
|
||||||
|
- `src/server/workspaces/workspaceService.test.ts` (new)
|
||||||
|
- `status.md` (leg tracking → last completed 1 / next 2, next task set to leg 2, added a
|
||||||
|
note about the new `WorkspaceGitPort` seam)
|
||||||
|
|
||||||
|
No changeset yet — charter allows it any time up to leg 3, and leg 3 owns one fragment for
|
||||||
|
the whole user-visible behavior.
|
||||||
|
|
||||||
|
### Blockers
|
||||||
|
|
||||||
|
None. Nothing ambiguous, no design decision needed, no intervention trigger fired.
|
||||||
|
|
||||||
|
### Handing off?
|
||||||
|
|
||||||
|
**Yes.** Work committed, packet updated, leg 2 dispatched via `spawn_session`.
|
||||||
|
|||||||
@@ -3,32 +3,39 @@
|
|||||||
## ✅ APPROVED — relay is live
|
## ✅ APPROVED — relay is live
|
||||||
|
|
||||||
The human approved the reduced scope in leg 0 and answered every open question. There are
|
The human approved the reduced scope in leg 0 and answered every open question. There are
|
||||||
no outstanding decisions. Run leg 1.
|
no outstanding decisions. Run leg 2.
|
||||||
|
|
||||||
## Current position
|
## Current position
|
||||||
|
|
||||||
Leg 0 (assessment/design) is complete. No production code has been written.
|
Leg 1 is complete and committed (`266f941`). The **server** side of the feature is done:
|
||||||
|
prunable worktrees are no longer reported as workspaces. Nothing in the client has changed
|
||||||
|
yet, so newly created worktrees still only appear on a full page load.
|
||||||
|
|
||||||
The recommendation is **reduced scope**: detection piggybacked on browser resume, plus a
|
The design remains reduced scope: detection piggybacked on browser resume. No watchers,
|
||||||
fix for the inverse (removed-worktree) case. No watchers, no timers, no new processes,
|
no timers, no new processes, no new push channel. Breakdown is in `plan.md`.
|
||||||
no new push channel. Rationale is in `log.md` leg 0; the implementation breakdown is in
|
|
||||||
`plan.md`.
|
|
||||||
|
|
||||||
## Leg tracking
|
## Leg tracking
|
||||||
|
|
||||||
- **Last completed leg:** 0 (assessment and packet creation)
|
- **Last completed leg:** 1 (server: hide removed worktrees)
|
||||||
- **Next leg to run:** 1
|
- **Next leg to run:** 2
|
||||||
|
|
||||||
## Next task — leg 1
|
## Next task — leg 2
|
||||||
|
|
||||||
Stop reporting worktrees whose checkout directory has been removed outside PI WEB.
|
Non-disruptive workspace topology refresh in the client.
|
||||||
|
|
||||||
See `plan.md` → "Leg 1". Summary: teach the `git worktree list --porcelain` parser about
|
See `plan.md` → "Leg 2" and **read it fully before writing the method** — it spells out the
|
||||||
the `prunable` and `locked` keys, exclude prunable linked worktrees from the workspace
|
plausible-looking wrong implementation and exactly why it is destructive. Summary: add
|
||||||
list, never filter the main worktree, never mutate the repo (no `git worktree prune`).
|
`refreshSelectedProjectTopology()` to `WorkspaceController` that re-lists the selected
|
||||||
|
project's workspaces and applies them through `applyProjectWorkspaces` **only** — never
|
||||||
|
through `selectWorkspace`, which has no already-selected guard and would clear the active
|
||||||
|
session and all workspace-scoped state on every alt-tab. Guard against machine/project
|
||||||
|
changing mid-flight; `console.warn` on failure, never `setState({ error })`.
|
||||||
|
|
||||||
Files: `src/server/workspaces/gitWorktreeDiscovery.ts`, `src/server/workspaces/workspaceService.ts`,
|
Files: `src/client/src/controllers/workspaceController.ts`,
|
||||||
new `src/server/workspaces/gitWorktreeDiscovery.test.ts`.
|
new `src/client/src/controllers/workspaceController.test.ts`.
|
||||||
|
|
||||||
|
Do **not** wire anything into `PiWebApp` in leg 2 — that is leg 3, deliberately after the
|
||||||
|
refresh is proven inert.
|
||||||
|
|
||||||
## Relevant context for the next runner
|
## Relevant context for the next runner
|
||||||
|
|
||||||
@@ -58,6 +65,13 @@ real git; do not re-derive them:
|
|||||||
- **Remote machines come for free.** `workspacesApi.workspaces(projectId, machineId)`
|
- **Remote machines come for free.** `workspacesApi.workspaces(projectId, machineId)`
|
||||||
routes through `machinePrefix`, and `GET /projects/:projectId/workspaces` is already in
|
routes through `machinePrefix`, and `GET /projects/:projectId/workspaces` is already in
|
||||||
`FEDERATED_HTTP_ROUTES` in `src/shared/federatedRoutes.ts`. No transport work needed.
|
`FEDERATED_HTTP_ROUTES` in `src/shared/federatedRoutes.ts`. No transport work needed.
|
||||||
|
- **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
|
- **The danger is `selectWorkspace`.** It calls `clearActiveSession()` and
|
||||||
`resetWorkspaceScopedState()`. A refresh must apply the new list via
|
`resetWorkspaceScopedState()`. A refresh must apply the new list via
|
||||||
`applyProjectWorkspaces` **only**, and must not route through `selectWorkspace` when the
|
`applyProjectWorkspaces` **only**, and must not route through `selectWorkspace` when the
|
||||||
@@ -87,4 +101,6 @@ real git; do not re-derive them:
|
|||||||
|
|
||||||
## Blockers
|
## Blockers
|
||||||
|
|
||||||
None. Leg 1 is clear to run.
|
None. Leg 2 is clear to run. The one thing to watch is leg 2's own intervention trigger:
|
||||||
|
if the refresh cannot be made non-disruptive without visible UI churn, stop rather than
|
||||||
|
working around it.
|
||||||
|
|||||||
Reference in New Issue
Block a user