# Close run-plan / harness-engine gaps (D, A, B, C) 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:** Fix the D2 crash class and bring the harness engine (`bin/runplan`/`src/runner.js`) to parity with the Workflow controller on decision gates, isolated runs, and risk-routed review escalation — without touching plan-emission (that's the separate, gated `harness-migration` follow-on).

**Architecture:** All code changes land in `src/runner.js` (sequential waves, since every code task touches this one file) plus one independent `bin/ensure-web.sh` fix and one independent `~/.claude/skills/run-plan/SKILL.md` doc update. No new files, no new scripts — this is entirely fixes and additive fields on the existing engine.

**Tech Stack:** Node.js (`src/runner.js`), bash (`bin/ensure-web.sh`, `lib/*.sh`), existing bash test harness (`test/*.sh`).

---

## Wave Plan

| Wave | Tasks | Files touched | Safe to parallelize? |
|------|-------|----------------|----------------------|
| 1 | Task 1, Task 2, Task 5 | `bin/ensure-web.sh`, `src/runner.js`, `~/.claude/skills/run-plan/SKILL.md` | ✅ no overlap |
| 2 | Task 3 | `src/runner.js` | single task — depends on Task 2 (same file) |
| 3 | Task 4 | `src/runner.js` | single task — depends on Task 3 (same file) |

**Execution strategy:** `dag-parallel` — Wave 1 holds 3 independent file-disjoint tasks.

## Decision-Enumeration Pass

No `gated` records authored for this plan. Checked each task against baseline categories (`irreversible | fork | input | policy | architecture`): none is a destructive/irreversible op, none forks the task graph, none needs a policy/architecture call beyond what the approved spec + `advisor()` review already settled (see `docs/specs/2026-07-01-run-plan-harness-gaps-design.md` §9 Architecture Decisions). Ship method (`merge-to-main`) already confirmed with the user and stamped in the session file's `meta.land_mode`.

---

### Task 1: `bin/ensure-web.sh` — dual-stack port probe (Gap D1)

**Wave:** 1
**Blocks:** —
**Blocked by:** —

**Files:**
- Modify: `bin/ensure-web.sh` — `find_free_port()` (lines 19–24) + wrap the script's executable body so it can be sourced for testing without starting a server
- Create: `test/ensure-web-port.sh`

**Contract:**
- New function `port_in_use(port) -> exit code`: returns 0 (in use) if EITHER `nc -z 127.0.0.1 "$port"` OR `nc -z ::1 "$port"` succeeds; 1 otherwise.
- `find_free_port()` unchanged in signature/output (still echoes the first free port starting from `${HARNESS_WEB_PORT:-4321}`), but its `while` loop condition calls `port_in_use "$port"` instead of the bare IPv4-only `nc -z` check.
- Wrap the script's existing top-level executable body (everything from `# Check if our previously-started server is still up` through the final `exit 1`) in `if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then ... fi` so `source bin/ensure-web.sh` from a test defines `probe_harness`/`port_in_use`/`find_free_port` WITHOUT executing them. No behavior change when the script is run directly (`bin/ensure-web.sh` invoked normally still does exactly what it does today).

**Behavior:** A listener bound only to `::1` (IPv6 loopback) on the candidate port must be treated as occupied, same as one bound to `127.0.0.1`. No change to the retry loop, PID/PORT files, or the `/api/health` identity probe.

**Acceptance:**
- Run: `bash test/ensure-web-port.sh`
- Expected: PASS — a background `python3 -c "import socket,time; s=socket.socket(socket.AF_INET6); s.bind(('::1', 39217)); s.listen(1); time.sleep(5)"` (or equivalent `::1`-only listener) started on a fixed test port, then `HARNESS_WEB_PORT=39217 bash -c 'source bin/ensure-web.sh; find_free_port'` prints a port strictly greater than `39217`, proving the `::1` listener was detected as occupied.

- [ ] Write `test/ensure-web-port.sh` covering the behavior above (start an `::1`-only listener, assert `find_free_port` skips it)
- [ ] Implement `port_in_use` + `find_free_port` update + the source-guard wrap in `bin/ensure-web.sh`
- [ ] Run `bash test/ensure-web-port.sh` → PASS
- [ ] Commit: `git add bin/ensure-web.sh test/ensure-web-port.sh && git commit -m "fix: ensure-web.sh port probe checks both IPv4 and IPv6 loopback"`

---

### Task 2: `src/runner.js` — fail-closed plan validation + resume-safe binding recovery (Gap D2a + D2b)

**Wave:** 1
**Blocks:** Task 3
**Blocked by:** —

**Files:**
- Modify: `src/runner.js` — `loadPlan` (lines 512–524), `runTask` (lines 162–168)
- Test: `test/runner-integration.sh`

