Repository navigation
ci: run main's checks to completion for every merge - #1589
Conversation
A push to main shared one concurrency group with the next push, so a merge that landed while the previous main run was still queued replaced it: #1573's main run on 586a3b8 was cancelled by #1576's merge and never said whether #1573 broke main. ci.yml and windows-install.yml now group a push by its commit; pull requests keep one group per branch and still cancel their superseded run. Adapted from oh-my-openagent's ci.yml concurrency (code-yeongyu/oh-my-openagent#10638).
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
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 1 files. (2 skipped: 2 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: ensuring that CI checks for every merge to main run to completion. Description check The description includes the required Change and Validation sections. It explains the problem, resulting behavior, affected workflows, pull-request behavior, rationale, cost, and test results. Verify … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required Change and Validation sections. It explains the problem, resulting behavior, affected workflows, pull-request behavior, rationale, cost, and test results. Verify the referenced incident date because October 22, 2026 is after the current date, October 21, 2026.
-
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 1 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- 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.
Comment @coderabbitai help to get the list of available commands.
Review of #1589: windows-install.yml runs on pull requests, a schedule and dispatch only, so the per-commit group never applied there. Only ci.yml (which holds the required Native Windows install job) runs on pushes to main.
Review of #1589: the text match also passed with the old per-branch group behind a comment, without the closing braces, or without the workflow name. The test now parses ci.yml, evaluates the group for three pushes and two runs of one pull request, and checks the push trigger; each of those mutants fails it.
Review of #1589 round 2: the JavaScript evaluator differs from GitHub's expression rules (case-insensitive ==) and its fixtures missed per-PR and per-caller isolation. The test now compares the parsed group and cancel-in-progress values whole; every reviewer mutant (case, literal workflow name, shared PR group, commented group) fails it.
|
Merge pass: CodeRabbit reviewed the workflow change at fbcd0f7 with no actionable comments. Later commits only revert windows-install.yml (it has no push trigger) and rewrite scripts/ci-concurrency.test.ts, so no new CodeRabbit review was requested. omo-native-code-reviewer PASS on fbcd0f7 and dc5d485; Codex gpt-6.1-sol Fast (xhigh) reviewed fbcd0f7 and 433258a, and both of its test findings are addressed in dc5d485 (every reviewer mutant fails the test). |
#1667) ## Change `.github/workflows/ci.yml` now groups pushes to main the way oh-my-openagent's `ci.yml` does: - one concurrency group per ref (`validation-${{ github.workflow }}-${{ github.ref }}`); - `cancel-in-progress` is still `github.event_name == 'pull_request'`. So a push run of main is never cancelled. The running one finishes, and GitHub keeps only the newest waiting one, which covers the merges in between. `scripts/ci-concurrency.test.ts` pins the new values. ## Why #1589 grouped each push to main by its own commit, so every merge's main run finished and a red main named its merge. It counted the cost as none because Linux jobs ran on GitHub-hosted runners then. Since 2026-10-22 the self-hosted runners are back on (`CI_SELF_HOSTED_LABELS`), and that cost now lands on the PC: - 9 merges in one afternoon queued 9 full main runs. - Each takes runners and one of the PC's 3 shared check slots from open pull requests: 19 of 22 recent heavy lanes logged "the 3-run limit is reached" before they started. - Pages refused to deploy each run that main had already moved past ("Refusing to deploy outdated SHA"). ## Trade-off In a burst of merges, a merge whose run was still waiting gets no run of its own. The newest run covers it, so a red main names the merges since the last green run rather than one merge. Merges spaced further apart than one CI run each still get their own run. Unchanged: - Pull requests still cancel their superseded run. - Release and Pages call `ci.yml` through `workflow_dispatch`, which already used the ref group and is never cancelled. - `windows-install.yml` has no push trigger. ## Validation - `bun run check fast` passes (actionlint, and `scripts/ci-concurrency.test.ts` with the new values). - Read against `pages.yml` and `release.yml`. `pages.yml` is the only workflow that waits on CI, and it deploys only a successful run of main. CI only; no changelog fragment.
Change
Every merge to
mainnow gets a CI run that finishes, so a redmainpoints at the merge that caused it..github/workflows/ci.ymland.github/workflows/windows-install.yml(the two required workflows that run on pushes tomain): a push is grouped by its commit (github.sha) instead of the branch. A later merge can no longer replace a queuedmainrun.scripts/ci-concurrency.test.tspins both expressions.Why: on 2026-10-22, #1573's
mainrun (586a3b8) was still queued when #1576 merged and was replaced, so no run ever said whether #1573 alone brokemain. The merge pass judges a PR's failure againstmain's result for the same tests, which needs a finished run per merge.Cost: each merge's
mainrun now completes, so a busy merge hour runs moremainCI. Linux jobs run on GitHub-hosted runners now (theCI_SELF_HOSTED_LABELSrepository variable was removed the same day), so this does not load the maintainer's PC.Reference: oh-my-openagent's
ci.ymlconcurrency (code-yeongyu/oh-my-openagent#10638), which also never cancels a running push run.No changelog fragment: nothing users see changes.
Validation
bun test scripts/ci-concurrency.test.ts scripts/pages-gate.test.ts scripts/conventions.test.ts: 60 pass, 0 fail.