[kyle-sexton]

Closes #12520
Refs: #12506

Summary

A job that finished every step but that GitHub recorded cancelled at its timeout-minutes no longer fails ci-status or evicts a healthy PR from the merge queue. This is the one required check, so the rule is deliberately narrow: a cancelled lane counts as passed only on proof that all its work ran.

Fix

• Each of the 12 lanes ci-status needs now ends in a Lane finished step. Its default if: success() means it runs only when no earlier step failed or was cancelled.
• ci-status's Check each skipped lane against scope step reads this run attempt's jobs when a lane is cancelled. The lane counts as passed only when each of its jobs (every matrix leg) passed, or was cancelled with Lane finished succeeded, no step worse than skipped, and Complete job last.
• A failed read, a lane with no job in the read, or a job without the step leaves the lane cancelled, so the aggregate stays red. With no cancelled lane there is no API call.
• Each cancelled lane writes a cancelled-after-success notice with counted-as-passed=true|false, the count to compare against the baseline on fix(ci): jobs recorded cancelled after all steps succeeded fail ci-status and evict PRs from the merge queue #12520.
• select-tests can count as passed under the rule, but the lanes it blocked stay skipped, so the run stays red. Their work never ran.

An earlier version used a 10 s gap after Complete job. It is dropped: jobs cut off between steps showed gaps up to 33 s in the measured sample (correction).

Verification

• Cause separated (measurement): in the merge queue, 31 of 31 such jobs ended at or past their limit (+1 s to +230 s, median +34 s), and a merge_group run's concurrency group is unique per run. The job logs end normally, so they do not show why GitHub left the job open.
• scripts/ci-ordered-skips.test.sh: 41 of 41 pass, running the step body with a stubbed jobs read. Rows cover a limit between two steps, a step cancelled mid-run, a failed step, a failed matrix sibling, a failed read, a name-prefix near match, a blocked select-tests, no API call without a cancelled lane, and every needed lane ending in Lane finished.
• Pre-push check-affected passed.
• Not shown yet: no real run has the new step, since saved runs predate it. From them I project 25 of the 29 affected merge-queue runs clearing, leaving 2 real failures and 2 select-tests hangs. The cancelled-after-success notices will show the real number.

Related

• ci: lint-repo hangs after its last step in the merge queue and is cancelled at the 7-minute limit #12506: the lint-repo instance of the same state, triaged a duplicate of fix(ci): jobs recorded cancelled after all steps succeeded fail ci-status and evict PRs from the merge queue #12520. Lane 1 closes it as a duplicate once this merges.
• feat(plugins): generate marketplace categories and tags from plugin.json #12493, chore(ci): read the post-merge mutation lane's config from docs/conventions/mutation-testing.yaml #12509: PRs evicted by this state.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQb9Q1Asjw331T5WUQHJyF

[kyle-sexton]

Second-reader review for Lane 1, at head 6b9cb15. Verdict: one small fix before merge, otherwise sound. I found no way for a lane with finished-but-unrun work to pass except the skipped branch below.

Fix before merge

A job that ran no steps can make a cancelled lane pass. In the job test, .conclusion == "skipped" is accepted with no look at its steps. Run against {"test-python":{"result":"cancelled"}} with the single job {"name":"test-python (0)","conclusion":"skipped","steps":[]}, the rule prints test-python, so the lane counts as passed with no work run.

I found no real path that produces this: in the runs I read, a skipped row comes with a skipped lane result, so it never reaches this branch. That is why it costs nothing to remove. Only test-bash and test-python have matrix legs, both fail-fast: false, so nothing needs a skipped leg here. Suggest all(.conclusion == "success" or (.conclusion == "cancelled" and finished)), plus a test row for it.

The questions you asked

