# Design: close the run-plan ↔ harness-engine gaps Slug: `run-plan-harness-gaps` ## 1. Context `/run-plan` was just wired (`~/.claude/skills/run-plan/SKILL.md`, "Harness engine path") to route native mega-plan-harness plans (`docs/plans/*.jsonl` with no `"schema"` key) to `bin/runplan`/`src/runner.js` instead of the Workflow controller. That wiring deliberately shipped without four things the Workflow path already has, plus one live bug found while testing it. This spec closes all four: - **D — web-UI port bug + fail-closed schema validation** (a real, reproduced crash) - **A — preflight decision gate** for the harness engine path - **B — `--isolate` clone-run** for the harness engine path - **C — risk-routed review escalation** in `src/runner.js`'s gate loop **Scope, split from the migration work:** this spec closes A-D only — the harness engine's own correctness (a real, reproduced crash in D, plus A/B/C parity gaps against the Workflow controller). A separate concern — flipping `brainstorm`/`handoff` to *target* this engine by default for all future plans — was originally drafted as Gap E/F in this same spec, then split out on review: E/F only make sense once A-D are proven, especially D2b (a resume-path bug that fires on any interrupted+resumed harness run — exactly what a migration would trigger constantly). Gaps E/F now live in `docs/specs/2026-07-01-harness-migration-design.md`, a follow-on spec gated on this one landing AND one real plan completing green end-to-end via `bin/runplan` (not just the targeted repros in §8 below). See that spec for full rationale and content. ## 2. Gap D: web-UI port detection + fail-closed plan validation Two independent bugs, confirmed by direct repro (`node src/runner.js run --plan docs/plans/2026-06-30-harness-gated.jsonl --preset cursor`). ### D1 — IPv4-only port probe `bin/ensure-web.sh`'s `find_free_port()` calls `nc -z 127.0.0.1 "${port}"`. A listener bound to the IPv6 loopback (`::1`) — e.g. another project's Astro dev server — is invisible to this probe, so an occupied port is reported free. Real bind then fails silently in the backgrounded `astro dev` process. **Fix:** `find_free_port()` must check both loopback families before accepting a port — a port counts as occupied if either `nc -z 127.0.0.1 "$port"` or `nc -z ::1 "$port"` succeeds. Structure as a helper `port_in_use()` returning 0/1, used by both the initial probe and the `while` loop condition. No other behavior of `ensure-web.sh` changes (retry loop, PID/PORT files, identity probe via `/api/health` stay as-is). ### D2 — no fail-closed validation on task seat/tier, AND a resume-path bug that reuses an unresolved binding Root-caused by tracing the actual crash stack (temporary `error.stack` instrumentation, removed after use), not just the surface symptom. Two distinct defects, confirmed by direct repro against `docs/plans/2026-06-30-harness-gated.jsonl` (a `session-state/v1` plan, task records with no `seat`/`tier`): **D2a — first-ever dispatch of a malformed task IS fail-closed today.** `resolveTaskSeat` shells to `lib/resolve-seat.sh`, which reads the task's `seat` field via `jq -r '.seat'`; when absent this yields the literal string `"null"`, which matches no preset seat, so `resolve-seat.sh` correctly `die_json`s with exit 2 and `runCli` rejects. On a **fresh** journal this throw propagates out of `runTask` cleanly (an ugly but non-crashing `resolve-seat failed with exit 2` message) — this path is not broken. **D2b — the actual crash requires a resumed/re-invoked run, and is a general bug independent of plan-schema correctness.** `runTask` (`src/runner.js:162-234`) writes a placeholder `binding` to the journal at the `"leased"` state (line ~182: `binding || { seat: task.seat, tier: task.tier || "regular" }`) *before* `resolveTaskSeat` has ever been called for that task. On the **next** invocation of `runTask` for the same task (crash-and-restart, `--resume`, or any re-run against an existing `runstate/.jsonl`), `readJournal` finds that `"leased"` entry, and line 167 does `let binding = taskHistory[last]?.binding || null` — picking up the placeholder as if it were a real resolved binding (it has `seat`/`tier` but no `wrapper`/`model`). Line 188's `binding = binding || (await resolveTaskSeat(...))` then short-circuits on this truthy placeholder, **skipping `resolveTaskSeat` and its fail-closed check entirely**, and the placeholder is passed straight to `dispatchWithFallback` → `runWrapper` → `spawn(binding.wrapper, ...)` with `wrapper === undefined` → the native `TypeError` this gap was originally reported as. Confirmed via stack trace: `spawnPassthrough (runner.js:656) ← runWrapper (434) ← dispatchWithFallback (317) ← runTask (190)` — `resolveTaskSeat`/`resolve-seat.sh` do not appear in the trace at all on the crashing call. This means **any interrupted harness-native run — well-formed plan or not — crashes the same way on resume**, once a task's journal history contains a `"leased"` entry with no later `"implemented"` entry. The malformed-plan case (D2a asymptotically feeding `"leased"` entries for every task in the wave before any dispatch attempt) is simply the easiest way to reproduce it, not the boundary of the bug. **Fix (two parts):** 1. **Resume-safety (the real bug, D2b):** a `"leased"` journal entry's `binding` field must never be treated as resolved on recovery. Change the binding-recovery read (`runTask`'s `taskHistory[last]?.binding`) to only trust a recovered binding when the sourcing entry's `state` is `"implemented"` or later — for a `"leased"`-only history, recovered `binding` stays `null`, so line 188 correctly re-runs `resolveTaskSeat` (idempotent — same seat/tier resolves the same binding again). 2. **Fail-closed validation (defense in depth, D2a's actionable-error half):** in `loadPlan`, after filtering `task` records, validate each has non-empty string `id`, integer `wave`, non-empty string `seat`, non-empty string `desc`. First violation → throw `Error("harness plan '': task missing required field '' — is this a session-state/v1 plan? harness plans use {type:'task', seat, tier, ...}, not run-plan.js's session-state schema.")`. This turns D2a's already-fail-closed-but-opaque shell error into an actionable one at load time, before any task is even leased — it does not by itself fix D2b, which is a resume-path defect that would affect a perfectly valid harness-native plan too. This is the ONLY schema check added — not a general JSON-Schema validator (that already lives in `spec/*.schema.json` for presets/runconfigs). ## 3. Gap A: preflight decision gate on the harness engine path The Workflow path's gate (SKILL.md step 4) reads `gated` records from the plan JSONL, scans for new ones, presents one `AskUserQuestion` sheet, and stamps answers back before launch. The harness-native plan schema currently has no `gated` records at all — this section adds the same mechanism, reusing the exact record shape (no new vocabulary): ### Schema addition (harness-native plan format) Add an OPTIONAL `gated` array of records, identical shape to `session-state/v1`'s `gated` record (`id`, `category`, `needs`, `why`, `blast_radius`, `options`, `default`, `status`, `answer`, `resolved_by`, `source`, `binds_meta`). Add an OPTIONAL `requires_decision` string field to `task` records, naming a `gated.id`. Absent `gated` array ⇒ no decisions ⇒ gate is a no-op (back-compat with every plan already in the repo). ### SKILL.md: new preflight step in "Harness engine path" Insert before step 1 (ship-freeze ladder): 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). No `task_graph_hash` scan-backstop for this path in this iteration — the harness plan's tasks are author-time-authored dispatch prompts (`desc`), not agent-judged multi-step work items, so the scan step (which reasons over task descriptions to auto-discover risk) has less signal here. Deferred; tracked as a follow-up, not silently dropped (state this explicitly to the user at launch: "decision gate covers plan-authored `gated` records only, no LLM scan backstop on this path yet"). ### `src/runner.js`: honor `requires_decision` in the dispatch prompt In `buildTaskPrompt(task, mode)`, when `task.requires_decision` is set and the plan's matching `gated` record has `status:"RESOLVED"`, append one line to the built prompt: `PRE-AUTHORIZED DECISION: → answer="". Apply this choice.` — same text pattern `run-plan.js`'s `implement()` already uses for its `gated` injection, kept identical so review/mental-model doesn't fork. **Fail-closed:** a task with `requires_decision` pointing at a `gated` record whose `status` is still `"OPEN"` at dispatch time (skill forgot to gate, or record added after launch) → `resolveTaskSeat`/dispatch throws before spawning any wrapper: `Error("task requires_decision '' still OPEN — decision gate was skipped")`. Mirrors the Workflow engine's trust-boundary statement ("the engine HALTs on unresolved decisions regardless of launch path"). ## 4. Gap B: `--isolate` for the harness engine path No new scripts. `~/.claude/workflows/lib/rp-isolate.sh` is already schema/engine-agnostic — it clones `origin` (the WHOLE repo, `bin/` included), seeds the plan jsonl, flips `land_mode` to `pr`, freezes a `pr` ship wrapper in the clone, and emits `{"clone","projects","jsonl","base","reused"}`. This works unmodified for a harness-native plan. ### SKILL.md: extend "Isolated run" section Add one clause to step 2 ("Rebind for steps 3–6"): when the classified path (SKILL.md step 3) is the Harness engine path, rebind means invoking `bin/runplan` from INSIDE the clone (`/bin/runplan --preset `, not `/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 (that mechanism is Workflow-engine-specific, since only `run-plan.js`'s `load()` needs an explicit repoRoot override). Ship-freeze (gap A/step-1-of-harness-path) also targets the clone's `.claude/scripts/ship.sh`, already frozen `pr` by `rp-isolate.sh setup`. Teardown ladder (`rp-isolate.sh teardown`/`gc`) is unchanged — already engine-agnostic. **Verified, not assumed:** the harness-native meta schema (`{type:meta, slug, base_branch, gate0_mode}`) has no `land_mode` field at all — checked against `test/runner-integration.sh`'s fixture. This is not a gap: `rp-isolate.sh`'s python flip (`o['land_mode']='pr'`) sets the key unconditionally, whether or not it pre-existed, so the clone's meta gets `land_mode` regardless. And the Harness engine path's ship-freeze ladder (step 1) already falls through to `ship-init.sh`'s interactive rung when `meta.land_mode` is absent/unset — exactly the harness-native case — so an un-isolated harness run freezes ship method correctly today, no schema change needed for this. ## 5. Gap C: risk-routed review escalation `lib/risk-router.sh` already classifies each task's diff `RISK=HIGH|LOW` + optional `TRUST_BOUNDARY` line, deterministically, and `lib/gates.sh`'s `risk_check` already relays it — but `runGateLoop` (`src/runner.js`) only checks that the call didn't throw; it never reads the classification. Reviewer tier is fixed at whatever the preset resolved, regardless of risk. ### Behavior change in `runGateLoop` Capture the risk call's `{stdout}` (already returned by `runCli`, currently discarded). Parse the first line for `RISK=HIGH` or `RISK=LOW`, and scan for a `TRUST_BOUNDARY` line. Return `{head, riskLevel, trustBoundary}` from `runGateLoop` instead of bare `head`. ### Behavior change in the task loop (around line 213's review dispatch) Before dispatching `reviewerBinding`: - `riskLevel === "HIGH"` → re-resolve the reviewer binding at `critical` tier instead of the task's own tier (`resolveOptionalSeat(context, task, "reviewer", "critical")` — `resolveOptionalSeat` gains an optional tier-override 3rd/4th param, defaulting to `task.tier || "regular"` exactly as today when omitted, so every existing call site is unaffected). **Caveat, verified against `presets/cursor.json`:** escalation only changes behavior on a TIERED preset (e.g. `codex`, which has distinct `regular`/`critical` bindings). On a flat preset like `cursor` (`resolve-seat.sh` rung 4, `preset.seats[reviewer]` with no tier split), `critical` resolves to the SAME binding as `regular` — escalation is a documented no-op there, not a bug. `TRUST_BOUNDARY` prompt injection (below) still fires regardless of preset shape. - `trustBoundary` present → append one line to the review prompt (`buildTaskPrompt(task, "review")`): `"TRUST_BOUNDARY: this diff touches an API/route/webhook surface — review with extra attention to authZ, input validation, and injection."` Purely an instruction addition, no new seat. **Journal:** record `riskLevel` (and `trustBoundary` if present) on the `"gated"` journal entry already appended after `runGateLoop` returns, so a resumed/replayed run can see why a task got escalated without re-running the risk router. No new config surface — this is fully automatic per the risk-router's existing deterministic output, matching its own doctrine ("a false positive costs one extra [higher-tier] pass; a false negative ships a sink unreviewed"). ## 6. Testing strategy - **D1:** extend `lib/test-resolve-seat.sh`-adjacent test or a new `test/ensure-web-port.sh`: start a dummy listener on `::1:` (bash `/dev/tcp` or `nc -l`), confirm `find_free_port` skips it. - **D2a:** extend `test/runner-integration.sh` with a fixture plan using session-state/v1-shaped tasks (missing `seat`); assert `loadPlan` throws the new named error, not a native `TypeError`. - **D2b:** extend `test/runner-integration.sh` with a well-formed harness-native task; run once far enough to reach `"leased"` (stub wrapper exits non-zero on first call to abort before `"implemented"`), then re-invoke the runner against the same journal; assert the stub wrapper's resolved binding is present (non-`undefined` `wrapper`/`model`) on the second call — i.e. `resolveTaskSeat` ran again instead of reusing the leased placeholder. - **A:** extend `test/runner-integration.sh` (or a new `test/gated-decision.sh`) with a task carrying `requires_decision` pointing at a `RESOLVED` gated record → assert the prompt sent to the stub wrapper contains the `PRE-AUTHORIZED DECISION` line; a second case with the same record `OPEN` → assert the run throws before the wrapper is invoked (grep the stub wrapper's invocation log — it must be empty). - **B:** manual/documentation-level only — `rp-isolate.sh` already has its own tests; no new isolate-specific test needed since gap B adds zero new code. - **C:** extend `test/runner-integration.sh` with a stub `risk-router.sh` returning `RISK=HIGH` + `TRUST_BOUNDARY`; assert the reviewer stub wrapper was invoked with the `critical`-tier model/wrapper from the active preset, and that its prompt contains the `TRUST_BOUNDARY` line. - **Exit criterion for this spec (gates the follow-on migration spec):** a deliberate, hand-authored harness-native plan (not just the targeted repros above — Gap E's producer doesn't exist yet, so this run cannot be brainstorm-generated) runs end-to-end through `bin/runplan` to completion — all waves reach `done`, no D2-class crash on any resume — before `docs/specs/2026-07-01-harness-migration-design.md` (Gap E/F) starts. ## 7. Architecture Decisions - **Gated records reuse `session-state/v1`'s exact shape rather than a harness-specific decision schema** — collapsed candidate: a slimmer harness-only gated record. Rejected: the SKILL.md gate-presentation logic (AskUserQuestion sheet shape) is shared prose across both paths; forking the record shape would fork that logic too for no behavioral gain. - **No LLM scan-backstop for the harness path's decision gate (deferred, not built)** — single-adapter test: the scan step has exactly one plausible use today (Workflow-path plans, where tasks are agent-judged multi-step work). Harness tasks are direct wrapper dispatch prompts; building a second scan adapter for a still-hypothetical need fails YAGNI. Revisit once a harness plan actually needs it. - **`--isolate` gets zero new code** — deletion test: an isolate-specific harness wrapper script was considered and rejected; deleting it changes nothing, since `rp-isolate.sh` + `bin/runplan`'s own relative-path resolution already compose correctly. Decorative seam avoided. - **Risk escalation lives inside `runGateLoop`, not as a new pipeline stage** — deepens an existing module (gate loop already owns gate0+risk sequencing) rather than adding a shallow new "review router" module whose entire body would be a 5-line risk-level branch. Fails the deletion test as a standalone module; folded in.