ci: a required context whose workflow is disabled can never report - #76
Merged
Conversation
THE DEFECT, measured on the sibling vault repo (wshallwshall/MessageFoundry) on
2026-07-30: 10 required contexts whose workflows were ALL `disabled_manually`. A
disabled workflow never dispatches, so those contexts never reported -- NO pull request
could merge, and every merge there had silently been riding admin bypass. It sat that way
unnoticed because the symptom presents to a human as "CI is stuck", not as "branch
protection is misconfigured". Found by the ASVS session, filed unclaimed, picked up here.
tests/test_required_contexts.py CANNOT see it. It resolves each required context against
job names in .github/workflows/, and a disabled workflow keeps its file and its job name
on disk -- so that check passes cheerfully in the healthy case AND the broken one. The
property it measures ("a job with this name exists in YAML") is true either way. Same
defect shape as a CI monitor polling for "nothing pending": sound about the thing it
looks at, blind to the thing that can fail. Workflow `state` is server-side, so no
file-based test can reach it.
SCHEDULED (07:00 UTC, an hour after the nightly CI cron), not per-PR: a workflow disabled
AFTER the last pull request is invisible to any per-PR check, because there is no PR to
run it on. The failure arrives while the repo is idle, which is exactly when nobody is
looking. It also runs on PRs touching required-contexts.txt or .github/workflows/** --
the one moment a human is changing the mapping, when a typo is cheapest to catch.
Scope is REACHABILITY, not outcome: whether a check passes is CI's job; whether it can
run at all is this one's. Needs only `actions: read` -- notably NOT the admin scope that
reading branch protection requires.
Reuses tests/_workflow_contexts.resolve rather than growing a second context->workflow
mapping, and a parity test fails if that import is ever dropped. One resolver, two
callers, because a second implementation is exactly the drift this repo keeps writing
parity tests to catch.
FAIL-CLOSED throughout: an unreadable payload, a failed API call, zero workflows
returned, or zero parsed contexts all exit 2. "We could not measure" must never read as
"everything is fine" -- that is the same blindness one level up.
The all-unreachable case is called out BY NAME, because it is the one that cost real
time: it means nothing can merge, and the only visible symptom is PRs hanging. A single
disabled workflow deliberately does NOT trigger that message (a test pins that), so it
cannot cry wolf.
VERIFIED end-to-end against the REAL live API payload, not a hand-written fixture:
live payload, all active -> exit 0
security.yml flipped to disabled -> exit 1, naming all 7 contexts it owns
every workflow disabled -> exit 1, with the vault-shape diagnosis
Exit codes captured WITHOUT a pipeline -- the first probe reported 0 for the failing case
because $LASTEXITCODE after a pipe reflects Select-String, not python. The probe lied
before the code did.
Measurement DATED so it cannot age into a false claim: on 2026-07-30 MEFORORG had 19
workflows, all active, and 13/13 required contexts resolved to active workflows. The
hazard is real but was not live here on that date.
wshallwshall
enabled auto-merge (squash)
July 30, 2026 17:36
…job does not have
My own check failed on its own PR, which is the check doing its job on the first thing
it was ever asked to run:
File "scripts/ci/check_required_workflow_state.py", line 48, in <module>
from tests._workflow_contexts import required_contexts, resolve
File "tests/_workflow_contexts.py", line 25, in <module>
import pytest
ModuleNotFoundError: No module named 'pytest'
`tests/_workflow_contexts.py` did `import pytest` at module scope for one reason:
`yaml = pytest.importorskip("yaml")`, keeping PyYAML optional for the test suite. That
was harmless while the module was test-only. It stopped being test-only the moment the
reachability script imported the same resolver -- deliberately, so there is ONE
context->workflow mapping rather than two that drift -- and that job installs
ci/locks/ci-scanners.lock, not the test toolchain.
A pure environment difference: it passed locally, where a dev venv has pytest, and died
in CI. Same class as the ruff-0.16-vs-0.15.22 drift and the actionlint-with-shellcheck
asymmetry earlier today -- a thing that behaves differently depending on what happens to
be installed where it runs.
FIX: import yaml directly and fall back to `pytest.importorskip` only when it is
genuinely absent. Behaviour under pytest is unchanged -- with PyYAML present the try
succeeds and pytest is never imported here; with it absent the importorskip still turns
the importing module into a SKIP, exactly as before.
The alternative was to give the script its own YAML parsing and drop the shared import.
Rejected: a second context->workflow mapping is precisely the drift this repo keeps
writing parity tests to catch, and `test_it_reuses_the_shared_resolver_rather_than_a_
second_copy` exists to prevent it.
GUARDED so it cannot regress: `test_the_script_imports_without_pytest_installed` imports
the resolver in a SUBPROCESS and asserts `pytest` never entered `sys.modules`. In-process
would be meaningless -- pytest is by definition already imported when a test runs, so the
assertion could never fail. Proven by reverting the module-scope import: caught, with the
right message.
Verified after: 33 passed across every suite that touches the shared resolver
(required-workflow-state, required-contexts, security-posture, ci-leg-data-class,
nightly-notice).
…EFORORG/MessageFoundry into claude/required-workflow-state
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.
Closes the gap the sibling vault repo exposed on 2026-07-30 — found by the ASVS session, filed unclaimed, picked up here.
The defect
The vault (
wshallwshall/MessageFoundry) had 10 required contexts whose workflows were alldisabled_manually. A disabled workflow never dispatches, so those contexts never reported: no pull request could merge, and every merge there had silently been riding admin bypass.It sat that way unnoticed because the symptom presents to a human as "CI is stuck" — not as "branch protection is misconfigured".
Why the existing guard can't see it
tests/test_required_contexts.pyresolves each required context against job names in.github/workflows/. Adisabled_manuallyworkflow keeps its file and its job name on disk, so that check passes cheerfully in the healthy case and the broken one.The property it measures — "a job with this name exists in YAML" — is true either way. Same defect shape as a CI monitor polling for "nothing pending": sound about the thing it looks at, blind to the thing that can fail. Workflow
stateis server-side, so no file-based test can reach it.Design
Scheduled (07:00 UTC, an hour after the nightly CI cron), not per-PR — a workflow disabled after the last pull request is invisible to any per-PR check, because there's no PR to run it on. The failure arrives while the repo is idle, which is exactly when nobody is looking. It also runs on PRs touching
required-contexts.txtor.github/workflows/**: the one moment a human is changing the mapping, when a typo is cheapest to catch.Scope is reachability, not outcome — whether a check passes is CI's job; whether it can run at all is this one's. Needs only
actions: read, notably not the admin scope reading branch protection requires.Reuses
tests/_workflow_contexts.resolverather than growing a second context→workflow mapping, with a parity test that fails if that import is dropped. One resolver, two callers.Fail-closed throughout: unreadable payload, failed API call, zero workflows returned, or zero parsed contexts all exit 2. "We could not measure" must never read as "everything is fine" — that's the same blindness one level up.
The all-unreachable case is called out by name, because it's the one that cost real time. A single disabled workflow deliberately does not trigger that message (a test pins it), so it can't cry wolf.
Verification — against the real live payload, not a hand-written fixture
security.ymlflipped todisabled_manuallyExit codes captured without a pipeline — my first probe reported
0for the failing case because$LASTEXITCODEafter a pipe reflectsSelect-String, not python. The probe lied before the code did.9 unit tests drive the shipped
main()through its--states-jsonseam rather than re-implementing the rule.disabled_inactivityis covered too — matching only the one state name seen on the vault would miss the 60-idle-day case, which is precisely the quiet period this check exists for.Measurement dated so it can't age into a false claim: on 2026-07-30 MEFORORG had 19 workflows, all
active, and 13/13 required contexts resolved to active workflows. The hazard is real but was not live here on that date.