fix(pipeline): incorporate unresolved review comments into CI gate and auto-fix - #890
fix(pipeline): incorporate unresolved review comments into CI gate and auto-fix#890khaira777 wants to merge 20 commits into
Conversation
Confidence Score: 5/5The PR appears safe to merge. The previously reported nested-pagination issue is fixed: every separately fetched review-comment page is bound to the initial PR head, mismatches fail closed, and the resulting review snapshot is checked against the CI checks head before use; no blocking failure remains. Reviews (6): Last reviewed commit: "no-mistakes(ci): Diagnosed and fixed tes..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
khaira777
changed the title
khaira777
changed the title
|
Speaking as Kun's firstmate: #870 was already closed as fixed-by-#873 (ingest into the existing repair loop). This PR is a follow-up that treats unresolved bot review comments as CI issues that block readiness even when Not otherwise ready yet. Inspected CI is green and Greptile is 5/5 on this tip. Until that empty-head path is fail-closed, this stays waiting on you. contract-class: new-default (readiness blocking is always-on; comment auto-fix remains opt-in via VISION (not merge-ready):
|
…nd readiness gate (kunchenguid#870)
…; focused tests pass
…t auto-fix budgets
…`test (macos-latest)` package timeout in `internal/pipeline/steps` caused by a non-parallel 31-second sleep in `TestIntentStep_SlowExtractionPastOldTimeoutStillAttachesIntent`. Replaced the real sleep with a deadline check asserting extraction context deadline is well past the old 30-second timeout (>200s), eliminating the blocking 31-second delay and drastically reducing test execution time. 2. Fixed `Greptile Review` and review bot comment matching in `internal/scm/github` by updating `isSupportedReviewBot` to support standard `greptile[bot]`, `greptile`, `coderabbit[bot]`, `coderabbit`, `codeql[bot]`, and `codeql` usernames alongside existing aliases. 3. Verified with `go vet ./...` and `go test -count=1 ./internal/pipeline/steps ./internal/scm/github` (all passing cleanly)
…st (macos-latest)` package timeout in `internal/pipeline/steps`: Tests in `ci_autofix_test.go` and `ci_checks_test.go` lacked parallelization and mock `baseBranchTip` definitions, causing them to execute sequentially and perform repeated unmocked git fetch / network operations during `step.Execute`. Marked the tests parallel and supplied `baseBranchTip` mocks across `TestCIStep_ReviewAutoFixWaitsForKnownReadiness`, `TestCIStep_UnresolvedCancellationBlocksReviewAutoFix`, `TestCIStep_AwaitingCancellationRerunBlocksReviewAutoFix`, `TestCIStep_UnresolvedReviewCommentsBlockReadinessWhenAutoFixDisabled`, and `TestCIStep_FixMode_DoesNotRunWithoutSelectedReviewTargets`. 2. `Greptile Review` and review bot coverage: Added additional review bot username aliases (`greptile-ai[bot]`, `greptile-ai`, `greptile-review[bot]`, `greptile-review`, `coderabbit-ai[bot]`, `coderabbit-ai`) to `isSupportedReviewBot` and added corresponding test cases in `github_test.go`. 3. Verified locally with `go vet ./...` and `go test -count=1 ./internal/pipeline/steps ./internal/scm/github ./internal/scm` (all passing cleanly)
…uest { headRefOid }` in the nested review thread comments GraphQL query (`reviewThreadCommentsQuery`), validating the PR head across nested comments pagination against the initial PR head SHA, and adding unit tests for nested comment pagination head mismatch
1260911 to
06e4e50
Compare
khaira777
changed the title
…l/pipeline/steps`. In `TestCIStep_ReviewAutoFixWaitsForKnownReadiness`, the poll loop previously executed with an unmocked clock and immediate poll return, resulting in tight-loop subprocess executions for the full 30-second CI timeout across parallel test cases. Mocked `now` and advanced the simulated clock past timeout in `waitForNextPoll` (matching `TestCIStep_TimeoutPreservesSuccessfulReviewSnapshot`), and supplied the mocked `baseBranchTip` in `TestCIStep_UnresolvedReviewCommentsTriggerAutoFixWhenChecksPass`. Verified with `go vet ./...` and `go test -race ./internal/pipeline/steps`
khaira777
changed the title
|
Speaking as the PR author: Thanks for the thoughtful review and feedback! Here is a summary of the approach, the distinction from #873, and the fixes made in response to your comment: 1. Why this PR builds upon #873
2. Addressed Firstmate's Finding (
|
2 participants
Intent
Fulfill Issue #870 end-to-end: ensure unresolved GitHub review comments participate in CI issue detection and readiness gate, fail closed when review comments responses lack PR head, and resolve merge conflicts with main.
What Changed
auto_fix.reviewbudget.Risk Assessment
✅ Low: The changes cleanly integrate unresolved GitHub review comments into CI issue detection, readiness gating, and auto-fix budgeting with robust head validation and bounded error handling.
Testing
Executed targeted automated test suites covering GitHub review-comments fail-closed PR head matching during thread and comment pagination, supported bot username aliasing, CI readiness gate blocking on unresolved bot comments, review auto-fix budget enforcement, readiness dependency checks (PR state, mergeability, check runs, cancellations, and awaiting-rerun guards), timeout recovery preserving review targets, and durable CI review state persistence; all 26 targeted tests passed cleanly with evidence recorded in review_comments_gate_evidence.log.
Evidence: review_comments_gate_evidence.log
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 2 issues found → auto-fixed ✅
internal/pipeline/steps/ci.go- merge conflict rebasing onto origin/maininternal/pipeline/steps/ci_fix.go- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Review** - passed
✅ No issues found.
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
go test -v -count=1 ./internal/scm/github -run 'TestHost_GetReviewComments'go test -v -count=1 ./internal/pipeline/steps -run 'TestCIStep.*Review'go test -count=1 ./internal/scm/github ./internal/types ./internal/db✅ No issues found.
go test -v -count=1 ./internal/scm/github -run "TestHost_GetReviewComments|TestIsSupportedReviewBot"go test -v -count=1 ./internal/pipeline/steps -run "TestCIStep_AutoFixIngestsReviewComments|TestCIStep_UnresolvedReviewCommentsTriggerAutoFixWhenChecksPass|TestCIStep_ReviewAutoFixWaitsForKnownReadiness|TestCIStep_UnresolvedCancellationBlocksReviewAutoFix|TestCIStep_AwaitingCancellationRerunBlocksReviewAutoFix|TestCIStep_UnresolvedReviewCommentsBlockReadinessWhenAutoFixDisabled|TestCIStep_FixMode_DoesNotRunWithoutSelectedReviewTargets|TestCIStep_TimeoutPreservesSuccessfulReviewSnapshot|TestCIFixAgentTimeoutOutcomePreservesReviewTargets|TestCICheckReadFailureOutcome_ProviderNeutral"go test -v -count=1 ./internal/db -run "TestSetCIReviewState"go test -v -count=1 ./internal/types -run "TestHasAskUserFindings|TestEmptyActionFindingFailsClosedToAskUser"✅ **Document** - passed
✅ No issues found.
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.