Skip to content

Promote develop to main: the blocked-merge reading and the open pull request sweep - #619

Merged
ptr727 merged 2 commits into
mainfrom
develop
Aug 8, 2026
Merged

Promote develop to main: the blocked-merge reading and the open pull request sweep#619
ptr727 merged 2 commits into
mainfrom
develop

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Promotes develop to main, carrying two merged pull requests.

What is being promoted

  • #59107ed74a, reading why a merge is blocked instead of reporting one word. scripts/pr_review.py gains the check rollup, the four stuck shapes it tells apart, and exit 44 for a review loop that closed against a check no waiting clears, plus the GOVERNANCE.md and runbook wording that says BLOCKED never names its own cause.
  • #618dd5fc90, the open pull request sweep in TODO.md's selection procedure, plus an amendment recording the upstream ask for a machine-readable Copilot review schema.

The exit code, since it changed meaning between branches

#591 was authored before the outage of 2026-08-06 and defined exit 42 for its check reading. #607, #608 and #613 took 42 for a round that read fewer files than the pull request changed, and 43 for a shape the reader does not know, while it sat. The forward-merge renumbered the check reading to 44 and ranked it under both: 43 says no field can be believed, 42 says part of the diff has no review, and only once those are clear is a wedged required check the thing worth reporting. wait returns the coverage and shape verdict where it has one and reads the checks only where it does not.

Review state

Both were reviewed and merged on their own pull requests, so this promotion carries no unreviewed change. #591 ran 18 rounds across its life, 5 of them after the revival, and #618 ran 5. Every finding was accepted except one on #614, which was declined with evidence and recorded under "Disproved Claims" in .github/copilot-instructions.md.

Why #591 was open long enough to need reviving

It was parked correctly during the GitHub Actions major outage, with the reason written on the pull request, and the reason then expired quietly. Three days later it was 20 commits behind develop, conflicting in six regions, and carrying an exit code that meant something else. #618 is the procedural answer to that, and it is in this same promotion.

Merge shape

This is a promotion, so it merges as a merge commit rather than a squash, per GOVERNANCE.md "Branching Model". Its head is develop itself, so it must not be merged with --delete-branch.

