Skip to content

ci: stop cancelling in-progress required runs on main - #2610

Open
Chris0Jeky wants to merge 3 commits into
mainfrom
issue-2582/main-run-no-cancel
Open

ci: stop cancelling in-progress required runs on main#2610
Chris0Jeky wants to merge 3 commits into
mainfrom
issue-2582/main-run-no-cancel

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Summary

ci-required.yml cancelled its own in-progress runs on refs/heads/main, so during a merge wave each
merge killed the previous merge's push run and no landed commit ended up with a completed required run.
This keeps cancellation for PR refs and turns it off on the default branch:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

A two-line comment above the block records why. The group key is unchanged, so main runs queue behind each
other on one ref rather than running in parallel. Nothing else in the workflow changed: no job, needs,
permissions, uses or trigger edits, and no other workflow, ci/policy.v1.json or scripts/ci/dev-up*
was touched. A new scripts/ci/smart-ci/required-concurrency.test.mjs locks the contract in place.

Root cause

The concurrency block was declared with an unconditional cancel-in-progress: true and no branch
condition. GitHub applies that to every event the workflow accepts, including push on main. Because the
group key is ${{ github.workflow }}-${{ github.ref }}, every push to main lands in the same group as the
previous push to main, and the newer run cancels the older one. On a PR ref this is the intended
behaviour (a new head supersedes the old one). On main there is no superseding: each merge commit is a
distinct artifact that needs its own evidence.

Evidence from the issue, six consecutive cancelled main runs on 2026-09-04:

Run ID Head Outcome
33923946316 df1559f cancelled
33924741549 46fb41d cancelled
33925232957 7155f10 cancelled
33925392077 ea3e39e cancelled
33925610356 0886b6c cancelled with 13 of 17 jobs already green
33926429510 330ccb4 cancelled

The last completed green main run before that sequence was 33886539482 at 61e94f6, 15:21Z. The same
pattern repeated at 01:12Z on 2026-09-05.

