# Live v2 seam audit — T-G / A1

audience: AI coding agents first.

Scope: `modules/harness/v2`, `modules/harness/v2/bin`, and wrappers/launcher skill named by `SPEC-P3-CHAINS-SEAMS.md` A1. `modules/harness/src` was neither read nor used as evidence. Branch/worktree audited: `factory/adoption`, `/home/user/wt-factory`.

Severity: `blocker` permits unreviewed or unvalidated code to integrate/land; `major` breaks required contract or protocol; `minor` creates drift or fail-late behavior without a demonstrated unsafe landing; `informational` is compliant and needs no fix.

## 1. Dispatch result → commit/integration/land

### S1 — Direct-dispatch tasks integrate without reply validation or review

- **Finding:** `meta.preset === undefined` selects direct-dispatch path. Successful child exit + real gate is sufficient to commit and call `integrateCheckpoint`; `attempt.reply.valid` is never read and no review runs.
- **Evidence:** `modules/harness/v2/run.js:runTask` lines 517–519 select direct path when preset is absent; lines 530–574 call `runDispatch`, test only `failedChild`, run gate, commit, write quality receipt, then call `integrateCheckpoint`. `modules/harness/v2/dispatch.js:emitAttemptEnd` lines 105–116 computes and journals `replyResult.valid` only. `modules/harness/v2/run.js:runPlan` lines 154–163 pushes integration branch and may call `landPlan` after successful task results.
- **Severity:** blocker.
- **A2 fix:** Delete direct quality bypass. Route every real dispatch through one trusted quality path that requires valid coder reply, independent review, deterministic gates, durable quality receipt, then integration.

### S2 — Preset/trusted tasks journal invalid coder replies but continue

- **Finding:** Trusted path also ignores coder reply validity. `runDispatch` returns child result without validation state; `runTrustedTask` checks process exit only before quality.
- **Evidence:** `modules/harness/v2/quality.js:validateDispatchReply` lines 60–73 computes `valid`; `modules/harness/v2/dispatch.js:emitAttemptEnd` lines 105–116 persists it; `modules/harness/v2/dispatch.js:runDispatch` lines 54–65 returns raw child result. `modules/harness/v2/run.js:runTrustedTask` lines 632–648 checks `failedChild(dispatch)` and proceeds to quality without reading `attempt.reply.valid`; lines 683–686 commit and integrate.
- **Severity:** blocker.
- **A2 fix:** Return validated reply result from `runDispatch`; fail closed before gate/review when reply is absent, malformed, or child exit is nonzero. Add integration test proving invalid reply cannot create commit, integration intent, checkpoint, push, or land.

### S3 — Normal-risk trusted tasks skip independent review

- **Finding:** Default task risk is `normal`; review is required only for `high`/`unknown`. Normal tasks can gate, commit, and integrate with an explicit `review.skipped` receipt.
- **Evidence:** `modules/harness/v2/run.js:runTrustedTask` lines 648–654 supplies `task.riskLevel || task.risk || 'normal'`. `modules/harness/v2/quality.js:reviewRequired` lines 118–120 returns true only for `high`/`unknown`; `runQualityPhase` lines 138–157 performs or skips review. `runTrustedTask` lines 671–686 accepts green quality, commits, and integrates.
- **Severity:** blocker.
- **A2 fix:** Require independent review for every code-producing task before quality can become green. If risk-tier review policy is intentional, document explicit product authorization in canonical spec; none exists in A1/reliability contract.

### S4 — Fixer mutations can integrate under stale pre-fix review

- **Finding:** Review runs before gates. When a gate fails, fixer mutates worktree; green gates return success without rerunning review. Commit can therefore contain code reviewer never saw.
- **Evidence:** `modules/harness/v2/quality.js:runQualityPhase` lines 138–154 performs review; lines 164–185 dispatches mutating fixer rungs; lines 186–188 returns green after gates only. `modules/harness/v2/run.js:runTrustedTask` lines 683–686 commits that final worktree and integrates it.
- **Severity:** blocker.
- **A2 fix:** Make final quality sequence mutation-aware: after every fixer mutation, rerun deterministic gates and independent review against exact final tree; bind quality receipt to reviewed commit/tree digest.