`mergeStateStatus` reports `BLOCKED` for a failed check, a required check nothing is running, an unresolved thread, and a missing approval alike, and the digest printed that word and stopped there.
Found by running into it. A run polled `BLOCKED` for twenty-five minutes on a pull request whose only unfinished check was an aggregator job GitHub dispatched and never assigned a runner, and the cause came from the maintainer rather than from any field in the digest. That is the defect: the digest named a state it could not explain.
This is the refusal defect of #584 one gate along. There, a review that covered nothing rendered as coverage. Here, a check nothing is running renders as patience, and both read as a pull request worth waiting on.
## What the digest says now
Reconstructed from the observed job states of the run this hit, rather than described:
```
repo=ptr727/ProjectTemplate pr=590 head=aaaaaaaa rounds=1 review_on_head=yes refusal=no
threads=0 unresolved=0 suppressed=0 (on_head=0 earlier=0) answered_outside_review=no
requested=no merge=BLOCKED checks=2/3 stuck=NOT_PICKED_UP
CHECK NOT PICKED UP ('Check pull request workflow status job', queued 15m with no runner
assigned): nothing here starts it, because the runner pool is GitHub-hosted, so re-run the
workflow or wait on that capacity
```
The rollup rides the existing full query, so reading the checks costs no extra round-trip, which is the reason this script exists at all.
## Three shapes, because each wants the opposite response
- **`NOT_PICKED_UP`** is a job GitHub dispatched and assigned no runner. Read from the queued state rather than from a runner name, which GraphQL does not carry, and the state suffices because a job held behind a `needs:` dependency does not enter the rollup until that dependency finishes, so there is no dependency-blocked queue to mistake for a starved one. Nothing agent-side starts it, since the pool is hosted, so the remedy is a re-run or that capacity, and **not** a re-request, a rebase, or an empty commit.
- **`RUNNING_LONG`** is deliberately the weaker reading and its wording says so, since duration alone cannot separate a hung job from a slow one. This repository's lint job legitimately runs nine to eleven minutes while its aggregator is a single shell conditional, so the threshold is generous, the elapsed time prints for the reader to judge against what the job costs, and nothing asserts a fault.
- **`FAILED`** is a verdict rather than a stuck check, reported so no reader deduces a red check from `BLOCKED`.
## Exit 44, and the boundary that makes it mean something
`wait` gains exit `44` for a review loop that closed against a check in one of those shapes, because `0` was saying the loop closing is the merge gate.
A check **merely still running normally is not 44 and exits 0.** That boundary is the whole design: the wait returns the moment coverage lands, which on almost every pull request is mid-CI, so taking `42` for a pending check would make `42` the ordinary outcome, and a code that fires always carries nothing. The grace is the pickup grace's five minutes for the reason that one is, and the stall is thirty because a fleet repository building and testing .NET runs longer than this one. Both are flags, and a negative value for either is rejected by name rather than rendering a digest that reports every check stuck from its first read.
## Two defects this caught in itself
- A finished check carrying **no conclusion yet** fell through to the unknown-conclusion branch and reported `FAILED`, inventing a red check out of a race in the API. Caught by the suite, which is also what said the first test helper was at fault rather than the code, since it failed eight cases at once.
- **`EXPECTED`** was missing from the unstarted set, so a required status nobody has posted reached that same branch and reported that same false failure. Caught reading the diff back.
Both have cases now. `SKIPPED` and `NEUTRAL` count as passes, since the fleet aggregator pattern skips the conditional jobs and four of the six checks on a green pull request here are skips. An unrecognized conclusion **is** reported, because a new enum member read as a pass is a red check rendering as a green digest.
## Docs
The runbook's `BLOCKED` bullet said the cause is "usually" unresolved review threads, which is true and is exactly what misled the run. The qualifier stays and the counter-case joins it rather than replacing it. `GOVERNANCE.md` merge-gate condition 1 now requires the reason to be **read** rather than inferred, and a new paragraph states that `BLOCKED` is no more self-explaining than `CLEAN` is sufficient, including that hosted-runner capacity is not a reason to weaken a gate.
`GOVERNANCE.md` is verbatim-carried, so a **fleet re-vendor is owed**, and the `TODO.md` re-vendor entry now names these sections.
## Verification
461 cases pass across the three `scripts/` suites, 217 of them in `test_pr_review.py`, which is 41 more than `develop` carries. The count read 291 when this was written and the suites have grown on both sides since, so it is restated against the merge rather than left as a number measured on a tree that no longer exists. `prose_lint.py --diff origin/develop`, `repo_gate.py`, `spec/validate.py`, and `markdownlint-cli2` over all 44 files are clean. A grep for restatements of the changed merge-gate wording across every markdown file found none stale, and the "all four preconditions" sentence still holds since condition 1 was amended rather than added to. The digest was also run live against a real pull request, where it reports `checks=6/6` and no stuck field.
## Renumbered from 42 to 44 on the forward-merge
This branch was parked on 2026-08-06 pending the GitHub Actions outage, and `develop` took exit `42` in the meantime for a round that read fewer files than the pull request changed, with `43` for a shape the reader does not know, both from #607, #608 and #613. The check reading here is `44`, ranked under both: `43` says no field can be believed, `42` says part of the diff has no review, and only once those are clear is a wedged required check the thing worth reporting. `wait` returns `report_verdict`'s code where it has one and reads the checks only where it does not.
Adds the missing half of the selection procedure's integrity check, and amends one cluster with an upstream reference.
## The gap
Step 1 confirms every open **issue** appears in this file. Nothing asked the same of an open **pull request**, so a pull request whose blocker has passed is invisible to the one procedure that would catch it. Nothing selects it, nothing closes it, and `develop` moves underneath it.
[#591](#591) is the worked example and is carried in the step as its evidence. It was parked correctly on 2026-08-06, during a GitHub Actions major outage, with the reason written on the pull request. The reason then expired quietly. Three days later it was 20 commits behind `develop`, conflicting in six regions, and its central exit code had come to mean something else, because #607, #608 and #613 had taken `42` for a different reading in the meantime.
The failure is not specific to this repository. The same shape was reported on Blog, where two pull requests were left open through the same outage and a day of new work landed on top of them.
## The rule
Step 2 asks that every open pull request carries a **stated active blocker**: stated where the pull request itself carries it rather than held in a session that has ended, and active only while the thing it names is still true. A landed review round, a merged dependency and a passed outage each stop being one, and what they leave is a forgotten pull request rather than a parked one. The remedy is to finish it, close it, or write the current blocker down.
## Measured before writing, not after
The lesson this repository keeps relearning about checkers is to measure the live corpus before shipping a rule, so it flags what it is for rather than the routine traffic:
| Reading | Result |
| --- | --- |
| Open pull requests right now | **1** (#591) |
| Dependabot pull requests, open to merged | ~**1 minute** (#611 and #612 both `02:24` to `02:25`) |
The merge-bot takes bot traffic inside a minute, so it never sits long enough to owe a blocker, and the rule's working set is the handful of human pull requests that actually linger.
## Second disposition in this change
**Amends "A Programmatic Reading of a Copilot Review"**. That cluster's open question is whether GitHub publishes anything but prose to read a Copilot review from, and its `Settled` line records that the public API does not. The ask is now filed upstream as [GitHub community discussion 204320](https://github.com/orgs/community/discussions/204320), which requests a versioned machine-readable schema carrying severity, category, suggestion and resolution state. It is unanswered, so the entry records it as a place to watch rather than a dependency to wait on.
## Verification
`prose_lint.py` clean including `sentence-split`, `editorconfig-checker` exit 0, `markdownlint-cli2` 0 issues across 44 files, `TODO.md` at 497 of 497 CRLF lines. The renumbering was checked against the file's own cross-references, and the only one that names a position is step 1 calling itself first, which it still is.
CopilotAI lite review requested due to automatic review settings August 8, 2026 13:42

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Promotes develop to main, carrying the already-merged work that teaches scripts/pr_review.py to read why a PR is mergeStateStatus: BLOCKED (via status-check rollups) and updates the repo’s documented selection/review procedures to account for open pull requests and blocked-merge causes.

Changes:

  • Extend scripts/pr_review.py to query and normalize status-check rollups, report checks=N/M plus per-shape stuck diagnostics, and return exit 44 from wait when the review loop is closed but a required check is stuck in a “no wait clears” shape under BLOCKED.
  • Add a focused test suite for the new check-shape and rollup behaviors in scripts/test_pr_review.py, including deterministic time helpers for age-based judgments.
  • Update process docs (TODO.md, scripts/README.md, GOVERNANCE.md, .github/copilot-instructions.md) to reflect the new blocked-merge reading and the open-PR sweep step.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
FileDescription
TODO.mdAdds an “open PRs must have an active blocker” sweep step; records upstream schema discussion link and adds new reference definitions.
scripts/test_pr_review.pyIntroduces rollup fixtures/time helpers and extensive coverage for check-shape classification, digest reporting, and wait exit 44 behavior.
scripts/README.mdDocuments the new wait exit 44 semantics and the check-rollup/stuck-shape reporting behavior.
scripts/pr_review.pyAdds check-rollup query and normalization, stuck-shape classification, digest checks= reporting, CLI flags for check thresholds, and wait exit 44 logic.
GOVERNANCE.mdTightens the merge gate wording to require reading the cause of BLOCKED and explicitly distinguishes blocked-merge causes.
.github/copilot-instructions.mdUpdates the runbook to warn that BLOCKED is not self-explaining and points at the new pr_review.py readings (including exit 44).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit b00de37 into mainAug 8, 2026
7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727