fix(ci): scope desktop-swift-ci main runs per-SHA so the release source gate can't deadlock - #10200
Conversation
…ce gate can't deadlock desktop-swift-ci grouped its concurrency by `github.event.pull_request.number || github.ref`. On a push to main there is no PR number, so every main push shared the single group `desktop-swift-refs/heads/main` with cancel-in-progress — each new push cancelled the previous commit's in-progress run. On a busy night the newest desktop commit never produced a green Desktop Swift run, so desktop_auto_release's newest-desktop-SHA source gate had nothing to accept and refused to cut the release (BasedHardware#10177: v0.12.91 blocked for hours, shipped only via break_glass). The non-PR fallback is now `github.sha`: each main commit's run gets its own group and completes instead of being cancelled by the next push. PR runs still group by PR number, so a newer push to the same PR still supersedes the older run. Regression guard added to the existing desktop-swift-ci contract test: the concurrency group must use github.sha (not github.ref) for the non-PR case, with an adversarial test that a revert to the shared per-ref group is detected. The guard fails on the pre-fix workflow by construction. Verification: python3 .github/scripts/test_desktop_swift_ci_contract.py → 17 passed (2 new); fails 1/17 when the fix is reverted; workflow YAML parses. Failure-Class: none Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Git-on-my-level
left a comment
There was a problem hiding this comment.
Maintainer review: this looks like the right targeted fix for #10177.
What I checked:
- The workflow keeps PR concurrency grouped by
github.event.pull_request.number, so newer pushes to the same PR still cancel older runs. - Main-branch pushes now fall back to
github.sha, which avoids unrelated later main pushes cancelling the desktop source-proof run needed by the release gate. - The added contract test covers the intended invariant, and I verified it locally with
python3 .github/scripts/test_desktop_swift_ci_contract.pyon8b9ab917ca3b7f4b11b2fabf554a9ea5f7906f4d(17 passed).
I do not see a code-level blocker. Because this touches CI/release workflow behavior, I’m leaving this as a positive signal rather than formal approval; a human maintainer should still confirm the release-gate behavior and wait for the queued Desktop Swift CI lane before merge.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
|
@Git-on-my-level need human response — CI/release-gate change (fixes the #10177 deadlock). Contract test added and verified locally (17 passed). Needs a human maintainer to confirm the release-gate behavior and merge. |
|
Obsolete — main already resolves #10177. The workflow now uses |
|
Hey @aryanorastar 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Before your next PR, please skim:
If this was declined for direction or taste, maintainers should cite an invariant ID or open a proposed one — ask if that citation is missing. Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! |
Summary
desktop-swift-ci.ymlgrouped its concurrency bygithub.event.pull_request.number || github.ref. On a push to main there is no PR number, so every main push shared the single groupdesktop-swift-refs/heads/mainwithcancel-in-progress: true— each new push cancelled the previous commit's in-progress run.On a busy night the newest desktop commit's
Desktop Swift Build & Testsrun kept getting cancelled before it finished (#10177:e298ad2331cancelled 3×), sodesktop_auto_release's "newest-desktop-SHA has a green source run" gate had nothing to accept and refused to cut v0.12.91 — it shipped only viabreak_glass.Fix: the non-PR fallback is now
github.sha, so each main commit's run gets its own concurrency group and completes instead of being cancelled by the next push.Fixes #10177.
Why this shape
github.event.pull_request.numberis still first, so a newer push to the same PR supersedes the older run (the desirable cancel-in-progress behavior).github.sha— unique per commit, so an unrelated later push can't cancel the run that the release source gate needs. This is exactly the issue's suggested direction ("scope the group per head SHA").cancel-in-progressstays: for a per-SHA group it only supersedes a manual rerun of the same commit, which is what you want.Failure class
Failure-Class: none
Product invariants affected
none (verified with
scripts/pr-preflight --suggest)Validation
test_desktop_swift_ci_contract.py(already wired into the checks manifest, triggered bydesktop-swift-ci.yml): the concurrency group must usegithub.shafor the non-PR case, plus an adversarial test that a revert to the shared per-ref group is detected.python3 .github/scripts/test_desktop_swift_ci_contract.py→ 17 passed (2 new).github.reffails the new guard (1/17) — it catches exactly the deadlock condition.yaml.safe_load).make preflight(local lane, this PR body) → all selected checks pass.Not exercised live: a real busy-night main-push storm (can't reproduce GitHub's concurrency scheduler locally). The mechanism is a documented GitHub Actions concurrency behavior; the contract test pins the group expression that produces the correct per-SHA scoping.
Out of scope, named