pr-watch: wait on green-but-BLOCKED, the needs: gap before a required check registers (improvise-zlj2, improvise-h3ni)
The required Browser harness check comes from a job that needs: build, so for
a moment per push every registered check is green and GitHub says
MERGEABLE/BLOCKED. #186 at 128a2a0 refused that and exited 1 on healthy PRs.
BLOCKED with no failed check is now waited on (exit 3 if it never clears).
Adds the gap sequence from PR #189's real rollup, and a head moving during
the UNKNOWN wait. Corrects tooling.md's claim that nothing else produces
BLOCKED, the evidence entry's rival-2 claims, and the fixture wording.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
+29
-8
@@ -259,7 +259,7 @@ the deploy's and pass. `README.md` therefore states the rule and deliberately
|
||||
**does not list the jobs**: a second copy with nothing holding it to the first
|
||||
is what went stale, twice.
|
||||
|
||||
### pr-watch and mergeability (improvise-edpl)
|
||||
### pr-watch and mergeability (improvise-edpl, improvise-zlj2)
|
||||
|
||||
`scripts/pr-watch` is what the orchestrator's merge procedure stands on, and
|
||||
until improvise-edpl its verdict failed only on `mergeable == CONFLICTING`.
|
||||
@@ -273,6 +273,9 @@ The rule now:
|
||||
|
||||
- **`UNKNOWN` in either field is not an answer, so it is waited on.** On
|
||||
timeout it exits 3, the same code as a check still running. It never exits 0.
|
||||
- **`MERGEABLE/BLOCKED` with no failed check is waited on too** (improvise-zlj2).
|
||||
A required check that never runs therefore times out at exit 3, and does not
|
||||
exit 1.
|
||||
- **A known answer ends the wait early**: a failed check (exit 1 whatever
|
||||
mergeability says, since waiting would turn a known red into a timeout), or
|
||||
`mergeable == CONFLICTING`.
|
||||
@@ -286,24 +289,42 @@ repos/fiddlerwoaroof/improvise/rules/branches/main`, read 2026-09-22) requires
|
||||
three status checks. It requires no approvals, and its required checks are not
|
||||
strict.
|
||||
|
||||
- `BLOCKED` with a `MERGEABLE` flag means a required check has not passed,
|
||||
either unrun or failing. With zero required approvals nothing else produces it
|
||||
here. It is refused, not waited on. A required check that never runs would
|
||||
otherwise hold the watch until the timeout, and exit 1 is the "not mergeable"
|
||||
answer.
|
||||
- `BLOCKED` with a `MERGEABLE` flag means a required check has not passed. A
|
||||
failing one is already in the rollup, so the script has exited 1 before it
|
||||
reads the state. What is left is a required check that **has not registered
|
||||
yet**, and every healthy push passes through that state. The required
|
||||
`Browser harness (OPFS contract + shell)` check comes from the `browser` job,
|
||||
which `needs: build`. GitHub creates its check run only after `build`
|
||||
completes. For a second or three per push, then, every registered check is
|
||||
`COMPLETED/SUCCESS`, the required one does not exist, and GitHub reports
|
||||
`MERGEABLE/BLOCKED`. The first version of this rule refused `BLOCKED`, and so
|
||||
exited 1 on a healthy PR whenever a poll landed in that gap. The reviewer put
|
||||
that at 5–15% of watches at the 20s interval. An "unrun" required check and a
|
||||
"not yet registered" one look the same from the rollup, so the state is
|
||||
waited on. A check that genuinely never runs costs a timeout, exit 3, not a
|
||||
false exit 1. An earlier version of this section said that nothing else
|
||||
produces `BLOCKED` here. That was wrong: the `needs:` gap produces it.
|
||||
- `DIRTY` can wear `MERGEABLE` for a moment while the two fields settle. It is
|
||||
refused.
|
||||
- `BEHIND` needs strict required checks, which this repo does not set. `DRAFT`
|
||||
cannot be merged. Both are refused.
|
||||
- `UNSTABLE` means a non-required check is unhappy. It stays a pass, as it was
|
||||
before. Any failed check in the rollup has already exited 1, so in practice it
|
||||
only means a non-required status still settling.
|
||||
before. It is safe because the loop only reads the state once every
|
||||
registered check is `COMPLETED`, and any failed one has already exited 1. So
|
||||
whatever `UNSTABLE` is reporting, it is not a check in the rollup that is
|
||||
still running or red.
|
||||
- `HAS_HOOKS` is GitHub Enterprise's "mergeable, with pre-receive hooks". It is
|
||||
harmless to allow.
|
||||
|
||||
The suite enumerates the whole `mergeable × mergeStateStatus` table, plus an
|
||||
unshipped value and `null`, rather than hand-picking cases. Why: improvise-90ea.
|
||||
|
||||
The suite also plays the `needs:` gap through, poll by poll: build running, then
|
||||
build done with no harness check, then the harness running, then `CLEAN`. That
|
||||
sequence must exit 0. The rollup it starts from is PR #189's, copied from
|
||||
`gh pr view --json`. Each earlier poll is derived from it with `jq`, by dropping
|
||||
entries or rewinding them to `IN_PROGRESS`.
|
||||
|
||||
### Why the Pages deploy fails on "Found 0 artifact(s)" (improvise-864a)
|
||||
|
||||
`actions/deploy-pages@v5` intermittently fails a push to `main` with *No
|
||||
|
||||
Reference in New Issue
Block a user