Worktree removal fault tolerance: delete first, deregister second

.specs/LESSONS.md

@@ -14,6 +14,12 @@ Corroborated across multiple features. Safe to apply as guidance.
 - evidence: src/main/workflow-ctx.ts:82,106 (CtxDeps.agent / CtxRuntime.signal SPEC_DEVIATION) (workflow-ctx) (+1 more)
 - last seen: 2026-07-06T16:15:40Z
 
+### L-005 — Before adding real-process or real-git tests, check whether existing suites already sit near the default per-test timeout: the extra parallel load alone can push them over it, turning a green gate red without any production change
+- signal: `gate_fail` · recurrence: 2 feature(s) · scope: `testing` · harmful: 0
+- features: worktree-post-create-hook, worktree-removal-fault-tolerance
+- evidence: validation.md round-2 gate section; tree.test.ts / worktree-manager.test.ts timeouts (testing) (+1 more)
+- last seen: 2026-07-31T12:27:40Z
+
 ## Candidates (under observation — do NOT load as guidance yet)
 
 Seen once or not yet corroborated. Tracked, not trusted.
@@ -36,11 +42,17 @@ Seen once or not yet corroborated. Tracked, not trusted.
 - evidence: mutant R1/M7; post-create-hook.test.ts output-tail test (testing)
 - last seen: 2026-07-29T22:37:04Z
 
-### L-005 — Before adding real-process or real-git tests, check whether existing suites already sit near the default per-test timeout: the extra parallel load alone can push them over it, turning a green gate red without any production change
-- signal: `gate_fail` · recurrence: 1 feature(s) · scope: `testing` · harmful: 0
-- features: worktree-post-create-hook
-- evidence: validation.md round-2 gate section; tree.test.ts / worktree-manager.test.ts timeouts (testing)
-- last seen: 2026-07-29T22:37:06Z
+### L-006 — Assert a returned payload field by its value, not by the value you handed an injected fake: a field that appears in the test only as a spy's input reads like coverage in review, but a mutation dropping it from the real return still passes
+- signal: `surviving_mutant` · recurrence: 1 feature(s) · scope: `testing` · harmful: 0
+- features: worktree-removal-fault-tolerance
+- evidence: round-1 mutant M6; worktree-manager.test.ts leftover: at :844/:867/:882 were spyDeleter inputs, not assertions - dropping the field from worktree-manager.ts:335-339 left all 80 tests green; closed by F1 124340c (testing)
+- last seen: 2026-07-31T12:27:40Z
+
+### L-007 — When writing a test to kill a specific surviving mutant, check the fixture does not encode that mutant's own blind spot: pick one whose readings differ under every wrong implementation, not just the one you saw. A directories-only residue pinned the recursive count yet let a directories-only count survive
+- signal: `surviving_mutant` · recurrence: 1 feature(s) · scope: `testing` · harmful: 0
+- features: worktree-removal-fault-tolerance
+- evidence: round-2 mutant N3 survived the round-1 fix F2 (dir-remover.test.ts:328-348 fixture wt/keep/a/b was directories-only); closed by F3 1abe8aa with a mixed chain giving 3/2/1/1 for every-entry/dirs-only/files-only/top-level (testing)
+- last seen: 2026-07-31T12:27:40Z
 
 ## Quarantined (failed when applied — ignore)
 

.specs/lessons.json

@@ -3,7 +3,7 @@
   "promote_threshold": 2,
   "window_days": 45,
   "quarantine_threshold": 2,
