[Bugfix #870] Wire up max_iterations as safety ceiling; treat COMMENT and REQUEST_CHANGES asymmetrically

[waleedkadous]

Fixes #870

Root cause

getMaxIterations() in packages/codev/src/commands/porch/protocol.ts:403 was exported but had zero callers — porch ignored the configured per-phase cap on every verdict-handling decision. The rebuttal-exists branch in next.ts also unconditionally advanced the phase even when reviewers had returned REQUEST_CHANGES, so the rebuttal cycle was effectively one-shot. The phantom comment at next.ts:617-618 referencing a "max_iterations safety valve" pointed at code that had never landed.

Fix

Adopts the asymmetric policy proposed on the issue. COMMENT is a non-blocking note, not a change-request:

• ADVANCE on no REQUEST_CHANGES (all APPROVE-or-COMMENT). Already handled by allApprove() in verdict.ts:57 (COMMENT was already counted as approve); this PR adds an explicit regression test for the asymmetry so future refactors do not silently regress it.
• RE-ITER on any REQUEST_CHANGES — no count limit in normal flow. The rebuttal-exists branch now increments state.iteration and clears build_complete so the next porch next emits a fresh build task carrying the previous reviews + rebuttal as context.
• SAFETY CEILING via getMaxIterations(). When state.iteration >= max_iterations and REQUEST_CHANGES persists, porch force-advances, records a force_advanced audit entry on status.yaml, and prepends a clear notice to the next task's description. The latest rebuttal file is preserved on disk as the audit trail.

The natural-experiment data cited on the issue (21 review iterations classified post-hoc) showed iteration-count is a poor signal — 52% of iters caught real bugs, 24% were COMMENT-cycling. A per-iter cap missed bugs that surfaced at iter-3, iter-4. The new policy keeps re-iter responsive to verdicts (which DO carry signal) and reserves max_iterations for runaway prevention only.

Defaults

• Default safety ceiling in protocol parsing bumped from 1 → 8 (protocol.ts:228).
• SPIR and ASPIR per-phase max_iterations bumped from 1 → 8 so build-verify cycles can re-iter on REQUEST_CHANGES.
• PIR left at max_iterations: 1 — its documented "single advisory pass" semantic is now mechanically enforced (REQUEST_CHANGES on iter 1 hits the ceiling and force-advances). BUGFIX/AIR are unaffected (their PR consultations are once-type phases, not build_verify).

Behavioral diff vs. before this PR

Scenario	Before	After
All APPROVE	Advance	Advance
Mix of APPROVE + COMMENT	Advance	Advance
Any REQUEST_CHANGES, no rebuttal yet	Emit "write rebuttal" task	Emit "write rebuttal" task (unchanged)
REQUEST_CHANGES + rebuttal, iter < ceiling	Advance (one-shot)	Re-iter (increment, rebuild with context)
REQUEST_CHANGES + rebuttal, iter >= ceiling	Advance silently	Force-advance + force_advanced audit + ceiling notice

Test Plan

• Existing next.test.ts rebuttal/advance tests still pass (max=1 protocol → iter=1 hits ceiling, force-advance path lands at same gate_pending response as before).
• New test: advances when reviewers return only COMMENT verdicts (no REQUEST_CHANGES) — asymmetric policy explicit.
• New test: re-iters when rebuttal exists and iteration is below the safety ceiling — confirms iter increments, build_complete cleared, no force_advanced.
• New test: force-advances with audit trail when REQUEST_CHANGES persists at the safety ceiling — confirms force_advanced state field, ceiling notice on task, rebuttal preserved on disk.
• Full porch test suite passes (319 tests).
• Full codev test suite passes (3141 tests, 0 failures, 13 unrelated skips).

CMAP Review

3-way consultation (Gemini, Codex, Claude) via consult --protocol bugfix --type pr:

Reviewer	Verdict	Confidence	Key Issues
Gemini	APPROVE	HIGH	None
Codex	APPROVE	HIGH	None
Claude	APPROVE	HIGH	None

Reviewers independently confirmed: (a) the fix correctly wires up the previously dead getMaxIterations as a safety ceiling, (b) the asymmetric COMMENT vs REQUEST_CHANGES policy matches the superseding policy from the issue's follow-up comment, (c) regression tests cover all three code paths (advance, re-iter, force-advance), (d) scope is tight, no debug code, no stray TODOs, (e) the existing rebuttal-advance test still passes because the test fixture's max_iterations: 1 puts iter=1 at the ceiling, producing the same gate_pending outcome through the new force-advance path. Reviews saved to codev/projects/bugfix-870-porch-max-iterations-enforceme/.

[waleedkadous]

Approving for merge — your dual-CMAP unanimous APPROVE earlier covered this thoroughly, and the asymmetric COMMENT/REQUEST_CHANGES policy is exactly the right answer for the data the reporter shared.

Please merge with --admin bypass.