Fix: Diagnose merge failures from live state, not the rejection

[ModeSevenIndustrialSolutions]

Closes #482.

A 157-PR owner-wide run reported 17 failures. Sampling four showed the stated cause was wrong in every case, and two were fully mergeable by the time the run printed them as failed.

A merge rejection is a snapshot. GitHub states what was unsatisfied at the instant the merge was attempted, which routinely names required checks that had merely not finished — and nothing re-read that before the summary printed.

PR	Reported as	Actually
test-python-project#396	Required workflows failed: Semantic Pull Request 🛠️	clean — nothing wrong with it
docker-workflows#70	Required workflows failed: Semantic Pull Request 🛠️	clean — judged before its rebase landed
python-workflows#84	3 required workflows not satisfied	All three pass; pre-commit.ci - pr was the blocker
workflows-template#55	AI Slop Scan 🧹, Zizmor Scan 🌈	pre-commit.ci was the blocker

All four are fixtures in tests/test_merge_failure_diagnosis.py.

---

What changes

1. A status check no longer hides behind a workflow

_format_failure_reason returned on the first group it found, so a rejection naming both showed only the workflows. Each kind is now listed, with its own verb read from its own clause — workflows that have not finished routinely accompany a context that has already failed.

2. Unfinished is not failed

A PR GitHub would merge now is recorded as UNSETTLED and counted apart from FAILED: one needs another run, the other needs a human. The confirmation step already re-read every reported failure, and the payload it fetches carries mergeable_state, so the reconciliation itself costs no extra request.

Three guards, each earned during review:

• Only a state-based refusal may be withdrawn. The run also reports FAILED for its own troubles — an exception, a rebase that did not complete, a 502, a missing token scope. Decided from the HTTP status by allowlist (405, 409), so an unrecognised status, or an exception carrying none, keeps its message rather than losing it.
• Only affirmative mergeability withdraws, matching _state_is_waitable. A closed PR never qualifies.
• Re-judged after every task finishes, not only when its own does. In an owner-wide run a PR refused at minute two can go green at minute ten; the earlier check alone could not see that. This was the issue's acceptance criterion.

3. Name what is blocking, without over-claiming

Read against the live head — a dependabot rebase moves it, and this tool requests those rebases. Three sources, weighted by what they prove:

Source	Carries	Proven blocker when…
commit status contexts	context name	the branch requires it
Actions workflow runs	workflow name:	GitHub quoted it as required
check runs	job name	never — reported as failing, not blamed

codeql.yml declares the workflow CodeQL and the job Audit Repository, so matching a rejection against check-run names compares two vocabularies that need not agree.

All three paginate; each reports whether it answered; a proven blocker stands on its own evidence, an unproven one requires every probe to have answered. Everything failing is named exactly once.

4. Retry a run that never reached a verdict

error is pre-commit.ci failing to complete, failure is the hooks reporting a real problem — only the first is retried. The repair gate covers unknown as well as blocked, though a missing status still needs a settled PR. Suppression is scoped to the incident by timestamp, and the poll ignores the pre-trigger reading — otherwise the first poll returns the very error being retried.

---

Corrections to the issue

Recorded in-thread:

• Cause 1 is smaller than described — the reconciliation hook already existed; it asked the wrong question, and ran at the wrong time.
• Cause 3's second half names the wrong state — blocked covers pending required checks; unknown was the gap.
• Criterion 6 needed no change — _pending_age already uses the status's own timestamp.
• Cause 4 has a naming trap — MergeStatus.PENDING is the initial non-terminal state.

Scope

Fifteen Copilot rounds produced 36 findings. Many were genuine defects in code this branch added while fixing an earlier one — the stale head, the over-broad refusal classification, the unpaginated status read (that endpoint defaults to 30), and the end-of-run reconciulation. Those are all here.

A cluster of hardening was implemented and then deliberately removed, about 680 lines. It addressed inputs that need a rule name containing our delimiters, a workflow name containing a comma, two workflow files sharing a name:, an app-pinned requirement, or more than a thousand status contexts. The machinery and its tests cost more than the failures they prevent are worth, and none of it reaches the reported cause — which a blocked PR now reads from live API state rather than from the rejection string. rule_violations.py is consequently left close to its original shape (+59 lines, against +171 at the peak).