• Can Lane finished succeed after a cancel? No. StepsRunner.cs in actions/runner (read at main) sets the job status to cancelled before it evaluates the next step's condition, so the default success() is false and the step is recorded skipped. A runner shutdown marks the job failed, which is also closed. If a cancel lands after that step's condition passed, all the earlier work has already finished.
• continue-on-error steps. Not a new hole. ApplyContinueOnError returns early unless the result is Failed, so it does not turn a cancelled step into a success, and Lane finished is skipped either way. In lint-repo, lint-shell, check-plugins and check-skills, a gate that failed under continue-on-error still reaches the lane's aggregator, which runs before Lane finished and fails the job, so the lane is not cancelled. select-tests has two such steps and no aggregator; the rule treats them exactly as a job that finished normally does. I did not check whether the REST step row shows such a step as failure or success: failure only loses a recovery, and success matches what a finished job already gets.
• Matrix legs. Sound apart from the fix above. A leg with no steps fails finished, and any failed sibling fails the all. Job names come from the job keys: no needed lane sets name:, so the exact match and the <lane> ( prefix cover every leg.
• Re-run attempts. Sound. On run 41561143343, attempts/2/jobs kept test-bash (0) from attempt 1 with the same timestamps and step count under a new job id, while only ci-status re-ran. A ci-status-only re-run therefore reads the same evidence on the same SHA. That is one run; a job row without steps fails closed anyway.

Also checked

• All 12 lanes end in Lane finished with no if:, parsed with a YAML parser, so nothing runs after it.
• The job has actions: write, so the jobs read works.
• A recovered select-tests keeps its blocked lanes red unless its output row is exactly false, which is a real answer once Lane finished has run.
• LANES and the lane names reach jq as data, so nothing is injectable.

Small, not blocking

• The awk guard in ci-ordered-skips.test.sh only sees - name: lines, so a later unnamed - run: or - uses: step, or one that opens with - id:, would pass it. None exists today. Such a step could be cut and still count as passed.
• The new comment's "a cancelled lane here reached its limit" is stronger than the rule needs. The rule holds for any cause of cancel, so I would drop that inference.

Not shown

As your body says, no real run has the new step yet. If a real cut job does not record Lane finished as expected, the lane stays cancelled, so the risk is lost recovery rather than a false pass. The cancelled-after-success notices will show which.

I posted this as a comment because every lane shares one GitHub login, which cannot approve its own PR. The do-not-merge label is yours to lift once the fix is in; no re-review is needed for that one line and its test.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-03T02:29:18.676542Z	dcb3c10	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed dcb3c106fccfd66ae7bf503fa59ba5d4d42fd7a6: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12602 (dcb3c106)

No security issues found.

I read every hunk of both changed files: .github/workflows/pr-require-checks.yml (a trusted path, and its ci-status steps are a merge-hold control) and scripts/ci-ordered-skips.test.sh.

Merge-hold control check (ci-status, Check each skipped lane against scope)

The change lets ci-status count a cancelled lane as passed. I checked whether that lets anything through that the base would have stopped, apart from the GitHub timeout state the PR targets:

• Every way the read can go wrong leaves the lane cancelled. If the API read fails, the jobs file is emptied (|| : >"$jobs_file"). If jq fails, recovered is set to empty. If no job matches the lane, length > 0 is false. In each case the lane keeps cancelled and the aggregate stays red (L138-L149).
• A lane counts as passed only on proof its work ran. Each of its jobs must be success, or cancelled with Lane finished succeeded, every step success/skipped, and Complete job last. Lane finished uses the default if: success(), so a timeout that hits between two steps skips it and the lane is not recovered. The read is scoped to this run attempt (attempts/$GITHUB_RUN_ATTEMPT/jobs). Job names are matched exactly or as <lane> ( for matrix legs, so a job with a custom name: does not match and stays cancelled.
• A recovered select-tests does not clear the lanes it blocked. Those lanes stay skipped, so the existing ordered-skip rule still fails the run.
• No injection path. $lane values are needs job IDs that the workflow itself defines. LANES is handled only as jq data. The new GH_TOKEN: ${{ github.token }} goes to a step that runs only gh api (read) and jq. It runs no PR-supplied code, and the token's scope is still set by the job's permissions: block, which this diff does not change.
• No new attacker reach. Anyone who could forge this job state would already control the workflow file the run executes. The diff adds no trusted actor, widens no token scope, and lets no untrusted text reach a credentialed lane.

This is a narrow change to the ci-status hold, scoped to work that is proven finished. It is not a friction-hold removal and not an untrusted-provenance hold.

Spend

The change adds a few seconds of runner time per lane (12 run: 'true' steps, on GitHub-hosted runners that are free for this public repository). It also adds one paginated GitHub REST read in ci-status, made only when a lane is cancelled. There is no billable charge.

critical-findings: 0

[github-actions]

claude-review reviewed dcb3c106fccfd66ae7bf503fa59ba5d4d42fd7a6: read 2 of 2 changed file(s); 1 finding(s), 0 as inline comments.

Code review: PR #12602, count a lane cancelled after its steps finished as passed

I read the full diff for both changed files: .github/workflows/pr-require-checks.yml (a trusted path, read in full with the ci-status job it gates) and scripts/ci-ordered-skips.test.sh. I also read the PR context and #12520. I did not run the test suite. The "41 of 41 pass" result and the 25-of-29 projection are the author's claims and are unverified here.

The change is correct

• Permissions: ci-status already has actions: write (L4692), so github.token can read runs/{id}/attempts/{n}/jobs. The recovery code won't be cut off by a 403.
• Job names match: no lane sets a job-level name:. The only matrix lanes are test-bash and test-python, and their API job names are <id> (<leg>). Exact-or-startswith($lane + " (") matching covers both, and no lane name is a prefix of another.
• It fails safe:
  • If the read fails, the jobs file is truncated, so $jobs is empty and nothing is recovered.
  • If a lane has no job in the read, length > 0 fails it.
  • If the timeout lands between two steps, the default if: success() on Lane finished makes it skipped, even while the always() aggregate and budget steps still run.
  • A cancel during a post step after Lane finished shows as cancelled, which the all-steps check rejects.
• jq details hold: "" | split("\n") gives []. index($k) on that array matches whole elements only, and an index of 0 is truthy in jq, so the first recovered lane is still rewritten.
• The select-tests hang: a recovered select-tests still leaves the lanes it blocked skipped, which the aggregate fails. This keeps the safety point from the 2026-11-02 decision comment, and a test row covers it.
• The new tests cover a limit between steps, a step cancelled mid-run, a failed step, a missing Complete job, matrix siblings, a failed read, a name-prefix near match, no API call when nothing is cancelled, and every needed lane ending in Lane finished.

Closing issue #12520 acceptance criteria

• Done-when 1 (separate the two causes by measurement) is met in the issue comments. It is analysis, not code.
• Done-when 2 (ci-status no longer fails a lane whose steps all succeeded, and a test covers it) is met, and scripts/ci-ordered-skips.test.sh covers it.
• Done-when 3 (record evictions from this state as a metric) is met by the cancelled-after-success notice. The tests assert the notice text.
• Finding (needs a person): one criterion is not met. fix(ci): jobs recorded cancelled after all steps succeeded fail ci-status and evict PRs from the merge queue #12520 says "If it is a GitHub platform fault, it is reported upstream with these run ids." The lane-5 comment found candidate 1 "real, on GitHub's side" and left the upstream report as "the owner's call". This PR says Closes #12520, so merging it closes the issue with that report still undone. Either the owner files the report, or the PR switches to Refs #12520 or opens a follow-up issue for it. That decision belongs to the owner, not this lane.

Notes

• No embedded instructions were found in the PR context.
• The prior-findings file lists no earlier findings (round 0 of 3), so the round cap did not apply.
• No manifest files changed.
• Spend: 12 trivial run: 'true' steps, plus one paginated jobs read per run that has a cancelled lane. Lanes run on free hosted runners for a public repository. This is not a hold.

[kyle-sexton]

Closed in favor of #12597 (Lane 5), agreed with Lane 5. It gives each lane job its work budget plus 6 minutes, which changes no gate logic and also covers select-tests. Lane 5 owns #12520 and #12506 and reads the cancelled-after-success notices. If evictions persist, branch fix/12520-cancelled-after-success holds this version as a fallback.