Repository navigation
test(dev-lead): guard the #443 concurrency fix (cancel-in-progress: false) - #1221
Conversation
…alse) The dev-lead multi-label issue-pickup race (#443) was fixed by #450's per-issue/ per-PR concurrency lanes with cancel-in-progress:false — same-lane labeled events queue behind the active run instead of cancelling it, so the dev-lead-triggered pickup always finishes. Add a regression guard that fails if anyone reintroduces cancel-in-progress:true or drops the per-issue lane. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Bats regression suite that validates dev-lead concurrency configuration and issue-based lane routing, and registers the suite in the lint workflow. ChangesDev-lead concurrency validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new BATS test file, tests/test_dev_lead_concurrency_guard.bats, to prevent regression of concurrency settings in the dev-lead reusable workflow. The reviewer noted that using [ "$status" -ne 0 ] to assert that a pattern is not found can cause false passes if the target file is missing or if grep fails for other reasons. They recommended asserting an exit status of exactly 1 instead.
There was a problem hiding this comment.
Pull request overview
Adds a Bats regression test intended to prevent a silent revert of the dev-lead concurrency fix from #450 by asserting cancel-in-progress: false and per-issue concurrency lane routing in the reusable workflow.
Changes:
- Introduces
tests/test_dev_lead_concurrency_guard.batsto assertcancel-in-progress: falseis present andcancel-in-progress: trueis absent indev-lead-reusable.yml. - Adds an assertion that issue-triggered events route to the
dev-lead-issue-<n>concurrency lane.
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1221 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
Superseded by automated re-review at
|
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: a432d862ed12559fbe3948be61f632fd5cbf6bc8
Review mode: triage-approved (single reviewer)
Summary
Test-only PR adding a Bats regression guard (tests/test_dev_lead_concurrency_guard.bats, 27 lines) for the #443/#450 dev-lead concurrency fix, plus a one-line registration in lint.yml's bats file list. All three assertions were re-verified against the current .github/workflows/dev-lead-reusable.yml: cancel-in-progress: false (line 65), no cancel-in-progress: true anywhere, and the per-issue dev-lead-issue- lane (line 60). The prior review (sha 20efbd4) escalated solely because the template-drift check was red from pre-existing drift merged in from main — template-drift now passes on this head, and the only changes since the prior reviewed SHA are merges from main (no test-file changes). All 5 review threads (grep ERE \s portability, CI registration, comment casing, negative-assertion exit code) are resolved and applied in the final test.
Linked issue analysis
No linked issues via closing references. The PR body references #443 (multi-label issue creation cancelling dev-lead dispatch), already fixed by #450 (separate per-issue/per-PR concurrency lanes with cancel-in-progress: false). This PR adds the regression guard so that fix cannot be silently reverted; the three assertions substantively cover the referenced problem.
Findings
- No security concerns: no auth/secrets/crypto/dependency changes; read-only grep assertions in a Bats test plus a test-list registration in lint.yml.
- Secret scan: gitleaks CI check passed. The run_secret_scanning MCP tool was not available in this environment (noted, not blocking).
- Prior review's single blocking finding (template-drift failure, pre-existing and unrelated to this PR) is resolved — template-drift is green on this head.
- Test quality: negative assertion uses exact exit-status 1, patterns use POSIX [[:space:]] (ERE-safe), and the new suite is registered in lint.yml so it actually runs in CI (bats check green, 3/3).
- Triage assessment confirmed: LOW risk, nothing missed.
CI status
All verification checks green: Lint, bats (includes the new suite), shellcheck/ShellCheck, CodeQL (actions + python), Secret scan (gitleaks), Agent Security Scan, SonarCloud (quality gate passed), agent-shield, holdout-guard, guard, unit-tests, gh-aw-compile, template-drift, validate-personas, validate-agent-profiles. CodeRabbit approved. The only cancelled runs are dev-lead / dispatch and dev-lead / ci-relay automation lanes (superseded agent runs, not required checks); dependency-audit jobs skipped (not applicable).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: a432d862ed12559fbe3948be61f632fd5cbf6bc8
Review mode: triage-approved (single reviewer)
Summary
Test-only PR adding a Bats regression guard (tests/test_dev_lead_concurrency_guard.bats) for the #443/#450 dev-lead concurrency fix, plus a one-line registration in the lint workflow's bats file list. All three assertions were verified against the current .github/workflows/dev-lead-reusable.yml: cancel-in-progress: false (line 65), no cancel-in-progress: true anywhere, and the per-issue dev-lead-issue- lane (line 60). The prior review's sole blocker — a red template-drift check caused by pre-existing drift on main, unrelated to this PR — is now green at the current head after the merge from main.
Linked issue analysis
No linked issues via closing references. The PR body references #443 (multi-label issue creation cancelling dev-lead dispatch), already fixed by #450; this PR adds the regression guard so that fix cannot be silently reverted. The guard substantively covers the referenced problem: presence of cancel-in-progress: false, absence of cancel-in-progress: true, and per-issue lane routing.
Findings
- Prior review (cycle 1, sha 20efbd4) escalated only because template-drift was red from unrelated drift on main; that check is SUCCESS at the current head. Changes since the prior reviewed SHA are merges from main only — the PR's own diff is unchanged. Resolved.
- No security concerns: no auth/secrets/crypto changes, no new dependencies, no executable logic — read-only grep assertions in a Bats test plus a test-list registration in lint.yml.
- Secret scan: gitleaks CI check passed. The run_secret_scanning MCP tool was not available in this environment (noted, not blocking).
- All 5 review threads are resolved: bot feedback was applied (POSIX [[:space:]] instead of \s in grep -E, exact-status [ "$status" -eq 1 ] for the negative assertion, lint.yml registration, comment casing fix).
- Triage assessment confirmed: low-risk, well-scoped, guard assertions verified accurate against the target workflow.
CI status
All required checks green (SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems). Also passing: Lint, bats (includes the new suite), shellcheck, ShellCheck, CodeQL actions+python, Secret scan (gitleaks), Agent Security Scan, SonarCloud quality gate (0 new issues), holdout-guard, guard, unit-tests, template-drift, gh-aw-compile, validate-agent-profiles, validate-personas, CodeRabbit (approved). The cancelled dev-lead / dispatch and dev-lead / ci-relay entries are the org's own automation runs superseded by newer same-lane events (non-required, not verification checks — ironically the exact queueing behavior this PR's guard protects). Dependency-audit ecosystem jobs skipped (not applicable).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
#443 (multi-label issue creation cancels dev-lead dispatch) was already fixed by #450 (2026-06-06) — separate per-issue/per-PR concurrency lanes with
cancel-in-progress: false, so same-laneissues:labeledevents queue behind the active run instead of cancelling the dev-lead-triggered pickup. (#443 was filed 2026-06-11 from pre-fix incident evidence.)This adds a regression guard so the fix can't be silently reverted: asserts the reusable keeps
cancel-in-progress: false, has nocancel-in-progress: true, and routes issue events to thedev-lead-issue-<n>lane. 3/3 green.🤖 Generated with Claude Code
https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s
Summary by CodeRabbit