### S5 — Review parser expects exact JSON but receives complete wrapper log

- **Finding:** Review parsing is fail-closed but protocol-incompatible. It calls `JSON.parse` on entire trimmed dispatch log, while wrappers emit headers, transport events, usage, and status lines.
- **Evidence:** `modules/harness/v2/quality.js:readDispatchLog` lines 53–57 reads full log; `parseReviewVerdict` lines 97–115 requires one exact JSON object; `reviewOutput` lines 214–217 returns dispatch output; `runQualityPhase` lines 145–151 parses it directly. Wrapper examples: `modules/harness/wrappers/ca.sh` lines 230–234 and 277–290; `codex.sh` lines 468–541; `pi.sh` lines 236–350; `na.sh` lines 227–250.
- **Severity:** major.
- **A2 fix:** Parse canonical validated terminal reply artifact, not whole log. Preserve exact review schema `{verdict,findings}` and test each wrapper through real `runDispatch` log shape.

### S6 — Stub gate path is not a dispatch-result bypass

- **Finding:** `gate.mode:"stub"` commits/integrates without review, but explicitly dispatches no agent; it is outside A1 question 1.
- **Evidence:** `modules/harness/v2/run.js:runPlan` line 69 reports no agent/no gate; `runTask` lines 493–508 executes `runStubGate`, commits, and integrates. No `runDispatch` call exists in this branch.
- **Severity:** informational.
- **Justification:** No A2 seam fix required for dispatch-result validation. Keep stub mode test-only; do not treat its receipt as evidence for real-mode quality.

## 2. Plan load, dependency/wave order, acceptance

### P1 — Dependency graph errors are rejected at parse/load

- **Finding:** Unknown dependencies and cycles fail in `parsePlan` before scheduling.
- **Evidence:** `modules/harness/v2/plan.js:parsePlan` lines 23–29 calls `validateTasks`; `validateTasks` lines 93–110 rejects duplicate IDs, unknown dependency IDs, and cycles.
- **Severity:** informational.
- **Justification:** No fix needed.

### P2 — Explicit dependency/wave conflicts are rejected at parse/load

- **Finding:** When task and dependency both declare waves, reverse/same-wave non-later ordering is rejected during `parsePlan`.
- **Evidence:** `modules/harness/v2/plan.js:parsePlan` lines 26–29 calls `validateWaves`; `validateWaves` lines 113–121 rejects `task.wave < dep.wave` and same-wave `task.phase <= dep.phase`.
- **Severity:** informational.
- **Justification:** No fix needed for fully explicit waves.

### P3 — Mixed explicit/inferred wave conflicts fail after run state initialization, not at load

- **Finding:** `validateWaves` checks only pairs where both waves are explicit. `topoOrder` catches mixed explicit/inferred conflicts later, after plan journal initialization, coordinator registration, and lock acquisition, though still before dispatch.
- **Evidence:** `modules/harness/v2/plan.js:validateWaves` lines 117–119 guards on both waves being defined; `topoOrder` lines 124–153 computes inferred positions and rejects conflicts at lines 138–140. `modules/harness/v2/run.js:runPlan` lines 52–74 initializes journal/registry/lock before calling `topoOrder` at line 86.
- **Severity:** minor.
- **A2 fix:** Compute and validate complete wave positions at plan trust boundary before journal, registry, branch, or lock mutation. Reuse one ordering validator from `parsePlan` and scheduler.

### P4 — Acceptance criteria are advisory text, not enforced contract

