Skip to content

ci: take Windows off the pull_request fan-out; require seven contexts - #4501

Open
BunsDev wants to merge 5 commits into
mainfrom
fix/cave-jtuu7-thin-ci-fanout
Open

ci: take Windows off the pull_request fan-out; require seven contexts#4501
BunsDev wants to merge 5 commits into
mainfrom
fix/cave-jtuu7-thin-ci-fanout

Conversation

@BunsDev

@BunsDev BunsDev commented Aug 10, 2026

Copy link
Copy Markdown
Member

Why

Every pull request scheduled 19 jobs, three of them Windows. On 2026-08-10 this repository held 39 queued Actions runs with ZERO in progress, the oldest stuck since 2026-07-18, while GitHub Actions was globally operational and a hosted runner completed an unrelated job mid-stall. The queue — not the suite — was the blocker, so every required check on every open PR sat pending indefinitely.

macOS was already pulled from these matrices for exactly this reason (the comment is still in ci.yml). Windows now follows it.

What changed

pull_request push to main release
conformance ubuntu only ubuntu + windows
sidecar-runtime ubuntu only ubuntu + windows
windows-native skipped runs
Windows MSI built by release.yml

main keeps identical platform coverage. Only the per-PR fan-out shrinks, by three jobs billed at 2x.

The two ways this wedges every PR, both handled here

  1. sidecar-runtime-required failed unless windows-native reported success. It now accepts skipped on pull_request only, and still hard-fails a skip on any other event — that is the event whose coverage the rollup exists to guarantee. Both rollups are already if: always(), so they keep reporting.

  2. Branch protection required the two (windows-latest) contexts, which no longer report on a PR. A required context that never reports is exactly the BLOCKED-with-nothing-failing wedge documented in CLAUDE.md. Protection must drop to the seven contexts that always report — see the checklist below. The two *-required rollups stay required, so Windows still gates main.

Required contexts 9 → 7: drop Cross-environment (windows-latest) and Sidecar runtime (windows-latest).

Verification

  • scripts/ci-recovery-workflow.test.mjs extended to pin the new shape. Its exact-string SHA-guard assertion becomes structural: a job may narrow itself further, but only by ANDing onto the guard, and a top-level || in the suffix is rejected so the head-moved protection cannot be silently weakened.
  • Mutation-tested — each of these fails a distinct assertion: reverting the windows-native PR skip; letting the rollup accept a skip on any event; swapping the && for ||.
  • Passing: ci-recovery-workflow, branch-to-merge-contract (20), ci-recovery (17), ios-build-ci, branch-cap-workflow, beads-familiar-workflow, dependency-policy, ui-consistency, check:tests-wired (1603 files wired).
  • ci.yml and release.yml both parse; release confirmed to build windows-latest (MSI) and both macOS arches.

Docs updated in the same change: CLAUDE.md, the branch-to-merge skill, and the dependency-policy comment noting the sharp-skew guard is now the only pre-merge signal for that class of defect.

Merge checklist

  • Merge this PR through the current nine contexts.
  • Immediately update protection to the seven contexts above, or subsequent PRs will sit BLOCKED with nothing failing.
gh api -X PATCH repos/OpenCoven/coven-cave/branches/main/protection/required_status_checks   -f 'contexts[]=Frontend build' -f 'contexts[]=Rust check'   -f 'contexts[]=E2E (Playwright)'   -f 'contexts[]=Cross-environment (ubuntu-latest)'   -f 'contexts[]=Cross-environment required'   -f 'contexts[]=Sidecar runtime (ubuntu-latest)'   -f 'contexts[]=Sidecar runtime required'

Follow-on

This is a safe subset of the approved release-candidate CI design (cave-7kix8, "approved design, not yet implemented"), whose Phase 1 reduces enforcement further to a single PR checks context once the release-candidate promotion gate exists. That larger change is not attempted here.

Bead: cave-jtuu7

Every pull request scheduled 19 jobs, three of them Windows. On 2026-08-10 the
repository held 39 queued Actions runs with ZERO in progress, the oldest stuck
since 2026-07-18, while GitHub Actions was globally operational and a hosted
runner completed an unrelated job mid-stall. The queue, not the suite, was the
blocker: every required check on every open PR sat pending indefinitely.

macOS was already pulled from these matrices for exactly this reason. Windows
now follows it, for the same reason:

- `conformance` and `sidecar-runtime` matrices are ubuntu-only on pull_request
  and ubuntu+windows on every other event.
- `windows-native` skips on pull_request.

`main` keeps identical platform coverage — all three Windows legs run in full on
push to `main`, and `release.yml` still builds the Windows MSI. Only the
per-PR fan-out shrinks, by three jobs billed at 2x.

Two consequences had to be handled together, because either one alone wedges
every PR:

- `sidecar-runtime-required` failed unless `windows-native` reported `success`.
  It now accepts `skipped` on pull_request ONLY, and still hard-fails a skip on
  any other event — that is the event whose coverage the rollup guarantees.
  Both rollups are already `if: always()`, so they keep reporting.
- Branch protection required `Cross-environment (windows-latest)` and
  `Sidecar runtime (windows-latest)`. Those contexts no longer report on a pull
  request, and a required context that never reports is precisely the
  BLOCKED-with-nothing-failing wedge documented in CLAUDE.md. Protection drops
  to the seven contexts that always report; the two `*-required` rollups stay
  required, so Windows still gates `main`.

`scripts/ci-recovery-workflow.test.mjs` pins the new shape. Its exact-string
SHA-guard assertion becomes structural: a job may narrow itself further, but
only by ANDing onto the guard, and a top-level `||` in the suffix is rejected so
the head-moved protection cannot be weakened. Verified by mutation — reverting
the PR skip, letting the rollup accept a skip on any event, and swapping the
`&&` for `||` each fail a distinct assertion.

Follows the approved release-candidate CI design
(docs/superpowers/specs/2026-08-05-release-candidate-ci-design.md, cave-7kix8),
which reduces enforcement further to a single `PR checks` context once the
release-candidate promotion gate exists.
Copilot AI lite review requested due to automatic review settings August 10, 2026 11:25
@BunsDev BunsDev self-assigned this Aug 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces GitHub Actions queue pressure by removing Windows runners from the pull_request CI fan-out while preserving Windows coverage on push to main and release, and updates the documentation/tests that pin the required-status-check contract.

Changes:

  • Update .github/workflows/ci.yml so conformance and sidecar-runtime run ubuntu-only on PRs and ubuntu+windows elsewhere; windows-native is skipped on PRs, with the rollup accepting skipped only for PR events.
  • Extend scripts/ci-recovery-workflow.test.mjs to pin the SHA-guard shape per job and assert the new PR-only Ubuntu policy and rollup gating semantics.
  • Update CLAUDE.md and the branch-to-merge skill/contract test to reflect seven required contexts (dropping the two Windows leg contexts).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/dependency-policy.test.mjs Updates the sharp-skew guard commentary to reflect Windows no longer running on PR CI.
scripts/ci-recovery-workflow.test.mjs Adds structural assertions for SHA-guarding and the new Windows/PR fan-out policy.
scripts/branch-to-merge-contract.test.mjs Adjusts the contract test to match the updated required-check wording in CLAUDE.md.
CLAUDE.md Updates branch-protection documentation from nine to seven required contexts and explains the Windows-on-PR change.
.github/workflows/ci.yml Implements the actual CI policy change (Ubuntu-only on PR for affected matrices; skip windows-native on PR; rollup gating update).
.agents/skills/branch-to-merge/SKILL.md Updates the operational skill to list seven required checks and removes PR-time Windows contexts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +58 to +81
const SHA_GUARD = "github.event_name != 'workflow_dispatch' || github.sha == inputs.expected_sha";

// Every job must refuse a recovery dispatch whose branch head has moved. A job
// may narrow itself further (for example, skipping on pull_request to keep a
// paid runner off the PR fan-out), but only by ANDing extra conditions onto the
// guard. A top-level `||` in the suffix could re-admit a stale dispatch, so the
// shape is pinned rather than merely searched for.
function assertShaGuarded(jobName, condition) {
assert.equal(typeof condition, "string", `${jobName} must declare an if: condition`);
if (condition === SHA_GUARD) {
return;
}
const prefix = `(${SHA_GUARD}) && `;
assert.ok(
condition.startsWith(prefix),
`${jobName} must not run a recovery dispatch after the branch head moves`,
);
const suffix = condition.slice(prefix.length);
assert.ok(suffix.length > 0, `${jobName} must not AND the guard against an empty condition`);
assert.ok(
!suffix.includes("||"),
`${jobName} must not weaken the head-moved guard with a top-level disjunction`,
);
}
Comment thread scripts/dependency-policy.test.mjs Outdated
Comment on lines +55 to +58
// `Sidecar runtime (windows-latest)` CI leg catches it — and since 2026-08-10 that
// leg runs on push to `main` and on release, NOT on pull requests. So this guard is
// now the only pre-merge signal for the skew; it fails fast in the required
// Frontend-build check. Fix a divergence by aligning `sharp`
…x non-exhaustive switch compile error

Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
Copilot AI and others added 2 commits August 10, 2026 12:42
Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
@BunsDev

BunsDev commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Triage (queue sweep, not a review): the failure here is a deliberate guard test firing against this PR's own intent, so it needs an author decision rather than a rerun.

Frontend tests (app):

✖ Windows conformance runs the native harness parser and DryRun fixture
  AssertionError [ERR_ASSERTION]: the conformance matrix must retain a real Windows runner

Something in the repo asserts the conformance matrix keeps a real Windows runner, and this change moves Windows coverage off it. That's exactly the kind of regression the test was written to block — so either the guard's premise is now obsolete and should be updated in this PR (with the reasoning recorded), or the change needs to keep a Windows runner in the matrix.

Frontend build is failing too and is the sole required check, so it must go green regardless.

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.

3 participants