feat(loop): fail-closed review gate + delta re-reviews (ADR 0078 Phase A2, v0.32.0)#72
Merged
Merged
Conversation
…e A2, v0.32.0) Two guards from the Quinn harvest (protoAgent ADR 0078): - A PARTIAL PANEL IS NOT A REVIEW (D3). The gate now reads the workflow result's failed-step list: any failed finder ⇒ fail closed (review-pending stays, reconcile retries) instead of judging a clean-looking report whose angles never ran — the exact hole observed live 2026-07-06 when two starved finders produced a promotable partial. Persistent unrunnability is bounded by review_run_max (default 3) → flag_blocked for the operator, so a dying workflow doesn't re-burn every poll forever. - DELTA RE-REVIEWS (D5). The gate remembers each round's parsed findings and passes them back as the recipe's prior_findings input on the next run, so a bounce re-review drops fixed findings and carries still-open ones instead of re-litigating from scratch. State cleared on both terminal edges. 4 new tests (partial-panel fail-closed with a live fake runner, run_max escalation, prior-findings carry across rounds, config floor). 302 passed; ruff + pinned format clean. v0.32.0 lockstep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
There was a problem hiding this comment.
QA Audit — PR #72 | feat(loop): fail-closed review gate + delta re-reviews (ADR 0078 Phase A2, v0.32.0)
VERDICT: WARN (CI settling — test queued; findings are non-blocking)
CI Status
- test: queued
Diff Review
loop.py(+115/-7): Two ADR 0078 guards. D3: partial-panel detection —result.failed⇒ gate treats as unreviewed (fail closed);review_run_max(default 3) bounds retries then escalates viaflag_blocked. D5: prior findings serialized asprior_findingsinput for delta re-reviews, with exception-safejson.dumpsbehindtry/except(memory is optimization, not gate).protoagent.plugin.yaml/pyproject.toml: version bump 0.31.0 → 0.32.0, newreview_run_maxconfig doc.tests/test_loop.py(+81): 4 new tests covering config floor (review_run_max: 0 → 1), partial-panel rejection, runner failure escalation, and delta carry._Finding.to_dict()helper added.
Observations
- LOW / CLAWPATCH: structural review returned
no features touched by diff— the PR is scoped to one module (loop.py) and its test; no cross-file hazards detected. - LOW:
testCI is still queued. Findings are from static review only. The approve-on-green policy promotes this review to APPROVED automatically once every check settles terminal-green — no re-review needed on a clean pass.
— Quinn, QA Engineer
|
Submitted COMMENT review on #72. |
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase A2 of the QA review takeover (protoAgent#1859, ADR 0078). Two guards from Quinn's failure ledger, applied to the M5 gate:
A partial panel is not a review (D3). The gate now consumes the engine result's
failedlist — any failed finder step ⇒ fail closed:review-pendingstays and the reconcile retries, rather than judging a clean-looking report whose review angles never actually ran. This closes the exact hole we hit live yesterday (two starved finders → partial report the gate would have trusted). Persistently unrunnable gates escalate viareview_run_max(default 3) →flag_blocked"needs operator attention" — Quinn's escalation rail, not an infinite workflow re-burn.Delta re-reviews (D5). Each round's parsed findings are remembered and passed back as the recipe's
prior_findingsinput (protoAgent#1860 threads it into the finder prompts), so a bounce re-review carries still-open findings and drops fixed ones instead of re-litigating from scratch.4 new tests incl. a fake host runner proving the partial-panel path never judges. Full suite 302 passed; pinned
ruff format --checkclean this time. v0.32.0.🤖 Generated with Claude Code