Skip to content

ci: run Check as a signal on ready main PRs - #7623

Merged
kixelated merged 4 commits into
mainfrom
quest/m0/ci/pr-check
Nov 2, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m0/ci/pr-check

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Check skips every main PR, losing the early changed-lane signal before the merge queue runs.

Approach

  • Run just ci against the fetched origin/main base for ready main PRs, including readiness transitions and subsequent pushes. Main drafts skip; release PRs retain --all, including drafts.
  • Keep cancellation on superseding events and cancel when a main PR returns to draft. Group by PR number so merging, which changes github.ref, also cancels the old run.
  • Keep the hosted runner, read-only token, non-persisted checkout credentials, and read-only cache setup. Main's live ruleset still requires only Queue; release still requires Check and Release gate. No ruleset changes.
  • Extend the existing CI event-policy tests, correct stale docs/comments, and remove the completed quest.

Impact

  • No public API or wire changes. Check is a signal only and never gates main's queue or its fixer.

Alternatives

  • Keep Check skipped or make it required: the recorded maintainer decision chose a signal on ready PRs using the gate's existing selection.
  • Keep ref-based concurrency: GitHub changes that ref on merge, losing the cancellation group.

Checks

  • Event matrix reproduced the missing ready-main run before the fix and passes afterward, covering ready/draft main PRs, release drafts, retargets, closes, and base selection.
  • Full local just ci passes in a clean Nix dev shell with mbx active (94 seconds on the final head), including all selected workflow, tooling, documentation, and worker checks.
  • quest check --base origin/main and git diff --check origin/main pass.

Review

  • Reviewer: Claude Code, Claude Opus 5.5 (claude-opus-5-5), medium effort, quest review --base origin/main, round 2.
  • Reviewed head: 361e6a20eef7164cc6ad19772d5b5dd26a550550. Findings: none.
  • Round 1: fixed fetched-base selection, stale root/infra README claims, and the branch-filter regression guard. Refuted skipping edit/readiness events: a skipped Check reports success and can replace a failed signal or required release result. Round 2 accepted both refutations.
  • Choices made without asking: retain edit reruns and run the release gate on readiness transitions in the shared workflow, preserving meaningful Check results; add converted-to-draft cancellation and group by PR number.

Follow-ups

  • No new quests. The broader credential migration remains with moq-dev/moq#7201: repository-level publishing secrets still exist. This workflow references only the existing MBX read key and enters no write Environment; cache-write's live branch policy excludes PR refs.
  • The inherited local environment still reproduces mbx 1.25.0 E2BIG in the Clippy-gate fixture. PATH deduplication alone does not clear it; the same fixture passes with mbx active when Nix starts from a clean outer environment. moq-dev/moq#7594 remains with its owner. No upstream changes or posts.

(Written by GPT-6.1 Sol)

kixelated and others added 3 commits November 2, 2026 03:27
Quest-Claim: 0f89e5cd158a75c1
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@kixelated kixelated changed the title Check runs on ready main PRs as a signal ci: run Check as a signal on ready main PRs Nov 2, 2026
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: main Check now selects against fetched origin/main instead of the event payload's base.sha. The payload can lag main when readiness or edit events run; the fetched branch gives the dispatcher the PR merge checkout's actual base ancestry. Release keeps --all. This change passed the full local gate and the second local review.

(Written by GPT-6.1 Sol)

@kixelated
kixelated marked this pull request as ready for review November 2, 2026 11:44

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: 361e6a2

No actionable bugs found. Reusing the existing dispatcher for a non-gating ready-main signal is appropriately small; fetched-base selection, release's full checks, and PR-number cancellation handle readiness, retargeting, and close events without a separate CI implementation. The event-matrix tests cover the policy changes, and read-only token/cache handling is preserved.

Verification limits: static GitHub review of the workflow, selector, cache action, tests, and documentation. I did not run tests or observe live Actions cancellation, and did not independently verify live ruleset or secret configuration.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

Reviewed the full head 361e6a2 (11 files): the ready-main Check signal, the fetched-base selection, the PR-numbered concurrency group, the rewritten event-policy tests, the doc/comment updates, and the deleted quest.

  • Ready-main signal — check.yml runs the gate's selection against origin/main for non-draft main PRs and --all for release, with closed still skipped.
  • Readiness triggers — ready_for_review and converted_to_draft added so a return to draft cancels the in-flight run while release keeps its required Check on every transition.
  • Concurrency keyed by PR number — verified against GitHub's docs that a merged pull request's github.ref becomes the merge target branch, so the old check-${{ github.ref }} group would miss the close cancelling the PR's own run.
  • Tests — the event matrix now sweeps action × base × draft and pins permissions, the absent Environment, and the checkout's fetch-depth/persist-credentials.
  • Docs and quest — nightly.yml, quest.yml, CONTRIBUTING.md, README.md, doc/setup/dev.md, and infra/README.md reworded to "signal on ready main PRs"; quest/m0/ci/pr-check.md deleted with no dangling references.

Local bun test sh/gh/queue.test.ts and sh/gh/setup.test.ts pass. Main-PR runs still use the read-only MBX_R2_* pair and name no cache-write Environment, so the secret posture is unchanged from release PRs.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5231e1e7-5783-4001-9814-88ac1ab894a0
Commits

Reviewing files that changed from the base of the PR and between f8c608a and 361e6a2.

Files selected for processing: 11
  • .github/workflows/check.yml
  • .github/workflows/nightly.yml
  • .github/workflows/quest.yml
  • CONTRIBUTING.md
  • README.md
  • doc/setup/dev.md
  • infra/README.md
  • quest/m0/ci/README.md
  • quest/m0/ci/pr-check.md
  • sh/gh/queue.test.ts
  • sh/gh/setup.test.ts
Files with no reviewable changes: 2
  • quest/m0/ci/pr-check.md
  • quest/m0/ci/README.md

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

Walkthrough

Check now runs for non-draft pull requests targeting main and for pull requests targeting release. It handles readiness and draft-status events, uses origin/main or --all as the CI base, and keys concurrency by pull-request number. Tests cover the workflow conditions and checkout settings. CI documentation now distinguishes Check from the merge queue gate.

Merge Risk: Minimal · up to 361e6

No actionable merge-blocking risk was identified.

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 with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: running Check as a signal for ready main pull requests.
Description check Passed The description directly explains the problem, implementation, impact, testing, and follow-ups for the workflow changes.
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.
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 with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Biome (2.5.15)
sh/gh/queue.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

sh/gh/setup.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

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

@kixelated
kixelated added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit c7da5b6 Nov 2, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m0/ci/pr-check branch November 2, 2026 16:47
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