fix(ci): stop body edits cancelling the base-re-point guard (RIG-3372) - #1060
Open
rigel-mintaka wants to merge 2 commits into
Open
fix(ci): stop body edits cancelling the base-re-point guard (RIG-3372)#1060rigel-mintaka wants to merge 2 commits into
rigel-mintaka wants to merge 2 commits into
Conversation
`pr-base-repoint.yml` keyed its concurrency group on the PR number alone, so the two `pull_request.edited` classes shared one cancel group. jj-vine edits every PR body ~2s after a base re-point, so the body-edit run cancelled the re-point run and then skipped itself (`changes.base == null`) — the PR kept its stale green checks and nothing went red. On PR #916 the cancelled run executed zero steps, dying before `dispatch`, so the verify step built to catch exactly this never ran to fail. Partition the group by event class, matching the job's own guard predicate: pr-base-repoint-<number>-<repoint|edit> `cancel-in-progress: true` is kept: a genuine second base re-point still supersedes the first, since both land in the `-repoint` group. Only the body-edit arm moves out of the way. The condition is `changes.base != null`, not a bare `changes.base`. GitHub's `&&`/`||` return the operand rather than a boolean, and objects are never converted to a string, so a bare truthiness test would put the `changes.base` OBJECT into the group name. Comparing to null yields a boolean, so both arms return strings. actionlint accepts either form — it does not catch this. Ledger-impact: none — a CI workflow concurrency-key fix; no design record touched.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-repo-rig-3372-repoin.compass-eng-docs.pages.dev Deployed from |
Review of #1060 found the justification in the previous commit's message is wrong, and narrowed what the fix delivers. The expression itself is correct and unchanged; this commit fixes the prose around it. **Correction.** The previous message claimed a bare `changes.base && 'repoint'` would put the `changes.base` OBJECT into the group name. It would not. GitHub's `&&` returns its RIGHT operand when the left is truthy, so the bare form also yields 'repoint'. The object could only reach the name by being the operand actually returned, which never happens here. Verified against GitHub's own reference evaluator (`@actions/expressions`) across six payload shapes: base re-point, body edit, title edit, `changes` absent, `changes` empty, and `changes.base` explicitly null. `!= null` remains the right form, for the real reason: it types the predicate as a boolean and mirrors the job's own `if:` conjunct verbatim, so the group and the guard are visibly the same test. **Scope of the fix.** The comment said the race left "nothing red to show for it", implying the fix restores a gate. It restores a *signal*: `rollup` is the only required context on `main`, so a red guard does not block the merge queue. The comment now says so. Also records why the key mirrors only the `changes.base` conjunct and not the same-repo one: a PR's head repo is fixed at creation, so a fork PR's runs only ever share a group with other fork runs of the same PR, all of which skip. Ledger-impact: none — comment and rationale only; no design record touched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the concurrency race in
pr-base-repoint.ymlthat left stacked PRs on stale checks (RIG-3372).The defect
Both a base re-point and a body edit arrive as
pull_request.edited, and jj-vine edits every PR body ~2s after a re-point. The group was keyed on PR number alone, so the two shared one cancel group: the body-edit run cancelled the base-re-point run and then skipped itself.The cancelled run died before its
dispatchstep, so the verify step built to catch exactly this never ran to fail. Reconstructed on #916:The fix
Partition the group by event class, mirroring the job's own guard predicate:
cancel-in-progress: trueis preserved: two genuine base re-points still share the-repointgroup and still supersede each other. Only the body-edit arm moves out of the way.Verification
The expression was evaluated against GitHub's own reference evaluator (
@actions/expressions) across six payload shapes — base re-point, body edit, title edit,changesabsent,changesempty, andchanges.baseexplicitly null — and yields the intended string in every case. Two re-points render an identical group; a re-point and a body edit render different ones.Scope — a signal, not a gate
rollupis the only required context onmain, so a red guard does not block the merge queue. This change restores the guard's ability to reach a verdict; it does not make that verdict gating. Whether it should gate is a separate policy call.A correction carried in the second commit
The first commit's message justified
!= nullby claiming a barechanges.basewould leak the object into the group name. That is wrong — GitHub's&&returns its right operand when the left is truthy, so the bare form also yields'repoint'.!= nullis still the right form, for the real reason: it types the predicate as a boolean and mirrors the job'sif:conjunct verbatim. The second commit corrects the rationale in-file.