fix(babysit-prs): merge gate is blind to an in-flight current-head review

[kyle-sexton]

🤖 Agent-authored — filed by the adversarial adjudication of #1614, from a code finding that neither the issue nor its synthesis had reached.

The merge gate can pass while the configured review bot is mid-review of the current head. Nothing in the gate asks whether the reviewer has reviewed this head, and the module that would answer cannot be configured for a reviewer that posts a review instead of a commit status.

The gap, in three parts

1. The merge gate never receives the reviewer configuration. babysit_review_bot_logins reaches exactly two consumers:

• plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py:82
• plugins/source-control/skills/babysit-prs/scripts/request_review.py:55

babysit_merge.py is not one of them. It fetches reviews, review threads, and the branch rules — including required_review_thread_resolution — but it has no notion of a configured reviewer, so it cannot ask the one question that matters here: has that reviewer reviewed the head I am about to merge?

2. The reviewer-awareness module is all-or-nothing and requires a commit-status context. babysit_review_trigger.py:51-53:

@property
def configured(self) -> bool:
    return bool(self.trigger_phrase and self.reviewer_logins and self.gate_context)

and review_gate_state() (same file, line 243) derives every signal — gate_state, workflow_state, request_signal_pending — by matching gate_context against StatusContext / CheckRun entries in the check rollup. Its own docstring: "With no configured contexts nothing matches, so every summarized state degrades to absent … the dormant default."

3. This reviewer publishes no such context. chatgpt-codex-connector posts a pull-request review. It creates no commit status and no check run, so there is no gate_context value that could be configured. The module is therefore not merely unconfigured here — it is unconfigurable for this reviewer, and reviewer_logins alone cannot activate it because configured demands all three slots.

Net effect: for a reviewer of this shape, no pending-engagement blocker can ever fire, and the merge gate is independently blind regardless.

Observed consequence — PR #1594

Codex reviewed five successive heads. Latency from commit to posted review, every round:

Head	Committed	Reviewed	Latency
dddce901	18:43:30Z	18:48:44Z	5m14s
81e5369d	19:56:53Z	20:01:47Z	4m54s
372b9872	21:24:27Z	21:28:29Z	4m02s
3ec45192	21:34:46Z	21:39:04Z	4m18s
edd9cbfb	21:44:00Z	21:49:06Z	5m06s

