feat(poller): CI-gate review readiness (red CI resumes to fix)#141
Conversation
A PR isn't reviewable until it builds, so CI standing now participates in the review verdict (the spec note from the reconcile PR, implemented): - PrSnapshot carries a collapsed `ci` (deriveCiStatus over statusCheckRollup: any failure → failing, else any unfinished → pending, else passing; no checks → none). findPrForEpic fetches it. - classifyReviewOutcome is CI-gated: explicit review feedback still wins; else failing CI resumes to fix it (CI_FAILED decision); an APPROVED/0-actionable verdict while CI is pending is held (null) so the loop never ends on an un-built PR; passing/none resolves. Absent CI is non-blocking — the pre-CI review loop is unchanged. - The resume brief branches on CI_FAILED: a fix-CI brief (pull failing checks via gh, fix the cause, push once) distinct from the address-review brief. Deferred (noted in spec): dead/stuck-CI → waiting-human needs a pending timeout.
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntegrates GitHub CI check status into the dispatcher's PR polling and review resume loop: adds deriveCiStatus() to normalize statusCheckRollup, embeds ci in PR snapshots, gates classifyReviewOutcome on CI (failing → CI_FAILED changes-requested; pending → hold), and adds CI-failure resume briefs in the workflow. ChangesCI Status Gating for PR Review Loop
Sequence Diagram(s)sequenceDiagram
participant Poller
participant Gateway as gh_pr_view
participant GitHub
Poller->>Gateway: request PR view with statusCheckRollup
Gateway->>GitHub: fetch PR checks
GitHub-->>Gateway: statusCheckRollup array
Gateway->>Gateway: deriveCiStatus(rollup)
Gateway-->>Poller: PrSnapshot { ..., ci }
sequenceDiagram
participant Poller
participant Classifier as classifyReviewOutcome
participant Snapshot
Poller->>Classifier: classifyReviewOutcome(snapshot)
Classifier->>Snapshot: read review verdicts
Classifier->>Snapshot: read ci
Note over Classifier,Snapshot: CHANGES_REQUESTED -> return review verdict
Note over Classifier,Snapshot: ci == failing -> return changes-requested with CI_FAILED_DECISION
Note over Classifier,Snapshot: ci == pending -> return null (hold)
Note over Classifier,Snapshot: else -> apply review-only classification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
✅ Actions performedReview triggered.
|
![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 `@packages/dispatcher/src/poller-gateway.ts`:
- Around line 23-24: STATE_OK currently includes "EXPECTED", causing
deriveCiStatus to treat StatusState.EXPECTED as passing; remove "EXPECTED" from
the STATE_OK set (or only include "SUCCESS" and "EXPECTED" must not be
considered OK) and update deriveCiStatus so that it treats entries with state
=== "EXPECTED" as pending (similar to how "PENDING" is handled). Then add a unit
assertion to the existing test "legacy StatusContext entries (state) are read
too" that calls deriveCiStatus([{ state: "EXPECTED" }]) and expects "pending" to
ensure the behavior. Reference: STATE_OK constant and deriveCiStatus function
and the existing test name to locate where to change and add the assertion.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 846eb745-6b16-4b1e-b442-2ec51f16fa10
📒 Files selected for processing (6)
packages/dispatcher/src/poller-gateway.tspackages/dispatcher/src/poller.tspackages/dispatcher/src/workflows/implementation.tspackages/dispatcher/test/implementation-workflow.test.tspackages/dispatcher/test/poller-gateway.test.tspackages/dispatcher/test/poller.test.ts
CodeRabbit on #141: deriveCiStatus put EXPECTED in STATE_OK, so an expected-but- not-yet-reported status read as a green gate. EXPECTED means a status is awaited and not final — treat it as pending (like PENDING); only an actual SUCCESS is passing.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
What
Make red CI a resume trigger and gate review-resolution on green CI — a PR can't be reviewed until it builds. Implements the spec note from #139.
How
PrSnapshot.ci— a collapsedCiStatus(passing/failing/pending/none) fromstatusCheckRollup, via the pure, unit-testedderiveCiStatus(any failure → failing; else any unfinished check → pending; else passing; no checks → none; handles CheckRun + legacy StatusContext).findPrForEpicnow fetches it.classifyReviewOutcomeis CI-gated: explicit review feedback still wins; else failing CI resumes to fix it (CI_FAILEDdecision); else anAPPROVED/0-actionable verdict while CI is pending is held (null) so the loop never ends on an un-built PR;passing/noneresolves. Absent CI is non-blocking — the pre-CI loop is unchanged.CI_FAILED— a fix-CI brief (gh pr checks/gh run view --log-failed, fix the cause, push once), distinct from the address-review brief.What to verify
deriveCiStatusacross CheckRun + StatusContext; failure outranks pending —poller-gateway.test.ts.CI_FAILED; APPROVED+pending→held; APPROVED+passing→resolved; review feedback wins; absent CI unchanged —poller.test.ts.CI_FAILEDresumes with the fix-CI brief, not the address-review one —implementation-workflow.test.ts.Deferred (noted in spec)
Dead/stuck CI →
waiting-humanneeds a pending-past-timeout detector; v1 re-polls a pending PR (cheap — no resume fired) until it reports.Summary by CodeRabbit
New Features
Tests