# bin/runplan Worktree Isolation Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use /ship (recommended) or /executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Give `src/runner.js` per-task git worktree + branch isolation (mirroring `~/.claude/workflows/run-plan.js`'s topology) so same-wave concurrent tasks no longer race against one shared tree, and `ship.sh land` has real integrated work to land. **Architecture:** One integration worktree (`/.wt--int` on branch `plan/`) plus one worktree per task (`/tmp/wt--` on branch `plan/--`), both deterministic from `(slug, taskId)` and never persisted. All `.git/worktrees`-mutating operations (create, remove, merge) are serialized under one mutex; implement/gate/review dispatch stays fully parallel across `runPool`'s existing concurrency limit. Full design: `docs/specs/2026-07-02-runplan-worktree-isolation-design.md`. **Tech Stack:** Node.js (`src/runner.js`), bash (`lib/gates.sh`, `lib/journal.sh`), git worktrees. --- ## Wave Plan | Wave | Tasks | Files touched | Safe to parallelize? | |------|-------|----------------|----------------------| | 1 | Task 1 | `src/runner.js` | single task | | 2 | Task 2 | `src/runner.js` | single task | | 3 | Task 3 | `src/runner.js` | single task | | 4 | Task 4 | `src/runner.js` | single task | | 5 | Task 5 | `src/runner.js` | single task | | 6 | Task 6 | `test/runner-integration.sh` | single task | All six tasks land in the same dependency chain (each wave's contract depends on the previous wave's seam existing), and waves 1–5 all touch `src/runner.js` — no two tasks share a wave. `meta.scheduler = sequential`. --- ### Task 1: Worktree/branch naming helpers + serialization mutex **Wave:** 1 **Blocks:** Task 2 **Blocked by:** — **Files:** - Modify: `src/runner.js` — add new pure helper functions near the existing `shellPath`/`gitHead` helpers (around line 723-731); no existing function bodies change in this task. **Contract (pin EXACTLY):** - `integrationWorktreePath(repoRoot, slug) -> string` — returns `path.join(repoRoot, `.wt-${slug}-int`)`. - `integrationBranchName(slug) -> string` — returns `` `plan/${slug}` ``. - `taskWorktreePath(slug, taskId) -> string` — returns `path.join(os.tmpdir(), `wt-${slug}-${taskId}`)`. - `taskBranchName(slug, taskId) -> string` — returns `` `plan/${slug}--${taskId}` ``. - `withWorktreeLock(fn) -> Promise` where `fn: () => Promise` — serializes execution of `fn` across concurrent callers within the same process using a module-level promise chain (`let worktreeLockTail = Promise.resolve(); withWorktreeLock = (fn) => { const run = worktreeLockTail.then(fn, fn); worktreeLockTail = run.catch(() => {}); return run; }` pattern, or equivalent — the requirement is FIFO mutual exclusion across `runPool`'s concurrent `worker()` invocations, not the exact chaining idiom). Must propagate `fn`'s resolved value or rejection to the caller; must NOT let one rejected `fn` permanently poison the chain for subsequent callers. **Behavior:** - All four naming functions are pure (no I/O, no side effects) and deterministic for a given `(repoRoot, slug, taskId)` — this is what makes resume recompute the same paths without persisting them in the journal. - `withWorktreeLock` is the SOLE serialization point for every `git worktree add`/`git worktree remove`/`git merge` call added in later tasks — Task 2's integration-worktree creation, Task 3's task-worktree creation, and Task 5's merge step must each wrap their worktree-mutating git calls in `withWorktreeLock(...)`. **Acceptance (one executable check):** - Run: `node -e "const r = require('./src/runner.js'); console.log(JSON.stringify([r.integrationWorktreePath('/repo','myslug'), r.integrationBranchName('myslug'), r.taskWorktreePath('myslug','t1'), r.taskBranchName('myslug','t1')]))"` - Expected: PASS — prints `["/repo/.wt-myslug-int","plan/myslug","/tmp/wt-myslug-t1","plan/myslug--t1"]` (tmpdir path may vary by OS; assert it equals `path.join(os.tmpdir(),'wt-myslug-t1')`). - Also export all four naming functions plus `withWorktreeLock` from `module.exports` (extend the existing export list at the bottom of the file) so the acceptance check and Task 6's tests can reach them. - [ ] Add the four naming functions + `withWorktreeLock`, matching the contract exactly - [ ] Add them to `module.exports` - [ ] Run acceptance check → expected output above - [ ] Commit: `git add src/runner.js && git commit -m "runner: add worktree/branch naming helpers + serialization mutex"` --- ### Task 2: Integration worktree lifecycle wired into run() **Wave:** 2 **Blocks:** Task 3 **Blocked by:** Task 1 **Files:** - Modify: `src/runner.js:14-50` (`main()`) — add `intWt`/`intBranch` to the `createRunner()` context object. - Modify: `src/runner.js:52-90` (`createRunner()`/`run()`) — ensure the integration worktree exists before the wave loop starts. **Contract:** - `context.intWt = integrationWorktreePath(context.repoRoot, context.slug)`, `context.intBranch = integrationBranchName(context.slug)` — computed once in `main()`, passed into `createRunner(context)` alongside the existing fields (`repoRoot`, `worktree`, `runId`, `branch`, `slug`, `journalPath`, `concurrency`, `plan`, `runconfig`). - New exported async function `ensureIntegrationWorktree(context) -> Promise` (returns `intHead` after ensuring the tree exists): under `withWorktreeLock`, checks whether `context.intWt` exists (`fs.existsSync`); if already present, skip `git worktree add` entirely and do NOT touch the branch. If absent: **NEVER use `-B`** (force-reset would drop any commits already integrated onto `plan/` from a prior run whose `.wt--int` directory was cleaned up — e.g. `git worktree prune` — while the branch itself survived; this is exactly the data-loss case the spec's Failure Modes section forbids). Instead: `git show-ref --verify --quiet refs/heads/` from `context.repoRoot` — branch exists → `git worktree add ` (attach to the existing branch as-is, no reset); branch absent → `git worktree add -b ` (fresh branch off `base_branch`). Returns `gitHead(context.intWt)`. - Call `ensureIntegrationWorktree(context)` once at the top of `run()` (in `createRunner()`), before the `waveMap` loop, and store the result nowhere new — each task will call it again cheaply in Task 3 to read the current `intHead` at lease time (idempotent no-op after the first call within a run since the worktree now exists). **Behavior:** - `base_branch` (`context.branch`, `main` in the existing test fixture) must remain untouched by this or any later task — only `plan/` moves. - A prior stalled run's `.wt--int` directory must be reused, never deleted and recreated (would drop in-flight merged work — see spec Failure Modes). **Acceptance:** - Run: `bash test/runner-integration.sh` (existing suite — will still pass at this point since nothing downstream reads `intWt` yet; this task is additive/dead-code-adjacent except for the new worktree appearing on disk). - Expected: `PASS= FAIL=0` (same N as baseline before this task — confirms no regression). - Manual check: after running the fixture once via `node runner.js run ...` in a scratch dir, `git -C worktree list` shows `.wt--int` on branch `plan/`, and `git -C rev-parse main` is unchanged from before the run. - [ ] Add `intWt`/`intBranch` to context construction in `main()` - [ ] Add `ensureIntegrationWorktree()`, export it - [ ] Call it at the top of `run()` - [ ] Run acceptance check → expected output above - [ ] Commit: `git add src/runner.js && git commit -m "runner: create/reuse plan/ integration worktree at run start"` --- ### Task 3: Task worktree lease + taskBase retargeting **Wave:** 3 **Blocks:** Task 4 **Blocked by:** Task 2 **Files:** - Modify: `src/runner.js:156-184` (`runTask()`, the `startState === "leased"` branch and the `taskBase` computation above it). **Contract:** - New exported async function `ensureTaskWorktree(context, task, intHead) -> Promise` (returns the task worktree path): under `withWorktreeLock`, computes `taskWt = taskWorktreePath(context.slug, task.id)` and `taskBranch = taskBranchName(context.slug, task.id)`. If `taskWt` exists on disk, reuse it unconditionally (branch may be ahead of `intHead` — that's the resume case). Else (worktree dir absent — covers BOTH the true first-lease case and the cron/reboot case where `/tmp` was cleared but the task branch survives): check `git -C context.repoRoot show-ref --verify --quiet refs/heads/` — branch exists → `git worktree add ` (attach to the existing branch, no `-b`, no reset — the branch may already hold committed work from before the reboot); branch absent → `git worktree add -b `. **Never use `-b` on an existing branch** — same rationale as `ensureIntegrationWorktree` (Task 2): it would silently discard commits already on the task branch. Returns `taskWt`. - **`taskWt`/`intHead` are resolved ONCE, unconditionally, for EVERY non-terminal entry into `runTask()` — not only fresh leases.** A resumed run can enter `runTask()` with journal state already `"leased"`, `"implemented"`, `"gated"`, or `"reviewed"`, and `nextApplicableState` dispatches straight into the corresponding branch WITHOUT passing through the `startState === "leased"` block — so gating the ensure-calls behind that block (as originally planned) leaves `taskWt` unresolved on those resume paths. Fix: reorder `runTask()` so the ensure-calls happen right after the existing `"done"` early return (line 171-173) and BEFORE the `taskBase` computation (currently line 161) — since `taskBase`'s fallback must become `intHead`, not `context.repoRoot` (see below), `taskBase`'s computation must move to AFTER the ensure-calls too. Concretely, `runTask()`'s top becomes: ``` const startState = nextApplicableState(state, { hasReviewer, riskLevel }); if (startState === "done") return; const intHead = await ensureIntegrationWorktree(context); const taskWt = await ensureTaskWorktree(context, task, intHead); const taskBase = taskHistory.find((entry) => entry.task === task.id)?.base ?? intHead; ``` Precise final ordering at the top of `runTask()`: `state`/`taskHistory`/`latestEntry` stay where they are (156-160); `binding` (162) and `riskLevel` (164) stay where they are (needed as-is by `nextApplicableState`); `reviewerBinding`/`startState` computation (currently 166-170) moves up to right after them; done-check follows; THEN `intHead`/`taskWt`/`taskBase` are computed; `currentHead` (currently line 163, `latestEntry?.head || taskBase`) moves down to right after `taskBase` since it reads `taskBase` — leaving it at its original position would reference `taskBase` before it's declared (`const` TDZ crash). Skipping the ensure-calls when `startState === "done"` is required, not optional — Task 6's "no lingering worktrees" acceptance (New case 2) depends on a completed task never re-creating its already-merged-and-removed `taskWt` directory. - The `if (startState === "leased")` block's `currentHead = await gitHead(context.repoRoot)` becomes `currentHead = await gitHead(taskWt)`, using the `taskWt` local resolved above. **Behavior:** - This task ONLY changes the top-of-`runTask()` reordering, the lease step, and `taskBase` sourcing. It does NOT yet retarget implement/gate/review/commit's I/O — those still run against `context.repoRoot` after this task (Task 4 does that). This task intentionally leaves the pipeline in a transiently inconsistent state (worktree is resolved and available, but implement/gate/commit still dispatch to `repoRoot`) — acceptable because Task 4 lands immediately after in the same sequential chain before any end-to-end test runs. **Acceptance:** - Run: `node -e "const path=require('path'); const r=require('./src/runner.js'); console.log(typeof r.ensureTaskWorktree)"` - Expected: PASS — prints `function` (confirms the export exists; full behavioral verification happens in Task 6's integration suite once the full pipeline is retargeted). - Add `ensureTaskWorktree` to `module.exports`. - [ ] Add `ensureTaskWorktree()`, export it - [ ] Retarget `taskBase` computation and the `leased`-state `currentHead` read as specified - [ ] Run acceptance check → expected output above - [ ] Commit: `git add src/runner.js && git commit -m "runner: lease per-task worktree, source taskBase from integration head"` --- ### Task 4: Retarget implement/gate/review/commit to the task worktree **Wave:** 4 **Blocks:** Task 5 **Blocked by:** Task 3 **Files:** - Modify: `src/runner.js:186-238` (`runTask()`'s implemented/gated/reviewed/committed branches) — replace `context.repoRoot` with the task worktree path in every implement/gate/review/commit call. - Modify: `src/runner.js:241-309` (`runGateLoop()`) — `gate0`/`risk` CLI calls take the task worktree as the `` arg, `context.repoRoot` stays as the `` arg (the two-arg split `lib/gates.sh` already supports). - Modify: `src/runner.js:440-472` (`runWrapper()`) — `--workspace` argument becomes the task worktree, not `repoRoot`. - Modify: `src/runner.js:474-486` (`commitTask()`) — `git diff`/`git add`/`git commit` run inside the task worktree. **Contract:** - `runWrapper(repoRoot, binding, taskId, prompt)` signature changes to `runWrapper(taskWt, binding, taskId, prompt)` — same shape, the first positional arg is now the task worktree path used both as `--workspace` value and as the `cwd` passed to `spawnPassthrough`. All 3 call sites in `dispatchWithFallback()` pass `context.repoRoot` today (line 328: `runWrapper(context.repoRoot, binding, task.id, prompt)`) — change to the task worktree path. Since `dispatchWithFallback(context, task, binding, prompt)` doesn't currently receive the task worktree, either (a) resolve it inline via `taskWorktreePath(context.slug, task.id)` (pure, no I/O — safe to call directly since the worktree is guaranteed to exist by the time dispatch runs, per Task 3's lease step) or (b) thread an explicit `taskWt` parameter through `dispatchWithFallback(context, task, binding, prompt, taskWt)`. Prefer (b) — explicit is clearer than an implicit re-derivation, and every call site in `runTask()` already has `taskWt` in scope from Task 3's lease step. - `runGateLoop(context, task, coderBinding, taskBase)` signature gains a `taskWt` parameter: `runGateLoop(context, task, coderBinding, taskBase, taskWt)`. Both `gate0` calls (lines 246-253 and 282-289) change their `` positional arg from `context.repoRoot` to `taskWt`; the `` positional arg (line 250/286) stays `context.repoRoot` unchanged. The `risk` calls (lines 256-261, 291-296) and `snapshotRiskHead(context.repoRoot)` calls (lines 255, 290) change their repo-path arg from `context.repoRoot` to `taskWt` — risk is diffed within the task worktree's branch, not the shared repoRoot. - `commitTask(context, task)` signature gains a `taskWt` parameter: `commitTask(context, task, taskWt)`. All three `spawnPassthrough`/`runCli` calls inside it (`git diff`, `git add`, `git commit`) use `taskWt` as the `cwd` instead of `context.repoRoot`. `gitHead(context.repoRoot)` inside it becomes `gitHead(taskWt)`. - Every remaining `gitHead(context.repoRoot)` call inside `runTask()` (the `implemented`/`reviewed` branches, lines 190 and 219) becomes `gitHead(taskWt)`. **Behavior:** - No behavior change to the state machine transitions (`nextApplicableState`, `shouldRunReview`, `shouldReviewForRisk`) — only the filesystem path each I/O call targets changes. - `taskWt` is the local resolved once at the top of `runTask()` per Task 3's reordering (via `ensureTaskWorktree`, not a bare `taskWorktreePath` call — Task 3 fixed this to guarantee the directory is actually attached/created, not just named, before ANY state branch runs, including resume entries that skip the `leased` block entirely). - **`commitTask` must be idempotent against an already-committed, clean task worktree** (needed for Task 5's crash-mid-merge resume): before running `git add`/`git commit` inside `taskWt`, check `git -C taskWt status --porcelain`. If it is empty (no staged or unstaged changes) AND `gitHead(taskWt) !== taskBase` (the branch already holds a commit beyond its fork point — i.e. a prior run already committed this task's work and crashed before the merge landed), SKIP the `git add`/`git commit` calls entirely and return the current `gitHead(taskWt)` unchanged. Otherwise (dirty tree, or clean tree with `gitHead(taskWt) === taskBase` meaning nothing was ever implemented) proceed with `git add`/`git commit` as before — a `git commit` failing on a genuinely empty diff with no prior commit is a real error and must propagate normally, not be swallowed by this tolerance check. **Acceptance:** - Run: `bash test/runner-integration.sh` - Expected: this WILL now show failures in `run_success_case`/`run_high_risk_review_case`/etc. because those tests still assert against `git -C "$dir"` (the fixture's `main` checkout) rather than the new task/integration branches — that's expected and intentional; Task 6 rewrites the assertions. For THIS task's acceptance, instead run: `cd && node src/runner.js run --plan plan.jsonl --runconfig runconfig.json; git branch -a` and confirm a `plan/runner-it--w3.p1.t1` branch exists with the stub's commit, and `git rev-parse main` still equals the pre-run seed commit (use `mkfixture` from `test/runner-integration.sh` manually via `bash -c 'source test/runner-integration.sh; mkfixture'` or an equivalent scratch fixture — do not skip this manual check even though the automated suite is red at this point in the chain). - [ ] Add `taskWt` parameter to `dispatchWithFallback`, `runGateLoop`, `commitTask`, `runWrapper`; retarget every listed call site - [ ] Retarget the remaining `gitHead(context.repoRoot)` calls inside `runTask()` - [ ] Run the manual acceptance check above → confirm task branch has the commit, `main` unchanged - [ ] Commit: `git add src/runner.js && git commit -m "runner: retarget implement/gate/review/commit dispatch to the task worktree"` --- ### Task 5: Merge-into-integration + resume ladder **Wave:** 5 **Blocks:** Task 6 **Blocked by:** Task 4 **Files:** - Modify: `src/runner.js:229-238` (`runTask()`'s `committed` branch) — fold in the merge step immediately after `commitTask()` returns. - Modify: `src/runner.js:54-62` (`reconcileJournal()`) — target `context.intBranch` instead of `context.branch`. - Modify: `src/runner.js:14-45` (`main()`) — the `reconcileJournal()` call site is unaffected in signature (still reads `context.branch` internally via the method), but `context.intBranch` must exist before `--resume` runs, so `ensureIntegrationWorktree(context)` must run before `runner.reconcileJournal()` is called when `args.resume` is true (currently reconcile runs before `run()`, which is where `ensureIntegrationWorktree` was placed in Task 2 — move or duplicate the ensure-call so reconcile always has a valid `intBranch` to check against, e.g. call `ensureIntegrationWorktree(context)` directly in `main()` before the `if (args.resume)` block, in addition to (or instead of) the call inside `run()`). - No change needed to `runTask()`'s lease step beyond what Task 3 already did — see Ownership below for why no separate resume branch-reuse check is added. **Contract:** - New exported async function `mergeTaskIntoIntegration(context, task, taskWt, taskBranch) -> Promise`: under `withWorktreeLock`, `git -C merge --no-ff `. On success (`rc === 0`): `git worktree remove ` (keep the branch). On failure: `git -C merge --abort`, leave `taskWt` and `taskBranch` intact, throw `new Error(`merge-conflict: ${task.id}`)` — this propagates up through `runTask()` exactly like the existing `gate-not-green-after-N` error does (uncaught → `main()`'s catch prints it via `formatError` and sets `process.exitCode = 1`; the task's journal already has its `committed` record, so it is NOT re-run from scratch on next invocation — see Ownership below for how the retry is picked up). - `runTask()`'s `committed` branch: after `currentHead = await commitTask(context, task, taskWt)` and its `appendJournal` call (both unchanged — `head` stays the task branch head per the spec's "Architecture Decisions" section), add `await mergeTaskIntoIntegration(context, task, taskWt, taskBranchName(context.slug, task.id))`. - `reconcileJournal()`: the `lib/journal.sh reconcile` call's branch argument (currently `context.branch`, line 60) becomes `context.intBranch`. **Ownership of crash-mid-merge recovery — single mechanism, no separate check in `runTask()`:** `reconcileJournal()` runs once, at `main()` startup, strictly BEFORE any `runTask()` call. It downgrades any journal `committed` record whose head is unreachable from `intBranch` (existing `_journal_reconcile`/`_head_reachable` behavior in `lib/journal.sh`, now checked against `intBranch` per this task) back to that task's last prior non-`committed` state (e.g. `reviewed`) — this is `_journal_reconcile`'s existing "downgrade" append, unchanged mechanically. **This means a resumed task's journal state is NEVER `"committed"`-with-unmerged-head by the time `runTask()` runs** — it always re-enters at the downgraded state instead. `nextApplicableState` then naturally drives it straight back to the `committed` branch (`"reviewed" → "committed"`), which re-runs `commitTask()` (now idempotent per Task 4's clean-tree tolerance — since `taskWt`'s work was already committed pre-crash, the tree is clean and `commitTask` returns the existing head untouched) and re-attempts `mergeTaskIntoIntegration()`. `ensureTaskWorktree` (Task 3, now called unconditionally for every non-done entry) already reattaches `taskWt` if its directory is gone but the branch survives. **No additional resume branch-reuse check is added to `runTask()`** — `reconcileJournal()` + Task 3's unconditional worktree-ensure + Task 4's idempotent `commitTask` are jointly sufficient; a second, independent "is this task committed-but-unmerged" check would be redundant and risks disagreeing with `reconcileJournal()`'s downgrade decision. If the re-attempted merge conflicts AGAIN, it fails closed exactly as on a first attempt (no silent retry loop, no auto re-implementation) — a deliberate, spec-documented limitation (see design doc's "Re-attempted merge that conflicts again is NOT auto-recovered"). - **Precondition on `reconcileJournal()` finding a valid downgrade target:** `_journal_reconcile` requires a prior non-`committed` journal line for the task to downgrade to (`_journal_prior_line`); if none exists it errors closed (`exit 3`, "cannot safely downgrade"). This is only reachable if a `committed` record exists with NO preceding `leased`/`implemented`/`gated`/`reviewed` line for that task — impossible in normal operation since `runTask()` always appends those in order before `committed`. Relevant for Task 6's fixture seeding: a test simulating crash-mid-merge must seed the FULL prior state sequence (not just a bare `committed` line), matching how the task would actually have gotten there. **Behavior:** - A task that reaches `committed` in the journal but whose branch is not yet an ancestor of `plan/` (detectable via `merge-base --is-ancestor`) is the crash-mid-merge signal — this only works because Task 4/this task's ordering guarantees the `committed` journal record is appended (inside `commitTask`'s caller, `runTask()`) strictly BEFORE `mergeTaskIntoIntegration` runs, matching the design doc's pinned ordering. - Merge conflict is never auto-resolved and never force-pushed over `plan/`. **Acceptance:** - Run: `bash test/runner-integration.sh` — still expected red on the pre-existing assertions (Task 6 fixes them); for this task, verify via a manual scratch run: run the fixture end to end, confirm `git -C log plan/ --oneline` contains the task's commit, `git branch --list 'plan/*--*'` shows the task branch still present (not force-deleted), and re-running with `--resume` after manually deleting the last journal line's merge effect (or after a simulated crash — kill before merge) correctly detects the unmerged-but-committed state (via `reconcileJournal()`'s downgrade, not a separate check) and completes the merge without re-implementing. - [ ] Add `mergeTaskIntoIntegration()`, export it, wire into the `committed` branch of `runTask()` - [ ] Retarget `reconcileJournal()` to `context.intBranch`; ensure `intBranch`/`intWt` exist before any `--resume` reconcile call in `main()` - [ ] Confirm no separate resume branch-reuse check is added — reconcile + Task 3's unconditional `ensureTaskWorktree` + Task 4's idempotent `commitTask` are the whole mechanism - [ ] Run the manual acceptance check above - [ ] Commit: `git add src/runner.js && git commit -m "runner: merge each committed task into plan/, add crash-mid-merge resume recovery"` --- ### Task 6: Rewrite runner-integration.sh for the new topology **Wave:** 6 **Blocks:** — **Blocked by:** Task 5 **Files:** - Modify: `test/runner-integration.sh` — every existing assertion that reads `git -C "$dir" ...` against the fixture's checked-out branch (`main`) needs to instead read from `plan/` (via `git -C "$dir" show plan/:`, `git -C "$dir" rev-list --count plan/`, etc.) or from the task worktree while it's still alive, per case. Add two new cases. **Contract — per existing case, the exact assertion change:** - `run_success_case` (lines 122-175): `commit_count` must now be checked as `git -C "$dir" rev-list --count plan/runner-it` (task's commit lands on the integration branch, not `main` — expect count relative to `plan/runner-it`'s own history, which forks from `main`'s single seed commit, so still `2` when counted via `git -C "$dir" rev-list --count plan/runner-it`). `git -C "$dir" show HEAD:task-file.txt` (lines 167-168) becomes `git -C "$dir" show plan/runner-it:task-file.txt`. The `final_head`/`previous_head` check (lines 158-165) reads `git -C "$dir" rev-parse plan/runner-it` and `plan/runner-it^` instead of `HEAD`/`HEAD^`. Add one new assertion: `git -C "$dir" rev-parse main` unchanged from the pre-run seed commit (capture the seed commit in `mkfixture` or right after it returns, before invoking the runner). - `run_high_risk_review_case` (lines 177-228): same `HEAD` → `plan/runner-it` substitution pattern for `commit_count` and the `task-file.txt`/`payment.js` content checks. - `run_exit3_case` (lines 230-274): the "no commit" and "no diff" assertions (lines 260-271) currently check `git -C "$dir" rev-list --count HEAD` and `git -C "$dir" status --short` on the fixture's `main` checkout — these stay valid AS-IS since exit-3 halts before any commit/merge ever happens (task never reaches the task-worktree lease's committed step) — no change needed to this case's git assertions, only confirm they still pass. - `run_invalid_plan_case` (lines 276-305): no git-topology assertions present — no change needed. - `run_resume_rebind_case` (lines 307-353): same `HEAD` → `plan/runner-it` substitution for `commit_count` (line 345-349); the seeded journal line's `base`/`head` (line 314) stays as `seed_head` (still valid — `seed_head` is `main`'s tip, which is also `plan/runner-it`'s fork point on a fresh fixture, since `ensureIntegrationWorktree` branches `plan/` from `main` when the integration worktree doesn't exist yet). **New case 1 — merge conflict fails closed:** - Function `run_merge_conflict_case()`: build a fixture via `mkfixture`, then seed a SECOND task in the same wave that the stub wrapper will make edit the SAME line of `task-file.txt` as the first task (deliberately violating the disjointness convention) — use two `plan.jsonl` task records both with `wave: 1`, both defaulting to `STUB_TARGET_FILE=task-file.txt`, and force `STUB_LINE` to the SAME fixed string for both via distinct env vars threaded per-task if the stub wrapper doesn't already support that, OR simpler: pre-seed `task-file.txt` with content on the integration branch AFTER task A's worktree already branched from an older `intHead`, by manually running task A to `committed` state via a crafted journal seed, then modifying `plan/` out from under it (append a conflicting commit directly to `intWt`) before running task B — pick whichever is less fixture-code; the acceptance below doesn't prescribe the exact conflict-construction mechanism, only the observable outcome. - Acceptance: task ends in journal state `committed` (not silently skipped, not `done`) with the runner process exiting non-zero and printing `merge-conflict:` in its stderr/stdout; `plan/` is unaffected by the conflicting task's changes (its tip is unchanged from before the conflicting merge attempt); the conflicting task's worktree and branch (`plan/--`) still exist on disk after the run. **New case 2 — no lingering worktrees on a clean run:** - Extend `run_success_case` (not a new function) with a final assertion after the existing ones: `git -C "$dir" worktree list` contains no `/tmp/wt-*` entries (only the fixture's own checkout and `.wt-runner-it-int`) — proves Task 5's `git worktree remove` on successful merge actually ran. **New case 3.5 — resume after task worktree dir is deleted but branch survives (`/tmp` cleared, e.g. reboot), AND crash-mid-merge is recovered by `reconcileJournal()`:** - Function `run_worktree_reboot_resume_case()`: run a task to `committed` state with the commit landed on `plan/--` but NOT YET merged into `plan/` — simulate by seeding the FULL prior journal sequence for the task (`leased`, `implemented`, `gated`, `committed` — matching Task 5's precondition that `_journal_reconcile` needs a valid non-`committed` prior line to downgrade to; a bare `committed` line alone makes reconcile error closed) plus a real commit on the task branch, extending `run_resume_rebind_case`'s seeding technique to the full sequence. Then `git worktree remove --force` (or manually `rm -rf`) the task's `/tmp/wt-*` directory while leaving the branch intact, then re-run with `--resume`. - Acceptance: the run does NOT crash with a `fatal: a branch named '...' already exists` error (the historical `-b`-on-existing-branch failure mode) — `ensureTaskWorktree` reattaches via plain `git worktree add `; the journal shows the task's `committed` record was downgraded by `reconcileJournal()` (a new `gated` or `implemented` record appended after the original `committed` line, per `_journal_reconcile`'s downgrade behavior) and then a fresh `committed` record appended again after resume re-runs `commitTask` (idempotent — no new commit created, `git -C "$dir" log --oneline` shows the SAME single commit before and after, proving no reset/data-loss and no duplicate commit); the run completes and the task's commit lands on `plan/` via merge. **New case 3 — concurrency proof:** - Function `run_concurrent_wave_case()`: seed a `plan.jsonl` with 2 tasks in `wave: 1`, both `seat: coder`, distinct `id`s, distinct `STUB_TARGET_FILE` values (so they're genuinely file-disjoint per planning convention) — run with default concurrency (no `--concurrency 1` override) and assert both tasks reach `committed` in the journal, both land on `plan/runner-it` (via two `--no-ff` merges), and neither task's worktree directory existed inside the other's (i.e., `git -C "$dir" worktree list` at no point during the run would have shown fewer than 2 `/tmp/wt-*` entries while both were mid-flight — since this is hard to observe from OUTSIDE the run without instrumentation, settle for the weaker but still meaningful assertion: both tasks' commits appear as separate commits authored against separate parent chains before merge, i.e. `git -C "$dir" log plan/runner-it-- --oneline` and `...--` each show exactly one commit past their shared fork point, proving neither task's worktree ever contained the other's uncommitted diff). **Acceptance (one executable check for the whole task):** - Run: `bash test/runner-integration.sh` - Expected: `PASS= FAIL=0` where N includes all 5 original cases (rewritten) plus the 4 new assertions/cases above (merge conflict, no-lingering-worktree, reboot-resume, concurrency). - [ ] Rewrite the 3 affected existing cases' git assertions per the substitution rules above - [ ] Add `run_merge_conflict_case()` - [ ] Add the no-lingering-worktree assertion to `run_success_case` - [ ] Add `run_worktree_reboot_resume_case()` - [ ] Add `run_concurrent_wave_case()` - [ ] Wire all new/changed cases into the script's final call list (after line 360's existing 5 calls) - [ ] Run acceptance check → `PASS= FAIL=0` - [ ] Commit: `git add test/runner-integration.sh && git commit -m "test: cover worktree-isolated runner topology, merge conflicts, and wave concurrency"` --- ## Self-Review **1. Spec coverage:** Naming convention (Task 1) ✓. Git topology per task — lease/implement/commit/merge (Tasks 2-5) ✓. Changes to existing seams — `reconcileJournal`, `runTask`, `runGateLoop` (Tasks 3-5) ✓. Resume behavior incl. non-auto-recovering re-conflict (Task 5) ✓. Concurrency claim (Task 6 new case) ✓. Failure modes — conflict fail-closed, stalled-run worktree reuse, crash mid-merge, branch-survives-worktree-deleted (Tasks 2, 3, 5, 6) ✓. Testing section's required extensions (fixture assertions, concurrency case, reboot-resume case) ✓ (Task 6). DRY/necessity justification and simpler-alternative-considered sections are prose-only spec content, not implementation surface — no task needed. **Second-pass correctness fixes (post-authoring, pre-launch):** two resume-path bugs found and fixed inline: (a) `taskWt`/`intHead` were originally only resolved inside the `startState === "leased"` block, which a resumed task entering directly at `implemented`/`gated`/`reviewed` skips entirely, leaving `taskWt` unresolved on those paths — fixed by moving the ensure-calls to the top of `runTask()`, right after the `done` early return, for every non-terminal entry (Task 3). (b) Task 5 originally proposed a SEPARATE resume branch-reuse check that re-attempts the merge directly on `state === "committed"`, while also retargeting `reconcileJournal()` to downgrade unreachable-head `committed` records against `intBranch` — the two contradicted each other (reconcile always fires first and rolls back the very state the other check needed to see), and neither `commitTask` nor the branch-reuse check tolerated the resulting clean-tree resume. Fixed by making `reconcileJournal()` the SOLE crash-mid-merge recovery mechanism, making `commitTask` idempotent on an already-committed clean tree (Task 4), and deleting the redundant branch-reuse check from Task 5. Task 6's case 3.5 acceptance and fixture-seeding requirements updated to match. **2. Vagueness + body-bloat scan:** Task 6's conflict-construction mechanism is deliberately left to the implementer's judgment ("pick whichever is less fixture code") since the OBSERVABLE acceptance criteria are fully pinned — this is a contract choice, not vagueness, matching the plan skill's "implementer writes the body" principle for a test-fixture-construction detail that has no bearing on `src/runner.js`'s contract. No task carries a full implementation body — all are seam + behavior + acceptance. **3. Contract/seam consistency:** `ensureIntegrationWorktree`, `ensureTaskWorktree`, `mergeTaskIntoIntegration`, `withWorktreeLock`, and the four naming functions are each defined exactly once (Task 1-3, 5) and referenced by the same names in every later task. `runWrapper`/`runGateLoop`/`commitTask`/`dispatchWithFallback` signature changes (Task 4) are stated once and consistently referenced in Task 5 (`commitTask(context, task, taskWt)` call site). **4. Wave plan check:** Every task has Wave/Blocks/Blocked-by. Table present. No two tasks share a wave (all single-file-chain, sequential). `meta.scheduler` will be stamped `sequential` in the session JSONL — no wave has ≥2 disjoint tasks.