Mean 4m43s, never below 4m02s. The PR merged at 21:48:40Z — 4m40s after its final commit, and 26 seconds before round 5 landed. Round 5 carried two valid findings, one of them a regression the PR itself introduced (both filed as #1613). The gate had no way to know a review was in flight, so it reported ready and the merge proceeded.

This was not a near-miss at the edge of a wide distribution. 4m40s sits below the mean of every prior observation on the same PR.

Note also that round 5 posted after head_ref_deleted. No push-side discipline can prevent a trailing round; only declining to merge inside the reviewer's latency window can.

Why configuration cannot close this

Setting babysit_review_bot_logins alone leaves ReviewTriggerConfig.configured false, so the module stays dormant. There is no gate_context to set. And even a fully configured module would only raise a snapshot blocker — babysit_merge.py would still merge on its own verdict, because it never sees the keys.

Proposed shape

A hold in the merge gate that does not depend on a status context: refuse ready when both hold —

• no submitted review or inline review comment whose own commit id equals the current head exists from any login in the configured reviewer set (the same current-head test review-trigger.md already specifies for completion: "a submitted review or inline review comment whose own commit ID equals the current head SHA and whose author has the authoritative GitHub Bot type plus an exact login from <review-bot-logins>"); and
• the current head is younger than a configured settle window.

Two conventions the design should follow rather than re-derive:

• Fail open when unconfigured. No reviewer set configured means no hold and no behavior change — mirroring how gate_context already degrades to absent. This keeps every repo that has not wired a reviewer exactly as it is today.
• Bounded, not indefinite. The hold expires with the settle window; a reviewer that never engages must not wedge the PR. review-trigger.md already takes this position for the trigger path: "If the reviewer does not engage after that one request, report it rather than retrying."

The change is strictly in the safe direction — it can only withhold a merge, never permit one the gate would otherwise refuse.

Whoever picks this up should decide the settle-window default against fresh measurements rather than inheriting the five samples above, and should confirm the fail-open-when-unconfigured reading before implementing. If either of those turns out to be a real policy call rather than a convention lookup, bounce this to status: needs-decision instead of guessing.

Acceptance criteria

• babysit_merge.py receives the configured reviewer set and holds ready per the rule above, with the settle window configurable through the existing effective-configuration seam.
• With no reviewer configured, gate behavior is byte-for-byte what it is today — covered by a test.
• A head with a current-head review from a configured reviewer merges immediately, with no settle wait.
• The hold expires with the window rather than persisting until a review arrives.
• reference/safety.md's "Two Gates, One Merge-Ready Authority" reflects the new policy blocker, since that section enumerates what the merge gate adds beyond GitHub's own mergeability.

Context

Surfaced while adjudicating #1614, which attributed the #1594 merge to an unbounded review-round loop. The round count was not the defect — every one of the 15 findings across five rounds was valid and distinct. The defect is that the merge fired inside a well-characterized reviewer latency window with no mechanism able to see it. Related: #1594 (the run), #1613 (the findings that arrived 26 seconds late).

[kyle-sexton]

🤖 Claimed by the autonomous adjudication lane (verify-1614-b), the context that filed this issue and holds the code map for it.

Implementing on fix/1629-merge-gate-review-blindness. Scope is exactly this issue's acceptance criteria — the merge gate learns the configured reviewer set and holds ready while a current-head review is plausibly in flight, fail-open when unconfigured, bounded by a settle window.

Per this issue's own instruction: if either pinned convention (fail open when unconfigured, bounded rather than indefinite) turns out to be a genuine policy call rather than a convention lookup, I stop and relabel status: needs-decision rather than guessing.

[kyle-sexton]

🤖 Claim released by the autonomous adjudication lane (verify-1614-b) — parked, not finished, on an operator wind-down order.

Status: implemented and durable, NOT merged. PR #1634 carries the work at head 9720486d: four commits, pushed, working tree clean, worktree removed, CI green at parking. Nothing is stranded on local disk.

Why it is not merged: four unresolved review threads, three of them P1. Two are fixed and awaiting an independent resolver — reference/safety.md's security/P1 escalation rule bars the authoring session from resolving its own P1 threads, and I am that session. Two more (one P1, one P2) arrived in a fourth review round during wind-down and are not fixed.

The full resume state — a per-round thread ledger, the two unfixed findings, and the design question that is now the actual blocker — is in a dedicated comment on PR #1634. A cold agent should be able to resume from that comment alone. Do not resume from this issue body; it predates three review rounds.

The short version of the blocker, because it changed shape: this is no longer "which timestamp measures head age". Round 4 showed that the review short-circuit and the age clock share one weakness — both key on the SHA, and a SHA is not an occurrence of that SHA as the PR head, so a force-push A → B → A defeats either alone. Whoever resumes should treat that as the open design question and should not inherit my round-3 rationale, part of which round 4 invalidated.

Unassigning. The issue stays open and status: ready.

Filed along the way and independent of the above: #1642 (reviewer evidence requires a Bot-typed author — pre-existing in the shared module, correctly deferred rather than fixed here).

[kyle-sexton]

Human floor: safety-sensitive / hard to reverse

This modifies the autonomous merge gate's hold logic. A defect here changes whether pull requests auto-merge without review, so it is not a safe autonomous edit.

Decision needed to unblock: Decide whether the merge lane's hold condition should widen, and define the fail-safe behavior when bot review state is unknown.

The autonomous worker lane stopped here. No code was written and no lease was taken. Re-label agent-ready once the decision is made.

Lane instance: MELO-LAP-001-worker-202607270855

[kyle-sexton]

Correction to the escalation above — this item is already implemented and awaiting review.

A concurrent lane instance escalated this issue to the human floor and stated that no code was written. That statement is not accurate on the record:

• PR fix(source-control): hold the merge gate while a current-head review is in flight #1634 — open, not a draft, and it closes this issue (Closes #1629 in its body, and GitHub resolves it as a closing reference). Its title: "fix(source-control): hold the merge gate while a current-head review is in flight." It was opened 2026-07-26 and implements exactly the hold described in the "Proposed shape" section above.

Verified again at the time of this comment: #1634 is OPEN, non-draft, and still lists this issue as a closing reference.

What this means for routing

The classification itself is sound — a change to the autonomous merge gate's hold logic is safety-sensitive and hard to reverse, and it does belong to a human. But the pending human action is a review and merge decision on PR #1634, not an issue-level design decision with nothing built yet. The two questions the escalation raises — whether the hold condition should widen, and the fail-safe when bot review state is unknown — are answerable against the diff that already exists, and the PR should be the place they are answered.

Concretely, needs-human on this issue with agent-ready removed drops it out of list-frontier --autonomous and surfaces it in the attended queue as an undecided design question. That misroutes it: the work is not waiting on a decision to start, it is waiting on review.

Recommended disposition

Restore this issue's role labels to their prior state (agent-ready, no needs-human) and let PR #1634 carry the human review gate, which already applies to it as a safety-sensitive change to the merge lane.

This lane instance deliberately did not apply the escalation labels here and did not modify this issue's labels, so the label state you see was set by the concurrent instance, not this one. Restoring it is left to the attended queue rather than resolved by a second automated edit in the opposite direction.

Lane state

No code was written by this instance, no branch or PR was opened, no labels were changed by this instance, and no lease was taken.

— instance MELO-LAP-001-worker-202607270855