ci: regression guard for path-filtered required checks (#286) - #828
Merged
claude[bot] merged 2 commits intoAug 27, 2026
Conversation
Issue #286: the branch ruleset's required status check "In-repo packages resolve from source" was produced by workspace-deps-check.yml, which was path-filtered to `**/package.json`. A path-filtered workflow never runs on a PR that touches none of its paths, so the check run is never created -- and GitHub treats a required-but-never-reported context as unsatisfied. Every PR that didn't touch a package.json sat permanently at "Expected -- waiting for status to be reported", with no red to click and nothing to re-run. That's why every merge needed --admin, defeating the auto-merge path CLAUDE.md documents. VERIFIED FIRST, PER INSTRUCTIONS: this exact defect no longer reproduces on master. - workspace-deps-check.yml's `on.pull_request` carries no `paths:` key today -- confirmed by reading the file at origin/master (2de4a1c). - The ruleset ("Protect Master", id 17974934) still requires the "In-repo packages resolve from source" context -- confirmed via `gh api repos/izzywdev/FuzeFront/rulesets/17974934` (branch-protection API 403'd for this token as anticipated, but the rulesets API worked). - Empirically confirmed on merged PR #818, which touches no package.json file: `In-repo packages resolve from source: success` is present in its check-runs, and the PR merged cleanly. - git blame shows the filter was already removed by #794 (2026-08-23, "ci: two required checks on master cannot deliver a correct verdict"), which chose exactly fix (a) from this issue's own writeup ("Always run, skip the work -- keeps the gate honest") over the path-filter- shim alternative, because the job is dependency-free and scans 49 manifests in under a second, so running it unconditionally costs nothing. That reasoning is sound and is not being revisited here. So there is no live code defect left in scope of #286 to fix -- #794 already applied the correct one of the two standard fixes and did not weaken the required set. What #286 leaves genuinely open is that nothing stops the exact same regression from recurring: the "no paths: filter" invariant lived only in prose (two workflow header comments) with no CI enforcement, for either of the two workflows whose comments state it (workspace-deps-check.yml, gate-sealed-keys.yml). This commit turns that prose into an enforced, dependency-free gate: - governance/required-check-triggers.json declares the (context, workflow file) pairs that must always run unconditionally on pull_request. - scripts/check-required-check-triggers.mjs scans each declared workflow for a paths:/paths-ignore: filter nested under its pull_request: trigger and fails if one is present. No YAML-parser dependency (same style as check-workspace-deps.mjs) -- a line-based indentation scan is enough for the one shape that matters, and it does not false-positive on an unrelated paths: filter under a sibling push: trigger (tested). - .github/workflows/gate-required-check-triggers.yml wires it in on every PR to master, itself with NO paths: filter -- it would be exactly the bug it exists to catch if it were path-filtered. Mutation-tested: re-adding the historical `paths: ['**/package.json', ...]` block to workspace-deps-check.yml's pull_request trigger is caught (exit 1, exact FuzeFront#286 message); the restored file passes (exit 0). Not done here, and why: promoting gate-required-check-triggers itself into the branch ruleset's required set needs ruleset-write access this session was not asked to exercise, and is a separate governance decision (which checks are required) from the workflow-level fix #286 asked for. Filed as a note for @izzywdev rather than actioned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
claude
Bot
changed the title
izzywdev
pushed a commit
that referenced
this pull request
Aug 27, 2026
…ing 2 of 38 #828 (issue #286) adds governance/required-check-triggers.json — a gate that fails if a required context's workflow ever gains a `paths:` filter on `pull_request:`. It is what stops the deadlock in §2 from being reintroduced, and its own header states the limitation: Adding a NEW required context to the ruleset? Add its workflow here in the same PR — the gate is only as complete as this list. It ships listing 2 workflows. Step 3 of this runbook takes the required set to 38. Applied without extending that list, the guard silently covers 2 of 38 and the other 36 can be re-filtered by anyone with nothing to catch it — a green gate measuring almost nothing, which is the shape of failure gate-vacuous-check exists to prevent. Adds step 3a: the full context -> producing-workflow mapping (seven workflows cover all 38), plus a verification snippet that prints required / guarded / UNGUARDED counts so the two files cannot drift silently. Mapping verified against the workflow files rather than written from memory. Two contexts are matrix expansions and needed checking by hand: `CodeQL Analysis (javascript)` comes from security.yml's `codeql-analysis` job (matrix: language) and `Lint & Test (24.x)` from ci.yml's `lint-and-test` (matrix: node-version). Also notes that gate-openapi-conformance.yml and gate-route-ownership.yml join the list once #844 lands, since unfiltering them is precisely the property this guard protects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
claude Bot
added a commit
that referenced
this pull request
Aug 27, 2026
…te-identifier are not among them (#843) * docs(governance): master requires 11 of 45 checks; gate-authz and gate-identifier are not among them CLAUDE.md states the gate set IS the production guard, now that CI may approve its own PRs: "gate-ds-conformance, gate-frames-first, gate-authz, gate-identifier, and the full CI matrix ... A bot-authored, bot-approved PR that clears every gate ships to prod." The ruleset does not enforce that. Measured against the live ruleset: 11 of 45 checks are required. gate-authz and gate-identifier — both named above — are not required. Neither is any backend, integration or e2e suite, nor CodeQL, Snyk, Container Security Scan, gate-toolchain, gate-version, gate-pagination or gate-vacuous-check. auto-merge.yml arms `gh pr merge --auto --squash`, and GitHub's auto-merge waits on REQUIRED contexts only. Master is deploy-on-push. So a PR merges and deploys the moment those 11 pass, while the rest are still running or already red. On 2026-08-27 the bot armed auto-merge on #836 while `Build chat-service` was failing on an unbuildable production image; that check is not required, so nothing would have stopped the merge. This commit is the applicable change, not a report of one: the rulesets API is write-blocked through the agent proxy (403), so it needs repo-admin rights. The desired set is data in governance/required-status-checks.json and the runbook carries the exact PUT, the verification, and the rollback. Audited every workflow trigger before choosing the set, because a required check that does not run on every PR is never created and pins the PR at "Expected" forever — this repo hit that on 2026-08-23 across four sampled PRs. Only unfiltered checks are included (39 contexts). `Generate SBOM` is excluded as it always concludes skipped. Two findings worth more than the count: - The most valuable gates CANNOT be required as they stand. `shipped code matches the frozen contract` — which would have caught config-service shipping two declared-but-unimplemented endpoints — is path-filtered, along with gate-frames-first, gate-route-ownership, image-reproducibility and helm-validate. Step 4 is to unfilter the trigger and filter inside the job, the correction workspace-deps-check.yml and gate-sealed-keys.yml already carry. - Merge queue must NOT be enabled yet, though it is the right fix for the release-commit treadmill. Zero of 58 PR-triggered jobs have a `merge_group` trigger, so queue entries would get no checks at all and time out; and gates that ratchet on the PR base would compute over an empty diff under `merge_group`, reporting green vacuously. Trigger support and base-ref handling land first, proven by a gate still FAILING under a merge_group event. Health-checked before proposing: 29 candidate contexts across all 20 open PRs produced 0 failures, so widening stalls nothing in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc * docs(governance): drop gate-code-review from the required set — it can hang, not fail Correcting my own recommendation in the previous commit. gate-code-review is `runs-on: fuzefront` — a SELF-HOSTED runner — with no `timeout-minutes`. Requiring it would let one offline runner block every merge in the repo, permanently, with no red anywhere to look at. Not hypothetical. Measured 2026-08-27 while draining the PR queue: the job completes in 12-14 seconds when the runner picks it up (five PRs did), but NOTHING has completed since 09:08Z and 17 PRs sit queued for up to an hour — including #840, the PR at the front of the merge queue, queued since 08:52Z. The runner is not slow; it stopped taking work. Meanwhile in-progress GitHub-hosted runs fell from 14 to 5, so this is not shared capacity. The health check in the previous commit read gate-code-review as 5 success / 0 fail / 15 pending and I counted it safe. That was the wrong reading: those pendings were not "slow", they were never going to complete. 0 failures is not evidence a check can fail. Hence the general rule now stated in the runbook, which is what I actually got wrong: A REQUIRED CHECK MUST BE ABLE TO FAIL. A check that can only pass or hang is worse than no check, because the deadlock it produces is indistinguishable from CI still running, so nobody knows to go look. gate-code-review and the path-filtered gates both fail that test, in different ways — one hangs waiting for a runner, the others are never created at all. Requireable again once it has a GitHub-hosted fallback, or a timeout that fails rather than hangs. Required contexts 39 -> 38; the verification count in step 3 is updated to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc * docs(governance): widening the required set leaves #828's guard covering 2 of 38 #828 (issue #286) adds governance/required-check-triggers.json — a gate that fails if a required context's workflow ever gains a `paths:` filter on `pull_request:`. It is what stops the deadlock in §2 from being reintroduced, and its own header states the limitation: Adding a NEW required context to the ruleset? Add its workflow here in the same PR — the gate is only as complete as this list. It ships listing 2 workflows. Step 3 of this runbook takes the required set to 38. Applied without extending that list, the guard silently covers 2 of 38 and the other 36 can be re-filtered by anyone with nothing to catch it — a green gate measuring almost nothing, which is the shape of failure gate-vacuous-check exists to prevent. Adds step 3a: the full context -> producing-workflow mapping (seven workflows cover all 38), plus a verification snippet that prints required / guarded / UNGUARDED counts so the two files cannot drift silently. Mapping verified against the workflow files rather than written from memory. Two contexts are matrix expansions and needed checking by hand: `CodeQL Analysis (javascript)` comes from security.yml's `codeql-analysis` job (matrix: language) and `Lint & Test (24.x)` from ci.yml's `lint-and-test` (matrix: node-version). Also notes that gate-openapi-conformance.yml and gate-route-ownership.yml join the list once #844 lands, since unfiltering them is precisely the property this guard protects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Summary
design-approval.yml's idempotency check ran after its staleness check, and only recognized a flow as "already approved" when the design-approval issue's ownstampstill matched the feature's current manifest stamp exactly.gate-frames-first(scripts/check-frames-first.mjs) gates implementation purely onbuild.flows[<flow>].approved === true— it never readsmanifest.stamp. So a flow's approval legitimately survives later, unrelated manifest edits that move the stamp (e.g. feat(frames-first): declare implementation.paths coverage for 19 features #717 / chore(frames-first): shrink uncovered residual 116→58, exclude app infra #734, which addedimplementation.pathscoverage across 19 features and re-stamped them).flow.approved === true→ close, no-op) now runs before staleness. Staleness still fully gates any flip toapproved: truefor a flow not yet approved.Context — issue #347
Investigating #347 (
design-approval: approve auth-experience / signup) found the flow was already approved by the owner via a direct commit to master (370dbe50, 2026-07-21) afterdesign-approval.yml's Contents-API write path was rejected by the branch-protection ruleset at the time.design/frames/auth-experience/manifest.jsonalready carriesapproved: trueforsignup(and the feature's other 3 flows) at a valid, up-to-datestamp— implementation is not blocked. Issue #347 itself couldn't self-close: its recorded stamp was a 12-char prefix from the original (failed) automated attempt, and the feature's stamp has since moved again via #717/#734's coverage-declaration edits. This fix lets the workflow reconcile that class of stale issue instead of permanently failing to close it.Test plan
node --test scripts/__tests__/stamp-frames.test.mjs— 3/3 pass (unaffected; no manifest content changed)node --test scripts/__tests__/check-frames-first.test.mjs— 19/19 pass (unaffected)node scripts/stamp-frames.mjs --check— all 21 feature stamps still up to datenode --check'd the modifiedactions/github-scriptblock (workflow YAML has no JS/YAML syntax checker in CI)Generated with Claude Code
Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
Generated by Claude Code