Skip to content

docs: fix pr_reviewer.repos semantics — empty means allow-all, not allow-none - #53

Merged
mabry1985 merged 1 commit into
mainfrom
docs/repos-allowlist-semantics
Sep 11, 2026
Merged

docs: fix pr_reviewer.repos semantics — empty means allow-all, not allow-none#53
mabry1985 merged 1 commit into
mainfrom
docs/repos-allowlist-semantics

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Summary

  • Corrects protoagent.bundle.yaml and README.md: both described an empty pr_reviewer.repos as "review nothing until the operator names them." That's backwards — the plugin (dispatch.py, documented since qaEngineer#38) has always treated empty as "whatever the GitHub App installation covers." GitHub's own install picker is the real gate; repos only narrows within it.
  • Closes out pr-reviewer-plugin#115, which I filed assuming this was a plugin bug — it isn't, the code and its docstring are correct and intentional.
  • Notes the actual drift: qaEngineer#38 (2026-08-17) explicitly decided qaEngineer itself should stay excluded from review ("she never reviews her own work"). The live config no longer reflects that (pr_reviewer.repos is globally empty). Confirmed with the operator (2026-09-11) that the current state — unrestricted scope, qaEngineer included — is now intended: her reviewing config/doc changes to her own deploy repo is useful and distinct from the "never approve a PR I personally authored" guard, and it's what makes this repo's branch-protection gate satisfiable at all (GitHub blocks a human PR author from self-approving).

Test plan

  • python3 -m unittest discover tests — 18 passed
  • Verified the real gate semantics by reading pr-reviewer-plugin's dispatch.py repos property docstring directly

🤖 Generated with Claude Code

https://claude.ai/code/session_01M1eVZNokjz3vjPaGqrC3RP

…low-none

The bundle.yaml comment and README both said "empty until the operator names
them," implying an empty repos list blocks review everywhere. The actual gate
(pr-reviewer-plugin dispatch.py, documented there since qaEngineer#38) has
always treated empty as "whatever the GitHub App installation covers" —
GitHub's own install picker is the real gate, repos only narrows it further.

Filed as pr-reviewer-plugin#115 assuming it was a plugin bug; it isn't, closed
not-planned. The actual drift was qaEngineer#38's "qaEngineer should stay
excluded" call, which the live config no longer reflects (repos is globally
empty) — confirmed with the operator this is now the intended state: Vera
reviewing config/doc changes to her own deploy repo is useful, and her
approval is what satisfies qaEngineer's branch-protection gate, since GitHub
blocks a human PR author from self-approving.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M1eVZNokjz3vjPaGqrC3RP

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review · head a747c48904e4 · formal

The panel found no defects across all four finder lanes; the PR is internally consistent. The one shared gap noted by the finders — independent verification of the "empty = allow-all" semantics in dispatch.py — was flagged as a coverage gap but did not rise to a finding because the behavior is consistent with the surrounding code and no contradicting evidence was produced. Verification confirmed the clean state: no claims to refute, qualify, or drop. No fix-first item; no panel disagreement.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promoting the PASS verdict for head a747c48904e4: all checks terminal-green, zero unresolved review threads. (approve-on-green)

@mabry1985
mabry1985 merged commit 0b82951 into main Sep 11, 2026
3 checks passed
@mabry1985
mabry1985 deleted the docs/repos-allowlist-semantics branch September 11, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant