Skip to content

ci(signal): turn a red check into a label a poller can find in one call - #716

Merged
wshallwshall merged 4 commits into
mainfrom
ci/failure-signal
Aug 31, 2026
Merged

ci(signal): turn a red check into a label a poller can find in one call#716
wshallwshall merged 4 commits into
mainfrom
ci/failure-signal

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Closes the signalling half of BACKLOG #1402 and #1403, both filed in PR #715.

The problem, measured. When a required check fails, nothing tells any session. Exactly two workflows carry an if: failure() step and both only fail the job. So the only route from a red to somebody acting on it is fetching the check rollup of every open pull request, one at a time, which is expensive enough that it does not get run.

What this adds. A workflow_run job that labels the pull request ci-red when CI, Security, CodeQL or backlog-hygiene fails. Noticing then costs one gh pr list --label call across all pull requests.

It is not a push, and the file header says so. GitHub still cannot reach into a session. This makes the poll cheap enough to run often, which is the achievable version.

Why workflow_run rather than a failure step in each workflow. A failure step cannot see other workflows' runs, and the required contexts span five of them. workflow_run observes all of them from one place, runs from the default branch with its own permissions, and never executes head code. nightly-notice.yml already uses the trigger.

The merge-queue case. A merge_group run has no github.event.pull_request, the same fact that collapsed backlog-hygiene's concurrency key until #711 keyed it on github.ref. Its ref does carry the number, as gh-readonly-queue/<base>/pr-<N>-<sha>, so an ejection is still attributable.

Reds with no pull request go to one standing issue, matched on exact title so search ranking cannot spawn a second tracker. Not a rare branch: 4 of the 9 failures in the last 200 runs had no PR behind them.

Hardening. Every github.event value reaches the scripts through env, since a branch name is chosen by whoever opened the branch. Top-level permissions are contents: read; the one job that writes elevates for itself. actionlint passes.

Not watched, deliberately: CLA Assistant. A CLA failure is the contributor's to resolve and needs no attribution; labelling it would train readers to ignore the label.

🤖 Generated with Claude Code

wshallwshall and others added 4 commits August 31, 2026 16:49
BACKLOG #1402 and #1403. When a required check fails, GitHub tells no session.
Measured 2026-08-31: exactly two workflows carried an if-failure step and both
only failed the job, so the only route from a red to somebody acting on it was
fetching the check rollup of every open pull request, one at a time.

A workflow_run job now labels the pull request ci-red on a failure of CI,
Security, CodeQL or backlog-hygiene. Noticing becomes one gh pr list --label
call. This is not a push and the header says so: GitHub still cannot reach into
a session. It makes the poll cheap enough to run often.

The merge-queue case is why head_branch is parsed. A merge_group run carries no
github.event.pull_request -- the same fact that collapsed backlog-hygiene's
concurrency key until #711 keyed it on github.ref -- but its ref encodes the
number as gh-readonly-queue/<base>/pr-<N>-<sha>, so an ejection can still be
attributed to the pull request it ejected.

A red with no pull request behind it goes to one standing issue instead, matched
on its exact title so search ranking cannot spawn a second tracker. That is not
the rare branch: 4 of the 9 failures in the last 200 runs were of that kind.

Every github.event value reaches the scripts through the environment, because a
branch name is chosen by whoever opened the branch. Top-level permissions are
contents: read and the one writing job elevates for itself. CLA Assistant is
deliberately not watched: a CLA failure is the contributor's to resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n what the justification rests on

zizmor reds failure-signal.yml under dangerous-triggers. workflow_run is the only trigger that can
observe another workflow's conclusion, and the contexts this watches span four workflows, so it is
the mechanism here rather than a convenience. Record it as a reviewed non-finding.

The objection is the "pwn request" class: workflow_run runs from the default branch with a
privileged token, so checking out and running the triggering pull request's code would hand any fork
pull request a write token. Four properties put that out of reach, and a test now holds each one
instead of the comment that claims it:

  * no checkout and no `uses:` at all, so nothing from the head is fetched, let alone run
  * file default contents: read, with one job taking pull-requests: write and issues: write, which
    cannot push, tag or modify code
  * every github.event value hoisted to env, never spliced into a run body
  * head_branch parsed only under RUN_EVENT = merge_group, an event a fork cannot produce

This is a weaker claim than nightly-notice.yml's, and the entry says so rather than implying
symmetry. That workflow gates on schedule, so no pull request reaches it at all. This one must react
to pull-request runs, because labelling the pull request is the entire point, so its fork path is
open. The four properties are what bound that cost, not a closed door. The entry also records that
GitHub leaves workflow_run.pull_requests empty for a fork, so a fork red falls through to the
standing-issue branch instead of labelling anything.

tests/test_failure_signal.py, reviewed before shipping: drop an assertion that could not fail -- it
ended in `if False else 0` and `or True`, so it graded nothing while reading as coverage. Rename the
watch-list test, which claimed every watched workflow produces a required context; CodeQL does not,
and .github/required-contexts.txt lists it under DELIBERATELY NOT REQUIRED. Add positive controls
where an empty scan and a clean scan rendered identically, look the resolve step up by id rather
than by position so a reorder fails where it happened, and assert the env hoist itself rather than
only the absence of interpolation. All six tests red under a targeted mutation of the workflow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tests/test_tooling_partition.py went red on the previous commit: a new test that imports no engine
module must land in tests/tooling_manifest.txt or in the explicit stay-list, and this one was in
neither. Measured on the CI run for 27a856d -- 1 failed, 11122 passed, and the one failure was
that classification gate.

The manifest is the right list. This test's subject is a workflow file, .github/workflows/
failure-signal.yml, exactly like tests/test_nightly_notice.py beside it. The stay-list exists for
tests that read messagefoundry/** off disk without importing it; this one never touches engine
source, so listing it there would widen that list's stated rule for no reason.

Listing it does not hide it. The tooling job's path filter fires on any change under .github/, and
separately on any path named in the manifest itself, so both the workflow this guards and the guard
itself trip the job that runs it. Confirmed locally: the six tests collect under `-m tooling` and
deselect under `-m 'not tooling'`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ation

The entry said the required contexts span five workflows. Neither half held: failure-signal.yml
watches four names -- CI, Security, CodeQL, backlog-hygiene -- and CodeQL carries no required
context at all, which .github/required-contexts.txt records deliberately.

Worth a commit of its own because of where the sentence sits. This file's header calls every entry
a reviewed, justified non-finding, so a reader checks the reasoning and carries the count with it. A
wrong number welded to sound reasoning is the half nobody re-checks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Reviewed, and labelled

Recording what I actually checked, so the label means something specific rather than "someone looked".

The measured claims, re-run against origin/main

Claim How I checked Result
Exactly two workflows carry if: failure(), and both only fail the job git grep -n "failure()" origin/main -- .github/workflows/ Holds. branch-leak-scan.yml:100 and security.yml:303, and both steps only echo
nightly-notice.yml already uses workflow_run read its on: block Holds. workflow_run over CI, Security, DAST
#711 keyed backlog-hygiene on github.ref backlog-hygiene.yml:43 Holds
Nothing on main reads head_ref git grep -n head_ref origin/main -- .github/ Holds. Zero hits
The four watched names resolve listed every workflow's name: at origin/main Holds. CI, Security, CodeQL, backlog-hygiene all exist

The zizmor.yml edit is scoped to dangerous-triggers only and adds one filename. Nothing else widened.

The tests can fail

The suppression rests on four properties, so I checked the tests that pin them are not vacuous. I extracted this branch's workflows plus tests/test_failure_signal.py into a scratch tree, confirmed 6 passed, then mutated the workflow once per test and restored between runs:

Mutation Test that went red
added - uses: actions/checkout@v4 test_it_pulls_in_no_third_party_actions
top-level contents: write test_it_is_least_privilege_and_cannot_modify_code
interpolated ${{ github.event.workflow_run.head_branch }} into a run: body test_every_event_value_reaches_a_script_through_env
dropped && [ "$RUN_EVENT" = "merge_group" ] test_the_merge_queue_parse_is_gated_on_an_event_a_fork_cannot_produce
added a watched name no workflow answers to test_every_watched_workflow_exists
added `

Each mutation reddened exactly the test that claims to guard it, and only that one. The suppression is pinned, not asserted.

The resolve script behaves as documented

Ran it standalone against seven payloads with a jq stub:

Input Result
[{"number":42}], pull_request 42
[], merge_group, gh-readonly-queue/main/pr-717-abc123 717
[], push, main empty
null, schedule empty
[], merge_group, ref with no number empty, and does not abort under pipefail
[], pull_request, branch named pr-999-evil empty. The hostile name never reaches the parse
[], merge_group, .../pr-1-x-pr-999-y 1, the queue's own prefix

Also green on this branch: zizmor, and repo harness tests (ubuntu-latest), which is the leg tests/tooling_manifest.txt routes the new file into.

Two things worth raising, neither changing the verdict

Nothing removes ci-red. When CI later goes green the label stays, so gh pr list --label ci-red accumulates pull requests that are already fixed. That is the same decay this file's own CLA paragraph refuses: labelling something that does not need attribution "would train readers to ignore the label". review-gate.yml solves the mirror problem by removing its label on synchronize. A sibling job on conclusion == 'success' that removes the label would close it, and I would file it rather than block on it.

The "4 of the 9 failures in the last 200 runs" figure did not reproduce, because the window has moved. Over the most recent 500 runs I count 28 failures: 20 pull_request, 5 merge_group, 2 push, 1 schedule. That still supports "not the rare branch" for the no-payload case. But 5 of those 8 are merge_group, which this workflow attributes from head_branch rather than sending to the standing issue, so the standing-issue branch itself was 3 of 28 in my window. The number is defensible; it reads as slightly larger than what it covers.

Merge order

The workflow header and the body cite BACKLOG #1402 and #1403, which PR #715 files and which are not on main yet. The numbers are allocated so they cannot mis-resolve onto somebody else's work. Land #715 first, or the citations point at nothing in the meantime. No gate will catch it: dangling_citation_check.py defaults to docs/**/*.md, and the diff-scoped citation gate only fires on a number added beside a ledger path.

