Skip to content

Require a unique full PR gate before merging - #28632

Merged
nwparker merged 4 commits into
mainfrom
nwparker/ci-unique-merge-gates
Oct 14, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/ci-unique-merge-gates

Conversation

@nwparker

@nwparker nwparker commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Require the complete PR checks under a new, unique name, so a passing Mobile check cannot unlock a PR whose main checks are pending or failed.

What Changed

Name the full PR aggregate PR checks passed, keeping its internal job ID and all dependencies unchanged. Move the workflow-name guard from #28607 into the always-required repository checks and reserve the context exclusively for the full PR aggregate across all workflows. The sparse checkout includes the workflow files; the pinned Bun parser runs without installing root dependencies. Document an agent merge rule that verifies the exact commit and workflow before merging, even while repository CI requirements are optional.

Why

#28543 merged after Mobile's verify passed while the full PR gate was still running and subsequently failed on obsolete translations. #28607 has merged after its full gate passed; it repairs the translations and renames Mobile to Mobile verify. A fresh required PR context also prevents older runs that still published the ambiguous verify from satisfying restored protection.

Linked Issue

Follow-up to #28543 and #28607, requested by a maintainer in the CI incident thread.

Visual Proof

N/A — CI naming and agent instructions only; no application UI or behavior changes.

Testing

  • 259 focused workflow, readiness, routing, preflight-admission and real aggregate-gate tests pass.
  • Deliberately assigning the required context to Mobile makes the regression guard fail.
  • The actual always-required CLI rejects missing contexts and contexts assigned to the lightweight detector; candidate files restored.
  • The guard passes in a real sparse filesystem fixture with no node_modules; tests cover both YAML parsers and unconditional wiring.
  • Existing aggregate tests reject failed, missing, cancelled and wrongly skipped required jobs.
  • Formatting, focused lint and git diff --check pass; CI exercises the changed workflow.
  • Verified incident timestamps and ruleset history. The full aggregate still depends on static analysis/typecheck, unit tests, conditional compatibility checks and packaging.

Review

The rename changes only the displayed status-check context. The ownership guard runs before path classification and readiness reuse, so workflow-only changes cannot skip it. Job-ID references and readiness reuse remain valid; source and workflow identity are still checked before reuse. Explicit user-authorized emergency exceptions remain possible; agents otherwise wait for the required aggregate on the exact head.

Review note: root pnpm test already delegates to config/scripts/run-vitest.mjs, which starts Bun and reports the pinned-runtime prerequisite when it is absent. The unit workflow installs that version before its test step. Cloud CI runs from cloud/ with its separate test suite, so it does not collect this root guard. The new test retains strict validation of the Bun entrypoint rather than skipping it.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied.

Notes

The prepared ruleset update requires PR checks passed and repository guards from GitHub Actions. It preserves the previous non-strict update policy and all other current protections. It will be applied only after the fix passes full CI, as authorized by the maintainer.

Checklist

  • Small and focused; before/after, mechanism and rationale explained
  • N/A visual proof with reason
  • Self-reviewed; platform and remote behavior unchanged
  • Full required PR gate passes before merge

Deployment verification

Merged after the full PR checks passed gate succeeded on exact head da45959f0e03fe5e28087e02a4df5202d7082bc7. The first run had one unchanged hidden-Electron native-control popup test fail; its unchanged shard rerun passed. No test or timeout was changed.

Restored the active main ruleset to require PR checks passed and detect changes and check repository guards, both from GitHub Actions (15368). Verified every other live protection unchanged and observed the previously nonblocking failed #28580 become blocked. The guard runs before path filtering and readiness reuse; the hosted setup took 2 seconds and validation finished within one second.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

The PR adds a script that checks workflow files for a single PR checks passed job, identified as verify in pr.yml. The code_paths job runs the check after setting up Bun. Tests cover the validator, its Bun execution, and the CI configuration. Merge guidance adds instructions for verifying checks on the exact commit.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to da459

Local tests that include the new check can fail on machines without Bun. CI has the prerequisite, so this is a bounded test-workflow risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly summarizes the primary change: requiring a unique full pull request gate before merging.
Description check Passed The description is detailed, on-topic, and covers the change, rationale, testing, visual proof, review notes, scope boundary, and checklist. The linked issue section references related pull requests r…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@nwparker nwparker changed the title Require full PR checks before agent merges Require a unique full PR gate before merging Oct 14, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 00068117-7711-46a9-8940-48f6044e3029
📥 Commits

Reviewing files that changed from the base of the PR and between b83a6f7 and da45959.

📒 Files selected for processing: 4
  • .github/scripts/check-required-pr-context.mjs
  • .github/workflows/pr.yml
  • AGENTS.md
  • config/scripts/required-pr-check-name.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@nwparker
nwparker merged commit f31aada into main Oct 14, 2026
65 of 67 checks passed
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.

1 participant