fix(ci): stop cancelling main's CI so the base branch is actually verified - #2124
Conversation
…ified `concurrency.cancel-in-progress` was a blanket `true`, so every push to main cancelled the previous main run. Measured over the last 30 pushes to main: 23 cancelled (77%), 6 success, 1 failure — main was unverified four times out of five. That is not a cost control, it is a hole in the safety net. Supersession is sound for a PR branch, where a newer head really does replace the work in flight. A merged commit has no newer head; cancelling its run skips no redundant work and simply discards the only verification main ever gets. Three defects from a single afternoon trace to it, each reaching a feature branch as first detection instead of the branch that introduced it: - The Lighthouse baseline drifted ~163ms on desktop /therapy-compass with no alert, so the next PR to complete a Lighthouse run inherited the drift as its own "regression" and was blocked by it. - Two @mockup assertions in ui-tools-search-mode-mockup.spec.ts broke on main and stayed broken until an unrelated PR surfaced them (repaired by #2115). - ci-triage could not baseline a single failure, reporting "Compared with main CI run #12038/#12062/#12087 (cancelled)" three times in one afternoon. Base-branch pushes are now exempt. `on.push.branches` is [main, release/**], so `event_name == 'push'` is precisely "a base-branch push"; pull requests and merge-queue runs keep cancel-in-progress unchanged. The cost is bounded to one concurrent run per merge, each already path-scoped by the `changes` job. A blanket `true` reads as harmless and is not, so `tests/ci-cache-safety.test.ts` pins both the exemption and the push-trigger scoping that gives it its meaning: widening the push trigger to feature branches would silently unscope it. AGENTS.md carried "do not disable cancel-in-progress" as unqualified guidance, which now contradicts the fix; scoped to pull requests, with the measurement and the reason the exemption must not be simplified away.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:47 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 102 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Uh oh!
There was an error while loading. Please reload this page.
Summary
concurrency.cancel-in-progresswas a blankettrue, so every push tomaincancelled the previousmainrun. Measured over the last 30 pushes tomain: 23 cancelled (77%), 6 success, 1 failure — the default branch was unverified four times out of five.mainever receives. Base-branch pushes are now exempt viacancel-in-progress: ${{ github.event_name != 'push' }}.on.push.branchesis[main, "release/**"], soevent_name == 'push'is precisely "a base-branch push"; pull requests and merge-queue runs are unchanged.desktop /therapy-compasswith no alert, so the next PR to complete a Lighthouse run inherited the drift as its own "regression" and was blocked by it (fix(home): give the shared mode home the full hero design on phones #2112).@mockupassertions inui-tools-search-mode-mockup.spec.tsbroke onmainand stayed broken until an unrelated PR surfaced them (repaired by test(tools): repair the three assertions the tools mockup spec had stale on main #2115).ci-triagecould not baseline a single PR failure, reportingCompared with main CI run #12038/#12062/#12087 (cancelled)three times in one afternoon.tests/ci-cache-safety.test.tspins the exemption and the push-trigger scoping that gives it meaning — wideningon.push.branchesto feature branches would silently unscope the exemption, so both are asserted in one case.AGENTS.mdcarried "do not disablecancel-in-progress" as unqualified guidance, which now contradicts the fix. Scoped to pull requests, with the measurement recorded and an explicit note not to simplify the exemption back to a blankettrue.Verification
npm run test:ci-workflows—Test Files 15 passed (15) / Tests 326 passed (326)npx vitest run tests/ci-cache-safety.test.ts—Tests 50 passed (50), re-run green after mergingmainnpm run check:github-actions—GitHub Actions pin check passed.npm run check:ci-scope—CI change scope self-test passed.(Mockup spec parity: 8 advisory specs all match mockupPatterns.)npx prettier --check .—All matched files use Prettier code style!The new contract test was confirmed to fail before it passed: its first version anchored on
on:, and the repo'ssourceSegmenthelper rejected it as ambiguous ("start marker occurs more than once … proves nothing about the rest of the file"). Re-anchored on the uniquepush:marker.Not run: browser, build, and provider-backed gates. This diff contains no application code — one workflow expression, one test, one documentation passage.
Risk and rollout
mainrun is no longer killed by the next merge. Worst case is concurrentmainruns, one per merge, each already path-scoped by thechangesjob — strictly cheaper than a defect landing unnoticed. PR and merge-queue concurrency is untouched, so the cancellation savings this repo relies on for feature branches are preserved.trueand behaviour is exactly as before.Notes
mainto go red for a while after this lands, and treat that as the fix working. The drift this was hiding is real:desktop /therapy-compasscurrently measures ~956 ms against a 793 ms baseline captured on 17 Aug, and no therapy-compass code has changed since — the growth came from shared chunks across the ~20 PRs merged in between. Deciding between refreshing the baseline and recovering the 163 ms is a separate, deliberate call that this PR intentionally does not make.mainitself failscheck:lighthouse-budgettoday (mobile-therapy-compassTBT +53.6%), with no feature-branch code present. That failure exists now; it has simply never been visible onmain.github.event_name != 'push'rather than agithub.refcomparison so it stays correct forrelease/**without enumerating branch patterns twice.Generated by Claude Code