- **Finding:** Live task schema requires only `id` and `desc`; no acceptance-criteria field is required or evaluated. Whole `desc` is sent to coder, while gates execute configured checks. Acceptance is enforceable only when author encoded it into deterministic gate commands; prose alone is advisory.
- **Evidence:** `modules/harness/v2/plan.js:normalizeTask` lines 60–90 validates ID, description, deps, wave, and file claims only. `modules/harness/v2/seats.js:buildPrompt` lines 150–157 declares `desc` whole specification and sends it to agent. `modules/harness/v2/quality.js:gatesGreen` lines 195–197 checks exact/strict gate results, not acceptance clauses. Production search under `modules/harness/v2` found no acceptance/criteria consumer.
- **Severity:** major.
- **A2 fix:** Define explicit acceptance contract at plan load and bind each criterion to deterministic evidence or final independent review. Reject missing/unbound criteria for code-producing tasks.

### P5 — Task checkpoint digest omits acceptance and file-claim fields

- **Finding:** Even if plan carries extra acceptance fields, checkpoint contract digest ignores them; changing them need not invalidate prior task checkpoint.
- **Evidence:** `modules/harness/v2/plan.js:taskDigest` lines 160–169 hashes only `id`, normalized `desc`, sorted deps, seat, and tier. `runPlan` lines 61–67 uses those digests in run identity; `integrateCheckpoint`/`writeCheckpoint` lines 422–428 and 470–488 persist them.
- **Severity:** major.
- **A2 fix:** After defining acceptance schema, include every execution/quality-affecting task field (`acceptance`, `files_modify`, risk, gate inputs, dispatch-affecting fields) in canonical task digest; add changed-contract resume tests.

## 3. Launch modes and foreground contract

### L1 — Live CLI exposes one executable submission path: direct foreground `runPlan`

- **Finding:** Current parser rejects queue, daemonized, and supervised launches. `main` parses then awaits `runPlan` in same process.
- **Evidence:** `modules/harness/v2/bin/runplan.js:parseArgv` lines 142–145 rejects `queue`, registry launch, `--daemonize`, and `--supervised`; lines 181–183 accepts optional `--foreground` marker. `main` lines 355–374 directly awaits `runPlan`. `USAGE` line 14 lists `--foreground`, not daemon/supervised/queue execution.
- **Severity:** informational.
- **Justification:** Code agrees with canonical foreground ownership. `--foreground` is redundant marker, not alternate execution mode.

### L2 — Queue and supervision modules retain state/projection APIs but cannot submit work

- **Finding:** `queue.js` can mutate backlog records; `supervision.js` can validate/update ownership and project restart history. Neither launches a coordinator. CLI queue and dormant systemd/supervised helpers throw before launch; queue worker is report-only.
- **Evidence:** `modules/harness/v2/queue.js:addQueue` lines 167–184, `startIntent` lines 207–218, and `updateQueue` lines 229–255 mutate state only. `modules/harness/v2/supervision.js:registerRun` lines 81–85 and `projectRestartHistory` lines 163–190 persist/project state only. `modules/harness/v2/bin/runplan.js:runQueue` lines 90–111, `runUnderSystemd` lines 235–248, and `launchSupervised` lines 261–265 throw unsupported. `modules/harness/v2/bin/queue-worker.js:reconcileQueue` lines 4–8 is report-only; `v2/test/foreground-only.test.js` lines 9–13 proves legacy schedulers cannot execute.
- **Severity:** informational.
- **Justification:** No current alternate submission path. State/projection utilities are harmless while entry points remain fail-closed and covered by foreground-only test.

### L3 — Dormant launcher bodies contradict no-daemon architecture and invite resurrection

- **Finding:** Unreachable `runUnderSystemd`, registry launch, and `launchSupervised` bodies remain exported behind unconditional throws. They encode prohibited coordinator ownership despite not being callable now.
- **Evidence:** `modules/harness/v2/bin/runplan.js:runUnderSystemd` lines 235–248 contains detached systemd launch after unconditional throw; `launchSupervised` lines 261–298 contains service registration/start after unconditional throws; module exports line 387 exposes both. Canonical reliability spec §6 says no daemon/timer/API/watcher may launch or relaunch a run.
- **Severity:** minor.
- **A2 fix:** Delete dormant execution bodies/exports and tests that preserve them; retain only explicit unsupported-option errors and observer-only record projection.

