feat(review-tools): formal verdict surface with below-the-model guards (v0.2.0)#21
Merged
Merged
Conversation
…s (v0.2.0)
ADR 0078 Phase B — the hands of the QA-review takeover:
- github_review_comment / github_review_approve / github_review_request_changes
post formal reviews via the GitHub Review API. The two blocking verdicts are
GUARDED inside the tool, where a prompt can't reach:
· CI-terminal (Quinn's #863): refused while any check run is queued/running,
AND when CI can't be read at all (403/error says nothing about pass/fail —
fail closed, never open). Refusals instruct comment-instead + never poll.
· Self-review: refused when the PR author is the token's own identity
(approve-your-own-work loops); commenting on own PRs stays allowed.
· No checks at all = terminal by definition.
- github_path_exists (read tool): the EXISTS/MISSING grounding probe for
review claims about external references — cross-repo capable; a tool error
reads as UNVERIFIED (a Gap), never a verdict.
- Review tools ride the github.write gate with the other mutating tools.
13 new tests (guards impossible to bypass: pending CI, unreadable CI, own PR;
COMMENT ungated; path probe EXISTS/MISSING/UNVERIFIED). 110 passed; ruff clean.
v0.2.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 #21 | feat(review-tools): formal verdict surface with below-the-model guards (v0.2.0)
VERDICT: WARN (CI settling — see below)
CI Status
test: in_progress ⏳
Diff Review
- 7 files, +426/-3: new
review_tools.py(219 lines),github_path_existsprobe inread_tools.py, version bump 0.1.6 → 0.2.0 acrosspyproject.toml+protoagent.plugin.yaml, and tool registration hook in__init__.py. - Architecture is clean: review tools register under the existing
github.writegate alongside merge/close tools — no new auth surface. - Guards are below-the-model:
_ci_state()returnsterminal/pending/unknown, andunknown(CI unreadable) is treated identically topending— fail-closed, per #863.
Observations
- INFO:
_viewer_login()returns""on failure. If the self-review guard compares this against PR author, an empty viewer will never match a real author (fail-open on that guard specifically). This is the right tradeoff — the alternative (blocking all verdicts on a transient user-API error) would be worse, and the CI-terminal guard is the primary safety net. - GAP: clawpatch structural review returned 2 report findings but the detail payload was null — could not surface the specific findings. The mapped features (9) and reviewed slice (1) suggest coverage of the new module; re-running with
provider: protoon a re-review pass would give deeper coverage. - GAP: The diff was truncated at 502 lines — the 3 tool definitions (
github_review_comment,github_review_approve,github_review_request_changes) and the test files are not fully visible. The 13 new tests claim full guard coverage (pending CI, unreadable CI, own-PR all refuse before reaching the Review API).
⏳ CI is still settling — the test check is in progress. Per the approve-on-green policy, this COMMENT review will be promoted to a formal APPROVE automatically once every check completes terminal-green with no blockers on this review. No re-review dispatch is needed.
— Quinn, QA Engineer
|
Submitted COMMENT review on |
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.
ADR 0078 Phase B — the verdict surface for the QA-review takeover (protoAgent
docs/plans/qa-review-takeover.md).What ships
github_review_commentgithub_review_approvegithub_review_request_changesgithub_path_exists(read)The guards (Quinn's failure ledger, enforced below the model)
Review tools register under the existing
github.writegate alongside merge/close.Tests
13 new — the point is that the guards are unbypassable: pending CI, unreadable CI, and own-PR all refuse before anything reaches the Review API (asserted via the captured call list). Full suite 110 passed, ruff clean. v0.2.0.
Next: B2 — protoPatch as a fifth, non-LLM panel member.
🤖 Generated with Claude Code