fix(review): allow fresh verdicts after thread responses

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

internal/gateio/gateio.go

@@ -917,7 +917,20 @@ 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 {
+		for _, latest := range latestVerdictReviewsByPostingIdentity(host.reviews, req.PostingIdentity) {
+			if latest.State != gitprovider.ReviewStateCommented {
+				continue
+			}
+			latestRecords := markerActionRecords(gateHostState{reviews: []gitprovider.Review{latest}}, req.PostingIdentity)
+			latestSummary := classifyMarkers(latestRecords, req.PR.Head.SHA, req.PR.Base.SHA)
+			if latestSummary.State != gate.PRStateCompleteReview {
+				return gate.PRSummary{State: gate.PRStateFresh}
+			}
+		}
+	}
+	return summary
 }
 
 func markerActionRecords(host gateHostState, posting gitprovider.Identity) []markerRecord {
@@ -990,30 +1003,44 @@ func latestCodereviewMarkerAt(host gateHostState, posting gitprovider.Identity)
 }
 
 func activeApprovalByPostingIdentity(reviews []gitprovider.Review, posting gitprovider.Identity) bool {
+	latest := latestVerdictReviewsByPostingIdentity(reviews, posting)
+	if len(latest) == 0 {
+		return false
+	}
+	for _, review := range latest {
+		if review.State != gitprovider.ReviewStateApproved {
+			return false
+		}
+	}
+	return true
+}
+
+// Review IDs are opaque, and tied timestamps do not establish chronology.
+// Keep every latest verdict so both fast paths handle ambiguity conservatively.
+func latestVerdictReviewsByPostingIdentity(reviews []gitprovider.Review, posting gitprovider.Identity) []gitprovider.Review {
 	var (
-		selected gitprovider.Review
-		found    bool
+		latest []gitprovider.Review
+		when   time.Time
 	)
 	for _, review := range reviews {
 		if !review.Author.Same(posting) {
 			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) {
-			selected = review
-			found = true
+		if len(latest) == 0 || review.SubmittedAt.After(when) {
+			latest = []gitprovider.Review{review}
+			when = review.SubmittedAt
+		} else if review.SubmittedAt.Equal(when) {
+			latest = append(latest, review)
 		}
 	}
-	return found && selected.State == gitprovider.ReviewStateApproved
+	return latest
 }
 
 func maybeExecuteApprovalOverride(ctx context.Context, opts Options, req Request, host *gateHostState) (Result, bool, error) {

internal/gateio/gateio_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -277,6 +278,158 @@ 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 TestEvaluateMarkedCommentedVerdictRemainsComplete(t *testing.T) {
+	fixture := newFixture(t)
+	submit := mustRenderAction(t, marker.ActionMarker{
+		RunID: "run-commented", ActionID: "submit-1", Kind: marker.ActionKindSubmitReview,
+		SHA: testHeadSHA, BaseSHA: testBaseSHA,
+	})
+	setReviews(t, fixture, []gitprovider.Review{{
+		ID: "review-commented", Author: fixture.req.PostingIdentity, Body: submit,
+		State: gitprovider.ReviewStateCommented, SubmittedAt: testNow,
+	}})
+
+	result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer releaseResultLock(t, result)
+	if result.Status != StatusEarlyExit || result.Decision.Kind != gate.DecisionEarlyExit {
+		t.Fatalf("Evaluate = %#v, want completed comment verdict to remain idempotent", result)
+	}
+}
+
+func TestEvaluateNewerMarkedCommentCompletesReplyRecovery(t *testing.T) {
+	fixture := newFixture(t)
+	submit := mustRenderAction(t, marker.ActionMarker{
+		RunID: "run-recovered", ActionID: "submit-1", Kind: marker.ActionKindSubmitReview,
+		SHA: testHeadSHA, BaseSHA: testBaseSHA,
+	})
+	setReviews(t, fixture, []gitprovider.Review{
+		{ID: "99", Author: fixture.req.PostingIdentity,
+			State: gitprovider.ReviewStateCommented, SubmittedAt: testNow.Add(-time.Minute)},
+		{ID: "100", Author: fixture.req.PostingIdentity, Body: submit,
+			State: gitprovider.ReviewStateCommented, SubmittedAt: testNow},
+	})
+	result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer releaseResultLock(t, result)
+	if result.Status != StatusEarlyExit || result.Decision.Kind != gate.DecisionEarlyExit {
+		t.Fatalf("Evaluate = %#v, want newer completed verdict to end reply recovery", result)
+	}
+}
+
+func TestEvaluateTiedApprovalsRemainIdempotent(t *testing.T) {
+	fixture := newFixture(t)
+	setReviews(t, fixture, []gitprovider.Review{
+		{ID: "99", Author: fixture.req.PostingIdentity,
+			State: gitprovider.ReviewStateApproved, SubmittedAt: testNow},
+		{ID: "100", Author: fixture.req.PostingIdentity,
+			State: gitprovider.ReviewStateApproved, SubmittedAt: testNow},
+	})
+	result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Status != StatusEarlyExit || result.Decision.Kind != gate.DecisionEarlyExit {
+		t.Fatalf("Evaluate = %#v, want unanimous tied approvals to remain idempotent", result)
+	}
+}
+
+func TestEvaluateTiedEmptyCommentPreventsMarkedCommentCompletion(t *testing.T) {
+	for _, reverse := range []bool{false, true} {
+		t.Run(fmt.Sprintf("reverse=%t", reverse), func(t *testing.T) {
+			fixture := newFixture(t)
+			submit := mustRenderAction(t, marker.ActionMarker{
+				RunID: "run-commented", ActionID: "submit-1", Kind: marker.ActionKindSubmitReview,
+				SHA: testHeadSHA, BaseSHA: testBaseSHA,
+			})
+			reviews := []gitprovider.Review{
+				{ID: "99", Author: fixture.req.PostingIdentity, Body: submit,
+					State: gitprovider.ReviewStateCommented, SubmittedAt: testNow},
+				{ID: "100", Author: fixture.req.PostingIdentity,
+					State: gitprovider.ReviewStateCommented, SubmittedAt: testNow},
+			}
+			if reverse {
+				reviews[0], reviews[1] = reviews[1], reviews[0]
+			}
+			setReviews(t, fixture, reviews)
+			result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer releaseResultLock(t, result)
+			if result.Status != StatusContinue || result.Decision.Kind != gate.DecisionFresh {
+				t.Fatalf("Evaluate = %#v, want fresh review after an ambiguous empty comment", result)
+			}
+		})
+	}
+}
+
+func TestEvaluateCommentedVerdictOnOldBaseRequiresFreshReview(t *testing.T) {
+	fixture := newFixture(t)
+	current := mustRenderAction(t, marker.ActionMarker{
+		RunID: "run-approved", ActionID: "submit-1", Kind: marker.ActionKindSubmitReview,
+		SHA: testHeadSHA, BaseSHA: testBaseSHA,
+	})
+	stale := mustRenderAction(t, marker.ActionMarker{
+		RunID: "run-commented", ActionID: "submit-2", Kind: marker.ActionKindSubmitReview,
+		SHA: testHeadSHA, BaseSHA: testOldBase,
+	})
+	setReviews(t, fixture, []gitprovider.Review{
+		{ID: "review-approved", Author: fixture.req.PostingIdentity, Body: current,
+			State: gitprovider.ReviewStateApproved, SubmittedAt: testNow.Add(-time.Minute)},
+		{ID: "review-commented", Author: fixture.req.PostingIdentity, Body: stale,
+			State: gitprovider.ReviewStateCommented, SubmittedAt: testNow},
+	})
+
+	result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer releaseResultLock(t, result)
+	if result.Status != StatusContinue || result.Decision.Kind != gate.DecisionFresh {
+		t.Fatalf("Evaluate = %#v, want fresh review after a stale-base comment verdict", result)
+	}
+}
+
 func TestEvaluateRetryPostsIgnoresActiveApprovalAndOverride(t *testing.T) {
 	fixture := newFixture(t)
 	run := fixture.allocateRun(t, "run-retry", testBaseSHA, ledger.PostModeLive)
@@ -446,6 +599,32 @@ func TestEvaluateSameTimestampChangesRequestedPreventsActiveApprovalExit(t *test
 	}
 }
 
+func TestEvaluateTiedNumericReviewIDsDoNotEstablishApproval(t *testing.T) {
+	for _, state := range []gitprovider.ReviewState{gitprovider.ReviewStateChangesRequested, gitprovider.ReviewStateCommented} {
+		for _, reverse := range []bool{false, true} {
+			t.Run(fmt.Sprintf("%s/reverse=%t", state, reverse), func(t *testing.T) {
+				fixture := newFixture(t)
+				reviews := []gitprovider.Review{
+					{ID: "99", Author: fixture.req.PostingIdentity, State: gitprovider.ReviewStateApproved, SubmittedAt: testNow},
+					{ID: "100", Author: fixture.req.PostingIdentity, State: state, SubmittedAt: testNow},
+				}
+				if reverse {
+					reviews[0], reviews[1] = reviews[1], reviews[0]
+				}
+				setReviews(t, fixture, reviews)
+				result, err := Evaluate(context.Background(), fixture.opts(), fixture.req)
+				if err != nil {
+					t.Fatal(err)
+				}
+				defer releaseResultLock(t, result)
+				if result.Status != StatusContinue || result.Decision.Kind != gate.DecisionFresh {
+					t.Fatalf("Evaluate = %#v, want fresh review for ambiguous tied verdicts", result)
+				}
+			})
+		}
+	}
+}
+
 func TestEvaluateAuthorOverrideAfterLatestMarkerApproves(t *testing.T) {
 	fixture := newFixture(t)
 	legacy := fixture.allocateRun(t, "legacy-override", testOldBase, ledger.PostModeLive)