-  "next_id": 6,
+  "next_id": 8,
   "lessons": [
     {
       "id": "L-001",
@@ -85,17 +85,55 @@
       "text": "Before adding real-process or real-git tests, check whether existing suites already sit near the default per-test timeout: the extra parallel load alone can push them over it, turning a green gate red without any production change",
       "signal": "gate_fail",
       "scope": "testing",
+      "status": "confirmed",
+      "features": [
+        "worktree-post-create-hook",
+        "worktree-removal-fault-tolerance"
+      ],
+      "recurrence": 2,
+      "harmful": 0,
+      "evidence": [
+        "validation.md round-2 gate section; tree.test.ts / worktree-manager.test.ts timeouts (testing)",
+        "two full runs of untouched main red (2 failed, then 14 failed across 5 files), all duration overruns 11430-15557ms; fixed by vitest.config.ts testTimeout/hookTimeout 30000 in 34f8970 (testing)"
+      ],
+      "created": "2026-07-29T22:37:06Z",
+      "last_seen": "2026-07-31T12:27:40Z"
+    },
+    {
+      "id": "L-006",
+      "key": "surviving_mutant::assert a returned payload field by its value not by the value you handed an injected fake a field that appears in the test only as a spy s input reads like coverage in review but a mutation dropping it from the real return still passes",
+      "text": "Assert a returned payload field by its value, not by the value you handed an injected fake: a field that appears in the test only as a spy's input reads like coverage in review, but a mutation dropping it from the real return still passes",
+      "signal": "surviving_mutant",
+      "scope": "testing",
       "status": "candidate",
       "features": [
-        "worktree-post-create-hook"
+        "worktree-removal-fault-tolerance"
       ],
       "recurrence": 1,
       "harmful": 0,
       "evidence": [
-        "validation.md round-2 gate section; tree.test.ts / worktree-manager.test.ts timeouts (testing)"
+        "round-1 mutant M6; worktree-manager.test.ts leftover: at :844/:867/:882 were spyDeleter inputs, not assertions - dropping the field from worktree-manager.ts:335-339 left all 80 tests green; closed by F1 124340c (testing)"
       ],
-      "created": "2026-07-29T22:37:06Z",
-      "last_seen": "2026-07-29T22:37:06Z"
+      "created": "2026-07-31T12:27:40Z",
+      "last_seen": "2026-07-31T12:27:40Z"
+    },
+    {
+      "id": "L-007",
+      "key": "surviving_mutant::when writing a test to kill a specific surviving mutant check the fixture does not encode that mutant s own blind spot pick one whose readings differ under every wrong implementation not just the one you saw a directories only residue pinned the recursive count yet let a directories only count survive",
+      "text": "When writing a test to kill a specific surviving mutant, check the fixture does not encode that mutant's own blind spot: pick one whose readings differ under every wrong implementation, not just the one you saw. A directories-only residue pinned the recursive count yet let a directories-only count survive",
+      "signal": "surviving_mutant",
+      "scope": "testing",
+      "status": "candidate",
+      "features": [
+        "worktree-removal-fault-tolerance"
+      ],
+      "recurrence": 1,
+      "harmful": 0,
+      "evidence": [
+        "round-2 mutant N3 survived the round-1 fix F2 (dir-remover.test.ts:328-348 fixture wt/keep/a/b was directories-only); closed by F3 1abe8aa with a mixed chain giving 3/2/1/1 for every-entry/dirs-only/files-only/top-level (testing)"
+      ],
+      "created": "2026-07-31T12:27:40Z",
+      "last_seen": "2026-07-31T12:27:40Z"
     }
   ]
 }

scripts/seed-smoke-remove.mjs

@@ -7,6 +7,10 @@
  *     api-feature-42/   clean worktree, branch feature/42
  *     api-chore-wip/    dirty worktree, branch chore/wip — mixed dirt:
  *                        a.txt modified, b.txt deleted, c.txt untracked