### L4 — Spec and code say foreground; installed `run-plan` skill advertises rejected `--daemonize`

- **Finding:** Skill launch command is foreground, but its v2 flag inventory still advertises `--daemonize`; code rejects it.
- **Evidence:** `/home/user/.claude/skills/run-plan/SKILL.md` lines 68–75 labels launch foreground and lists `--daemonize`. `modules/harness/v2/bin/runplan.js:parseArgv` lines 142–145 rejects any argv containing `--daemonize`. `SPEC-P3-CHAINS-SEAMS.md` A1 and canonical reliability spec §6 require one blocking foreground owner.
- **Severity:** major.
- **A2 fix:** Remove `--daemonize` from installed/source `run-plan` skill and add contract test comparing documented live flags with `runplan --help`/parser.

## 4. Wrapper status + usage parity

Expected contracts:

- Reply reader accepts only top-level JSON carrying one marker in `clean|findings|committed|steer|kill|noop`: `modules/harness/v2/quality.js:isReply` lines 8–50.
- Review reply is exact `{verdict:"PASS|FAIL",findings:[...]}`: `quality.js:parseReviewVerdict` lines 97–115.
- Usage reader scans every log line for `{"kind":"usage","usage":{...}}`: `quality.js:extractAttemptUsage` lines 76–88.
- Shared status builder uses same reply markers and emits fallback `{ok,detail}` only when structured reply absent: `modules/harness/wrappers/lib/verdict-status.py:is_reply` lines 15–21 and `main` lines 196–210.

### W1 — `ca.sh` successful dispatch parity is correct

- **Finding:** Success/failure after cursor dispatch emits usage then final status. Successful structured agent reply is normalized by shared status builder; final JSON is reply reader's last recognized object.
- **Evidence:** `modules/harness/wrappers/ca.sh:status_json` lines 196–204 invokes `verdict-status.py`; terminal block lines 271–290 emits usage at line 289 and status at line 290.
- **Severity:** informational.
- **Justification:** No fix needed for post-dispatch terminal path. Pre-dispatch failures are engine/precondition failures, not completed attempts.

### W2 — `codex.sh` success parity is correct; generic failure omits status

- **Finding:** Success emits status, usage, then repeats status, which keeps reverse reply parsing correct while usage remains discoverable. Generic nonzero exit emits usage only.
- **Evidence:** `modules/harness/wrappers/codex.sh` lines 459–468 builds/emits shared status; lines 469–538 emits canonical usage; line 541 repeats status. Generic failure branch lines 448–456 emits only unknown usage then exits.
- **Severity:** major.
- **A2 fix:** Centralize one terminal finalizer used by success, rate-limit, timeout, and generic failure. Emit canonical usage then canonical status exactly once at end.

### W3 — `pi.sh` usage-after-status ordering is not itself a reader bug; status shape is incompatible

- **Finding:** Known ordering claim is **refuted as root cause**: reply reader scans JSON objects backwards and skips usage objects because they carry no reply marker; usage reader scans all lines. Actual defect: Pi builds ad hoc status accepting `result`/`status`, adds `session_id` instead of `thread_id`, and does not use `verdict-status.py`; such status can be invisible to `isReply`. Generic nonzero exit emits usage only.
- **Evidence:** `modules/harness/v2/quality.js:extractReplyJson` lines 42–50 and `extractAttemptUsage` lines 76–88 establish order-independent scanning. `modules/harness/wrappers/pi.sh` lines 289–345 accepts `clean|findings|result|status`, falls back to `{ok,detail}`, and adds `session_id`; lines 349–350 emit status then usage. Lines 272–274 emit usage only on generic failure. Shared contract requires marker set and `thread_id`: `verdict-status.py` lines 3–21, 196–210.
- **Severity:** major.
- **A2 fix:** Replace Pi status builder with shared `verdict-status.py`; map continuity to `thread_id`; use common finalizer on every post-dispatch exit. Prefer usage then final status for uniformity, though ordering alone is not currently unsafe.

### W4 — North `na.sh` emits usage but no terminal status on success/generic failure