**Contract:**
- `loadPlan(planPath)`: after building `tasks` (line 516–522, unchanged), validate each task has non-empty string `id`, integer `wave` (already enforced by existing `integerOrThrow` — no change there), non-empty string `seat`, non-empty string `desc`. First violation found (in task array order) throws:
  ```js
  throw new Error(`harness plan '${planPath}': task ${task.id || index} missing required field '${field}' — is this a session-state/v1 plan? harness plans use {type:'task', seat, tier, ...}, not run-plan.js's session-state schema.`);
  ```
  `field` is the name of the first missing/invalid field checked in this order: `id`, `seat`, `desc`. Return shape (`{ meta, tasks }`) unchanged.
- `runTask(context, task)`: line 167 currently reads `let binding = taskHistory[taskHistory.length - 1]?.binding || null;` — this trusts a `"leased"`-state journal entry's placeholder `binding` (`{seat, tier}`, no `wrapper`/`model`) as if resolved. Add a module-level constant `const RESOLVED_BINDING_STATES = new Set(["implemented", "gated", "reviewed", "committed"]);` and change the read to:
  ```js
  const lastEntry = taskHistory[taskHistory.length - 1];
  let binding = lastEntry && RESOLVED_BINDING_STATES.has(lastEntry.state) ? lastEntry.binding : null;
  ```
  Every other line in `runTask` is unchanged — `binding = binding || (await resolveTaskSeat(...))` at lines 188/202 now correctly re-runs `resolveTaskSeat` whenever the only history is a `"leased"` entry.

**Behavior:**
- D2a: a task missing `seat` or `desc` (or non-string `id`) throws the named error at `loadPlan` time, before any task is leased — never reaches `resolveTaskSeat`/`spawn`.
- D2b: a task whose journal history ends at `"leased"` (no later `"implemented"` or beyond) must have its binding re-resolved via `resolveTaskSeat` on the next `runTask` invocation for that task, not reused from the placeholder. A task whose history ends at `"implemented"` or later still recovers its real resolved binding unchanged (no regression to the resume-continues-past-implement path).

**Acceptance:**
- Run: `bash test/runner-integration.sh`
- Expected: PASS, including two new cases —
  1. A fixture task record missing `seat` → `loadPlan` throws an `Error` whose message contains `"missing required field 'seat'"` and `"session-state/v1"`, not a native `TypeError`.
  2. A well-formed harness-native task dispatched against a stub wrapper that exits non-zero on its first invocation (aborting before `"implemented"` is journaled, leaving only a `"leased"` entry) — re-invoking the runner against the same journal for the same task asserts the stub wrapper's second invocation received a resolved binding with non-`undefined` `wrapper` and `model` fields (i.e. `resolveTaskSeat` ran again).

- [ ] Add both fixtures + assertions above to `test/runner-integration.sh`; run it, confirm both NEW cases fail against current code (missing-field case throws a raw shell/TypeError instead of the named Error; resume case gets `undefined` wrapper/model)
- [ ] Implement the `loadPlan` validation and the `runTask` binding-recovery fix in `src/runner.js`
- [ ] Run `bash test/runner-integration.sh` → PASS
- [ ] Commit: `git add src/runner.js test/runner-integration.sh && git commit -m "fix: harness engine — fail-closed plan validation + resume-safe binding recovery (D2a/D2b)"`

---

### Task 3: `src/runner.js` — preflight decision gate honored at dispatch (Gap A)

**Wave:** 2
**Blocks:** Task 4
**Blocked by:** Task 2

**Files:**
- Modify: `src/runner.js` — `loadPlan` (lines 512–524), `buildTaskPrompt` (line 670) and its 3 call sites (lines 189, 214, 271)
- Test: `test/gated-decision.sh` (new)

**Contract:**
- `loadPlan(planPath)`: additionally collect `gated` records — `const gated = records.filter((record) => record.type === "gated");` — and return `{ meta, tasks, gated }`. `gated` record shape is identical to `session-state/v1`'s (`id`, `category`, `needs`, `why`, `blast_radius`, `options`, `default`, `status`, `answer`, `resolved_by`, `source`, `binds_meta`) — no new vocabulary, just a new top-level JSONL record `type`. `task` records may now carry an OPTIONAL `requires_decision` string field naming a `gated.id`; no validation change needed in `loadPlan` for this field (absence is the common case).
- `buildTaskPrompt(context, task, mode)` — signature grows a leading `context` param. Behavior:
  ```js
  function buildTaskPrompt(context, task, mode) {
    const base = mode === "review" ? `Review task ${task.id}: ${task.desc}`
      : mode === "fix" ? `Fix gate failures for task ${task.id}: ${task.desc}`
      : `Implement task ${task.id}: ${task.desc}`;
    if (!task.requires_decision) return base;
    const gate = (context.plan.gated || []).find((g) => g.id === task.requires_decision);
    if (!gate || gate.status !== "RESOLVED") {
      throw new Error(`task ${task.id} requires_decision '${task.requires_decision}' still OPEN — decision gate was skipped`);
    }
    return `${base}\nPRE-AUTHORIZED DECISION: ${gate.needs} → answer="${gate.answer}". Apply this choice.`;
  }
  ```
- Update all 3 call sites to pass `context` as the first argument: line 189 (`buildTaskPrompt(context, task, "implement")`), line 214 (`buildTaskPrompt(context, task, "review")`), line 271 (`buildTaskPrompt(context, task, "fix")`).

**Behavior:** A task with no `requires_decision` is unaffected (identical prompt to today). A task with `requires_decision` pointing at a `RESOLVED` gated record gets one extra line appended to EVERY prompt built for it (implement/review/fix — the decision applies for the task's whole lifecycle, not just one dispatch). A task with `requires_decision` pointing at a still-`OPEN` (or missing) gated record throws BEFORE any wrapper is spawned, since `buildTaskPrompt` runs before `dispatchWithFallback`/`runWrapper` at every call site.

**Acceptance:**
- Run: `bash test/gated-decision.sh`
- Expected: PASS —
  1. Task with `requires_decision:"g1"` + a `gated` record `{id:"g1", status:"RESOLVED", needs:"...", answer:"proceed", ...}` → assert the stub wrapper's recorded prompt contains the exact string `PRE-AUTHORIZED DECISION:`.
  2. Same task, `gated` record `status:"OPEN"` → assert the run throws an Error containing `"still OPEN"`, and the stub wrapper's invocation log file is empty (never invoked).

- [ ] Write `test/gated-decision.sh` with both fixtures above; run it, confirm it fails against current code (no `gated` array parsed, `buildTaskPrompt` takes no `context`)
- [ ] Implement the `loadPlan`/`buildTaskPrompt`/call-site changes in `src/runner.js`
- [ ] Run `bash test/gated-decision.sh` → PASS, and re-run `bash test/runner-integration.sh` → still PASS (no regression from the `buildTaskPrompt` signature change)
- [ ] Commit: `git add src/runner.js test/gated-decision.sh && git commit -m "feat: harness engine — honor requires_decision/gated records at dispatch (Gap A)"`

---

### Task 4: `src/runner.js` — risk-routed review escalation (Gap C)

**Wave:** 3
**Blocks:** —
**Blocked by:** Task 3

**Files:**
- Modify: `src/runner.js` — `runGateLoop` (lines 237–297), `resolveOptionalSeat` (lines 392–401), `runTask`'s review-dispatch block (around line 213–223, exact line offset shifted by Task 2/3's edits — locate by the `reviewerBinding &&` conditional)
- Test: `test/runner-integration.sh`

**Contract:**
- `runGateLoop` currently returns bare `head` from both its success-path `return head;` statements (lines 257 and 288). Capture the risk call's `{stdout}` (already returned by `runCli`, currently discarded) at both call sites, parse its first line for `RISK=HIGH` or `RISK=LOW` and scan for an optional `TRUST_BOUNDARY: <text>` line, and return `{ head, riskLevel, trustBoundary }` (`trustBoundary` is the matched line's text or `null`) instead of bare `head` from both return points.
- `resolveOptionalSeat(context, task, seat, tier = task.tier || "regular")` — add the 4th `tier` parameter with that exact default (preserves every existing 3-arg call site, e.g. the `"fixer"` call in `runGateLoop` line 265, unchanged). Pass `tier` through to the existing `resolveSyntheticSeat(context, task, seat, tier)` call (replacing the hardcoded `task.tier || "regular"` there).
- `runTask`'s call to `runGateLoop` (currently `currentHead = await runGateLoop(...)`) destructures `{ head, riskLevel, trustBoundary }` instead; `currentHead = head`. The `"gated"` journal entry appended right after (currently `{state:"gated", base, head, binding, rc:0}`) gains `riskLevel` and `trustBoundary` fields (`trustBoundary` may be `null`).
- In the review-dispatch block: if `riskLevel === "HIGH"`, re-resolve `reviewerBinding` via `resolveOptionalSeat(context, task, "reviewer", "critical")` (overriding the binding computed earlier in `runTask` at line 170, which used the default tier) before dispatching the review. If `trustBoundary` is truthy, append `\nTRUST_BOUNDARY: this diff touches an API/route/webhook surface — review with extra attention to authZ, input validation, and injection.` to the built review prompt.

**Behavior:** `riskLevel === "LOW"` (or risk-router output absent/unparseable) → no change from today's behavior. `riskLevel === "HIGH"` on a TIERED preset (e.g. `codex`) escalates the reviewer to its `critical` binding; on a FLAT preset (e.g. `cursor`, single binding per seat) `critical` resolves to the same binding as `regular` — a documented no-op there, not a bug. `trustBoundary` injection fires independent of preset shape.

**Acceptance:**
- Run: `bash test/runner-integration.sh`
- Expected: PASS, including a new case — a stub `lib/risk-router.sh` (or an override producing `RISK=HIGH` + a `TRUST_BOUNDARY` line as the risk `runCli` output) → assert the reviewer stub wrapper was invoked with the `critical`-tier `model`/`wrapper` values from the active preset fixture, and that its recorded prompt contains the exact string `TRUST_BOUNDARY:`.

- [ ] Add the stub-risk-router fixture + assertion above to `test/runner-integration.sh`; run it, confirm it fails against current code (risk output discarded, reviewer always dispatched at task tier)
- [ ] Implement the `runGateLoop`/`resolveOptionalSeat`/review-dispatch changes in `src/runner.js`
- [ ] Run `bash test/runner-integration.sh` → PASS (including Task 2/3's cases — no regressions)
- [ ] Commit: `git add src/runner.js test/runner-integration.sh && git commit -m "feat: harness engine — risk-routed review escalation (Gap C)"`

---

### Task 5: `~/.claude/skills/run-plan/SKILL.md` — decision-gate step + isolate clause (Gap A doc half + Gap B)

**Wave:** 1
**Blocks:** —
**Blocked by:** —

**Files:**
- Modify: `~/.claude/skills/run-plan/SKILL.md` — insert a new step 0 before "Harness engine path"'s step 1 (line 115), and add one clause inside "Isolated run (`--isolate`)"'s step 2 (line 94)

**Contract (exact text to insert):**

Insert as a new numbered step 0, immediately before the existing step 1 in "## Harness engine path" (before line 115), renumbering existing 1–5 to 1–5 unchanged (step 0 sorts before step 1, no renumber needed):

```
0. **Decision gate.** Native Read the harness plan JSONL. Collect `gated` records with `status:"OPEN"`. None → skip to step 1. Any → present ONE `AskUserQuestion` sheet (same shape as the Workflow path's step 4.3: `needs`/`why`/`blast_radius`/`options`, recommended first; go/no-go entries offer exactly `proceed`/`abort`). Write each answer back to its `gated` line (`status:"RESOLVED"`, `answer`, `resolved_by:"user"`) via a small `jq`/`python3` in-place edit (single line rewrite, same pattern the Workflow path already uses for its own jsonl).
```

Also update the "Scope, stated plainly" sentence at line 127 (which currently says "no preflight decision gate, no `--isolate` clone-run, no per-task risk-routed review on this path") to drop "no preflight decision gate, no `--isolate` clone-run, no per-task risk-routed review" since all three now exist after this plan lands — replace with: `"Scope, stated plainly: decision gate covers plan-authored gated records only, no LLM scan backstop on this path yet — tracked as a follow-up, not silently dropped."`

Add one clause inside "## Isolated run (`--isolate`)" step 2 ("Rebind for steps 3–6"), after its existing bullets (after line 97): when the classified path (step 3) is the Harness engine path, rebind means invoking `bin/runplan` from INSIDE the clone (`<clone>/bin/runplan <slug> --preset <preset>`, not `<repoRoot>/bin/runplan`) so `bin/runplan`'s own `REPO_ROOT` resolution (`cd "$SCRIPT_DIR/.." && pwd`) naturally resolves to the clone — no `--projects`/`RUN_PLAN_PROJECTS` flag needed (Workflow-engine-specific). Ship-freeze (Harness engine path step 1) also targets the clone's `.claude/scripts/ship.sh`, already frozen `pr` by `rp-isolate.sh setup`.

**Acceptance:** manual/documentation-level — no automated test (this is a skill doc, not code). Verify by re-reading the edited SKILL.md and confirming: (1) step 0 appears before step 1 under "Harness engine path", uses the exact `AskUserQuestion` sheet shape described, (2) the isolate clause appears under step 2 of "Isolated run", (3) the "Scope, stated plainly" line no longer claims gaps this plan just closed.

- [ ] Apply both SKILL.md edits above (apply inline, no dispatch — doc-only, no code/tests)
- [ ] Re-read the edited file, confirm the 3 acceptance points above
- [ ] Commit: `git add ~/.claude/skills/run-plan/SKILL.md && git commit -m "docs: run-plan SKILL.md — decision gate step + isolate clause for harness engine path"`