@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Aug 31, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit 947df9d Aug 31, 2026
41 of 43 checks passed
@wshallwshall
wshallwshall deleted the ci/failure-signal branch August 31, 2026 23:00
wshallwshall pushed a commit that referenced this pull request Sep 1, 2026
… branch

Sixth review pass on #715. Two blocking findings, both the same failure:
asserting state without reading the tree the claim sits in.

#1402 AND #1403 DESCRIBED A GAP THIS BRANCH HAS ALREADY CLOSED. PR #716
merged failure-signal.yml on 2026-08-31, this branch then merged main, so the
workflow is in its own tree while the items said no workflow labels a pull
request and no ci-red label exists. The workflow's header even names #1403 by
number. Both items now record the signal half as shipped and verified firing
-- github-actions[bot] labelled 718, 719 and 721 -- and state plainly that
what remains is the watcher, which is claude-multisession#108. Neither should
close until that lands, and each says so.

#1404 BLESSED PR #718 WHILE #718 CARRIES THE PREMISE #1404 RETRACTS. Step 2
said "DONE in PR #718". Reading its diff rather than its body: it rewrites
the DELIBERATELY NOT REQUIRED entry that step 1 says must not be edited, adds
a block asserting "THEY BLOCK A MERGE TODAY ... branch protection enforces
SIXTEEN", and adds a _LIVE_CODEQL_CONTEXTS constant naming both CodeQL
contexts. All false against a 14-context server. This item voided PR #700's
premise for precisely that reason and never applied the same test to the PR
stacked on its own branch. Step 2 now says so and says #718 needs correcting
first, since the two land together.

I took #718's PR body as evidence for what #718 does. That is the same
mistake as taking an item's prose as evidence for what the code does.

Also fixed: commit 11199d2 replaced a table but left one row behind it, so
a bare pipe row sat under prose with no header. Removed; the prose that
follows already carries it.

Verified: 41 tests pass across backlog-status and required-contexts; the
citation gate passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed A reviewer has read this. Removed automatically when new commits arrive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant