fix(ci): require fail-closed aggregate-check evidence

[luvs01]

Summary

• make Cross-platform CI start for every pull request while keeping expensive jobs behind the existing read-only changes filter;
• validate changes.outputs.ci as exactly true or false, re-emit only that validated value, and fail the producer job on missing or malformed output;
• keep an always-created aggregate ci result, including documentation-only pull requests whose expensive jobs are intentionally skipped;
• require positive, latest, GitHub-Actions-produced ci evidence and fail closed when the filtered response is missing, unreadable, foreign, pending, unsuccessful, conflicting, or truncated;
• grant the trusted readiness job the explicit checks: read permission required by its live check lookup;
• preserve the generic paginator harness while adding exact filtered-response cardinality coverage.

Why

The readiness gate previously treated missing aggregate evidence as green. Requiring a check without changing the trigger would deadlock documentation-only pull requests, because the workflow-level path filter created no ci result for them.

This change always creates the pull-request workflow, scopes costly work inside the read-only changes job, validates the filter output before downstream jobs consume it, and requires complete positive evidence before accepting the contributor CI claim.

Verification

• Bun 1.3.14: tests/ci-workflows.test.ts — 130/130 passed.
• Bun 1.4.0-canary.1 (b22e0e6d0): the same file — 130/130 passed.
• bun x tsc --noEmit: passed.
• bun scripts/privacy-scan.ts: passed.
• git diff --check: passed.
• Independent full-file reviews: no diff-introduced P0-P3 findings.
• Codex Security diff scan: complete coverage of all four changed files, 0 reportable findings, no deferred work.
• Windows full suite: attempted, not passed, and not counted as verification. The run hit unrelated pre-existing Windows/Bun isolate and storage-fixture failures before Bun 1.3.14 terminated with an internal assertion.

Security boundary

The readiness writer remains a pull_request_target workflow executing trusted default-branch code. Untrusted values are transported through step environment variables instead of shell-source interpolation. The Cross-platform CI workflow retains read-only repository permissions for pull requests, uses hosted runners for PR jobs, exposes no secrets, and keeps the persistent self-hosted Windows lane dispatch-only.

This GitHub Actions policy change remains Draft pending exact-head upstream CI and the repository-required maintainer security review.

Readiness checklist

• All CI tests are green on my local testing.
• I pushed my PR to the latest dev commit.
• I resolved all correct Codex and CodeRabbit findings.
• My PR is ready for review.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0b78f9af-5156-47a1-8bc5-02990c772a7d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚠️ Deterministic hygiene checks failed.

• unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: .github/workflows/enforce-pr-target.yml.

[luvs01]

Updated on exact head 9d2839446509ecf46355979523383bdec3765e74.

• Added strict true|false validation and re-emission for the CI scope output, so a missing or malformed filter result fails the changes job instead of silently skipping all expensive jobs.
• Replaced the impossible decoy-pagination fixture with complete filtered check evidence, added fail-closed total_count mismatch coverage, and preserved the generic paginator harness.
• Added explicit checks: read permission to the readiness job.
• Merged current dev 14e94852517875592d4a228d09ff010ec73b55f2; the PR is 0 commits behind.
• Bun 1.3.14 and Bun 1.4.0-canary.1 focused runs both pass 130/130; typecheck, privacy scan, and diff check pass.
• Final Codex Security diff scan completed with all four files covered and 0 reportable findings.

The PR remains Draft. Exact-head upstream Actions and maintainer security review are still required; the Windows full suite is not claimed green.

[lidge-jun]

Reviewed this one closely because it changes when CI runs at all, which MAINTAINERS.md puts in the security-review class. The problem it solves is real — a docs-only PR currently produces no ci check rather than a passing one, which is harmless today and a trap the moment the check becomes required. Moving scope gating into changes is the right shape.

One blocker before it can land, and it is in the fail-closed property the PR is named after.

changes.outputs.ci is forwarded without validation. It is exposed as ${{ steps.filter.outputs.ci }} and every expensive job gates on needs.changes.outputs.ci == 'true'. If changes succeeds but that output is missing or malformed — a dorny/paths-filter upgrade renaming an output, a filter-syntax slip, an edge case where the step yields nothing — then:

1. every expensive job evaluates '' == 'true' → skipped;
2. the aggregate ci gate treats skipped as a pass, deliberately and correctly, since that is how it recognises trigger-scoped jobs;
3. ci reports green with nothing having been tested.

The changes job failing is handled fine — the aggregate catches it. It is changes succeeding with an unusable output that slips through. Right now paths in the trigger makes this impossible to reach; after this PR the output becomes the single point of truth, so it needs to be load-bearing on purpose.

Suggested fix inside changes, after the filter step:

      - name: Assert the scope output is usable
        shell: bash
        run: |
          set -euo pipefail
          case "${{ steps.filter.outputs.ci }}" in
            true|false) ;;
            *) echo "::error::changes.outputs.ci was '${{ steps.filter.outputs.ci }}', expected true or false"; exit 1 ;;
          esac

That turns the silent-skip case into a failing changes, which the aggregate already handles correctly.

Two smaller notes:

• The harness scenario with 100 decoy-* checks does not model the real request. Production queries with check_name: "ci" and filter: "latest", so the API does not return that population — the pagination path may be unreachable under the narrowed query. Either exercise the filtered endpoint semantics or drop the pagination claim.
• This overlaps enforce-pr-target.yml with fix(ci): promote workflow comment-spam hardening to main - Hotfix #1265, which merged into main but is not on dev (dev and that commit are diverged, 6 ahead / 155 behind). Whichever lands second inherits a conflict, so the ordering needs deciding before either is republished.

Holding rather than approving. CI here is cancelled rather than failing, so there is no failure of yours to chase — the base branch had an unstable shard today. Happy to take the validation step myself if you would rather not respin.

[lidge-jun]

Two corrections to my comment above, one of which is a defect in the snippet I suggested.

The validation step I wrote interpolates the expression directly into shell. case "${{ steps.filter.outputs.ci }}" in … substitutes the value into the script before bash parses it, which is the same shape as the injection pattern this repository's workflow hardening exists to avoid. That the value comes from a SHA-pinned action makes it low risk here, not correct. Pass it through the environment instead, so bash sees data rather than source:

      - name: Assert the scope output is usable
        shell: bash
        env:
          CI_SCOPE: ${{ steps.filter.outputs.ci }}
        run: |
          set -euo pipefail
          case "$CI_SCOPE" in
            true|false) ;;
            *) echo "::error::changes.outputs.ci was '$CI_SCOPE', expected true or false"; exit 1 ;;
          esac

Worth being explicit about what this does and does not buy: it closes the malformed output hole — empty string, renamed output, unexpected value — and nothing more. It cannot tell you a syntactically valid false is the semantically right classification. That still rests on the pinned dorny/paths-filter and on the filter-list tests, which is the part worth keeping direct coverage on.

I also overstated the #1265 overlap. I said whichever of #1259 and #1265 lands second inherits a conflict. I checked the branch relationship, not the hunks — both touch enforce-pr-target.yml, but I did not show their changed regions actually collide. Accurate statement: they may conflict, and the integration order needs deciding, with any conflict resolved at that point.

The main blocker stands unchanged: changes succeeding with an unusable outputs.ci currently ends in a green ci with nothing tested.