Also declined, with reasons: rulesets are not enumerated to resolve required workflow names (a ruleset names a workflow by file path while a run carries its name:, and GET /orgs/{org}/rulesets returned 403 throughout the 503-PR run in docs/BULK_RUN_PERFORMANCE_AUDIT.md); stale is not treated as failing (GitHub applies it to a run a later push made irrelevant).

One review finding is correct but pre-existing and out of scope — required checks from rulesets and branch protection are cumulative, and only one is consulted. Filed as #494 rather than folded in: it touches four callers, costs a request per repo/branch, and tightens reliable for all of them.

Validation

Check	Result
pytest	2438 passed, 20 skipped (baseline 2317)
basedpyright --warnings src tests	0 errors, 0 warnings
mypy src tests	Success, 283 files
ruff check / format --check	All checks passed
reuse lint	Compliant
aislop	100 / 100, 0 errors, 0 warnings — unchanged against main
prek	All hooks passed

Every fix was reverted in isolation and its tests shown to fail. _precommit_ci.py crossed the 400-line gate during review and was split rather than suppressed, following #475's precedent: _precommit_status.py (what pre-commit.ci said) and _precommit_wait.py (reading it, and waiting).

One existing test asserted that a pre-commit.ci error posts no comment, and now asserts the opposite, with the distinction it turns on named in the class docstring.

Also here

#429 closed as completed — verified that shorthand targets and local-checkout defaulting both landed with #475.

[Copilot]

🔵 Needs a closer look

Live blocker attribution can be wrong for app-pinned checks and can degrade during final reconciliation.

Review details

Suppressed comments (2)

src/dependamerge/merge_manager/_not_mergeable.py:246

• The end-of-run pass no longer has the original rejection in result.error: the first diagnosis replaced it with blocked by required workflow: … and preserved the rejection in warning. Parsing the rewritten text yields no required workflow names, so a still-failing workflow is downgraded on the second pass to merely failing checks. Reuse the preserved rejection when present so required-workflow attribution remains stable across reconciliation.

        rejection = result.error or ""

src/dependamerge/merge_manager/_live_blockers.py:190

• This drops integration_id, so an app-pinned requirement is treated as required by context name alone. A same-named failing status from another GitHub App does not satisfy or block that requirement, but this code reports it as the proven blocker. Since the failing-status probe carries no source identity, exclude app-pinned entries from promotion (as the PR description requires) or propagate app identity through the probe.

        return {
            str(entry.get("context", "")).strip()
            for entry in required
            if isinstance(entry, dict) and entry.get("context")
        }, True

• Files reviewed: 30/30 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[ModeSevenIndustrialSolutions]

Review follow-ups captured

Fifteen review rounds produced 36 findings. The substantive ones are fixed on this branch; everything declined or deliberately removed now has an issue, so nothing rests only in a resolved thread.

Issue	What it holds	Why not here
#494	Required checks from rulesets and branch protection are not combined	Pre-existing on main; four callers, costs a request per repo/branch, tightens reliable for all of them
#495	Seven low-probability diagnosis imprecisions	Implemented then removed — ~680 lines for inputs that essentially do not occur
#496	Recreate dispatch cannot tell a refusal from a failed attempt	Needs a change to the recreate contract; this PR is scoped to diagnosis

The removed work in #495 is archived with its tests at archive/merge-diagnosis-hardening (4606bd4), so anything worth reinstating can be lifted rather than rewritten.

Declined outright, not deferred

Three findings are decisions rather than debts, and are recorded in the commit body so they are not re-litigated by a future reviewer:

• Resolving required workflow names from rulesets — a ruleset names a workflow by file path while a run carries its name:, so joining them means fetching and parsing each file; and GET /orgs/{org}/rulesets returned 403 throughout the 503-PR run in docs/BULK_RUN_PERFORMANCE_AUDIT.md. The rejection message is both cheaper and more reliable.
• Treating stale as failing — GitHub applies it to a run a later push made irrelevant, so reading it as a failure would let a superseded run block a commit it never examined.
• Disambiguating an unbalanced quote inside a rule name — recorded as item 1d of Blocker diagnosis is imprecise for pathological names and configurations #495. GitHub escapes neither delimiter, so the input is not reliably parseable by any rule.

A note for whoever reviews next

If Copilot is asked for another pass, it will re-raise most of #495 — those findings were valid, and we judged the machinery not worth it rather than judging them wrong. Worth reading as a recorded trade rather than as regression.