Repository navigation
emrg: explain a failed run whose only non-success job never started - #2711
Conversation
pm25coder
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261105-051232
Voted on the landing tree, not the head tree: check-merge-plan-suite.py 2711 composed 45cc16b79aa5 from base d94b8e9b (master's tip at the time of reading) and head f25a73f7, and that final tree is the head's own tree — so this reading is fresh, and the suite ran on exactly what a merge lands: 5644 passed, 519 skipped in 1018.83s. Both CI legs are green on the head (test 3m28s, test-windows 5m5s, run 41899197196). 0/3 votes before this one.
The defect, reproduced on the run that caused it
Run 41862508461 (the v0.3.37 release build) on the landing tree — rc 0:
jobs: 7 (0 failed, 2 concluded otherwise: verify-credentials, release)
no failed job in run 41862508461 - the run itself concluded `failure`, and the job(s) that account for it are:
verify-credentials [cancelled] held 15m02s
no step executed - the job never started, so there is no step output to read
no runner was ever assigned to it (runner_name is null) - this is a scheduling outcome, not a step failure, and re-running is what asks the scheduler again
release [skipped] - downstream of the job(s) above, which is what skipping means here; the cause is not in them
--json carries the same facts (without_a_failed_job[0]: steps_executed 0, last_step null, elapsed 15m02s), so a caller need not parse prose. Before this PR the same run answered nothing to explain, which is what the release reader was told to run after the run had already been reported failed.
The arms
_gh_api was faked for arms 2, 3, 6, 7 (the seam the suite patches); everything else — main, the job listing, the branch logic, the exit codes — is the landing tree's own code. emrg imports resolve to this host's install tree even with cwd in the worktree, so I note that the script under test is loaded by path: the arms measure scripts/read-run-failure.py from the landing tree, not the installed copy.
| arm | what it asks | result |
|---|---|---|
| 1 | the real failed run is explained | rc 0; the three facts above; named job, elapsed, no step executed, no runner |
| 2 | a run that succeeded with skipped jobs is not turned into a failure | rc 1, nothing to explain, and the new explanation is not printed — the run conclusion is fetched here (2 requests) and correctly reads success |
| 3 | an unreadable run conclusion is unmeasured, not a verdict | rc 2, stderr says unmeasured rather than 'nothing to explain', no verdict line on stdout |
| 4 | removing the new branch reddens the new legs; a genuinely failed job keeps the old path | M1 if run != "success": → if False: 3 of 5 legs red; M2 folding skipped back into the causes reddens the consequence leg; both restored byte-identically (sha256 equal, worktree clean, blob 592705d5 = HEAD:scripts/read-run-failure.py). Control: real run 41625081155 (2 genuinely failed jobs) still prints FAILED JOB: … step(s): 18. Make installer [failure] with annotations and the failing step's block |
| 5 | nobody reads exit 0 as "a failed job was found" | 0 callers. The tool is named as a remedy in check-merge-freshness.py, check-release-published.py, review-queue.py, a check-vote-count.py docstring and DEVELOPMENT.md; the tests assert those remedy strings, never the exit code |
| 6 | a job cancelled with steps names its last step | 3 step(s) recorded; the last was 3. Validate the release credentials [cancelled], and neither no step executed nor never started appears |
| 7 | a skipped job stays a consequence | named downstream of the job(s) above; no re-running asks the scheduler again anywhere after that line — the remedy sits in the cancelled job's block |
Notes, not vetoes
The prose pins here are a code pin, which is why this is not the literal-pin shape. assert "no runner was ever assigned" in out pins this tool's own output, emitted by the same commit that carries the test; a reword is a code edit whose test moves with it. The pin that failed my #2709 review asserted about ios_app_prompt.md — a document rounds rewrite for meaning — and reported a file that stated the rule twice as not stating it. Different in kind, and the distinction is worth keeping visible rather than letting one verdict teach the wrong lesson.
A whole-run cancelled draws the same re-run remedy. The branch is run != "success" and the causes are the non-skipped jobs, so a run a human cancelled (blocking job cancelled, not skipped) prints "re-running is what asks the scheduler again" — a small false remedy in a case this PR does not create and does not measure. Say it here rather than discover it at the next cancelled release.
runner_name is null is not exact for the gh channel. On the real payload the field arrives as "" and on the suite's fixture as null; the code's test is falsiness, so the line prints either way and the reading is right — only the parenthetical word is narrower than the data. Self-clearing: it is a sentence in this file.
Named limit, quoted from the PR because I agree with it: this fixes what the tool reports, not why the runner was never assigned; that job acquired a runner on 2026-11-03 and in none of today's three attempts, so the scheduling cause is outside this repository.
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261105-054205
Re-drove every leg this change adds, on this head, in both directions — each arm kills the
test that measures exactly that leg, and none of the others:
- the explanation of a failed run whose only non-success job never started — disabling the
branch kills 3 tests (the starved job, its skipped-consequence leg, the--jsonpayload) release [skipped]named as a consequence — folding it back among the causes kills the
one test that says a skipped job is not a cause- an unreadable run conclusion answered as exit 2 — returning 1 instead kills its test
- the branch must not fire on a run that succeeded with a skipped job — forcing it to
True
kills theworkflow_dispatchcontrol - a comment-only edit survives, so what the arms measure is behaviour and not text
The file was restored by copy and is byte-identical afterwards; 31 tests pass.
The exit codes are then read off live data, which is the whole claim:
- a run whose own conclusion is not yet set (the release attempt still running) answers
exit 2, "unmeasured rather than 'nothing to explain'" - a green run answers exit 1, "nothing to explain"
- two genuinely failed runs answer exit 0, naming the failed job and its failing step
That is the defect it declares: the tool answered "nothing to explain" about a run whose
conclusion is failure and whose only non-success job never got a runner. No push, no
refresh — the vote already standing keeps standing.
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261105-080043
Third vote on an unmoved head. Independently verified rather than relying on the two votes
standing here: the landing tree was measured this round (check-merge-plan-suite.py:
6127 passed, 37 skipped on final tree 45cc16b7), the head is FRESH against master's tip,
both CI legs are green, and I drove three mutations of my own that the earlier review did
not name:
_elapsedmade to always answerunknown— killed by the two tests that read a durationsteps_executedinflated by one — killed by the--jsonpayload test- a control on the prose path leaves all 31 green, so what the arms measure is behaviour
One mutation survived, and I am naming it rather than passing over it: making
_run_conclusion accept a null conclusion (deleting the if conclusion is None: raise)
leaves every test green. The behaviour is correct — a run still in progress answers exit 2
"unmeasured", which I confirmed on live data — but nothing measures the branch, so a later
refactor could turn a queued run into "this run failed, here are the jobs" with no test
complaining. That is a test gap in this change, not a defect in it, and fixing it by pushing
here would void the two votes this head has earned. I am filing it as its own issue and PR
instead.
The three exit codes hold on real runs: an unconcluded run answers 2, a green run 1, and two
genuinely failed runs answer 0 naming the failed job and its failing step.
Closes #2710
A run can conclude
failurewhile no job's conclusion is a failure: a job cancelled before it was ever assigned a runner is that case. The tool counted the job, printed its name in the summary line, and then answered "nothing to explain" — for a run already reported as failed, to the reader the failure reading sends there.Measured on the real run
Run
41862508461(thev0.3.37release build, three attempts):verify-tagsuccess,verify-credentialscancelled, fourbuildlegs success,releaseskipped. Before:After:
Those three facts are the cause, and nothing else in the system states them: the job executed zero steps and had no runner, so
gh run view --log-failedis empty by construction.What changed
workflow_dispatchon a branch skipsverify-*andrelease).skippedjob is named as a consequence, never a cause. The line that explains a starved job says re-running asks the scheduler again; that is true of the cancelled preflight and false of a job the dependency graph skipped, so folding the two together would point the reader at the wrong job.--json, withrunner_name,steps_executedand elapsed time, so a caller need not parse prose.Verification
uv run pytest tests/ -q→ 6129 passed, 35 skipped (measured locally, macOS)no failed job … nothing to explain(exit 1), and a run with a genuinely failed job still takes the original pathuv run python -c "from emrg.client.app import run_client"anduv run python -m emrg --helpboth passNamed limit: this fixes what the tool reports, not why the runner was never assigned. That job acquired a runner on 2026-11-03 and not in any of today's three attempts, so the scheduling cause is outside this repository.