[Phase 4] pr-review defers indefinitely on another agent's own check run (dev-lead/dispatch) and on zombie checks — ci-pending has no timeout

[don-petry]

Story

As a maintainer waiting for pr-review to approve a PR whose tests are green,
I want the "CI still pending" deferral to ignore other agents' own check runs and to time out on a check that never completes,
so that ordinary dev-lead activity — or a single stuck check — cannot defer the review indefinitely.

Problem — two ways a PR gets stuck at ci-pending forever

scripts/review-one-pr.sh skips with reason:"ci-pending" whenever compute_ci_status sees a non-completed check, posting a <!-- pr-review-agent ci-pending-ack --> comment that says "re-mention @donpetry-bot once they complete." scripts/lib/ci-status.sh deliberately filters the cascade's own check runs (review, review / review, PR Review*) so pr-review never blocks on itself (#469). It does not filter any other agent's checks, and the pending state has no upper bound — a FORCE_REVIEW mention polls only 10×30s before giving up.

Case A — dev-lead's own check defers pr-review (agent-vs-agent interference)

Observed on PR #1420 at 07:48Z: 57 checks COMPLETED, all tests green (bats, unit, unit-tests all SUCCESS), zero unresolved threads, zero genuine failures — and one check IN_PROGRESS: dev-lead / dispatch, started 07:39:01Z. pr-review posted ci-pending-ack at 07:45:08Z and declined to review.

The loop is self-sustaining, and it is easy to enter by trying to help:

1. Someone comments on the PR (including "@donpetry-bot review", the remedy the ack itself recommends)
2. The comment is an issue_comment event → dev-lead fires → dev-lead / dispatch check goes IN_PROGRESS
3. pr-review sees a pending check → defers → posts another ci-pending-ack
4. Because dev-lead's concurrency is cancel-in-progress: false, queued runs keep at least one dispatch check pending across the window, so step 3 repeats

Every attempt to re-engage the reviewer re-arms the condition that makes it defer. During the observed window, two mention-driven attempts to obtain an approval both ended in ci-pending-ack rather than a review.

Case B — a zombie check never completes

Observed on PR #1426 at 07:48Z: Validate AW specs, docs, and workflows reports status: IN_PROGRESS with conclusion: SUCCESS, started 06:57:49Z — inconsistent, and ~50 minutes stale. A check in this state will not transition, so the ci-pending skip is permanent: pr-review will defer on every future trigger, forever, with no timeout to rescue it.

Both PRs were otherwise fully merge-ready. Nothing has merged since 05:49Z.

Acceptance Criteria

