Skip to content

emrg: explain a failed run whose only non-success job never started - #2711

Merged
how2how2how2-arch merged 1 commit into
masterfrom
fix/a-failed-run-with-no-failed-job
Nov 5, 2026
Merged

how2how2how2-arch merged 1 commit into
masterfrom
fix/a-failed-run-with-no-failed-job

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2710

A run can conclude failure while 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 (the v0.3.37 release build, three attempts): verify-tag success, verify-credentials cancelled, four build legs success, release skipped. Before:

jobs: 2 (0 failed, 1 concluded otherwise: verify-credentials)
no failed job in run 41862508461 - nothing to explain

After:

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

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-failed is empty by construction.

What changed

  • The run's own conclusion is read — fetched only on this branch, so the ordinary paths still make one request. It is what separates a run that failed from a run that succeeded with jobs skipped by design (workflow_dispatch on a branch skips verify-* and release).
  • A skipped job 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.
  • The explanation is also machine-readable under --json, with runner_name, steps_executed and elapsed time, so a caller need not parse prose.
  • Exit code 2 when the run's conclusion cannot be read while a job did not succeed — whether the run failed is the question, so failing to read it is unmeasured, not a verdict.

Verification

  • uv run pytest tests/ -q → 6129 passed, 35 skipped (measured locally, macOS)
  • Both controls on real runs: a green run still answers no failed job … nothing to explain (exit 1), and a run with a genuinely failed job still takes the original path
  • Both directions driven: disabling the explanation fails three legs; folding skipped jobs back into the causes fails the one leg that pins the distinction. Each mutation restored byte-identically
  • uv run python -c "from emrg.client.app import run_client" and uv run python -m emrg --help both pass

Named 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.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ 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 how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ 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 --json payload)
  • 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 the workflow_dispatch control
  • 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 how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ 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:

  • _elapsed made to always answer unknown — killed by the two tests that read a duration
  • steps_executed inflated by one — killed by the --json payload 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.

@how2how2how2-arch
how2how2how2-arch merged commit a2eef6e into master Nov 5, 2026
2 checks passed
Sign up for free to 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.

read-run-failure.py says a failed run has nothing to explain when its only non-success job was cancelled

3 participants