- **Finding:** North has no shared status normalization. Non-rate success and generic failure emit usage then exit; only rate-limit branch prints an `{ok:false}` line, which is not a recognized reply marker.
- **Evidence:** `modules/harness/wrappers/na.sh:usage_event_json` lines 143–214 emits canonical usage; terminal block lines 239–251 prints status only inside rate-limit branch lines 246–249, always emits usage at line 250, then exits. No `verdict-status.py` reference exists in wrapper.
- **Severity:** major.
- **A2 fix:** Add shared status builder/finalizer. Emit canonical normalized reply after usage for every post-dispatch exit; add North fixture using real Claude/OpenRouter transport envelope shape without provider call.

### W5 — All four wrappers emit usage in reader-compatible shape

- **Finding:** `ca.sh`, `codex.sh`, `pi.sh`, and `na.sh` normalize usage as `{"kind":"usage","usage":{...}}`, with `{unknown:true,model}` fallback.
- **Evidence:** `ca.sh:usage_event_json` lines 136–193; `codex.sh` lines 469–538; `pi.sh:usage_event_json` lines 55–149; `na.sh:usage_event_json` lines 143–214. Reader contract: `quality.js:extractAttemptUsage` lines 76–88.
- **Severity:** informational.
- **Justification:** No usage-shape fix needed. Preserve canonical envelope while unifying terminal ordering/status.

### W6 — Existing v2 tests do not exercise real wrapper output shapes

- **Finding:** Attempt protocol test feeds a synthetic two-line reply+usage log; it cannot catch Codex failure omission, Pi ad hoc schema/order, North missing status, or full-log review parsing.
- **Evidence:** `modules/harness/v2/test/attempt-records.test.js` test `attempt journal payloads match schema and create artifacts`, lines 88–128, writes synthetic `{"clean":...}` then usage and stubs `runChild`. Repository search across `modules/harness/v2/test` found no invocation of `wrappers/ca.sh`, `codex.sh`, `pi.sh`, `na.sh`, or `verdict-status.py`.
- **Severity:** major.
- **A2 fix:** Add provider-free wrapper fixtures for success, malformed reply, timeout, rate-limit, and generic failure. Assert `validateDispatchReply`, review parser, and `extractAttemptUsage` against captured stdout/log for every wrapper.

## A2 task inventory

1. `quality-single-path`: fix S1–S5; one final-tree reply/review/gate contract before receipt/integration.
2. `plan-contract-validation`: fix P3–P5; validate full order at trust boundary and hash explicit acceptance contract.
3. `foreground-surface-cleanup`: fix L3–L4; remove dormant launchers and stale daemon skill flag.
4. `wrapper-terminal-contract`: fix W2–W4; shared finalizer/status schema across Codex, Pi, North.
5. `wrapper-contract-tests`: fix W6; provider-free end-to-end protocol fixtures.

No finding is unassigned: informational findings P1/P2/L1/L2/W1/W5 and scoped exclusion S6 are justified above; every blocker/major/minor maps to an A2 task.

## P4 review round — dispositions

### Fixed

- **`plan.js requiresVerify` exempted real-mode `test` and `discovery` seats.** N1 requires verification for any task reaching a dispatch seat, regardless of seat name. Exemption dropped; only `gate.mode:"stub"`, which dispatches no agent, is exempt. All three dispatch seats now reject a plan carrying no `verify`, and `runVerify` precedes both the N4 ladder and `runQualityPhase` in `run.js`, so rung 3 cannot run on an unverified path.
- **`docs/plans/*.jsonl` verify migration wrote acceptance prose into `verify` as if it were shell** (`2026-07-21-offload-phase3`, `2026-07-28-ai-os-foundation`) and placeholder `git diff --check` (`2026-07-22-overdeck-observability-b3`). Reverted on all 14 archived plans. Those plans fail to parse on `acceptance`/`wave` before `verify` is reached, on `origin/main` and here alike, so the migration was unreachable noise, and a `git diff --check` standing in for a behavioural acceptance bar is a green gate that checks whitespace. `verify` is retained only on `2026-08-04-pi-envelope-fixture.jsonl`, the one plan that loads, where the command is real. Plan-load parity with `origin/main` preserved: 1 loaded, 14 failed.
- **`codex.sh await_output_pipeline` leaked its watchdog `sleep` on every dispatch.** `kill "$watchdog"` signals the subshell but not the `sleep $((TIMEOUT + 30))` running as its child, so each run orphaned one sleep. `command-supervisor` then correctly refused to call the run clean and returned 254 on a green suite; two agents this round read that as a red tree and burned fix rounds on nothing. The subshell now traps `TERM` and forwards the kill to its own sleep. Regression test `codex: the output-pipeline watchdog leaves no orphaned sleep behind` uses a unique timeout so the assertion cannot collide with concurrent runs; verified to fail without the fix (20/21) and pass with it (21/21).

