Skip to content

feat(poller): CI-gate review readiness (red CI resumes to fix)#140

Closed
thejustinwalsh wants to merge 1 commit into
feat/reconcile-merged-parksfrom
feat/ci-gated-review
Closed

feat(poller): CI-gate review readiness (red CI resumes to fix)#140
thejustinwalsh wants to merge 1 commit into
feat/reconcile-merged-parksfrom
feat/ci-gated-review

Conversation

@thejustinwalsh

@thejustinwalsh thejustinwalsh commented May 25, 2026

Copy link
Copy Markdown
Owner

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 added in #139.

Stacked on #139 (feat/reconcile-merged-parks) — both touch the poller's PR observation. Base retargets to main when #139 merges; review the CI-only diff here.

How

  • PrSnapshot.ci — a collapsed CiStatus (passing/failing/pending/none) from statusCheckRollup, via the pure, unit-tested deriveCiStatus (any failure → failing; else any unfinished check → pending; else passing; no checks → none; handles both CheckRun and legacy StatusContext shapes). findPrForEpic now fetches it.
  • classifyReviewOutcome is CI-gated, with clear precedence:
    1. explicit review feedback (CHANGES_REQUESTED / label) still wins — addressing it should green CI too;
    2. else failing CI resumes the agent to fix it (CI_FAILED decision);
    3. else 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 (undefinednone) is non-blocking, so the pre-CI review loop is unchanged.
  • Resume brief branches on CI_FAILED — a fix-CI brief (pull the failing checks via gh pr checks / gh run view --log-failed, fix the cause, push once), distinct from the address-review brief.

What to verify

  • deriveCiStatus: none/passing/failing/pending across CheckRun + StatusContext; failure outranks pending — packages/dispatcher/test/poller-gateway.test.ts.
  • classifyReviewOutcome CI gate: failing→CI_FAILED; APPROVED+pending→null; APPROVED+passing→resolved; review feedback wins over red CI; absent CI unchanged — poller.test.ts.
  • A CI_FAILED verdict resumes with the fix-CI brief (gh pr checks, "Push once"), not the address-review brief — implementation-workflow.test.ts.
  • Full suite 584 pass, lint/typecheck/format clean.

Deferred (noted in spec)

Dead/stuck CI → waiting-human needs a pending-past-timeout detector; v1 re-polls a pending PR (cheap — no resume fired) until it reports.

Summary by CodeRabbit

Release Notes

  • New Features

    • Workflows now incorporate GitHub CI check status when resuming pull requests.
    • Failed CI checks trigger a resume with investigation guidance for fixing checks.
    • Pending CI checks prevent resume until checks complete.
    • Passing or absent CI checks allow normal workflow resolution.
  • Tests

    • Added coverage for CI status detection and CI-gated resume workflows.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d790f224-4611-447e-9993-1a1524ef647b

📥 Commits

Reviewing files that changed from the base of the PR and between 350dabe and 51ace4f.

📒 Files selected for processing (6)
  • packages/dispatcher/src/poller-gateway.ts
  • packages/dispatcher/src/poller.ts
  • packages/dispatcher/src/workflows/implementation.ts
  • packages/dispatcher/test/implementation-workflow.test.ts
  • packages/dispatcher/test/poller-gateway.test.ts
  • packages/dispatcher/test/poller.test.ts

📝 Walkthrough

Walkthrough

This PR adds CI status detection and resume gating to the dispatcher. GitHub PR check status is parsed from statusCheckRollup and collapsed into a CiStatus. Resume verdicts now gate on CI state: failing checks trigger changes-requested decisions, pending checks hold approvals, and passing/absent checks allow normal flow. The workflow handles CI-failure resumes with a specialized prompt.

Changes

CI Status Detection and Resume Gating

Layer / File(s) Summary
CI Status Modeling and GitHub Data Fetching
packages/dispatcher/src/poller.ts, packages/dispatcher/src/poller-gateway.ts, packages/dispatcher/test/poller-gateway.test.ts
Adds CiStatus type and extends PrSnapshot with optional ci field. Exports CheckRollupEntry and deriveCiStatus() which parses GitHub status checks into passing/failing/pending/none. Updates ghPollGateway.findPrForEpic() to fetch statusCheckRollup from gh pr view and compute CI status. Tests cover success, failure, pending, precedence, and legacy state-based inputs.
CI-Gated Resume Verdict Logic
packages/dispatcher/src/poller.ts, packages/dispatcher/test/poller.test.ts
Updates classifyReviewOutcome() to apply CI precedence: explicit review feedback takes priority, failing CI triggers changes-requested with synthetic CI_FAILED_DECISION, pending CI suppresses approval until checks complete, and passing/none CI allows normal flow. Exports CI_FAILED_DECISION constant. Tests verify verdict outcomes across all CI states and review feedback combinations.
CI-Failure Resume Brief and Workflow Integration
packages/dispatcher/src/workflows/implementation.ts, packages/dispatcher/test/implementation-workflow.test.ts
Imports CI_FAILED_DECISION and extends writeResumeBrief() to detect and handle CI-failure resumes with a specialized prompt (round/cap context and check investigation steps) that bypasses the standard address-review brief. End-to-end test verifies resume continuation correctly uses CI-fix framing and settles after approval.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

dogfood

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

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.
@thejustinwalsh thejustinwalsh force-pushed the feat/ci-gated-review branch from 7c3f37c to 51ace4f Compare May 25, 2026 06:54
@thejustinwalsh thejustinwalsh deleted the branch feat/reconcile-merged-parks May 25, 2026 07:37
@thejustinwalsh

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant