Non-required failing check silently deadlocks every merge: template-drift fails on all PRs, review agent skips as ci-failing, no approval can land #1795

Story

As a maintainer whose PRs are all green on every gate that actually blocks merge,
I want a non-required failing check to stop silently withholding the review agent's approval,
so that an advisory signal cannot convert itself into an unbreakable merge block.

The deadlock, observed live 2026-09-13

Every open PR in this repo is currently unmergeable, and no gate that the ruleset enforces is red.

1. template-drift fails on every PR. scripts/template_stub_drift.sh compares blobs committed in
   petry-projects/repo-template against this repo's standards-derived baseline:

   ::error::Template stub .github/workflows/copilot-setup-steps.yml has DRIFTED (9c8f93e7146c != 77c99a427679)
   ::error::Template stub .github/workflows/sonarcloud.yml         has DRIFTED (687bf511b01f != d7aa265ffdfe)
   ::error::Template stub .github/dependabot.yml                   has DRIFTED (23d611e8f038 != 3e2daee1f7f8)
   ::warning::Template stub .github/workflows/agent-ingress.yml is MISSING from petry-projects/repo-template
   

   repo-template has not been touched since 2026-08-31 (363190b2). The template did not drift —
   the baseline moved underneath it, and nothing re-seeded it. Same cross-repo shape as the ADR-0006
   schema change on 2026-09-07, where an org-repo commit instantly red-lit every open PR here.

2. review-one-pr.sh then refuses to review. Its CI gate is all-or-nothing:

   skip: CI checks are failing — will re-evaluate after fixes are pushed
   {"decision":"skip","reason":"ci-failing"}
   

3. No approval is ever produced, and required_approving_review_count: 1 means the PR cannot merge.

4. Nobody sees it on main. lint.yml is pull_request-triggered, so template-drift never runs
   on main. Main is permanently green while every PR is blocked — the failure is invisible exactly where
   people look for repo health.

Why this is a defect and not the gate working as intended

template-drift is not a required status check. The main ruleset requires only SonarCloud,
CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems, and
duplicate-decl-gate. GitHub would merge these PRs today if an approval existed.

So the block is created entirely by the review agent treating a non-blocking check as blocking. An
advisory check has escalated itself into a hard merge gate, repo-wide, with no signal saying so.
review-one-pr.sh already reasons in exactly these terms for SKIPPED and NEUTRAL conclusions
("NEUTRAL covers informational checks that don't gate merging") — it just has no notion of
non-required as a third such category.

Acceptance Criteria

1. The immediate outage is cleared: petry-projects/repo-template is re-seeded via
   scripts/seed-repo-template.sh so template-drift passes, including the missing
   .github/workflows/agent-ingress.yml. This is cross-repo; sequence it as its own step and confirm a
   PR here goes green before closing.
2. The CI gate distinguishes required from non-required failures. A failing check that the branch
   ruleset does not require must not by itself produce reason: ci-failing. Derive the required set
   from the ruleset/branch-protection API — do not hardcode a list of check names, which would drift
   the same way the template just did.
3. The gate is not weakened for anything that does block. A failing required check still skips the
   review, unchanged. This changes which failures count, never what the gate does when one counts. The
   FORCE_REVIEW break-glass (Self-host deadlock: force_review bypasses the advisory gate but not the CI gate, so CI-gate fixes can't merge through ring-0 #619) stays exactly as-is.
4. A non-required failure is reported, not ignored. When the agent proceeds past one, it names the
   check in its review body, so "we reviewed despite a red advisory check" is visible rather than
   silent. Downgrading a signal must not delete it.
5. The invisibility is fixed too. template-drift (or an equivalent scheduled check) runs somewhere
   that is not pull_request-only, so a standards/template divergence is caught against main instead
   of surfacing as an unexplained block on every contributor's PR.
6. Covered by tests: a PR whose only failing check is non-required is reviewed; a PR with a failing
   required check is still skipped; the required set is read from the API, not a literal. Offline,
   stubbed — no network.

Dev Notes

• Do not fix this by deleting or disabling template-drift. It is doing its job — it found real
  drift ([Phase 1] Drift-guard CI: fail when template stubs diverge from standards/ #969's whole purpose). The defect is the coupling, not the check.
• Do not fix it by making template-drift a required check. That inverts the bug into a permanent
  hard block and would make this outage worse, not better.
• scripts/review-one-pr.sh lines ~121–160 hold the CI_STATUS classification and the ci-failing
  skip sentinel (exit 100). The passing set already special-cases SUCCESS, SKIPPED, NEUTRAL;
  non-required is the missing fourth case.
• Branch rules come from GET /repos/{owner}/{repo}/rules/branches/{branch} — this repo uses a
  ruleset, not classic branch protection, so /branches/{branch}/protection returns 404. Read the
  ruleset, or handle both.
• Related: Self-host deadlock: force_review bypasses the advisory gate but not the CI gate, so CI-gate fixes can't merge through ring-0 #619 (the FORCE_REVIEW break-glass this issue makes rarely necessary), [Phase 1] Drift-guard CI: fail when template stubs diverge from standards/ #969 (introduced
  template-drift), pr-review-sweep's stop condition mistakes a marker for a standing approval — a dismissed approval strands a green, auto-merge-armed PR indefinitely #1665 (the sweep's stop condition — a different way approvals failed to land).

References

• scripts/review-one-pr.sh — the ci-failing gate
• scripts/template_stub_drift.sh, scripts/seed-repo-template.sh
• .github/workflows/lint.yml — template-drift job, pull_request-only trigger
• Blocked PRs at time of filing: feat: implement issue #1778 — issue-triage applies needs-human-review as a blanket hold on every bug report — well-specified issues are parked until a human clears the label by hand #1783, feat: implement issue #1775 — Preserve the advisory when the persona post fails — a 403 must cost a retry, not the work (split from #1734 AC #4) #1788, feat: implement issue #1647 — [qa-lead S4] Make persona.yml's declared surfaces agree with what is actually deployed #1790 (all green on every required check)

Evidence added 2026-09-13 ~10:10Z: the model judges "CI green" too, so AC2 alone won't clear it

After stale cancelled checks were cleared on several PRs (09:14–10:04Z), pr-review ran; the code gate was not the blocker there, since FORCE_REVIEW=true on the sweep bypassed it. The deep-review model then escalated on template-drift itself:

• feat: implement issue #1775 — Preserve the advisory when the persona post fails — a 403 must cost a retry, not the work (split from #1734 AC #4) #1788, fix-requested (cycle 3/3), 10:00Z: "Escalating (not to Tier 3): CI is not green — the template-drift Lint check FAILED and merge is BLOCKED"
• feat: implement issue #1778 — issue-triage applies needs-human-review as a blanket hold on every bug report — well-specified issues are parked until a human clears the label by hand #1783, fix-requested (cycle 3/3), 09:57Z: logic fully verified, then "Escalating only because a gate fails"

The source is the prompts, not only review-one-pr.sh:

• prompts/deep-review.md:108, deep-review-logic.md:63, -performance.md:63, -security.md:77, -style.md:64: "Check statusCheckRollup for CI status."
• prompts/deep-review.md:197, -logic.md:136, -performance.md:125, -style.md:124: approve only if "all gates pass (CI green, issue addressed, …)"

statusCheckRollup includes non-required checks. Suggested AC addition: the harness computes the ruleset-required set once (AC2's API read) and passes it into the prompt context. Every deep-review prompt defines "CI green" as required checks passing, and lists non-required failures as informational to name in the review body (AC4), never as a reason to escalate. Test: a non-required-only failure yields approve-eligible, not escalate.

Knock-on cost, observed: dev-lead can't fix a check the PR doesn't cause, so it answers no-changes. Each round burns a review cycle, and PRs hit the 3/3 escalation cap in about 20 minutes (#1788, #1783, #1782).

⚠ AC conflict with #1791 — needs a decision before release

• This issue, AC1: re-seed repo-template "including the missing .github/workflows/agent-ingress.yml".
• template-drift fails on every PR since standards/v1-stable moved (2026-09-13 04:03Z) — repo-template carries Dependabot bumps standards/ lacks #1791, AC3: "The fix does not add .github/workflows/agent-ingress.yml to repo-template (see [Phase 3] Collapse the first repo's Class-1 event-driven stubs into agent-ingress.yml and rename the required checks #1729 AC12)", because the reference file is a live workflow (on: triggers, a dev-lead job, secrets: inherit, write perms).

These can't both hold. Decide first whether template-drift should expect agent-ingress.yml in repo-template at all during the ADR-0007 migration (#1723). Then align the other issue's AC before either is released to dev-lead.

Delivery brief — 2026-09-19 (read this first; implement it, do not re-ask)

Scope for this PR is AC #2, #3, #4 and #5 only. AC #1 (re-seed repo-template) is explicitly OUT OF SCOPE — do not run scripts/seed-repo-template.sh. #1812 is still open: the seeder destroys files it does not own (it wiped repo-template's 420-line managed .gitignore and overwrote the drift-allowlisted ci.yml). Running it would trade this outage for a worse one. Leave AC #1 to #1812 and say so in the PR description.

The deadlock is still live, re-measured 2026-09-19

Four PRs (#1783, #1790, #1814, #1815) have every required check green — SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems, duplicate-decl-gate — and template-drift red. A forced review run (35441769067) produced exactly this:

skip: CI checks are failing — will re-evaluate after fixes are pushed
{"pr":".../pull/1783","decision":"skip","reason":"ci-failing", ...}
Summary: 0 reviews posted, 1 no-ops skipped, 0 failures (1 deferred pending CI/checks)

template-drift is not in the ruleset's required set. A purely advisory check is holding every merge in the repo.

What to implement

1. compute_ci_status (scripts/review-one-pr.sh, called at line ~176) must ignore failures of checks the branch ruleset does not require. A red non-required check alone must not yield ci-failing.
2. Read the required set from the API — never hardcode check names. This repo uses a ruleset, so GET /repos/{owner}/{repo}/rules/branches/{branch} is the source; /branches/{branch}/protection returns 404 here. Today that set is exactly the five named above. Hardcoding would drift the same way the template baseline just did — that is the bug, not the fix.
3. Fail closed on an unreadable required set. If the ruleset API call fails or returns nothing, treat every failing check as blocking (today's behaviour). An API hiccup must never silently widen what gets approved — same reasoning as the router's unreadable-label guard.
4. Do not weaken anything that genuinely blocks. A failing required check still skips with ci-failing. A failing non-required check must still be visible: log it and name it in the review body, so "ignored" never means "unmentioned".
5. Keep ci-pending behaviour as it is. The bounded deferral from [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 #1427 is separate; do not fold it into this change.

Tests (bats, in the existing suite)

• a red non-required check with all required green → reviews (no ci-failing skip)
• a red required check → still skips with ci-failing
• ruleset API unreadable → fails closed, skips
• the non-required failure is named in the output

Budget guard

1. The gate change plus its tests. This is the commit that matters — a green partial PR beats a timed-out run.
2. Logging/surfacing polish and the PR-description note that AC test issue from agent #1 stays with seed-repo-template.sh destroys files it does not own: wipes the 420-line managed secrets .gitignore baseline and overwrites the drift-allowlisted ci.yml #1812.

Stop after step 1 if budget runs low and list the rest as a follow-up.