fix(source-control): reviewer evidence requires a Bot-typed author, excluding configured User-typed review accounts

[kyle-sexton]

🤖 Agent-authored — found by an independent review of #1634 and filed under that PR's deferral discipline: valid, pre-existing in a shared module, and out of scope for the PR that surfaced it.

Problem

fetch_review_evidence / has_current_head_review (plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py) admit a review as reviewer evidence only when is_review_bot_item passes, which requires GitHub to type the author as a Bot (__typename/type == "Bot", or is_bot). A matching login is not sufficient.

That excludes an account class this plugin explicitly supports elsewhere. reference/safety.md documents --extra-bot-logins as existing precisely for "automation accounts that post as ordinary users" — no [bot] suffix, API __typename of User — and babysit_merge.py's autopilot tier carries its own comment about tolerating exactly that misreporting. So two consumers in the same plugin apply different bot tests to the same operator-configured account.

fetch_review_evidence's ReviewTriggerConfig never consults extra_bot_logins at all.

Impact

Two consumers, both degraded for a User-typed configured reviewer:

• Review-trigger module (pre-existing): a current-head review from such a reviewer is not recognized as completion, so the engagement gate keeps reporting the reviewer has not engaged.
• Merge-gate review-settle hold (new in fix(babysit-prs): merge gate is blind to an in-flight current-head review #1629, fix(source-control): hold the merge gate while a current-head review is in flight #1634): the hold never clears early. Every merge waits the full settle window even though the reviewer already reviewed the live head, and each gate run additionally spends a gh api repos/{repo}/commits/{sha} request it did not need.

Fail-closed in direction — it holds longer, never merges sooner — but permanently slower, and silently so.

Verified by probe: the same review on the live head, with only the API-reported author type differing.

• type: "Bot" → reviewSettle.state: reviewed, ready: true
• type: "User" → reviewSettle.state: settling, ready: false

Why this was not fixed in #1634

It is not a defect that PR introduced — the constraint is pre-existing in the shared module, and the reviewer that motivated #1629 (chatgpt-codex-connector) is genuinely Bot-typed, so the shipped feature works for it. Fixing it means changing a predicate shared with the review-trigger flow, which is a wider blast radius than that PR's scope. #1634 documents the limitation at the hold's safety.md section rather than leaving it implicit, and points here.

Suggested direction (not prescriptive)

Login-only matching is defensible for this specific purpose: the reviewer login set is operator configuration, a GitHub login is unique, and a PR author cannot author a review as someone else. So the authoritative-Bot test buys little here that the configured allowlist does not already provide.

Options, for whoever takes it:

1. Thread an explicit allowance (e.g. require_bot_type: bool = True) through is_review_bot_item and fetch_review_evidence, and have the settle hold pass it as False. Narrow; leaves the trigger flow's behavior untouched unless it opts in too.
2. Have ReviewTriggerConfig consult extra_bot_logins the way the rest of the plugin does, so a registered non-structural bot account is classified consistently across every consumer. Wider, and arguably the real fix — the inconsistency, not the strictness, is the defect.
3. Leave the strictness and document it as intended. Cheapest, but it means the --extra-bot-logins seam does not reach two consumers that look like it should.

Whoever takes this should decide whether the review-trigger flow wants the same widening; changing it for one consumer and not the other reintroduces the inconsistency at a different seam.

Acceptance criteria

• A configured reviewer that GitHub types as User is recognized as having reviewed the live head, in whichever consumers the chosen option covers.
• Regression tests with "type": "User" for each covered consumer.
• If the review-trigger flow is deliberately left strict, that asymmetry is recorded at both sites with the reason.
• reference/safety.md's Review-Settle Hold section drops its limitation note and its pointer here.

Related

#1634 (the PR that surfaced it), #1629 (the hold), #1614 (the adjudication that produced both).

[kyle-sexton]

🤖 Claimed: implementing in branch fix/1642-extra-bot-logins-review-evidence.

[kyle-sexton]

🤖 Wind-down (session usage limit). No code landed — branch fix/1642-extra-bot-logins-review-evidence was pushed but sits at origin/main with zero commits; analysis and design are complete and recorded here for a cold resume.

Coordination finding: PR #1634 is still open, so the review-settle hold does not exist on main. The acceptance criterion "safety.md's Review-Settle Hold section drops its limitation note" is unsatisfiable against origin/main — implement the shared-module fix against main and let the hold pick it up when #1634 merges (recommended), or rebase onto #1634.

Verified design (from full reads of babysit_review_trigger.py, pr_queue_snapshot.py, request_review.py, babysit_merge.py, babysit_classify.py):

• ReviewTriggerConfig gains extra_bot_logins: frozenset[str] = field(default_factory=frozenset).
• is_review_bot_item (babysit_review_trigger.py:69-81) becomes: authoritative Bot-typed OR normalized login in normalize_login_set(config.extra_bot_logins), AND login in reviewer_logins — mirrors actor_kind/is_bot in babysit_classify.py; Bot-typed path byte-for-byte unchanged. All consumers (fetch_review_evidence, has_current_head_review, fetch_review_reactions) route through this one predicate.
• Config threading: pr_queue_snapshot.py::build_config (~line 80) already parses --extra-bot-logins into FeedbackConfig — add the same _csv(...) to its ReviewTriggerConfig(...); request_review.py::build_trigger_config (line 52) plus a new --extra-bot-logins CLI flag there.
• Tests to add: User-typed configured login accepted as evidence; unconfigured User-typed login still refused. Locate the review-trigger test file in scripts/tests/ first (test_review_trigger_race.py / test_babysit_delta.py adjacent); read test_skill_contract.py before touching pinned prose; no net lines to SKILL.md (chore(source-control): babysit-prs/SKILL.md also sits 1 line under the 500-line hard cap #1626 cap).
• Gates at resume: Python suite, scripts/engine.test.sh, check-changed-skills.sh origin/main, markdownlint.

[kyle-sexton]

[kyle-sexton]

🤖 Handoff: #1703 opens with the shared-predicate fix (option 2), independently reviewed (APPROVE WITH FINDINGS, all fixed); AC4 deferred — safety.md's Review-Settle Hold section ships with #1634, not on main.