+ *     api-lock-me/      clean worktree, branch lock/me, holding an empty sub/
+ *                        for the WRFT-06 blocked-removal flow (the smoke parks
+ *                        a holder process's cwd there). Empty directories are
+ *                        invisible to git status, so the worktree stays clean.
  *
  * Usage:
  *   node scripts/seed-smoke-remove.mjs [baseDir]
@@ -32,6 +36,7 @@ const wsPath = join(base, 'wtm-smoke-seed')
 const repo = join(wsPath, 'api')
 const cleanWt = join(wsPath, 'api-feature-42')
 const dirtyWt = join(wsPath, 'api-chore-wip')
+const lockWt = join(wsPath, 'api-lock-me')
 
 // Fresh on every run so the dirty state is deterministic.
 rmSync(wsPath, { recursive: true, force: true })
@@ -54,6 +59,14 @@ writeFileSync(join(dirtyWt, 'a.txt'), 'alpha edited\n') // modified
 rmSync(join(dirtyWt, 'b.txt')) // deleted
 writeFileSync(join(dirtyWt, 'c.txt'), 'scratch\n') // untracked
 
+// Clean sibling worktree (lock/me) for the WRFT-06 blocked-removal flow. `sub`
+// is the directory the smoke's holder process sits in: an external cwd is the
+// only honest lock fixture (Node's own handles open with FILE_SHARE_DELETE and
+// never block a delete). It stays empty so git still reports the worktree clean,
+// which keeps the first Remove click on the direct path, not the confirm dialog.
+git(repo, 'worktree', 'add', lockWt, '-b', 'lock/me')
+mkdirSync(join(lockWt, 'sub'))
+
 // Register the workspace folder in the app config. Mirrors WorkspaceRegistry.add:
 // id = lowercased absolute path, displayName = folder basename.
 const appData = process.env.APPDATA ?? join(homedir(), 'AppData', 'Roaming')
@@ -83,6 +96,7 @@ console.log(
   dirtyWt,
   '(chore/wip — a.txt modified, b.txt deleted, c.txt untracked)'
 )
+console.log('  lock worktree:   ', lockWt, '(lock/me — empty sub/ for the holder process)')
 console.log('Registered in:     ', configPath)
 console.log('')
 console.log('Next: start the app with --remote-debugging-port=9222, then run')

scripts/smoke-remove.mjs

@@ -1,16 +1,24 @@
 /* CDP smoke for delete-worktree (DLWT-01..04) + force-remove-worktree
- * (FRWT-01..04). Assumes the app is running with --remote-debugging-port=9222
+ * (FRWT-01..04) + worktree-removal-fault-tolerance (WRFT-06).
+ * Assumes the app is running with --remote-debugging-port=9222
  * and a seeded workspace named wtm-smoke-* containing repo `api` (branch main)
- * plus a clean linked worktree `api-feature-42` (branch feature/42) and a dirty
- * linked worktree `api-chore-wip` (branch chore/wip). For the fullest FRWT
- * coverage, seed chore/wip with mixed dirt — a modified tracked file, an added
- * untracked file, and a deleted tracked file — so the confirm dialog renders
- * Modified/Added/Deleted rows; any non-empty dirt also passes.
+ * plus a clean linked worktree `api-feature-42` (branch feature/42), a dirty
+ * linked worktree `api-chore-wip` (branch chore/wip) and a clean linked
+ * worktree `api-lock-me` (branch lock/me) holding an empty `sub/`. For the
+ * fullest FRWT coverage, seed chore/wip with mixed dirt — a modified tracked
+ * file, an added untracked file, and a deleted tracked file — so the confirm
+ * dialog renders Modified/Added/Deleted rows; any non-empty dirt also passes.
  * Seed it with: node scripts/seed-smoke-remove.mjs   (run before launching the app)
  * Run: node scripts/smoke-remove.mjs
+ *
+ * MANUAL ONLY — never CI (TESTING.md): this drives a live Electron app over CDP
+ * on a real desktop session, against real on-disk state that the run destroys.
+ * Every removal here is one-shot; re-seed before each run.
  */
 
+import { spawn } from 'node:child_process'
 import { existsSync } from 'fs'
+import { join } from 'node:path'
 
 const PORT = 9222
 
@@ -69,6 +77,10 @@ const selectExpr = (branch) => `(async () => {
   }
 })()`
 
+/* Git's porcelain paths and the path Node reports inside an fs error can differ
+ * in separators and case, so compare them the way the main process does. */
+const norm = (p) => p.replaceAll('/', '\\').replace(/\\+$/, '').toLowerCase()
+
 const checks = []
 function check(name, ok, detail = '') {
   checks.push({ name, ok })
@@ -247,6 +259,112 @@ check(
 )
 check('dirty worktree folder gone from disk (FRWT-03)', !existsSync(dirtyWt.path))
 
+// WRFT-06: a removal blocked by a live lock names what blocked it, keeps the
+// row (git is never asked to deregister anything), and the same button is a
+// working retry once the holder is gone. The fixture is an external process
+// whose cwd sits inside the worktree — the real agent-terminal case, and the
+// only honest one, since Node's own handles never block a delete.
+const lockWt = api.worktrees.find((w) => w.branch === 'lock/me')
+check('seeded lock/me worktree present for the blocked-removal flow', Boolean(lockWt))
+const heldDir = join(lockWt.path, 'sub')
+
+const holder = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 60000)'], {
+  cwd: heldDir,
+  stdio: 'ignore'
+})
+try {
+  await new Promise((r) => setTimeout(r, 400)) // measured settle before the lock is held
+
+  // First Remove: the deleter exhausts its 3 s budget and gives up before git
+  // runs, so the Danger section gets the structured leftover block.
+  await evaluate(ws, selectExpr('lock/me'))
+  const blocked = await evaluate(
+    ws,
+    `(async () => {
+       document.querySelector('.detail-remove-btn').click()
+       await new Promise((r) => setTimeout(r, 6000)) // 3s retry budget + IPC round-trip
+       return {
+         note: document.querySelector('.detail-danger-leftover .detail-danger-note')
+           ?.textContent ?? null,
+         path: document.querySelector('.detail-danger-path')?.textContent ?? null,
+         disabled: document.querySelector('.detail-remove-btn')?.disabled ?? null
+       }
+     })()`
+  )
+  check(
+    'blocked removal names the blocked path inline (WRFT-06 AC 1)',
+    blocked.path !== null && norm(blocked.path) === norm(heldDir),
+    JSON.stringify({ shown: blocked.path, expected: heldDir })
+  )
+  check(
+    'blocked removal reports what is left and that it stays registered (WRFT-06 AC 1)',
+    /\d+ items? still on disk/.test(blocked.note ?? '') &&
+      /still registered/.test(blocked.note ?? ''),
+    JSON.stringify(blocked.note)
+  )
+  check(
+    'remove button is enabled again after the failure (WRFT-06 AC 3)',
+    blocked.disabled === false,
+    JSON.stringify(blocked.disabled)
+  )
+
+  // The row must survive a real tree refresh — the defect this feature fixes is
+  // that it used to vanish while the folder stayed on disk.
+  const survived = await evaluate(
+    ws,
+    `(async () => {
+       document.querySelector('.topbar-icon-btn').click()
+       await new Promise((r) => setTimeout(r, 1500))
+       return [...document.querySelectorAll('.sidebar-worktree-branch')]
+         .some((b) => b.textContent === 'lock/me')
+     })()`
+  )
+  check('blocked worktree is still listed after a tree refresh (WRFT-06 AC 1)', survived === true)
+  check('blocked worktree folder is still on disk (WRFT-02 AC 1)', existsSync(heldDir))
+
+  // Release the lock and retry from the same row — no restart, no cleanup.
+  await new Promise((resolve) => {
+    holder.once('exit', resolve)
+    holder.kill()
+  })
+  await evaluate(ws, selectExpr('lock/me'))
+  const retried = await evaluate(
+    ws,
+    `(async () => {
+       document.querySelector('.detail-remove-btn').click()
+       await new Promise((r) => setTimeout(r, 1000))
+       // The blocked attempt already deleted everything it could reach, the
+       // worktree's .git link included, so the row can read either clean (direct
+       // remove) or dirty (confirm dialog). Confirm it if it opened.
+       document.querySelector('.dialog-btn-danger')?.click()
+       await new Promise((r) => setTimeout(r, 2500))
+       return {
+         rowGone: ![...document.querySelectorAll('.sidebar-worktree-branch')]
+           .some((b) => b.textContent === 'lock/me'),
+         toast: document.querySelector('.toast')?.textContent ?? null
+       }
+     })()`
+  )
+  check(
+    'retry after the holder exits removes the row (WRFT-06 AC 2)',
+    retried.rowGone === true,
+    JSON.stringify(retried)
+  )
+  check(
+    'retry toast names the branch (WRFT-06 AC 2)',
+    /Removed lock\/me/.test(retried.toast ?? ''),
+    JSON.stringify(retried.toast)
+  )
+  check(
+    'blocked worktree folder gone from disk after the retry (WRFT-06 AC 2)',
+    !existsSync(lockWt.path)
+  )
+} finally {
+  // A failed check above must not leave a node.exe parked in the worktree —
+  // it would block every later run and the seed's own rmSync.
+  holder.kill()
+}
+
 ws.close()
 const failed = checks.filter((c) => !c.ok).length
 console.log(`\n${checks.length - failed}/${checks.length} checks passed`)