fix(auto-rebase): surface skipped-but-conflicting PRs instead of silent rot (#711 Part C) - #713
Conversation
…ing them rot (#711 Part C) The review-ready eligibility gate (#465) skips any PR that isn't approved or labeled. A skipped PR that is ALSO conflicting is in a silent deadlock: auto-rebase won't update it, and pr-review won't approve a red PR, so it never progresses and emits no signal — the root of the fleet's weeks-stale CONFLICTING+REVIEW_REQUIRED cohort (#711). When a PR is skipped by the gate AND mergeable_state is 'dirty', post a one-time manual-attention notice (idempotent via the <!-- auto-rebase-stuck --> sentinel) explaining the three ways to unblock it (add the ready label / get it approved / resolve manually). mergeable_state is read from the PR_JSON already fetched, so there is no extra API call; posting reuses auto_rebase_post_comment_best_effort, so the 2500-comment cap can't abort the step. Complements Part A (.github-private#1201, which labels new dev-lead PRs ready). 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. |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Warning Review limit reached
Next review available in: 43 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 (1)
📝 WalkthroughWalkthroughThe reusable auto-rebase workflow now identifies skipped pull requests with a dirty merge state and posts a one-time comment containing manual unblocking options and conflict-resolution commands. ChangesAuto-rebase conflict visibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
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 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s
There was a problem hiding this comment.
Pull request overview
Adds a safety-net escalation to the auto-rebase eligibility gate so that PRs which are skipped (not review-ready) but already conflicting don’t silently rot: it posts a one-time “manual attention needed” notice (idempotent via <!-- auto-rebase-stuck -->) explaining how to unblock.
Changes:
- Detect
mergeable_state == "dirty"for PRs skipped by the eligibility gate and trigger a one-time stuck notice. - Add an idempotency sentinel (
<!-- auto-rebase-stuck -->) to avoid reposting the notice. - Post the notice via
auto_rebase_post_comment_best_effortso comment-cap/API failures don’t abort the run.
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: 36fd5ff7ff2fc50dc5d0f9e7e259a254a8937368
Review mode: triage-approved (single reviewer)
Summary
Adds a one-time, idempotent 'stuck' notice for PRs that the auto-rebase eligibility gate skips while they are also conflicting (mergeable_state=dirty) — implementing option C of issue #711. The block fires only inside the existing skip branch, reuses PR_JSON already fetched (no extra call for mergeable_state), and posts via the existing best-effort comment helper so comment-cap/API failures cannot abort the run. It mirrors the file's two established sentinel patterns exactly. Triage assessment confirmed; all CI green.
Linked issue analysis
No closing issue reference (intentional — #711 remains open for Parts A and follow-ups). The PR substantively implements option C described in #711: surface gate-skipped + CONFLICTING PRs once so they cannot rot invisibly. The diff matches the issue's described deadlock and the PR body's design notes.
Findings
- Secret scanning MCP tool unavailable in this environment; gitleaks CI check passed — no secrets in the diff (comment text and shell logic only).
- No security concerns: no new permissions/secrets/triggers; the sentinel interpolated into the jq filter is a hardcoded literal; backticks in the comment body are escaped; all expansions quoted. ShellCheck, CodeQL, AgentShield, and SonarCloud (0 issues, 0 hotspots) all green.
- Minor (non-blocking): the sentinel-presence check adds one
gh pr viewAPI call per skipped+dirty PR per run, so the PR body's 'no extra API cost' claim applies only tomergeable_stateitself. Acceptable — the cohort is small and the call pattern is identical to the two existing sentinel checks. - Minor (non-blocking, pre-existing pattern):
gh pr view --json commentsmay not return all comments on very long PRs, which could cause a duplicate notice; this limitation is shared with the existingauto-rebase-blockedandauto-rebase-conflictsentinels and posting is best-effort, so worst case is one redundant comment.
CI status
All checks green: CI (Lint, ShellCheck, Agent Security Scan, gitleaks), Auto-rebase Tests (lint + bats), CodeQL, AgentShield, SonarCloud quality gate passed (0 new issues), Dependency audit, CodeRabbit status SUCCESS. Remaining entries SKIPPED (not applicable). mergeStateStatus=BEHIND but MERGEABLE.
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/auto-rebase-reusable.yml:
- Around line 164-166: Update the STUCK_POSTED assignment in the PR-processing
loop to guard the gh pr view command against failures under set -e by
suppressing its error output and falling back to -1. Preserve the existing
zero-value check so failed lookups bypass notification handling without aborting
the loop or causing an empty-value arithmetic test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0265384-0e96-4e32-8382-1c11c00a485a
📒 Files selected for processing (1)
.github/workflows/auto-rebase-reusable.yml
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #713 |
|
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: 27a3a61a6d4dd63a6728815765b17a5bb225da21
Review mode: triage-approved (single reviewer)
Summary
Adds a one-time, idempotent manual-attention comment when the auto-rebase eligibility gate skips a PR that is also CONFLICTING (mergeable_state == dirty), breaking the silent skipped+conflicting deadlock diagnosed in #711 (Part C). +30/-0 in a single file (.github/workflows/auto-rebase-reusable.yml). Reuses the already-fetched PR_JSON (no extra API cost), reuses auto_rebase_post_comment_best_effort (no new permissions, failures logged and swallowed), and guards the sentinel lookup so an API failure errs toward not posting rather than failing the step.
Linked issue analysis
References issue #711 (open) — the auto-rebase review-ready gate deadlock. This PR implements Part C (safety net for skipped+conflicting PRs) exactly as scoped in the issue and PR body; it intentionally does not close #711, since Part A (.github-private#1201) and label-management follow-ups remain. Substantively addresses its declared slice.
Findings
No blocking findings.
- Security: No new permissions, no untrusted-input execution. The stuck sentinel is a hardcoded literal (no injection into the jq filter). Interpolated values (BASE_BRANCH, READY_LABEL, eligibility flags) come from workflow config/gate output, not attacker-controlled PR content, and land only in a markdown comment body. Secret-scanning MCP tool not available in this environment; gitleaks CI check is green.
- Correctness: Idempotency check on the sentinel falls back to "-1" on gh/API failure, which fails the -eq 0 test — safe default (skips posting rather than spamming or aborting under set -e). This resolves CodeRabbit's Major finding (guard gh pr view), which was fixed in commit 27a3a61 and the review dismissed. mergeable_state == "unknown" is handled by deferring to a later run, as documented inline.
- Resolved threads: Copilot's idempotency-slice and warning-message threads are resolved (one outdated by the fix commit). Worst case of a comment-slice miss is a single duplicate notice — acceptable for a best-effort signal.
- Open thread (non-blocking): Copilot suggested extracting the stuck-notice logic into a bats-covered helper under .github/scripts/auto-rebase/. The repo owner explicitly deferred this to a follow-up PR with rationale and left the bot-started thread open for tracking. Maintainability suggestion only — not blocking.
- Risk note: Triage classified this low-risk; I rate it MEDIUM (non-trivial logic in an org-wide reusable workflow), which still auto-approves. Blast radius is limited to posting one extra comment per stuck PR.
CI status
All checks green: ShellCheck, Lint and bats, CodeQL (actions), SonarCloud, gitleaks secret scan, npm audit, AgentShield, Agent Security Scan, CodeRabbit — all SUCCESS; ecosystem-specific audits (pip/cargo/go/pnpm) skipped as not applicable. mergeStateStatus is BLOCKED only pending review approval.
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 |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Problem
The auto-rebase
review-readyeligibility gate (#465) skips any PR that isn't approved or ready-labeled. A skipped PR that is also conflicting is in a silent deadlock: auto-rebase won't update it, andpr-reviewwon't approve a red PR — so it never progresses and emits no signal. This is the root of the fleet's weeks-staleCONFLICTING + REVIEW_REQUIREDcohort (full diagnosis: #711).Change
When a PR is skipped by the gate and
mergeable_state == "dirty", post a one-time manual-attention notice (idempotent via the<!-- auto-rebase-stuck -->sentinel) listing the three ways to unblock it:auto-rebase:readylabel (opts it into auto-rebase without approval),Design notes:
mergeable_stateis read from thePR_JSONalready fetched per PR.auto_rebase_post_comment_best_effort, so a comment-cap/API failure is logged and swallowed (can't abort the step or starve other PRs, per auto-rebase: unguarded conflict-comment aborts the whole run when a PR hits the 2500-comment cap — blocks the ring1→stable soak gate #594).mergeable_statemay beunknownuntil GitHub computes it; a later run catches those.Validation
bash -nclean; shellcheck clean.comments.bats+eligibility.batsgreen (unchanged libs).Relationship
Complements Part A (.github-private#1201) which labels new dev-lead PRs
auto-rebase:readyso they never reach this deadlock. Part C is the safety net for PRs that slip through (e.g. legacy PRs, or ones at the comment cap). Remaining #711 follow-ups: standardizeauto-rebase:readyin label management, and apply it in the other PR-opening agents.🤖 Generated with Claude Code
https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s
Summary by CodeRabbit