fix(ci): classify stalls and failures instead of guessing (REQ-317, REQ-316) - #890
Merged
Conversation
📐 Rivet artifact delta
Graphgraph LR
REQ_316["REQ-316"]:::modified
REQ_317["REQ-317"]:::modified
REQ_323["REQ-323"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Modified
Posted by |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…EQ-316)
Two CI signals were sending readers to the wrong conclusion, and both had
already produced a published misattribution.
REQ-317 — the liveness probe had three defects, all fixed:
1. Its diagnose hint told every reader to run
`gh api repos/<repo>/actions/runners --jq .total_count` and read `0 = pool
offline`. Runners here are ORG-registered, so that command returns 0 on a
perfectly healthy pool and can never say otherwise. The alert's own hint
was the trap.
2. It reported a duration and nothing else, so hosted starvation and a dead
self-hosted pool produced identical alerts — and hosted capacity appears
in the runners API at NO scope, so no runner count can tell them apart.
3. It counted queue age on runs that had already COMPLETED: #867 reported
"queued 177m" for a cancelled run that was never going to start.
REQ-316 — a job killed by a fleet restart reports `failure` with NO failed
step, because the steps end `skipped`. GitHub renders that identically to a
broken test, which is how a wrong cause got published on #855.
Both are now shell functions in tools/ci-diagnose/diagnose.sh with fixtures:
classify_stall no-queue | hosted-starved | pool-offline | label-saturated
| capacity-available
classify_failure job-passed | runner-vanished | setup-failed | job-failed
A FIFTH failure mode turned up while building this and is covered: label-
partitioned saturation. On 2026-09-02 the pool read online=12 busy=8 — a third
of the fleet idle — while every rust-cpu job queued, because the four idle
runners carried only lean-mem. A global busy-versus-online count cannot see
that, so capability is computed PER LABEL against the labels of what is queued.
Validated against live data, not only fixtures: with 5 hosted and 3 rust-cpu
jobs queued and only lean-mem and light runners idle, classify_stall returns
label-saturated; the control that filters to just the hosted jobs returns
hosted-starved. classify_failure on the real Kani job from #888 returns
setup-failed — the same discriminator applied by hand all cycle, now mechanical.
Negative-controlled: ignoring per-label capability, losing the runner-vanished
case, and dropping the hosted short-circuit each redden the oracle.
The tests run in the YAML Lint job, which IS in CI Gate's needs. A classifier
that never executes is documentation, not a gate. They are shell, and their
`rivet: verifies` markers are picked up by `coverage --tests` only because
REQ-319 taught the scanner to read shell scripts earlier in this same round —
before that they would have been invisible.
REQ-316 is SPLIT, decided with the maintainer. This carries the legibility
half. Finding the cause on the runner hosts — unattended upgrades, a systemd
timer, config management or an autoscaler reclaim, all consistent with the
04:30-05:00 UTC window both incidents fall in — needs shell access to those
machines and is deferred to REQ-323 at v0.37.0. Recorded, not dropped.
Confirmed with the diagnose oracle (10 cases), cargo fmt --check, cargo test
--workspace (exit 0, 66 ok), rivet validate, rivet docs check, yamllint on the
CI paths — all exit 0. Both workflow files parse as YAML and every embedded
shell block passes `bash -n`.
Implements: REQ-316, REQ-317
Verifies: REQ-316, REQ-317
Answered on #862 by the infrastructure maintainer, not from this repository. `needrestart` runs as a dpkg Post-Invoke hook and, without a TTY under unattended-upgrades, restarts every service mapping an upgraded library. `github-runner@*.service` was never excluded, so a library upgrade stops and starts all twelve runners mid-dpkg. `apt-daily-upgrade.timer` fires 06:00-06:45 CEST — 04:00-04:45 UTC, the window both incidents fall in. All three occurrences match to the second, each upgrading a library Runner.Listener links for HTTPS: curl at +13s, openssl at +21s, libssh-4 at +30s. Negative-controlled: on 2026-09-04 the upgrade installed only gnupg, which the runners do not map, and no restart followed. The mechanism explains the non-events as well as the events, which is what makes it a diagnosis rather than a correlation. Recorded as `implemented`, NOT `verified`: the drop-in is deployed and parses, but suppression of a real restart has not been demonstrated — that needs an actual library upgrade, and the maintainer is watching the next apt-daily-upgrade. The acceptance clause is the suppression, not the diagnosis. This also corrects #839: exit 143 on Kani in that window is this needrestart, not hosted-runner reclaim — confirming REQ-316's note that the shutdown message alone does not identify which pool restarted. It removes A cause, not THE cause. Runners also vanish on a broker-service wedge and under host memory pressure, so the REQ-316 legibility work stays worth having on its own. Refs: REQ-323, REQ-316
avrabe
force-pushed
the
fix/req-316-317-ci-signal-honesty
branch
from
September 5, 2026 15:43
d3034c7 to
3a14293
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 3a14293 | Previous: 16bff52 | Ratio |
|---|---|---|---|
traceability_matrix/1000 |
59674 ns/iter (± 599) |
46077 ns/iter (± 202) |
1.30 |
query/10000 |
352091 ns/iter (± 3835) |
244880 ns/iter (± 1851) |
1.44 |
This comment was automatically generated by workflow using github-action-benchmark.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two CI signals were sending readers to the wrong conclusion, and both had
already produced a published misattribution.
REQ-317 — three defects in the liveness probe
gh api repos/<repo>/actions/runners --jq .total_countand read0 = pool offline. Runners here are org-registered, so that returns0on a perfectly healthy pool and can never say otherwise. The alert's own
hint was the trap.
self-hosted pool produced identical alerts — and hosted capacity appears in
the runners API at no scope, so no runner count can separate them.
"queued 177m" for a cancelled run that was never going to start.
REQ-316 — a red X that means "the runner left"
A job killed by a fleet restart reports
failurewith no failed step — thesteps end
skipped. GitHub renders that identically to a broken test, which ishow a wrong cause got published on #855.
Both are now classifiers with fixtures
A fifth failure mode turned up while building this and is covered:
label-partitioned saturation. On 2026-09-02 the pool read
online=12 busy=8—a third of the fleet idle — while every
rust-cpujob queued, because the fouridle runners carried only
lean-mem. A global busy-vs-online count cannot seethat, so capability is computed per label against the labels of what's
queued.
Validated against live data, not only fixtures
That last one is the discriminator I'd been applying by hand all cycle, now
mechanical.
Negative-controlled: ignoring per-label capability, losing the runner-vanished
case, and dropping the hosted short-circuit each redden the oracle.
The tests actually run
They're wired into the YAML Lint job, which is in CI Gate's needs. A
classifier that never executes is documentation, not a gate.
They're shell — and their
rivet: verifiesmarkers are picked up bycoverage --testsonly because REQ-319 taught the scanner to read shellscripts earlier in this same round. Before that they'd have been invisible and
neither requirement could have reached
verifiedby any honest route. Which isexactly what the #870 reporter said would happen.
Scope split — please note
REQ-316 asked for two things. This carries the legibility half. Finding the
cause on the runner hosts (unattended upgrades, a systemd timer, config
management, an autoscaler reclaim — all consistent with the 04:30–05:00 UTC
window both incidents fall in) needs shell access to those machines, so it's
deferred to REQ-323 at v0.37.0. Recorded, not dropped.
Gates
diagnose oracle (10 cases) ·
cargo fmt --check·cargo test --workspace(exit 0, 66 ok) ·
rivet validate·rivet docs check·yamllint— allexit 0. Both workflow files parse as YAML and every embedded shell block passes
bash -n.Fourth of the v0.36.0 feature-loop run.