1. compute_ci_status excludes other first-party agent check runs from the pending calculation — at minimum dev-lead / *, generalised from the existing self-filter rather than hard-coded per agent. A writing agent's own job must not gate the reviewing agent.
2. The exclusion is derived from a declared source (the interaction contracts from [Phase 2] Author a machine-readable interaction contract for every agentic role, per the standard #1404 name each role's workflows), not a second hand-maintained name list that can drift from the first.
3. The ci-pending deferral is bounded: after a configurable age, pr-review either proceeds on the completed checks or escalates once, rather than deferring indefinitely. A check that is IN_PROGRESS while carrying a terminal conclusion is treated as complete.
4. The bound is fail-safe, not fail-open: proceeding past a genuinely pending required check must remain impossible — the timeout applies to the review decision, not to the merge gate, which the rulesets still enforce independently.
5. The ci-pending-ack comment stops recommending a remedy that re-triggers the condition. Either the ack is dropped for the self-inflicted case, or its text stops advising a re-mention when the only pending check belongs to another agent.
6. Tests cover: pending dev-lead check + otherwise-green PR → review proceeds; zombie check (IN_PROGRESS + conclusion) → treated as complete; a genuinely pending required check → still defers; the deferral timeout path.

Tasks / Subtasks

• Generalise the ci-status.sh self-filter into an agent-check filter sourced from the interaction contracts. (AC: test issue from agent #1, Go-live improvements for PR review agent #2)
• Add the bounded-deferral rule and the terminal-conclusion-wins rule to compute_ci_status / review-one-pr.sh. (AC: Add @claude delegation, auto-merge, and rebase handling #3, Optimize review: small-PR and incremental fast paths #4)
• Revise the ci-pending-ack copy so it never recommends an action that re-arms the block. (AC: feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
• Bats coverage for all four cases, using the feat: implement issue #1410 — [Phase 4] Add a stall canary + rollback trigger for the Class-2 timer changes, before narrowing them #1420 and feat: implement issue #1409 — [Phase 5] Wire the standard into AGENTS.md and per-agent docs, and reconcile the ci-failure-analyst spec vs deployed divergence #1426 states as fixtures. (AC: feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6)

Dev Notes

• The existing self-filter in scripts/lib/ci-status.sh (pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs) #469) is the precedent and the right shape — this story widens its scope rather than inventing a mechanism. Note it also already treats CANCELLED as non-blocking (pr-review CI gate should gate on REQUIRED checks only — non-required CANCELLED *and* FAILED checks falsely block reviews #608), which is the same class of judgement.
• Do not fix Case A by giving dev-lead cancel-in-progress: true. That is what pr-review: per-PR cancel-in-progress cancels racing runs (concurrency thrash) #534 fixed for pr-review and it kills in-flight work; the queueing is deliberate. The fix belongs in what pr-review counts, not in how dev-lead queues.
• Case B is a GitHub-side inconsistency we cannot prevent, only tolerate — hence AC Add @claude delegation, auto-merge, and rebase handling #3's "terminal conclusion wins" rule. Treating a check with a conclusion as complete is safe: a conclusion is by definition terminal.
• Interaction with [Phase 4] pr-review runs are ~49% cancelled-while-pending — cancel-in-progress:false protects in-flight only, and cancelled renders as a failing check #1422: that issue covers pr-review runs cancelled while pending; this one covers pr-review runs that complete but decline to review. Both end with "no approval lands", and both feed the approve→push→dismiss cycle, but the mechanisms and fixes are separate.
• The observed instances are clean fixtures: feat: implement issue #1410 — [Phase 4] Add a stall canary + rollback trigger for the Class-2 timer changes, before narrowing them #1420 — 57 COMPLETED / 1 IN_PROGRESS (dev-lead / dispatch, started 07:39:01Z), all tests green; feat: implement issue #1409 — [Phase 5] Wire the standard into AGENTS.md and per-agent docs, and reconcile the ci-failure-analyst spec vs deployed divergence #1426 — Validate AW specs, docs, and workflows with status: IN_PROGRESS + conclusion: SUCCESS, started 06:57:49Z.

Project Structure Notes

scripts/lib/ci-status.sh (the filter and the pending calculation), scripts/review-one-pr.sh (the skip path and the ack copy); tests under tests/, registered in lint.yml's bats list.

References

• scripts/lib/ci-status.sh (compute_ci_status; the pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs) #469 self-filter; the pr-review CI gate should gate on REQUIRED checks only — non-required CANCELLED *and* FAILED checks falsely block reviews #608 CANCELLED rule)
• scripts/review-one-pr.sh (ci-pending skip, the ci-pending-ack comment, the 10×30s FORCE_REVIEW poll)
• .github/workflows/dev-lead-reusable.yml (concurrency cancel-in-progress: false)
• [Phase 4] pr-review runs are ~49% cancelled-while-pending — cancel-in-progress:false protects in-flight only, and cancelled renders as a failing check #1422 (cancelled-while-pending review runs), [Phase 4] A review bot that can create blocking threads but is not in dev-lead's TRUSTED_BOTS creates an unclearable merge block (graphite-app; root cause of the 2026-08-02 deadlock) #1425 (unclearable third-party threads) — sibling causes of "no approval lands"
• Observed on PR feat: implement issue #1410 — [Phase 4] Add a stall canary + rollback trigger for the Class-2 timer changes, before narrowing them #1420 and PR feat: implement issue #1409 — [Phase 5] Wire the standard into AGENTS.md and per-agent docs, and reconcile the ci-failure-analyst spec vs deployed divergence #1426, 2026-08-02 07:31–07:48Z

Likely target surface

• scripts/lib/ci-status.sh, scripts/review-one-pr.sh

---

Filed from overnight monitoring of epic #1402. Found while diagnosing why two fully-green PRs would not attract a code-owner approval: the reviewer was deferring on the writing agent's own check run, and each attempt to re-engage it re-armed that deferral.

[github-actions]

Thanks for the exceptionally thorough write-up — the two observed fixture PRs, the self-reinforcing loop diagram, and the zombie-check edge case make the failure modes very clear. Before we move to implementation, one question on AC #2: the interaction contracts from #1404 are listed as the intended source for the agent-check filter — are those contracts currently stored in a machine-readable format (e.g., structured YAML/JSON) that ci-status.sh can parse directly, or would sourcing from them require reading prose documentation first? That'll determine whether the generalised filter can be data-driven from day one or needs an intermediate step.

[don-petry]

Trigger-source data, and a hypothesis I checked and discarded

Discarded: a check_run self-trigger loop. I suspected dev-lead was re-triggering itself via its own dev-lead / * check runs (it creates four on a PR and subscribes to check_run). It is not that. The ci-relay job's guard requires conclusion == 'failure' and !startsWith(name, 'dev-lead / ') (.github/workflows/dev-lead-reusable.yml:424-428), and dev-lead's own checks complete skipped/success — so they cannot relay. 8 check_run-triggered runs in an hour on a PR carrying 57+ checks is ordinary. Recording the negative result so nobody re-derives it.

What the data does show. dev-lead runs on PR #1420 in the 07:20–08:17 window, by trigger:

event	runs
issue_comment	10
check_run	8
pull_request_review	2
pull_request	2
issues	1
total	23

There were 6 comments on the PR in that window. So comments are the single largest trigger source, and the reviewer's own activity is the dominant contributor — my two @donpetry-bot review mentions plus dev-lead's own marker comments each start a dev-lead run. Note a run starts before the skip decision is made: dev-lead-intent.sh skips comments authored by BOT_USER (:376-377), but only after the workflow has already begun and created its dev-lead / dispatch check.

Why that matters for this issue. Each of those runs holds a dev-lead / dispatch check IN_PROGRESS, and per AC #1 that is what defers pr-review. So the failure mode is self-reinforcing through ordinary review traffic rather than through any loop: commenting on a PR to ask for a review is itself the thing that defers the review. On #1420 the two mention attempts both ended in ci-pending-ack instead of a review, and donpetry-bot has still never reviewed that PR.

Immediate mitigation, no code required: stop commenting and let the dev-lead queue drain, at which point no dev-lead check is pending and pr-review can proceed. I am doing that now for #1420 and #1426. That is a workaround, not a fix — AC #1 remains the fix, because "reviewers must stay silent for the reviewer to work" is not a usable contract.

---

Generated with Claude Code

[don-petry]

Mitigation confirmed empirically — going quiet unblocked both PRs

Recording the result, because it is the strongest evidence yet for AC #1.

At 08:17Z both #1420 and #1426 had been stuck for hours: fully merge-ready (green tests, zero unresolved threads, zero genuine failures) but each holding one non-completed check — dev-lead / dispatch on #1420, the zombie Validate AW specs on #1426 — so pr-review kept answering with ci-pending-ack instead of reviewing. donpetry-bot had never reviewed #1420 across roughly three hours and two mention attempts.

I stopped commenting on both. Result:

PR	outcome
#1420	merged 08:32:02Z (~15 min after the last comment)
#1426	approved 08:39:57Z, merged 08:39:59Z (~22 min)

Nothing else changed — no code push, no configuration change, no human intervention. The only variable was the absence of new comments, which let the dev-lead queue drain so that no dev-lead check was pending, which let pr-review proceed.

This confirms the diagnosis and inverts the ergonomics. The remedy the ci-pending-ack itself recommends — "re-mention @donpetry-bot" — is counter-productive whenever the pending check belongs to dev-lead: the mention starts a dev-lead run whose dispatch check is pending, which is precisely what defers the review. Silence is the effective remedy and the ack advises the opposite. That is AC #5's concern, now demonstrated rather than argued.

It also means the failure is self-limiting on a quiet PR and self-sustaining on an active one — it bites hardest exactly when a PR is under active review, which is when it matters most.

[don-petry]

Maintainer decision — full fix: exclude agent checks + bounded deferral

Scope confirmed by the maintainer. Implement the complete AC set:

• AC test issue from agent #1/Go-live improvements for PR review agent #2 — compute_ci_status excludes other first-party agent check runs, with the exclusion sourced from the [Phase 2] Author a machine-readable interaction contract for every agentic role, per the standard #1404 interaction contracts (each role declares its workflows) rather than a fourth hand-maintained name list. Generalises the existing pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs) #469 self-filter instead of duplicating it.
• AC Add @claude delegation, auto-merge, and rebase handling #3 — bounded deferral: after a configurable age pr-review proceeds on the completed checks or escalates once, and a check that is IN_PROGRESS while carrying a terminal conclusion is treated as complete (the zombie case, observed 80 minutes stale on feat: implement issue #1409 — [Phase 5] Wire the standard into AGENTS.md and per-agent docs, and reconcile the ci-failure-analyst spec vs deployed divergence #1426).
• AC Optimize review: small-PR and incremental fast paths #4 — fail-safe, not fail-open: the timeout governs the review decision only. A genuinely pending required check must still block the merge, which the rulesets enforce independently.
• AC feat: add Copilot engine support via REVIEW_ENGINE toggle #5 — the ci-pending-ack stops advising a re-mention when the pending check belongs to another agent. This is the trap that cost roughly three hours on feat: implement issue #1410 — [Phase 4] Add a stall canary + rollback trigger for the Class-2 timer changes, before narrowing them #1420: the ack's own advice re-arms the deferral.
• AC feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6 — tests for all four cases, using the feat: implement issue #1410 — [Phase 4] Add a stall canary + rollback trigger for the Class-2 timer changes, before narrowing them #1420 and feat: implement issue #1409 — [Phase 5] Wire the standard into AGENTS.md and per-agent docs, and reconcile the ci-failure-analyst spec vs deployed divergence #1426 states as fixtures.

The empirical confirmation is already on this issue: going silent merged #1420 in ~15 min and #1426 in ~22 min with no other change. That is the behaviour the fix must make unnecessary — "the reviewer must stay silent for the reviewer to work" is not a usable contract.

Unblocking for implementation.

[don-petry]

Additional AC — a non-required check failure silently halts all review

Same file, same class of defect as this issue's pending-deferral, so folding it in rather than filing separately.

Observed now on PR #1434. It has no pending checks and zero unresolved threads, yet donpetry-bot has never reviewed it. The cause is one failing check — template-drift — which is:

• not a required check (main's rulesets require only SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems), and
• not caused by this repo at all — it reports copilot-setup-steps.yml and sonarcloud.yml as DRIFTED in the repo-template repo, files no epic PR touches. It began failing ~09:54Z and has failed on every PR since. feat: implement issue #1415 — [Phase 4] Close the maintainer review-gate gap — dev-lead PRs cannot be blocked by the owner (shared identity + agent-resolved threads) #1433 merged with it red, confirming it does not gate the merge.

compute_ci_status (scripts/lib/ci-status.sh:20,85) returns failing for anything that is not SUCCESS / SKIPPED / CANCELLED, with no notion of required. review-one-pr.sh:141 then skips — and that skip is silent: it writes only to the run log, posting nothing on the PR. So an unrelated external drift stops every review in the repo with no signal anywhere the reader will look.

This is the same failure shape as the pending-check deferral: pr-review declines to act on a signal that should not gate it, and says nothing.

Proposed AC #7

7. A non-required failing check does not by itself prevent a review. compute_ci_status distinguishes required from non-required checks (the ruleset's required set is queryable), and pr-review gates only on the required ones. Where a non-required failure does cause a skip, the skip is visible — one deduped marker naming the check — rather than silent, so a reader can tell why nothing is happening.

Note the existing break-glass (FORCE_REVIEW bypasses ci-failing, #619) is documented for exactly this case — "a fix to the CI gate itself can be reviewed" — but it requires a human mention, and per this issue's own findings a mention re-triggers dev-lead and can re-arm the pending deferral. The break-glass should be the exception, not the routine remedy for an unrelated red check.

Separate follow-up, outside this epic: the repo-template drift itself needs a re-seed via scripts/seed-repo-template.sh. That is maintenance in another repo, not an interaction-model defect.

[don-petry]

The cause of the template-drift failure cited in AC #7 above is now filed separately as #1435: Dependabot is bumping action pins inside repo-template, a byte-identity distribution artifact, so the shipped files drift from the standards-derived baseline by construction (the 09:49Z actions/checkout 6.0.2→7.0.1 bump landed five minutes before drift began failing; standards pins v7.0.0).

The two issues are complementary and both wanted: #1435 stops the drift being generated; AC #7 here stops a non-required red check from silently halting every review. Either alone leaves half the failure — a different non-required check will eventually go red for an unrelated reason, and without AC #7 the same silent halt recurs.

[don-petry]

The #619 break-glass did not bypass ci-failing when dispatched — observed, mechanism unconfirmed

Relevant to AC #7. I tried to review PR #1434 through the documented break-glass and it did not work.

What I ran:

gh workflow run pr-review-trigger.yml \
  -f pr_url=https://github.com/petry-projects/.github-private/pull/1434 \
  -f dry_run=false -f force_review=true

What the run decided (run 30749802155, workflow_dispatch, conclusion success):

review decision: REVIEW_REQUIRED
skip: CI checks are failing — will re-evaluate after fixes are pushed
{"pr":".../pull/1434","sha":"c2c43a2b…","decision":"skip","reason":"ci-failing"}

That is the else branch at scripts/review-one-pr.sh:156 — the path taken when FORCE_REVIEW is not true. The force-review branch (:142-154) emits a distinctive ::warning::force-review: CI is failing but FORCE_REVIEW=true …, which does not appear anywhere in the run log.

What I checked and could not fault:

• pr-review-trigger.yml declares the force_review dispatch input and forwards it: force_review: ${{ inputs.force_review || '' }}.
• The reusable declares force_review as a workflow_call input.
• The env wiring is identical at main and at the pinned channel @pr-review/next (ded84ce4820d): FORCE_REVIEW: ${{ github.event_name == 'repository_dispatch' && 'true' || inputs.force_review || 'false' }}.

So the plumbing reads correctly end-to-end, yet the script behaved as though FORCE_REVIEW=false. I am not asserting a mechanism — I could not confirm the value inside the run before hitting an API error, and I have been wrong twice tonight when I inferred a mechanism without proof. Recording it as a reproducible observation for whoever picks up this issue.

Why it matters here. AC #7 argues that a non-required red check should not silently halt review, and notes the break-glass exists for exactly this case. If the break-glass does not work from workflow_dispatch, then there is currently no working escape from a ci-failing skip other than making the unrelated check green — which is precisely the situation that has held this repo at zero merges since 09:55Z. That strengthens AC #7 from "nice to have" to "the only remaining lever".

Worth a targeted reproduction: dispatch with force_review=true against any PR with a red non-required check and confirm whether the force-review warning appears.

[don-petry]

Dev-Lead Implementation Plan

Issue: #1427 — [Phase 4] pr-review defers indefinitely on another agent's own check run (dev-lead/dispatch) and on zombie checks — ci-pending has no timeout

Scope

Stop compute_ci_status from gating pr-review on another first-party agent's own orchestration check runs (sourced from the interaction-contracts/), treat a check carrying a terminal conclusion as complete, and bound the ci-pending deferral so a genuinely stuck check can no longer defer the review forever.

Implementation Checklist

• scripts/lib/ci-status.sh: derive first-party agent role slugs from interaction-contracts/*.yml; add an is_agent_check filter (generalised from the pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs) #469 self-filter) applied in compute_ci_status and classify_rollup_eventability (AC test issue from agent #1/Go-live improvements for PR review agent #2)
• scripts/lib/ci-status.sh: is_pending — a check carrying a terminal conclusion is never pending (zombie/terminal-conclusion-wins, AC Add @claude delegation, auto-merge, and rebase handling #3)
• scripts/lib/ci-status.sh: add ci_pending_age_exceeded — true only when every remaining pending external check is older than a configurable age; unknown age is fail-safe not-exceeded (AC Add @claude delegation, auto-merge, and rebase handling #3/Optimize review: small-PR and incremental fast paths #4)
• scripts/review-one-pr.sh: bounded-deferral — when the pending state is stuck past CI_PENDING_MAX_AGE_SEC, proceed on the completed checks (review decision only; merge gate still enforced by the ruleset) and escalate once via a deduped comment (AC Add @claude delegation, auto-merge, and rebase handling #3/Optimize review: small-PR and incremental fast paths #4)
• scripts/review-one-pr.sh: revise the ci-pending-ack copy so it no longer recommends a re-mention that re-arms the block (AC feat: add Copilot engine support via REVIEW_ENGINE toggle #5)

Tests to Write

• tests/test_ci_status.bats: dev-lead / dispatch (IN_PROGRESS) + otherwise-green → passing (filtered); zombie (IN_PROGRESS + conclusion: SUCCESS) → treated complete; genuine external pending required check → still pending; ci_pending_age_exceeded true/false paths (update the now-outdated "dev-lead NOT filtered" cases)
• tests/test_review_one_pr_ci_pending.bats (or a new bats): stuck pending → review proceeds (no ci-pending skip); ack copy contains no re-mention advice

Test Command

bats tests/test_ci_status.bats tests/test_review_one_pr_ci_pending.bats and bash .dev-lead/scripts/dev-lead-lint.sh