Skip to content

fix(source-control): reconcile review-trigger 'pending or failing' gate contract with pending-only code #324

Description

@kyle-sexton

Problem

The AI-review-trigger contract and its implementation disagree on whether a failing
<review-gate-context> (not just a pending one) should make a PR a review-trigger candidate.

plugins/source-control/skills/babysit-prs/reference/review-trigger.md (line 58) documents the
trigger signal as:

the explicit pending engagement signal — <review-gate-context> pending or failing while
no current-head review from the configured reviewer exists …

But the code accepts only pending at three sites:

  • scripts/babysit_review_trigger.py:387 — candidate predicate: check_state["gate_state"] == "pending"
  • scripts/babysit_review_trigger.py:388request_signal_pending is derived only from a
    StatusContext whose effective_state == "PENDING" (see review_gate_state, ~line 288)
  • scripts/babysit_delta.py:500review_trigger["gate_state"] == "pending" gates the
    "awaiting requested review" material signal

summarized_state (scripts/babysit_checks.py:87) returns "failing" for a failing gate
StatusContext, so a review gate that reports "missing reviewer" as a failing status never
becomes a candidate and request_review.py never posts the trigger — even after the anti-flap
window. Reported by Codex on #322 (P2, inline on babysit_review_trigger.py).

The decision to make (do not resolve by inference)

This is a genuine design fork, not an obvious bug:

  • Honor the doc (expand the code): treat a failing engagement-gate (with no current-head
    review) as a trigger candidate at all three sites, and define an analogue of
    request_signal_pending for the failing case. Correct iff the gate context a real
    deployment wires reports FAILING (not PENDING) on "no reviewer engaged yet".
  • Honor the code (narrow the doc): babysit_review_trigger.py:437-440 already surfaces
    failing as a distinct reported state (the else branch passes it through, unlike the
    successgate_success_unverified branch), which suggests excluding failing from
    candidacy may be deliberate — in which case review-trigger.md:58's "or failing" is the
    error and should be narrowed to "pending".

Do not silently edit the doc to match the code — that would erase authored intent to hide the
gap. The tiebreaker is empirical: does the review-gate-context that the Phase-3 cutover config
wires report FAILING or PENDING when no reviewer has engaged?
The dotfiles skill's current gate
configuration is the fact that settles it.

Acceptance

  • Decide honor-doc vs honor-code, with the tiebreaker evidence recorded.
  • Make all three sites (+ request_signal_pending) and review-trigger.md:58 mutually
    consistent.
  • Add a regression test for the chosen failing-gate behavior in test_review_trigger_race.py.

Related

Work-class: C3 (bug-fix-shaped) — attended triage 2026-07-23, operator-ratified. 🤖

Metadata

Metadata

Assignees

Labels

agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions