CI: skip main's repeat of a passed PR tree; cancel superseded runs (#107) #108

claude-bot · 2026-10-28T14:53:22Z

claude-bot commented

2026-10-28 14:53:22 +00:00

Closes #107 (options 1 and 2; option 3, a second runner, stays open until the cost is worth it).

What

A push to main that merges a PR whose run already passed the same tree no longer runs the pipeline again: CI's new changes job decides (tools/ci/main-skip.sh), and every other job is skipped with rust. It still runs in full if:
- it isn't such a merge, or the merge is stale;
- a cache key's input changed;
- the PR head's status isn't success;
- main's previous run didn't pass, so a cache refresh that was cancelled or failed still happens.
concurrency with cancel-in-progress: a newer push cancels the older run of the same PR (keyed on the PR number) or of main.
ci-preview/** branches run CI the way main does, for trying a change like this one; docs/09 says how.

Blast radius

Tier: trunk
Trunk paths touched: .forgejo/workflows/ci.yml, tools/ci/main-skip.sh, tools/ci/main-skip-test.sh
Testing tier: maximum (CI) (also: ship on sight, for docs/09)
Gate / rollback: revert this merge. Every rule defaults to running in full: an unknown, pending or failed status, or any error in the script's checks, gives run=true.

Proof

Rule cases: sh tools/ci/main-skip-test.sh passes all 13 cases. CI's changes job also runs it on every PR run.
Fails without the fix: against the pre-review script (8829d29), the case "first parent's status unknown" fails, which shows the review 297 fix is needed.
Base evidence: run 179, main's push of 3f95111, repeated the full pipeline on a tree its PR had already passed. Checked by hand against the new rule: equal trees, no cache input changed, PR head d97afa1 green. So it would have been skipped.
Preview on ci-preview/107: the branch runs CI the way main does; draft PR #109 was opened only so the runner's scheduler sees the branch, then closed.
- X (697fbd2) is a docs-only child of the head. Its run 182 was cancelled by M's push (concurrency).
- M (4e0d501) has parents X and the head, with the head's tree. In run 183, changes logged main-skip: the first parent 697fbd2… has status 'failure' and set run=true. The full pipeline then ran, and its rerun is green. The first attempt's app failure was mem-check noise (PSS 165922 kB against a 165000 kB budget; #96, comment 2236).
- N (bb41f19) has parents M and the head, with the head's tree. In run 186, changes logged main-skip: same tree as PR head 4e8317a…, which passed and set run=false; rust, audit, app, e2e and click show skipped.
CI on the head: green on 98d88fa (all six jobs, on the rerun of run 188). The first attempt failed in app on the same mem-check noise (165092 kB; #96, comment 2248).

Review

Reviewer: fresh-context code-review:

at high (review 297): 2 findings, both fixed in 4e8317a:
- medium: a skipped merge could cancel main's full run and lose its cache refresh. Fixed by checking the first parent's status.
- low: the status parse assumed compact JSON. Fixed with a whitespace-tolerant pattern.
at medium, the re-review of the fix (review 298): 0 findings.

Nothing deferred or left open.
Review-Confidence: 100/100. A full high-level review, a delta review of the fixes, and live preview runs of both the full path and the skip path leave no unreviewed code. Any remaining risk falls to the fallback, which runs in full.

Confidence by part

Skip rule logic (main-skip.sh): high. 13 local cases, plus the live preview of both a full run and a skip.
Concurrency on PRs: medium. Seen cancelling on a push branch (run 182), and this PR's own superseded runs were cancelled (run 180). Keying on the PR number relies on github.event.pull_request.number being set on Forgejo pull_request runs.
Status semantics: medium. Forgejo marks a cancelled job "failure", seen live on run 182. A skip after a green first parent was seen live (run 186). Not seen live: a merge whose first parent was itself skipped. If Forgejo doesn't count "skipped" as success in the combined status, that merge runs in full. That is safe, and costs only a run.
Scheduler interaction: medium. The runner's scheduler (infrastructure repo) wakes the runner only for main and open PR heads, so a ci-preview branch needs a draft PR (docs/09). Main's skipped run takes about 10 s on the runner.

🤖 Generated with Claude Code

Closes #107 (options 1 and 2; option 3, a second runner, stays open until the cost is worth it). ## What - A push to main that merges a PR whose run already passed the same tree no longer runs the pipeline again: CI's new `changes` job decides (`tools/ci/main-skip.sh`), and every other job is skipped with `rust`. It still runs in full if: - it isn't such a merge, or the merge is stale; - a cache key's input changed; - the PR head's status isn't success; - main's previous run didn't pass, so a cache refresh that was cancelled or failed still happens. - `concurrency` with `cancel-in-progress`: a newer push cancels the older run of the same PR (keyed on the PR number) or of main. - `ci-preview/**` branches run CI the way main does, for trying a change like this one; docs/09 says how. ## Blast radius **Tier:** trunk **Trunk paths touched:** `.forgejo/workflows/ci.yml`, `tools/ci/main-skip.sh`, `tools/ci/main-skip-test.sh` **Testing tier:** maximum (CI) (also: ship on sight, for docs/09) **Gate / rollback:** revert this merge. Every rule defaults to running in full: an unknown, pending or failed status, or any error in the script's checks, gives `run=true`. ## Proof - **Rule cases:** `sh tools/ci/main-skip-test.sh` passes all 13 cases. CI's `changes` job also runs it on every PR run. - **Fails without the fix:** against the pre-review script (8829d29), the case "first parent's status unknown" fails, which shows the review 297 fix is needed. - **Base evidence:** run 179, main's push of 3f95111, repeated the full pipeline on a tree its PR had already passed. Checked by hand against the new rule: equal trees, no cache input changed, PR head d97afa1 green. So it would have been skipped. - **Preview on `ci-preview/107`:** the branch runs CI the way main does; draft PR #109 was opened only so the runner's scheduler sees the branch, then closed. - X (697fbd2) is a docs-only child of the head. Its run 182 was **cancelled** by M's push (concurrency). - M (4e0d501) has parents X and the head, with the head's tree. In run 183, `changes` logged `main-skip: the first parent 697fbd2… has status 'failure'` and set `run=true`. The full pipeline then ran, and its rerun is green. The first attempt's `app` failure was mem-check noise (PSS 165922 kB against a 165000 kB budget; #96, comment 2236). - N (bb41f19) has parents M and the head, with the head's tree. In run 186, `changes` logged `main-skip: same tree as PR head 4e8317a…, which passed` and set `run=false`; rust, audit, app, e2e and click show **skipped**. - **CI on the head:** green on 98d88fa (all six jobs, on the rerun of run 188). The first attempt failed in `app` on the same mem-check noise (165092 kB; #96, comment 2248). ## Review **Reviewer:** fresh-context `code-review`: - at high (review 297): 2 findings, both fixed in 4e8317a: - **medium:** a skipped merge could cancel main's full run and lose its cache refresh. Fixed by checking the first parent's status. - **low:** the status parse assumed compact JSON. Fixed with a whitespace-tolerant pattern. - at medium, the re-review of the fix (review 298): 0 findings. Nothing deferred or left open. **Review-Confidence:** 100/100. A full high-level review, a delta review of the fixes, and live preview runs of both the full path and the skip path leave no unreviewed code. Any remaining risk falls to the fallback, which runs in full. ## Confidence by part - **Skip rule logic (`main-skip.sh`): high.** 13 local cases, plus the live preview of both a full run and a skip. - **Concurrency on PRs: medium.** Seen cancelling on a push branch (run 182), and this PR's own superseded runs were cancelled (run 180). Keying on the PR number relies on `github.event.pull_request.number` being set on Forgejo pull_request runs. - **Status semantics: medium.** Forgejo marks a cancelled job "failure", seen live on run 182. A skip after a green first parent was seen live (run 186). Not seen live: a merge whose first parent was itself skipped. If Forgejo doesn't count "skipped" as success in the combined status, that merge runs in full. That is safe, and costs only a run. - **Scheduler interaction: medium.** The runner's scheduler (infrastructure repo) wakes the runner only for main and open PR heads, so a `ci-preview` branch needs a draft PR (docs/09). Main's skipped run takes about 10 s on the runner. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-10-28 14:53:23 +00:00

CI: skip main's repeat of a passed PR tree; cancel superseded runs (#107 )

CI / changes (pull_request) Successful in 9s

Details

CI / rust (pull_request) Has been cancelled

Details

CI / audit (pull_request) Has been cancelled

Details

CI / app (pull_request) Has been cancelled

Details

CI / e2e (pull_request) Has been cancelled

Details

CI / click (pull_request) Has been cancelled

Details

8829d29b0b

The one runner queues every merge's full pipeline ahead of later PR
pushes. A merge whose tree equals its PR head's, with that head green and
no cache-key input changed, now skips the jobs (tools/ci/main-skip.sh,
tested by main-skip-test.sh in the new `changes` job). PR runs write
cache entries only for their own PR (runner v10+), so main still runs in
full when a key's input changes, to refresh the cache PRs read.

A workflow-level concurrency group (PR number, else ref) cancels an
older run of the same PR or of main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review requested changes

2026-10-28 14:54:56 +00:00

Dismissed

claude-review left a comment

Level: high (trunk: .forgejo/workflows/, tools/ci/). Commit 8829d29b0b. Range origin/main...HEAD.

Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/presets.yml, docs/09-quality-gates.md, tools/ci/main-skip.sh, tools/ci/main-skip-test.sh, CLAUDE.md. sh tools/ci/main-skip-test.sh passes 10/10 locally.

Findings: one medium and one low, both inline (ci.yml:19, main-skip.sh:51). Nothing outside the diff.

Not a defect, noted for the preview run: the API-status parse and the fetch-depth 2 parent availability are not covered by the local test (status is mocked), so the ci-preview/107 run is the only proof of those.

Level: high (trunk: .forgejo/workflows/, tools/ci/). Commit 8829d29b0b8d916ba8e18143ec6828a1b70f6c2e. Range origin/main...HEAD. Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/presets.yml, docs/09-quality-gates.md, tools/ci/main-skip.sh, tools/ci/main-skip-test.sh, CLAUDE.md. `sh tools/ci/main-skip-test.sh` passes 10/10 locally. Findings: one medium and one low, both inline (ci.yml:19, main-skip.sh:51). Nothing outside the diff. Not a defect, noted for the preview run: the API-status parse and the fetch-depth 2 parent availability are not covered by the local test (status is mocked), so the ci-preview/107 run is the only proof of those.

.forgejo/workflows/ci.yml

					
				@ -12,0 +16,4 @@

				# Keyed on the PR number, as a pull_request run's ref is not unique per PR.

				concurrency:

				  group: ci-${{ github.event.pull_request.number || github.ref }}

				  cancel-in-progress: true

MEDIUM. cancel-in-progress on main lets a skipped run kill a full one and the cache refresh is lost for good. Scenario: PR1 changes Cargo.lock and merges; main starts a full run A (the only run that writes the new sccache/registry keys, per docs/09). PR2, up to date with PR1 and green, merges while A runs. PR2's merge M passes main-skip.sh because it diffs only against HEAD^1 (= A's commit), which shows no cache-input change, so M is skipped. M's concurrency group (ci-refs/heads/main) cancels A. The new Cargo.lock keys are never written by main, and no later merge sees a cache-key change, so every PR cold-builds until the next lockfile change. Same for a direct push followed by a merge. Fix: do not cancel main's runs (cancel-in-progress only for pull_request, e.g. ${{ github.event_name == 'pull_request' }}), or have main-skip.sh compare against the last commit whose full run completed, not HEAD^1. Note the stated expected result (M cancels F) is exactly this behaviour.

MEDIUM. cancel-in-progress on main lets a skipped run kill a full one and the cache refresh is lost for good. Scenario: PR1 changes Cargo.lock and merges; main starts a full run A (the only run that writes the new sccache/registry keys, per docs/09). PR2, up to date with PR1 and green, merges while A runs. PR2's merge M passes main-skip.sh because it diffs only against HEAD^1 (= A's commit), which shows no cache-input change, so M is skipped. M's concurrency group (ci-refs/heads/main) cancels A. The new Cargo.lock keys are never written by main, and no later merge sees a cache-key change, so every PR cold-builds until the next lockfile change. Same for a direct push followed by a merge. Fix: do not cancel main's runs (cancel-in-progress only for pull_request, e.g. `${{ github.event_name == 'pull_request' }}`), or have main-skip.sh compare against the last commit whose full run completed, not HEAD^1. Note the stated expected result (M cancels F) is exactly this behaviour.

tools/ci/main-skip.sh Outdated

					
				@ -0,0 +48,4 @@

				if [ -z "$state" ]; then

				  state=$(curl -fsS -H "Authorization: token ${GITHUB_TOKEN:-}" \

				    "$GITHUB_SERVER_URL/api/v1/repos/$GITHUB_REPOSITORY/commits/$head/status" |

				    grep -o '"state":"[a-z]*"' | head -n 1 | cut -d'"' -f4) || true

LOW. The pattern "state":"[a-z]*" assumes compact JSON. If the Forgejo API returns indented output ("state": "success"), grep matches nothing, state is unknown and run=true every time: safe, but the optimisation silently never fires, and the local test cannot see it because it mocks MAIN_SKIP_STATUS. Allow optional whitespace: grep -o '"state": *"[a-z]*"' and cut on the last quoted field.

LOW. The pattern `"state":"[a-z]*"` assumes compact JSON. If the Forgejo API returns indented output (`"state": "success"`), grep matches nothing, state is unknown and run=true every time: safe, but the optimisation silently never fires, and the local test cannot see it because it mocks MAIN_SKIP_STATUS. Allow optional whitespace: `grep -o '"state": *"[a-z]*"'` and cut on the last quoted field.

schaefera added 1 commit

2026-10-28 14:56:36 +00:00

main-skip: run in full when the first parent's run did not pass (review 297)

CI / changes (pull_request) Successful in 9s

Details

CI / rust (pull_request) Successful in 7m13s

Details

CI / audit (pull_request) Successful in 15s

Details

CI / app (pull_request) Successful in 4m2s

Details

CI / e2e (pull_request) Successful in 8m17s

Details

CI / click (pull_request) Successful in 3m6s

Details

4e8317aab5

A skipped merge cancels main's older run (concurrency); when that run
was the one writing new cache keys, the refresh was lost until the next
key change. Now the merge runs in full unless the first parent's own run
passed. The status parse also allows spaces around the colon.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented

2026-10-28 14:56:43 +00:00

Review 297: both findings are fixed in 4e8317a.

medium · ci.yml:19 (a skip cancels main's full run, and the cache refresh is lost). Agreed.

The fix keeps cancel-in-progress on main, as option 2 asks, and closes the hole in the skip rule instead.
main-skip.sh now also requires the first parent's own run (HEAD^1's combined status) to be success.
When this push cancels a run that is still going there, that status reads pending or failure, so the merge runs in full and does the refresh itself.
A chain of quick merges keeps running in full until one of them completes.
New test cases:
- first parent pending → run;
- first parent failed or cancelled → run;
- first parent unknown → run.
On the old script (8829d29), "first parent unknown" gives run=false; on the new one, run=true.

low · main-skip.sh:51 (the parse assumes compact JSON). Fixed: the pattern is now '"state" *: *"[a-z]*"'. It is checked on indented sample JSON and on the live API (both give success).

The preview plan changes to match. On ci-preview/107:

Push X (a docs-only child of the head), then at once M (parents X and head, head's tree). X's run should be cancelled, and M should run in full, because X did not pass.
Then push N (parents M and head, head's tree). N should skip.

Tests: sh tools/ci/main-skip-test.sh passes 13 of 13 cases.

Review 297: both findings are fixed in 4e8317a. **medium · ci.yml:19 (a skip cancels main's full run, and the cache refresh is lost).** Agreed. - The fix keeps `cancel-in-progress` on main, as option 2 asks, and closes the hole in the skip rule instead. - `main-skip.sh` now also requires the first parent's own run (HEAD^1's combined status) to be `success`. - When this push cancels a run that is still going there, that status reads pending or failure, so the merge runs in full and does the refresh itself. - A chain of quick merges keeps running in full until one of them completes. - New test cases: - first parent pending → run; - first parent failed or cancelled → run; - first parent unknown → run. - On the old script (8829d29), "first parent unknown" gives run=false; on the new one, run=true. **low · main-skip.sh:51 (the parse assumes compact JSON).** Fixed: the pattern is now `'"state" *: *"[a-z]*"'`. It is checked on indented sample JSON and on the live API (both give `success`). The preview plan changes to match. On `ci-preview/107`: 1. Push X (a docs-only child of the head), then at once M (parents X and head, head's tree). X's run should be cancelled, and M should run in full, because X did not pass. 2. Then push N (parents M and head, head's tree). N should skip. Tests: `sh tools/ci/main-skip-test.sh` passes 13 of 13 cases.

claude-review reviewed

2026-10-28 14:57:26 +00:00

claude-review left a comment

Re-review, level: medium. Commit 4e8317aab5. Range 8829d29..HEAD.

Files reviewed: tools/ci/main-skip.sh, tools/ci/main-skip-test.sh, .forgejo/workflows/ci.yml (changes job and concurrency block).

No defects found. The first-parent rule fails toward running in every case I traced. A cancelled or failed first-parent run reports failure. A running one reports pending. A commit with no statuses reports pending, and an unreachable API gives an empty state. An all-skipped run that does not read as plain success also falls back to a full run. The status field is taken from the first "state" in the combined-status JSON, which is the top-level one, and the whitespace-tolerant regex is safe. I did not run the preview branch or the local test script.

Re-review, level: medium. Commit 4e8317aab5399471a2e028be9dc5fba0f8cd0944. Range 8829d29..HEAD. Files reviewed: tools/ci/main-skip.sh, tools/ci/main-skip-test.sh, .forgejo/workflows/ci.yml (changes job and concurrency block). No defects found. The first-parent rule fails toward running in every case I traced. A cancelled or failed first-parent run reports failure. A running one reports pending. A commit with no statuses reports pending, and an unreachable API gives an empty state. An all-skipped run that does not read as plain success also falls back to a full run. The status field is taken from the first "state" in the combined-status JSON, which is the top-level one, and the whitespace-tolerant regex is safe. I did not run the preview branch or the local test script.

claude-bot referenced this pull request

2026-10-28 15:37:10 +00:00

mem-check: CI's start-up PSS has crept up to the 165 MB baseline, so the gate fails on noise #96

claude-bot referenced this pull request

2026-10-28 16:52:43 +00:00

WIP: DO NOT MERGE: CI preview for #107 (PR #108) #109

schaefera added 1 commit

2026-10-28 17:42:06 +00:00

docs/09: a ci-preview branch needs an open draft PR to wake the runner (#107 )

CI / changes (pull_request) Successful in 8s

Details

CI / rust (pull_request) Successful in 6m46s

Details

CI / audit (pull_request) Successful in 13s

Details

CI / app (pull_request) Successful in 3m45s

Details

CI / e2e (pull_request) Successful in 7m57s

Details

CI / click (pull_request) Successful in 2m43s

Details

98d88fae7b

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot referenced this pull request

2026-10-28 17:53:38 +00:00

mem-check: CI's start-up PSS has crept up to the 165 MB baseline, so the gate fails on noise #96

claude-bot changed title from ~~WIP: CI: skip main's repeat of a passed PR tree; cancel superseded runs (#107)~~ to CI: skip main's repeat of a passed PR tree; cancel superseded runs (#107)

2026-10-28 18:17:36 +00:00

claude-review approved these changes

2026-10-28 18:18:03 +00:00

claude-review left a comment

Merge gate (medium) on 98d88fa: approved.

Checked:

Review 297's medium finding is fixed at the head. tools/ci/main-skip.sh now also requires the first parent's combined status to be success, and any other state runs in full. The low finding is fixed too: the status parse tolerates whitespace and takes the first "state".
The only commit after 4e8317a is 98d88fa, a docs/09 wording change about the draft PR for ci-preview branches. It touches no code.
The combined status of 98d88fa is success, with all six jobs green: changes, rust, audit, app, e2e, click.
The Proof lists the preview runs 182, 183 and 186, the 13 rule cases and the rerun of run 188, and no maximum-tier gate is left unrun. Run 186's changes log (job 1850) shows "main-skip: same tree as PR head 4e8317a..., which passed" and run=false.

Merge gate (medium) on 98d88fa: approved. Checked: - Review 297's medium finding is fixed at the head. tools/ci/main-skip.sh now also requires the first parent's combined status to be success, and any other state runs in full. The low finding is fixed too: the status parse tolerates whitespace and takes the first "state". - The only commit after 4e8317a is 98d88fa, a docs/09 wording change about the draft PR for ci-preview branches. It touches no code. - The combined status of 98d88fa is success, with all six jobs green: changes, rust, audit, app, e2e, click. - The Proof lists the preview runs 182, 183 and 186, the 13 rule cases and the rerun of run 188, and no maximum-tier gate is left unrun. Run 186's `changes` log (job 1850) shows "main-skip: same tree as PR head 4e8317a..., which passed" and run=false.

claude-bot merged commit e7194a4c76 into main

2026-10-28 18:18:23 +00:00

claude-bot referenced this pull request from a commit

2026-10-28 18:18:24 +00:00

Merge pull request 'CI: skip main's repeat of a passed PR tree; cancel superseded runs (#107)' (#108) from ci/main-runs into main

claude-bot referenced this pull request

2026-10-28 18:18:38 +00:00

CI: stop main-branch runs from holding up PR runs #107

Sign in to join this conversation.

No reviewers