Each merged PR head carried its own green required run, so the merges themselves were gated. What is
missing is the post-merge proof of the combined tip, which the readiness view's clause 4, the SC-4
observation window and the CI-03 landed-commit verifier (#2327) all read.

Verification

Run from the worktree at .worktrees/codex-2582-main-run-no-cancel.

Red first, against the unmodified workflow (test committed before the fix, dbfe5c6):

node --test scripts/ci/smart-ci/required-concurrency.test.mjs

3 pass, 1 fail. The failing assertion:

in-progress cancellation is disabled on the default branch
  AssertionError: ci-required.yml must not cancel an in-progress run on refs/heads/main (#2582)
  actual: 'true'
  expected: "${{ github.ref != 'refs/heads/main' }}"

After the workflow change (a9f03a9):

  • node --test scripts/ci/smart-ci/required-concurrency.test.mjs — 4 tests, 4 pass, 0 fail.
  • node --test scripts/ci/smart-ci/*.test.mjs — 95 tests, 95 pass, 0 fail (91 before this branch, plus
    the 4 new ones). smart-ci-self-test.yml line 41 already runs exactly this glob, so the new file is
    wired with no workflow edit; confirmed by grepping .github/workflows for it.
  • node scripts/check-docs-governance.mjs — "Docs governance check passed."
  • git diff --check — clean.
  • YAML sanity parse of the edited file (yaml.safe_load): the concurrency mapping reads back as
    {"group": "${{ github.workflow }}-${{ github.ref }}", "cancel-in-progress": "${{ github.ref != 'refs/heads/main' }}"}
    and the job count is still 13.

Not verified

  • actionlint is not installed on this machine, so no local workflow lint ran. Hosted Workflow Lint is
    the proof for the YAML.
  • This is an R4 CI-control change and qualifies hosted-only. Hosted Workflow Lint, Smart CI / Planner
    Self-Test and ci-required at the exact head are the R4 proof and were not observed at the time of
    writing; the local runs above are additive.
  • The behavioural claim, that a main run now queues instead of being cancelled, can only be observed
    after this merges, on the next merge wave. Nothing local can exercise the GitHub concurrency engine.
  • No backend or frontend code is touched, so no dotnet and no vitest or Playwright run applies.

Risk notes

  • What changes for PR refs: nothing. github.ref on a pull_request event is
    refs/pull/<n>/merge, so the expression evaluates to true and a new push to a PR still cancels the
    older run exactly as before. merge_group refs are likewise unaffected.
  • What changes for main: runs queue. A wave of N merges now costs N Linux required runs instead of one
    surviving run, and the last one finishes some minutes after the wave ends rather than being killed.
  • Minutes under SC-3: the Windows legs run only while the repository is public and are already slated to
    move to local runners, so the marginal cost here is the Linux legs. Queued main runs are the price of
    having one completed run per landed commit, which is what the landed-commit verifier needs.
  • Alternative not taken: a per-SHA group such as ${{ github.workflow }}-${{ github.sha }} on main.
    That would let every merge's run start immediately and finish sooner, but it runs the whole wave in
    parallel and multiplies peak runner usage. Queueing keeps peak concurrency at one per ref, which the
    issue records as the preferred trade.
  • Reversible by restoring cancel-in-progress: true and deleting the test file.

Closes #2582

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Review (agent half of the ADR-0066 gate; Codex credits exhausted, SC-9). One fresh-context reviewer (read-only, Opus 5) at exact head a9f03a9: verdict SHIP, no CRITICAL/HIGH. This PR is CI-control (ci-required.yml, a declared control path, T2): it stays parked for the maintainer's review under OUTSTANDING_TASKS SC-10 and is not merged by an agent.

Confirmed: the diff is the concurrency block plus its comment (3 insertions, 1 deletion) and one new contract test; no job, needs, permissions, uses or trigger changed; the group key is byte-identical, so PR refs (refs/pull/N/merge) and merge-queue refs keep cancelling exactly as before; cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} is the documented expression form and a valid YAML plain scalar (PyYAML readback recorded by the worker); the test is wired through the existing Planner Self-Test glob and discriminates on a deleted or inlined block, a changed group key, and a hardcoded true. Red-first evidence on the PR: assertion (c) failed on the unmodified workflow, then passed; Smart CI suite 95/95; docs governance green.

Findings and dispositions:

  1. MEDIUM, fixed in 0fc8fb2: the workflow comment and the test's header said main runs are "never cancelled" and "queue behind each other". GitHub keeps one in-progress plus one pending run per concurrency group and cancels a previously pending run when a newer one arrives, so with the per-ref group the in-flight main run completes and the newest tip runs next, but intermediate pending main runs are still superseded. Both texts now say exactly that. What this PR guarantees is therefore: no in-flight main run is killed by the next merge, and the tip always gets a completed run once the wave drains. If the maintainer wants a completed run for every landed commit (what the CI-03 landed-commit verifier ideally consumes), the one-line alternative is a per-SHA group on main (${{ github.workflow }}-${{ github.ref }}-${{ github.sha }}), which runs them in parallel and costs one Linux run per merge; that is a ruling for the SC-10 review, recorded on [CI][Control plane] ci-required cancel-in-progress on main discards the tip's evidence during a merge wave (six consecutive main runs cancelled 2026-09-04) #2582.
  2. LOW, recorded as a deliberate choice: the master push trigger is not covered by the guard (only refs/heads/main); main is the default branch.
  3. LOW, recorded: if the top-level block is deleted, tests 2 and 3 fail with a TypeError instead of a named message (test 1 still fails cleanly), and a flow-mapping concurrency: would not be parsed; diagnostic quality only.
  4. LOW, recorded: the contract test runs in the advisory Planner Self-Test lane, like every other workflow-contract test in that directory; registration of that lane is the SC-4 decision.

Merge gate: maintainer review (SC-10) plus hosted Workflow Lint, Smart CI / Planner Self-Test and ci-required green at the final head 0fc8fb2.

Chris0Jeky added a commit that referenced this pull request Sep 5, 2026
…ation-2026-09-05

docs: coordination lane in the eleventh STATUS block, SC-10 #2608/#2610, D-12 unclaimed PR author
Chris0Jeky added a commit that referenced this pull request Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending

1 participant