fix(actions): coalesce superseded agent mentions while they are queued - #1964
Conversation
Both agent-mention dispatchers carried their concurrency group on the single `validate-and-forward` job. A job-level group is not evaluated while the whole run waits behind the organization job ceiling, so a superseded mention keeps its queue slot until a runner frees up and only cancels once it is already running -- which is after the slot has been held, not instead of holding it. Moving the group to workflow level coalesces the older run while both are still queued. Measured on `opencode-review-dispatch.yml`, which carried the identical defect until #1958, over the 39.7 hours ending 2026-09-06T12:41Z: 24 pairs of runs for one pull request overlapped in time, and none was coalesced. The five whose older run ended `cancelled` were cancelled 0.7 to 2.9 hours after the newer run was created, which is a sweep rather than concurrency; the other 18 ran to `failure` while already superseded, the longest living 9.4 hours past its successor's arrival (#1529). `agent-mention-router.yml` is deliberately left alone: it has two jobs with different groups, and its organization sweep sets `cancel-in-progress: false` on purpose. A workflow carries at most one workflow-level group, so hoisting either one would give the sweep the route's cancelling policy and let a later comment kill a sweep part way through the organization. A test now pins that. The group text is unchanged and stays on one line, so the string `test_agent_mention_downstream_idempotency.py` pins still matches verbatim. That test's `assert "concurrency:" not in header` is retargeted rather than deleted: `git log -S` shows it arrived with 109d79b ("replace unsupported queue concurrency"), which removed a workflow-level block using `queue: max` -- a key GitHub Actions does not support -- and parked the group on the job on the way past. What that commit pins is the absence of `queue:`, not the level, so the assertion now reads `assert "queue:" not in header` plus a job-level absence check. Controls both ways: reverting to job level fails the retargeted contract and the new test; reintroducing `queue: max` still fails the retargeted contract. Gate: 2959 passed, 1 skipped, coverage 100%, interrogate 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 24 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| group: agent-mention-noema-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
🟡 Delayed mentions replace newer requests
When the sweep rediscovers an older unclaimed comment, cancel-in-progress lets its delayed dispatch cancel a newer request. The concurrency key lacks comment ordering, so arrival order decides which response survives.
Prompt for agents
The workflow-level PR group coalesces by dispatch arrival, but agent_mention_sweep.py can redispatch any older comment that has no artifact. A wrapper canceled while queued creates no artifact, so an older comment can arrive after and cancel a newer invocation for the same PR. Update both agent-mention wrapper workflows and the router/sweep contract so cancellation is based on source-comment ordering, not dispatch arrival, while retaining workflow-scope admission and exact-invocation artifact idempotency. Add a regression covering a delayed older sweep dispatch arriving after a newer local dispatch.
Was this helpful? React with 👍 or 👎 to provide feedback.
…urrency Both sides added tests in the same region of tests/test_required_workflow_queue_contract.py; the resolution keeps both. This branch's two assertions move onto workflow_level_concurrency_group(), the helper #1970 introduced, rather than staying on the raw concurrency block. Correcting a claim I made when merging #1970: this branch's comment block does NOT quote the expressions its tests assert on -- it names other workflow files and explains the mechanism in prose -- so a collapsed key here would have been caught by the raw-block form too. The move is defence against a future comment that does quote them, not a fix for a present hole. Control on the prospective shape (collapse the key, move the expressions into the comment beside it): the helper form fails, while all four raw-block assertions -- group prefix, target_repository, pr_number || github.run_id, and cancel-in-progress: true -- would have passed. cancel-in-progress is a sibling key and therefore outside the group value, so it is asserted separately, anchored to its own line at the block's indent where a comment cannot satisfy it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Resolved against ResolutionBoth sides added tests in the same region of This branch's assertions move off the raw concurrency block and onto Correcting something I claimed when merging #1970I said there that this branch's comment blocks quote the expressions its tests assert on, making them The comments name other workflow files and explain the mechanism in prose. A collapsed key here would So the move onto the helper rests on a weaker, accurate footing: defence against a future comment that ControlProspective shape — collapse the key to the repository alone and move the expressions into the comment Gate
|
교차 세션 검증 — 네 축 통과, 후속 범위 하나 발견저자(peer 세션)와 분리된 세션이 저자 철회 재확인저자가 "이 브랜치의 주석이 단언 표현식을 인용해 방패가 된다"는 앞선 주장을 철회했는데, 독립으로 확인했습니다. 두 워크플로의 주석 줄만 추출해 세 표현식을 검색한 결과 여섯 조합 전부 축별 결과해소 범위 — 해소 커밋의 이전 헤드( 헬퍼 이전 — 이 PR 의 두 테스트는 판별력 — 저자 변이를 재사용하지 않고 독립 변이 둘을 만들어 실행했습니다. 기준선 63 passed.
줄 앵커( 후속 범위 — 이 PR 의 결함은 아닙니다같은 파일에서 필수 리뷰 워크플로의
게이트
검증하지 않은 것변이 A·C 는 판정: 병합 가능. 위 후속 범위는 이 PR 을 막는 사유가 아닙니다. 🤖 Generated with Claude Code |
|
Merged as Gated on the merged tree, not on the head.
Tree identity proven after the merge. The squash commit's tree is Change verified as described. Both workflows now carry the group at workflow level with Why this merged while blocked. Of the 12 required contexts on Independent field evidence for the mechanism. Measured on the already-shipped workflow-level group in Author and verifier were other sessions; this session only merged. 🤖 Addressed by Claude Code |
1 participant
Both agent-mention dispatchers carried their concurrency group on the single
validate-and-forwardjob. A job-level group is not evaluated while the whole run waits behind the organization job
ceiling, so a superseded mention keeps its queue slot until a runner frees up and only cancels once
it is already running. Moving the group to workflow level coalesces the older run while both are
still queued, which is where the slot is actually held. Same defect and same fix as #1958.
Evidence that the defect is real
Measured on
opencode-review-dispatch.yml, which carried the identical defect until #1958, over the39.7 hours ending 2026-09-06T12:41Z. Runs grouped by the FULL
<owner>/<repo>#<pr>parsed from therun name -- the central dispatcher runs in
.githubon behalf of sibling repositories, so groupingon the bare number merges different pull requests.
cancelledThe five cancellations lagged +0.7, +1.2, +2.1, +2.8 and +2.9 hours behind the newer run's creation.
Coalescing puts that lag at roughly zero; hours means a sweep. The other 18 overlapping runs ran to
failurewhile already superseded, the longest living 9.4 hours past its successor's arrival(#1529).
An overlap count of 24 rather than 23 is a correction from a peer session: a run that has not
concluded was open at every earlier instant, so
older.created_at < newer.created_atsettles theoverlap and no end time is needed. My first pass compared
updated_atfor every pair, which bothused a field that carries no liveness information for a queued run -- across 100 currently queued
runs the
updated_at - created_atoffset is 0 s for 80, 1 s for 10 and 2 s for 10, never above 60 s-- and let a 34-second margin decide whether a row entered the sample.
What is deliberately not changed
agent-mention-router.ymlkeeps its two job-level groups. It has two jobs with different groups andits organization sweep sets
cancel-in-progress: falseon purpose. A workflow carries at most oneworkflow-level group, so hoisting either would hand the sweep the route's cancelling policy and let a
later comment kill a sweep part way through the organization.
test_agent_mention_router_keeps_its_two_distinct_job_groupspins this.The nine central workflows with
cancel-in-progress: falseare also untouched. Every one of themmutates state (merge scheduler, autofix, hourly repair, Cloudflare DNS, SBOM scheduler), where a
mid-flight cancel leaves partial state.
falsethere is the safe design, not a defect.The retargeted contract
Hoisting failed
test_agent_mention_downstream_idempotency.py, which carried a flatassert "concurrency:" not in header.git log -Son that assertion shows it arrived with109d79b7("replace unsupported queue concurrency"), which deleted a workflow-level block usingqueue: max-- a key GitHub Actions does not support -- and parked the group on the job on the waypast. What that commit pins is the absence of
queue:, not the level; five other queue-bearingworkflows here (
strix.yml,noema-review.yml,opencode-review.yml,codeql-scan-dispatch.yml,opencode-review-dispatch.yml) key their groups at workflow level.So the assertion is retargeted rather than deleted, to
assert "queue:" not in headerplus ajob-level absence check. The group text itself is unchanged and stays on one line, so the exact
string that test pins still matches verbatim.
Controls
queue: maxat workflow level: the retargeted contract still fails, so109d79b7'sactual finding remains pinned.
git checkout HEAD --.Gate
coverage run -m pytest tests2959 passed, 1 skipped, 21 subtests passed; coverage 100%;interrogate 100%.
Developer experience
A superseded mention no longer occupies a runner slot for hours before being discarded, so the
queue drains faster for everyone waiting behind it. The retargeted assertion keeps the original
queue: maxprohibition enforced, so a future session cannot reintroduce the unsupported key.User experience
An @-mention that is superseded by a newer comment stops producing a second, stale agent response.
Reviews arrive against the comment the author actually meant.
🤖 Generated with Claude Code