Repository navigation
feat: implement issue #405 — petry-projects — workflow failures detected 2026-05-28 - #528
Conversation
…ted 2026-05-28
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 49 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds guards against processing already-closed or merged PRs in two key paths: intent classification emits ChangesClosed PR Safe-Exit Guards
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
![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 introduces early-exit guards to gracefully handle closed or merged pull requests, preventing checkout failures when a branch has been deleted. It adds checks in both the intent classifier and the worktree checkout scripts, along with corresponding unit tests and fixtures. The review feedback suggests a valuable optimization to combine multiple gh pr view calls in pr-worktree.sh into a single API request to save network round-trips and avoid rate-limiting, which also requires updating the test stubs accordingly.
There was a problem hiding this comment.
Pull request overview
Implements a guard for issue #405 to prevent dev-lead runs from crashing when a trusted-bot comment arrives on an already closed/merged PR (where gh pr checkout would fail due to a deleted branch), and adds regression tests/fixtures for that behavior.
Changes:
- Add an early-exit in
checkout_pr_in_worktreewhen PR state is notOPEN. - Add intent-classifier fast-path to skip
issue_commentevents on closed PRs (pr-already-closed). - Add unit tests and an event fixture covering the closed/merged PR skip paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/lib/pr-worktree.sh |
Adds early PR-state detection and clean exit before attempting checkout on merged/closed PRs. |
scripts/dev-lead-intent.sh |
Skips processing issue_comment events targeting already-closed PRs to avoid wasted runs/crashes. |
tests/dev-lead/unit/test_pr_worktree.bats |
Extends gh stub to support PR state queries; adds regression tests for merged/closed PR early exit. |
tests/dev-lead/unit/test_intent_reviews.bats |
Adds regression test ensuring classifier emits skip/pr-already-closed for closed PR bot comments. |
tests/dev-lead/fixtures/events/issue_comment_sonarqube_closed_pr.json |
New fixture representing a SonarQube bot comment on a merged/closed PR. |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
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. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #528 |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #528 |
|
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. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #528 |
|
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. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 1c4b0a7a351f52b8629f550346162088fdc6d69d
Cascade: triage → deep (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7)
Summary
Defensive bug fix for issue #405 that prevents crashes on merged/closed PR checkout. All CI gates pass (shellcheck, bats, unit-tests, SonarCloud, CodeQL, gitleaks all green). The coderabbitai CHANGES_REQUESTED is stale — both items it flagged ([ ] vs [[ ]] in dev-lead-intent.sh and pr-worktree.sh) are already addressed in the current HEAD, and donpetry-bot has approved the final state.
Findings
- INFO: Early-exit guard in dev-lead-intent.sh checks issue.state == 'closed' (lowercase) which matches the GitHub webhook payload format; the downstream check in pr-worktree.sh uses OPEN/MERGED/CLOSED (uppercase) matching the gh CLI JSON output — both are correct for their respective data sources. (
scripts/dev-lead-intent.sh:309) - INFO: exit 0 inside checkout_pr_in_worktree (a sourced library function) exits the calling shell rather than returning. This is intentional and documented: the comment explains it must fire before the EXIT trap is registered to avoid triggering the caller's trap prematurely. Consistent with existing patterns in this library. (
scripts/lib/pr-worktree.sh:64) - MINOR: coderabbitai[bot] has an unresolved CHANGES_REQUESTED review (submitted 2026-06-10T18:55:18Z against commit 059dd2e) causing mergeStateStatus=BLOCKED. Both actionable items it requested (replace [ ] with [[ ]] in dev-lead-intent.sh and pr-worktree.sh) are fully implemented in the current HEAD. The review was never re-evaluated or dismissed after the fixes were pushed. Human action required to dismiss the stale bot review to unblock merge.
- INFO: Two new bats tests cover MERGED and CLOSED state early-exit paths in checkout_pr_in_worktree; one new intent-classifier test covers the issue_comment-on-closed-PR scenario. Regression fixture includes _test_expected_intent sentinel for automated fixture validation. (
tests/dev-lead/unit/test_pr_worktree.bats:229) - INFO: Gemini advisory feedback implemented: gh pr view now fetches state and headRefName in a single API call (--json state,headRefName), eliminating a redundant round-trip. Test stub updated to return the combined JSON object matching the new call signature. (
scripts/lib/pr-worktree.sh:62)
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7). Reply if you need a human review.
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #528 |
|
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. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 7be6ddbdd44f6304da6bef61d8fa7e14c60e18e4
Review mode: triage-approved (single reviewer)
Summary
Confirms triage assessment: a focused, well-tested bug fix for issue #405. Adds an early skip in dev-lead-intent.sh for trusted-bot comments on closed/merged PRs and a merged/closed-PR guard in checkout_pr_in_worktree (combined into a single gh pr view call), preventing the 'couldn't find remote ref' crash when a PR branch was deleted after merge. Includes regression fixtures and bats tests for both layers. The pr-review.yml change is comment re-indentation only.
Linked issue analysis
Issue #405 (fleet monitor: dev-lead workflow failures from 2026-05-28) is substantively addressed. The root cause — dev-lead reacting to bot comments on already-merged PRs and crashing at checkout because the branch was deleted — is fixed at two layers: a fast-path skip in the intent classifier (emit_skip pr-already-closed) and a defensive early-exit in checkout_pr_in_worktree before the EXIT trap is registered. Regression tests cover MERGED and CLOSED states plus the classifier skip path.
Findings
No blocking findings. Verified directly against the head diff: (1) both gemini-code-assist suggestions (single combined gh pr view call; combined-JSON test stub) are implemented even though the threads were not marked resolved in the UI; (2) both CodeRabbit nitpicks ([ ] to [[ ]]) are implemented and their threads resolved — the standing CHANGES_REQUESTED review predates these fixes and is stale; (3) shell changes are quote-safe, fail-open on gh API errors, and shellcheck passes; (4) the test stub exports PR_STATE_API unconditionally in setup(), preventing runner-env leakage. Note: CodeRabbit's stale CHANGES_REQUESTED keeps reviewDecision blocked until dismissed or re-reviewed — that is a GitHub mechanic, not an unaddressed concern.
CI status
All substantive checks green: shellcheck, bats/unit-tests, validate-fixtures, gh-aw-compile, CodeQL, SonarCloud, gitleaks, Agent Security Scan, workflow-structure checks. The two CANCELLED dev-lead jobs (dispatch, ci-relay) are concurrency-cancellation artifacts (per issue #608) and non-blocking. The in-progress 'review / review' check is this review run itself.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: MEDIUM
Reviewed commit: 7be6ddbdd44f6304da6bef61d8fa7e14c60e18e4
Review mode: triage-approved (single reviewer)
Summary
Adds two-layer protection against dev-lead crashing on trusted-bot comments to closed/merged PRs (issue #405): a fast-path skip in the intent classifier when the event's issue state is closed, and an early-exit guard in checkout_pr_in_worktree that detects non-OPEN PR state before attempting checkout of a possibly-deleted branch. Includes regression bats tests and a new event fixture. The pr-review.yml change is comment re-indentation only. Since the prior approval at 1c4b0a7, the only new commits are merges of main (#609, #618) — no PR-content changes.
Linked issue analysis
Issue #405 is a fleet-monitor failure report covering dev-lead workflow failures; the root cause addressed here is the crash ("fatal: couldn't find remote ref") when a SonarQube/trusted-bot comment lands on an already-merged PR whose branch was deleted. The PR substantively fixes that failure mode at both the classifier level (skip with reason pr-already-closed, avoiding a wasted agent run) and the worktree library level (graceful exit 0 with a ::notice), each covered by dedicated regression tests.
Findings
- No new issues found. Shell changes are well-quoted, use jq for JSON extraction, and combine the previous two gh pr view round-trips into one (--json state,headRefName), as suggested by gemini-code-assist.
- The two formally-unresolved gemini-code-assist threads (combine gh pr view calls; update the test gh stub to return combined JSON) are both implemented in the current diff and answered in-thread — they are stale-unresolved markers, not outstanding feedback.
- CodeRabbit's CHANGES_REQUESTED review (2026-06-10) had two trivial nitpick threads ([ ] vs [[ ]]); both threads are resolved and the style point was addressed. The review state itself was never dismissed, which keeps reviewDecision at CHANGES_REQUESTED — a mechanical leftover, not substantive outstanding feedback.
- The intentional exit 0 (rather than return) inside checkout_pr_in_worktree terminates the calling script by design (nothing to do on a closed PR); trap-ordering implications are documented in a comment and exercised by tests.
CI status
All required checks green: shellcheck, bats, unit tests, CodeQL, SonarCloud, gitleaks, agent-shield, fixture/profile validation all SUCCESS; dependency-audit matrix legs SKIPPED (no matching ecosystems). The two CANCELLED checks (dev-lead/dispatch, dev-lead/ci-relay) are the coding agent's own concurrency-cancelled dispatch jobs — the known false-blocker addressed by #608 — and 'review / review' IN_PROGRESS is this review run itself.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
|
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. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #528 |
|
@dev-lead - Please fix the findings and blockers here |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 5bcfcb49379aaabe0ec943b170b0f8c70ba778aa
Review mode: triage-approved (single reviewer)
Summary
Two-layer guard so trusted-bot comments (e.g. SonarQube) on already-merged/closed PRs no longer crash the dev-lead agent at checkout (issue #405). Adds an early skip in the intent classifier and a defensive early-exit in checkout_pr_in_worktree, with regression tests at both layers. Triage cleared this as low-risk; confirmation review agrees.
Linked issue analysis
Closes #405 (dev-lead crash: trusted-bot comment on a merged PR whose branch was deleted → gh pr checkout fails with 'fatal: couldn't find remote ref'). Substantively addressed in two independent layers: (1) scripts/dev-lead-intent.sh emits skip pr-already-closed when .issue.state == "closed" (lowercase, correct for webhook payloads) before spending tokens; (2) scripts/lib/pr-worktree.sh exits 0 when gh pr view --json state returns anything other than OPEN (uppercase, correct for the gh API). Casing is handled correctly on both sides. Each layer has a dedicated regression test plus a new fixture.
Findings
No blocking findings.
- scripts/lib/pr-worktree.sh: the new
gh pr view --json state,headRefNamecall folds the previously-separate headRefName lookup into one round-trip;_pr_branchis reused downstream so behavior of the stale-worktree release (#470) is preserved. - The early
exit 0is correctly placed before the EXIT trap registration; the inline comment documents why (avoids the trap replaying the caller's handler — observed previously as bats 'Executed 2 instead of 1 tests'). - .github/workflows/pr-review.yml change is cosmetic only (de-indenting a comment block to column 0); no permissions, secrets, or behavior change.
- Tests export
PR_STATE_APIunconditionally in setup() to prevent runner-env leakage into unrelated tests — good defensive hygiene. - All 6 review threads (gemini, copilot, coderabbit) are resolved; no unresolved or unanswered human-reviewer questions.
CI status
All substantive checks green: shellcheck/ShellCheck, bats, unit/unit-tests, validate-fixtures, CodeQL, SonarCloud, gitleaks, agent-shield, validate-agent-profiles, caller/toplevel-permissions, gh-aw-compile, etc. The CANCELLED entries (dev-lead / dispatch, dev-lead / ci-relay) are concurrency-cancelled duplicates — each shares a name with a SUCCESS/SKIPPED companion run — a known false-positive pattern (cf. #608). The only non-terminal check is review / review IN_PROGRESS, which is this review itself. mergeStateStatus=BLOCKED pending this review.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — on-mention (no-changes)Engine ran but made no changes. |
Closes #405
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
Bug Fixes
Tests
Chores