From 18d95d74724a03da5cfd1c58529d59a50bee5e92 Mon Sep 17 00:00:00 2001 From: piekstra Date: Tue, 15 Sep 2026 09:57:52 -0400 Subject: [PATCH] Fix review reruns after thread responses --- internal/cmd/reviewcmd/reviewcmd.go | 4 +++- internal/gateio/gateio.go | 24 ++++++++++++++------ internal/gateio/gateio_test.go | 35 +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/internal/cmd/reviewcmd/reviewcmd.go b/internal/cmd/reviewcmd/reviewcmd.go index 4949407a..29cb8111 100644 --- a/internal/cmd/reviewcmd/reviewcmd.go +++ b/internal/cmd/reviewcmd/reviewcmd.go @@ -35,7 +35,9 @@ const reviewLong = `Run an automated pull-request review. Live review checks local and host state before starting the reviewer loop. By default, if the posting identity has already approved the PR, cr exits before any LLM classifier or reviewer work, even if newer commits made that approval -stale. Use --rerun to bypass these local gates and force a new live review. +stale. A newer COMMENTED review from the posting identity supersedes that fast +path so thread-response activity can be followed by a fresh verdict. Use +--rerun to bypass these local gates and force a new live review. Session reuse is independent of local review gates. Plain follow-up reviews and --rerun reuse the PR's original reviewer cohort and each reviewer's provider diff --git a/internal/gateio/gateio.go b/internal/gateio/gateio.go index be561b76..51266ad4 100644 --- a/internal/gateio/gateio.go +++ b/internal/gateio/gateio.go @@ -917,7 +917,14 @@ func readGateHostStateWithReviews(ctx context.Context, provider outbox.LiveProvi func summarizePRFromHost(host gateHostState, req Request) gate.PRSummary { records := markerActionRecords(host, req.PostingIdentity) - return classifyMarkers(records, req.PR.Head.SHA, req.PR.Base.SHA) + summary := classifyMarkers(records, req.PR.Head.SHA, req.PR.Base.SHA) + if summary.State == gate.PRStateCompleteReview { + latest, found := latestVerdictReviewByPostingIdentity(host.reviews, req.PostingIdentity) + if found && latest.State == gitprovider.ReviewStateCommented { + return gate.PRSummary{State: gate.PRStateFresh} + } + } + return summary } func markerActionRecords(host gateHostState, posting gitprovider.Identity) []markerRecord { @@ -990,6 +997,11 @@ func latestCodereviewMarkerAt(host gateHostState, posting gitprovider.Identity) } func activeApprovalByPostingIdentity(reviews []gitprovider.Review, posting gitprovider.Identity) bool { + selected, found := latestVerdictReviewByPostingIdentity(reviews, posting) + return found && selected.State == gitprovider.ReviewStateApproved +} + +func latestVerdictReviewByPostingIdentity(reviews []gitprovider.Review, posting gitprovider.Identity) (gitprovider.Review, bool) { var ( selected gitprovider.Review found bool @@ -999,21 +1011,19 @@ func activeApprovalByPostingIdentity(reviews []gitprovider.Review, posting gitpr continue } switch review.State { - case gitprovider.ReviewStateApproved, gitprovider.ReviewStateChangesRequested: - case gitprovider.ReviewStateCommented, gitprovider.ReviewStateDismissed, gitprovider.ReviewStatePending: + case gitprovider.ReviewStateApproved, gitprovider.ReviewStateChangesRequested, gitprovider.ReviewStateCommented: + case gitprovider.ReviewStateDismissed, gitprovider.ReviewStatePending: continue default: continue } if !found || review.SubmittedAt.After(selected.SubmittedAt) || - (review.SubmittedAt.Equal(selected.SubmittedAt) && - selected.State == gitprovider.ReviewStateApproved && - review.State == gitprovider.ReviewStateChangesRequested) { + (review.SubmittedAt.Equal(selected.SubmittedAt) && string(review.ID) > string(selected.ID)) { selected = review found = true } } - return found && selected.State == gitprovider.ReviewStateApproved + return selected, found } func maybeExecuteApprovalOverride(ctx context.Context, opts Options, req Request, host *gateHostState) (Result, bool, error) { diff --git a/internal/gateio/gateio_test.go b/internal/gateio/gateio_test.go index 7374a064..09bc4544 100644 --- a/internal/gateio/gateio_test.go +++ b/internal/gateio/gateio_test.go @@ -277,6 +277,41 @@ func TestEvaluateActivePostingIdentityApprovalExitsBeforeOverrideReads(t *testin } } +func TestEvaluateNewerCommentedReviewDoesNotUseApprovalFastPath(t *testing.T) { + fixture := newFixture(t) + submit := mustRenderAction(t, marker.ActionMarker{ + RunID: "run-approved", + ActionID: "submit-1", + Kind: marker.ActionKindSubmitReview, + SHA: testHeadSHA, + BaseSHA: testBaseSHA, + }) + setReviews(t, fixture, []gitprovider.Review{ + { + ID: "review-approved", + Author: fixture.req.PostingIdentity, + Body: submit, + State: gitprovider.ReviewStateApproved, + SubmittedAt: testNow.Add(-time.Minute), + }, + { + ID: "review-commented", + Author: fixture.req.PostingIdentity, + State: gitprovider.ReviewStateCommented, + SubmittedAt: testNow, + }, + }) + + result, err := Evaluate(context.Background(), fixture.opts(), fixture.req) + if err != nil { + t.Fatalf("Evaluate: %v", err) + } + defer releaseResultLock(t, result) + if result.Status != StatusContinue || result.Decision.Kind != gate.DecisionFresh { + t.Fatalf("Evaluate = %#v, want fresh review after newer commented review", result) + } +} + func TestEvaluateRetryPostsIgnoresActiveApprovalAndOverride(t *testing.T) { fixture := newFixture(t) run := fixture.allocateRun(t, "run-retry", testBaseSHA, ledger.PostModeLive)