feat: the findings loop — review → fix → re-review in the engine, roles untouched

[saucam]

Problem

A read-only review phase (reviewer, adversary, verifier) had no way to get its findings acted on. The pipeline had no backward edge: onFail was halt, retry-the-same-phase, or abort, and Revise re-ran the same phase. So the only exits from a review with findings were "approve anyway" or "re-run the reviewer, who still cannot edit". In practice the model in the review phase asked to be re-run under a write-capable role, which is the one thing the role model exists to prevent.

The two obvious fixes each lose what the review rounds are for. Giving reviewers write access means findings stop being written down and the closeout phase has nothing to verify. Hand-authoring fix phases in every pack pushes engine work into prose with no validation that a finding was actually answered.

Change: the findings loop (docs/findings-loop.md)

A phase declares that it produces findings and who resolves them:

- id: review
  skill: review
  role: reviewer            # unchanged: read-only
  gate: bench_clear         # kind: review — now a real verdict
  findings:
    fixWith: implementer    # a write-capable pack role (checked at load)
    blocking: [critical, high]
    maxRounds: 2
    gate: tests_pass        # optional, evaluated after each fix leg

The engine runs the loop, one persisted step per leg:

1. Review leg under the phase's own role, with a contract appended: end the report with a fenced ```findings block (JSON; empty when clean). A missing or malformed block is a format failure with one bounded retry on the engine's own channel, then onFail.
2. Fix leg on the same bound session under fixWith's role, driving the built-in findings-fix skill with the findings and a disposition contract. The engine validates the answer: every blocking finding has exactly one disposition (fixed, not_a_finding, declined, deferred), a reason unless fixed, no unknown ids, no duplicates, no bare deferrals. One bounded retry, then the phase halts with the ledger.
3. Fix gate (optional) on the fix leg: a fixer that broke the build halts the phase with that reason.
4. Re-review under the reviewer's role with the ledger: verify each fix, accept or re-raise rejected ones, report new findings, and list only what remains open. The latest round is the open set, so the reviewer decides convergence and the engine enforces it.
5. Bounded by maxRounds (0 = audit only). Blocking findings left open fail the exit boundary with the ledger. review-kind gates finally carry a real verdict (S4 in docs/pipeline-run.md); a findings phase with no gate gets the same verdict from the engine directly.

Also: the fix leg gets its own model binding through the usual six rungs with the same skip rules, persisted on the def; load refuses a read-only or unknown fix role; the wire carries counts plus a markdown ledger per phase (additive PipelinePhaseWire.findings); codeoid pipeline status prints them.

What is enforced, and where

Rule	Enforced by
The reviewer cannot write	the capability role, unchanged
The fixer is a different, write-capable role	loadPack
Findings are structured	parseFindings (zod), format retry, then onFail
Every blocking finding is answered, with a reason unless fixed	validateDispositions, format retry, then halt with ledger
The fix did not break the build	fixWith.gate on the fix leg
The fix is real	the re-review leg under the reviewer's role
The loop terminates	maxRounds; open blockers fail the boundary
The human sees what happened	halt reason, wire findings, CLI status

Verification

• bun x tsc --noEmit on the CLI, protocol, and core packages: clean.
• bun run lint: clean.
• bun run test: 2534 pass, 12 skip, 0 fail. 27 new tests: the pure parts (block extraction, schema parsing, the disposition contract, ledger rendering, contracts), the engine loop with a scripted runner (role swap and fix-leg binding, clean first report, budget exhausted with halt and abort, audit-only, fixer format retry then halt, reviewer format retry then halt, fix gate failure, serialize/parse round-trip between legs, phases without findings untouched), and the pack side (schema with defaults and pins, read-only or unknown fix role refused at load, review gate verdict, create-time fix-gate validation, fix-leg tier binding, cross-provider skip).

Follow-ups (separate PRs)

• ai-factory packs/org-dev: findings: on the review phase.
• The private yash-dev pack: findings: on review and adversary, with fixWith: implementer and gate: tests_pass.

🤖 Generated with Claude Code

Audit (second commit)

An independent adversarial review of the first commit found, and this PR now fixes:

• High: a failing fix gate on the last budgeted leg was a dead end (leg counted before the gate ran; every exit handed the reviewer a red tree). The gate now runs before the leg commits, with one same-leg repair.
• Medium: onFail: retry re-ran the read-only reviewer and pasted the ledger into the human's revise notes. A findings phase now handles retry as another fix loop with a fresh budget, reason carried on the engine's channel.
• Medium: a bare-marker turn after a nudge dropped the findings block (summary was only the last message). Summaries are now the whole of what the model said across the leg.
• Medium: explicit-phases plans accepted findings with no pack role check. Rejected at create.
• Medium: Revise could not reach the fixer. Human notes now travel to every fix leg.
• Low: entry gates no longer re-evaluated on loop legs; loader refuses skipWhenSatisfied, review-kind entry gates, and non-deterministic fix gates on a findings phase; gate-failure halt keeps the summary; re-review wording and "fixed" counting corrected; web cockpit keeps ledger line breaks and shows counts; CHANGELOG wording.

Verified after the fixes: tsc clean on CLI, protocol, core, and web; lint clean; full suite 2543 pass, 0 fail (9 more tests, including the same-leg gate repair, retry-as-fresh-loop, human notes reaching the fixer, entry gate evaluated once, the loader rejections, the explicit-plan rejection, and the bare-marker summary path through the real phase host).