### Justified, not fixed

- **N3/N5 reported unimplemented against `234071d7`.** Reviewed a tree predating `8617ace0`, which wires `buildPrompt` into `bindingDispatch`; the finding itself notes the later unreviewed change.
- **`scout.test.js` ran in no gate.** `modules/harness/v2/test/index.js` is a hand-maintained manifest and `node --test modules/harness/v2/test/` collects only that file, so all scout tests were invisible to every acceptance command — a green suite said nothing about scout. Collected now, and `suite-manifest.test.js` asserts every `*.test.js` on disk is listed, so the trap cannot re-arm.
- **`scout` snapshot hashed `.git/index` bytes.** git rewrites the index stat cache on a stat-dirty tree, so a genuinely read-only recon that merely ran `git status` would trip `scope-violation`. The raw metadata walk now skips `index`/`index.lock`; `gitState` still compares index CONTENT via `ls-files --stage`, so a real index change is still caught.
- **`scout` journalled `run.complete` for a non-succeeded reply.** A `blocked` scout returned `blocked` and exited 1 while /plans showed the run complete. Non-succeeded replies now journal `run.failed` with `failureClass: scout-<status>`; no schema change was needed because `failureClass` is a free-form string.

### T1 exit criterion — real dispatch (found after P4 landed)

Every scout test stubbed `runDispatch`, so T1's "scout end-to-end on a real repo, repo untouched" was never exercised. An end-to-end test against a real wrapper found two defects that made `scout` non-functional in production, both now fixed:

- **`scout` never provisioned its workspace.** `runDispatch` asserted a provisioning receipt unconditionally, so every real invocation died `dispatch-error` before reaching the wrapper. T1 forbids paying full-engine cost and `repoRoot` carries `pnpm-lock.yaml`, so provisioning scout's workspace would run `pnpm install` in the live repo. The assertion now scopes to writable workspaces: a `readOnlyWorkspace` child gets `cwd` bind-mounted read-only and cannot install anything, so requiring it to be prepared is unsatisfiable. The reviewer path still provisions through `run.js`.
- **`scout`'s log violated the read-only workspace contract.** `runDir` resolves inside `repoRoot`, and `child.js` rejects a read-only dispatch whose `logPath` is within `cwd`. The dispatch log now lives under `harnessRoot/runs/scout/<runId>`; the journal stays in `runstate/` so the run remains visible in /plans.
- **`scout`'s CLI defaults pointed nowhere.** `harnessRoot` defaulted to the harness home, which holds no `presets/` or `wrappers/` — those ship inside the read-only engine bundle — and the default preset name `default` matches no preset file. A bare `scout.js <prompt>` therefore always died on `unknown preset`. `harnessRoot` now self-locates to the bundle containing the file, the default preset is `codex` (the same default the plan launcher applies), and the dispatch log moved to the writable harness home, since the bundle is `dr-x`.
- **`scout`'s snapshot crashed on an unreadable directory.** The live repo contains `dr-x------` jail work directories, so the pre-dispatch walk threw `EACCES` before any dispatch. The walk now stops at a directory it cannot read; the parent already records that directory's type and mode, which is the only property a scout that also cannot read it could change.