Repository navigation
chore: supervisor: PR-approval Telegram ping fires before all CI checks exist (check-run snapshot race) - #688
Conversation
Fixed the false-positive-early-ping bug in `decide_from_checks` (`src/worktree/mod.rs:350`), the shared decision function for both call sites (`pr_ci_status_impl`'s REST path and `pr_ci_status_from_batch`'s GraphQL batch path). **Change:** `decide_from_checks` now takes an additional `mergeable_state: Option<&str>` param. When the fetched check-runs all look green (0 pending, 0 failed) but GitHub's own `mergeable_state` is `"blocked"`, it now returns `PrCiStatus::Pending` instead of `Passing` — since "blocked" means GitHub's required-checks/branch-protection view still has outstanding items the sweep's check-run snapshot hasn't picked up yet (e.g. gated `build` matrix jobs not yet created). Both call sites already had `mergeable_state` in hand (used for the existing `Behind` check), so no new API calls needed. **Tests:** Added `pr_ci_status_from_batch_reports_pending_when_blocked_despite_green_checks`, mirroring the live repro (green checks + `mergeable_state: "blocked"` → `Pending`, not `Passing`). All 8 tests in `worktree::tests` pass, including the pre-existing ones (`clean` mergeable_state still reports `Passing`/`Failing`/`Pending` as before). **Verification run synchronously:** - `cargo build --lib` — clean - `cargo test --bin agentflare pr_ci_status` — 8/8 pass - `cargo fmt --check -- src/worktree/mod.rs` — clean except one pre-existing, unrelated diff (import ordering at line 51) confirmed present on the base branch before my changes (verified via stash) - `cargo clippy --bin agentflare --all-targets` — no new warnings (only pre-existing unrelated `unsafe`-block warnings elsewhere) **Concerns:** None blocking. One judgment call worth flagging: I scoped the cross-check to `mergeable_state == "blocked"` only, per the issue's repro. Other transient states like `"unknown"` (GitHub still computing mergeability) or `"dirty"` weren't touched — the issue didn't report those as problematic and expanding scope there felt speculative rather than evidence-driven. Agentflare-Branch: task/259-supervisor-pr-approval-telegram-ping-fir Agentflare-Item: 259
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughCI status evaluation now passes GitHub’s ChangesCI status evaluation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No merge-blocking risk is established for the reported CI-status correction. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use Comment |
|
Tracking this PR for automated review ( |
Status
Fixed the false-positive-early-ping bug in
decide_from_checks(src/worktree/mod.rs:350), the shared decision function for both call sites (pr_ci_status_impl's REST path andpr_ci_status_from_batch's GraphQL batch path).Change:
decide_from_checksnow takes an additionalmergeable_state: Option<&str>param. When the fetched check-runs all look green (0 pending, 0 failed) but GitHub's ownmergeable_stateis"blocked", it now returnsPrCiStatus::Pendinginstead ofPassing— since "blocked" means GitHub's required-checks/branch-protection view still has outstanding items the sweep's check-run snapshot hasn't picked up yet (e.g. gatedbuildmatrix jobs not yet created). Both call sites already hadmergeable_statein hand (used for the existingBehindcheck), so no new API calls needed.Tests: Added
pr_ci_status_from_batch_reports_pending_when_blocked_despite_green_checks, mirroring the live repro (green checks +mergeable_state: "blocked"→Pending, notPassing). All 8 tests inworktree::testspass, including the pre-existing ones (cleanmergeable_state still reportsPassing/Failing/Pendingas before).Verification run synchronously:
cargo build --lib— cleancargo test --bin agentflare pr_ci_status— 8/8 passcargo fmt --check -- src/worktree/mod.rs— clean except one pre-existing, unrelated diff (import ordering at line 51) confirmed present on the base branch before my changes (verified via stash)cargo clippy --bin agentflare --all-targets— no new warnings (only pre-existing unrelatedunsafe-block warnings elsewhere)Concerns: None blocking. One judgment call worth flagging: I scoped the cross-check to
mergeable_state == "blocked"only, per the issue's repro. Other transient states like"unknown"(GitHub still computing mergeability) or"dirty"weren't touched — the issue didn't report those as problematic and expanding scope there felt speculative rather than evidence-driven.Opened by
claude-codeon flared:51bb8de6c33b for item #259 via agentflare.Summary by CodeRabbit