147 lines
14 KiB
Markdown
147 lines
14 KiB
Markdown
# Worktree remove / prune
|
||
|
||
Delete losing worktrees and prune stale ones from any device. Backend adds `removeWorktree`/`pruneWorktrees` to `src/http/worktrees.ts` (same execFile-no-shell + timeout + containment machinery as the shipped `createWorktree`), two Origin-guarded routes in `src/server.ts`, and the FE makes the already-rendered `locked`/`prunable`/main tags in `public/projects.ts` `makeWorktreeRow` (lines 492–502) actionable.
|
||
|
||
## Contract
|
||
|
||
### New backend functions — `src/http/worktrees.ts`
|
||
|
||
Reuse the private helpers already in the file: `isGitRepo` (line 168), `listWorktrees`/`parseWorktrees` (lines 29, 69), `extractStderr` (line 180), `resolveRealPath` (line 118). Match `createWorktree`'s structured-result-never-throws contract (line 219).
|
||
|
||
```ts
|
||
export interface RemoveWorktreeOptions { readonly force?: boolean; readonly timeoutMs: number }
|
||
export async function removeWorktree(
|
||
repoPath: string, targetPath: string, opts: RemoveWorktreeOptions,
|
||
): Promise<RemoveWorktreeResult>
|
||
|
||
export interface PruneWorktreesOptions { readonly timeoutMs: number }
|
||
export async function pruneWorktrees(
|
||
repoPath: string, opts: PruneWorktreesOptions,
|
||
): Promise<PruneWorktreesResult>
|
||
```
|
||
|
||
`removeWorktree` algorithm (the security spine):
|
||
1. `isGitRepo(repoPath)` false → `{ ok:false, status:404, error:'Not a git repository.' }`.
|
||
2. `typeof targetPath !== 'string'` or empty → `{ ok:false, status:400, error:'Worktree path is required.' }`.
|
||
3. `listWorktrees(repoPath)` → find the entry whose **realpath equals** `resolveRealPath(targetPath)` (canonical compare, not raw-string compare, to defeat symlink tricks). No match → `{ ok:false, status:404, error:'That path is not a worktree of this repository.' }`.
|
||
4. Matched entry `.isMain === true` → `{ ok:false, status:400, error:'Cannot remove the main worktree.' }`.
|
||
5. Matched entry `.locked` → `{ ok:false, status:409, error:'This worktree is locked; unlock it in a terminal first.' }` (never double-force `-f -f`).
|
||
6. Run `git worktree remove` **with the canonical path from git's own list entry** (`match.path`), never the raw user string: `['worktree','remove', ...(force?['--force']:[]), '--', match.path]` via `execFileAsync` (cwd `repoPath`, `timeout: opts.timeoutMs`, `maxBuffer: WORKTREE_MAX_BUFFER`). On success `{ ok:true, path: match.path }`.
|
||
7. On error → `classifyRemoveError(err)` (new; sibling of `classifyWorktreeError` line 193): stderr containing `contains modified or untracked files` / `use --force` / `is dirty` → `{ ok:false, status:409, error:'Worktree has uncommitted changes — force required.' }`; `not a working tree`/`is not a working tree` → `{ ok:false, status:404, ... }`; else `{ ok:false, status:500, error:'Failed to remove the worktree.' }`. Never leak raw stderr (SEC-M10).
|
||
|
||
`pruneWorktrees` algorithm: `isGitRepo` gate (404), then `git worktree prune -v` via execFile (timeout/maxBuffer). Parse verbose lines (`Removing worktrees/<name>: <reason>`, emitted on stdout/stderr — capture both) into `pruned: string[]` (best-effort; empty when nothing prunable — idempotent). Error → `{ ok:false, status:500, error:'Failed to prune worktrees.' }`.
|
||
|
||
### Changed message types — `src/types.ts` (coordination edit)
|
||
|
||
Add next to `CreateWorktreeResult` (line 485):
|
||
|
||
```ts
|
||
export interface RemoveWorktreeResult { ok: boolean; path?: string; status?: number; error?: string }
|
||
export interface PruneWorktreesResult { ok: boolean; pruned?: string[]; status?: number; error?: string }
|
||
```
|
||
|
||
`WorktreeInfo` (line 290), `Config` worktree fields (lines 67–69) unchanged. Option interfaces stay local to `worktrees.ts` (mirrors `CreateWorktreeOptions`, line 207).
|
||
|
||
### New routes — `src/server.ts`
|
||
|
||
Insert both immediately after the create route (ends line 725), reusing `requireAllowedOrigin` (line 352), `sanitizeForLog` (line 162), `cfg.worktreeEnabled`, `cfg.worktreeTimeoutMs`. Add `removeWorktree, pruneWorktrees` to the import at line 43.
|
||
|
||
| Route | Body / gate | Response |
|
||
|---|---|---|
|
||
| `DELETE /projects/worktree` | `express.json({limit:'4kb'})`; `{ path, worktreePath, force? }`. Guard order: `requireAllowedOrigin` → `worktreeEnabled` (403) → `path`+`worktreePath` present (400). Audit-log via `sanitizeForLog`. | `result.ok` → `200 {ok:true,path}`; else `result.status ?? 500` + `{error}` |
|
||
| `POST /projects/worktree/prune` | `express.json({limit:'4kb'})`; `{ path }`. Same guard order (path required → 400). | `200 {ok:true,pruned}` or `result.status` + `{error}` |
|
||
|
||
`force` coerced strictly: `const force = body['force'] === true`.
|
||
|
||
### Env vars — `src/config.ts`
|
||
|
||
**None new.** Reuse `worktreeEnabled` (line 372, gates all worktree writes), `worktreeTimeoutMs` (line 374) for remove/prune timeouts.
|
||
|
||
### FE — `public/projects.ts`
|
||
|
||
- New module-level fetch helpers (siblings of `killSession`, line 275): `removeWorktreeReq(repoPath, worktreePath, force)` → `DELETE /projects/worktree` with JSON body, returns `{ok, status, error}`; `pruneWorktreesReq(repoPath)` → `POST /projects/worktree/prune`, returns `{ok, pruned?, error?}`. Both same-origin (Origin guard passes), best-effort catch.
|
||
- `makeWorktreeRow` (line 492) gains an optional `actions?: { onRemove:(wt)=>void }` param. When `actions` present and **not** `wt.isMain` and **not** `wt.locked`: append a `proj-wt-remove` `✕` button (`aria-label` "Remove worktree", `title` "Remove this worktree"). Locked rows keep the `locked` tag but no remove button (tooltip explains). Prunable rows still get remove.
|
||
- `DetailCallbacks` (line 666) gains `onRemoveWorktree:(worktreePath:string)=>void` and `onPruneWorktrees:()=>void`. `renderProjectDetail` (line 672) threads `actions` into the worktree-list loop (line 721) and, when `detail.worktrees.some(w=>w.prunable)`, renders a section-level `Prune stale worktrees` button beside the "Worktrees" title (line 712) wired to `cb.onPruneWorktrees`.
|
||
- `mountProjects` (line 820) implements the confirm→force→refresh flow (encapsulated here, like `killAndRefresh` line 897), passed into `renderDetail` (line 959):
|
||
- `onRemoveWorktree`: `confirm("Remove worktree at <path>? This deletes the working tree.")` → `removeWorktreeReq(detailPath, wtPath, false)`; on `409` → second `confirm("Uncommitted changes will be lost. Force-remove?")` → retry with `force:true`; on other failure show error via `textContent` (never innerHTML, SEC-L3/H6); then `void refresh()`.
|
||
- `onPruneWorktrees`: `confirm("Prune worktrees whose folders are gone?")` → `pruneWorktreesReq(detailPath)` → `refresh()`.
|
||
- `public/diff.ts`: **no change** — `grep` confirms it renders no worktree rows/tags; the task's mention is covered entirely by `projects.ts`. (Note this deviation from the task wording in the log.)
|
||
|
||
## Files to change
|
||
|
||
| Path | Change |
|
||
|---|---|
|
||
| `src/types.ts` | **Coordination edit:** add `RemoveWorktreeResult`, `PruneWorktreesResult` after line 491. |
|
||
| `src/http/worktrees.ts` | Add `removeWorktree`, `pruneWorktrees`, `RemoveWorktreeOptions`, `PruneWorktreesOptions`, `classifyRemoveError`; reuse existing `isGitRepo`/`listWorktrees`/`resolveRealPath`/`extractStderr`. |
|
||
| `src/server.ts` | Import (line 43) + two routes after line 725 (`DELETE /projects/worktree`, `POST /projects/worktree/prune`). |
|
||
| `public/projects.ts` | `removeWorktreeReq`/`pruneWorktreesReq` helpers; extend `makeWorktreeRow` (492), `DetailCallbacks` (666), `renderProjectDetail` (672), `mountProjects` (820). |
|
||
| `test/http/worktrees-remove.test.ts` | **New** — node, real temp repos; unit + integration of the two functions. |
|
||
| `test/integration/worktree.test.ts` | Extend with `DELETE`/prune route cases. |
|
||
| `test/worktree-form.test.ts` | **New describe blocks** — jsdom FE row/confirm/prune tests. |
|
||
|
||
## TDD steps (ordered)
|
||
|
||
Backend pure/logic — `test/http/worktrees-remove.test.ts` (node env, mirror `worktrees-create.test.ts`: `makeRepo()` helper, `gitAvailable` guard, real temp repos, no network):
|
||
|
||
1. Test `removeWorktree` returns `{ok:false,status:404}` for a non-git dir → implement `isGitRepo` gate.
|
||
2. Test empty/missing `targetPath` → `{ok:false,status:400}` → implement guard.
|
||
3. Test a path not in `git worktree list` → `{ok:false,status:404}` → implement realpath-match against `listWorktrees`.
|
||
4. Test main worktree (the repo root) → `{ok:false,status:400,error:/main/}` → implement `isMain` reject.
|
||
5. Test happy path: create a worktree via `createWorktree`, then `removeWorktree` (clean tree) → `{ok:true}`; assert `git worktree list` no longer contains it.
|
||
6. Test dirty worktree (write an untracked file into it) → `removeWorktree(force:false)` → `{ok:false,status:409}`; then `force:true` → `{ok:true}` → implement `classifyRemoveError` + force arg.
|
||
7. Test error message never contains `fatal:`/`error:` (SEC-M10) → assert on the 409 case.
|
||
8. Test locked worktree (`git worktree lock`) → `{ok:false,status:409,error:/locked/}` → implement locked reject.
|
||
9. Test symlink alias: pass a symlink pointing at a real worktree as `targetPath` → still matches via realpath and removes git's canonical path → verify (M2-style containment).
|
||
10. Test `pruneWorktrees` on a repo with a manually-`rm -rf`'d worktree dir → `{ok:true, pruned:[...]}` length ≥1; on a clean repo → `{ok:true, pruned:[]}` (idempotent). Non-git → `{ok:false,status:404}`.
|
||
|
||
Backend route — extend `test/integration/worktree.test.ts` (reuse `spawnServer`, `makeRealRepo`, `itGit`):
|
||
|
||
11. `DELETE /projects/worktree` foreign Origin → 403; missing Origin → 403.
|
||
12. `WORKTREE_ENABLED=0` → 403.
|
||
13. Missing `worktreePath` → 400.
|
||
14. Attempt to remove main worktree → 400.
|
||
15. `itGit`: create worktree via `POST /projects/worktree`, then `DELETE` it (clean) → 200 `{ok:true}`; `git worktree list` no longer lists it.
|
||
16. `itGit`: dirty worktree → DELETE without force → 409; with `force:true` → 200.
|
||
17. `POST /projects/worktree/prune`: Origin 403, disabled 403, and `itGit` prune-after-manual-delete → 200 with `pruned`.
|
||
|
||
FE — `test/worktree-form.test.ts` (jsdom; extend, reuse `makeDetail`, `makeHooks`, `makeCbs`, stubbed `fetch`):
|
||
|
||
18. `makeWorktreeRow` for a non-main non-locked wt with `actions` → contains a `.proj-wt-remove` button; main and locked rows → no button.
|
||
19. `renderProjectDetail` with a prunable worktree → a `Prune stale worktrees` button exists; without → absent.
|
||
20. Clicking Remove: stub `window.confirm=()=>true`, stub `fetch` → `{ok:true}`; assert `fetch` called `DELETE /projects/worktree` with `force:false` in body.
|
||
21. Dirty retry: `fetch` first resolves `{ok:false,status:409}` then `{ok:true}`; `confirm` returns true twice → assert second `fetch` body has `force:true`.
|
||
22. `confirm` returns false → assert `fetch` **not** called (no accidental deletion).
|
||
23. Error render: `fetch` → `{ok:false,status:500,error:'boom'}` → error shown via `textContent` (assert `.textContent`, no HTML injection).
|
||
|
||
Run `npm test`; keep the 80% gate — the new functions and both routes carry direct + error-path coverage; FE branches (button presence, confirm true/false, force retry, error) are all exercised.
|
||
|
||
## Edge cases & failure modes
|
||
|
||
- **Main worktree** — always rejected (400); the repo root can never be deleted.
|
||
- **Not-a-worktree path** — arbitrary FS path (e.g. `/etc`) never matches the list → 404, git never invoked against it.
|
||
- **Dirty tree** (modified tracked or untracked files) — git refuses without `--force`; surfaced as 409 → explicit second confirm before force.
|
||
- **Locked worktree** — 409 with a "unlock first" message; UI hides the remove button; never auto-escalate to `-f -f`.
|
||
- **Removing the worktree you're viewing** (`isCurrent` but not `isMain`) — allowed; after refresh the detail path may 404 → detail shows "Project not found" (existing null branch, line 691). Acceptable.
|
||
- **Already-removed / concurrent delete** — git errors "not a working tree" → 404 safe message; the follow-up `refresh()` reconciles the UI.
|
||
- **Prune with nothing prunable** — `{ok:true, pruned:[]}`, no error (idempotent).
|
||
- **git binary missing / timeout** — execFile rejects → 500 safe message; timeout bounded by `worktreeTimeoutMs`.
|
||
- **Path with control chars / flag-like leading `-`** — never reaches argv as-is: we pass git's own canonical list path, and `--` terminates options; audit log runs through `sanitizeForLog`.
|
||
- **DELETE-with-body stripped by an intermediary** — same-origin fetch, no proxy in the LAN threat model; body reliably delivered. (If ever a concern, mirror as query params — noted, not implemented.)
|
||
|
||
## Security
|
||
|
||
- **Origin/CSRF:** both routes call `requireAllowedOrigin` first (line 352) — destructive state change, mandatory (SEC-C3). Integration tests 11–12, 17 assert 403 for foreign/missing Origin.
|
||
- **Feature gate:** `cfg.worktreeEnabled` (403 when off) governs remove/prune exactly as it governs create.
|
||
- **No-shell exec:** `execFile('git', [...])` only; never a shell string. `timeout` + `maxBuffer` bound resource use.
|
||
- **Path containment (the core defense):** the target is accepted **only** if its realpath matches an entry git itself reports in `worktree list`, and the command runs against **git's canonical path**, not the user string — so no traversal/symlink/arbitrary-path deletion is reachable (M2-consistent). `--` belt-and-suspenders before the path arg.
|
||
- **Main-worktree protection:** `isMain` reject prevents deleting the repository itself.
|
||
- **Safe error messages:** `classifyRemoveError`/prune mapping return fixed strings; raw git stderr never returned (SEC-M10) — asserted in test 7.
|
||
- **FE injection:** error/label rendering uses `textContent` only (SEC-L3/H6), asserted in test 23.
|
||
- **Destructive-intent confirmation:** browser `confirm()` before any delete, plus a **second** confirm before `force` on a dirty tree — no single-click data loss.
|
||
- **Rate-limit:** inherits the app's per-connection posture; these are Origin-gated same-origin calls. (No new limiter added — consistent with the create route.)
|
||
|
||
## Effort & dependencies
|
||
|
||
- **Effort:** ~1.5–2 days (backend + routes ~0.75d, FE wiring + confirm flow ~0.5d, tests ~0.5d).
|
||
- **Depends on:** W4 *create worktree* (shipped `createWorktree`, whose `isGitRepo`/`listWorktrees`/`resolveRealPath`/`classify*` are reused) and the v0.6 project-detail worktree list (`makeWorktreeRow`).
|
||
- **Unlocks:** full worktree lifecycle from any device (create → work → **remove/prune losers**), and pairs naturally with W4 *stage/commit/push from the diff viewer* (issue #13) to close the "spin up a worktree, land the winner, delete the rest" loop. |