cancel review

internal/agents/review.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"math"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -92,6 +93,17 @@ func (m *Manager) afterSnapshot(snap *api.Snapshot) {
 	if task.Status != api.TaskStatusActive {
 		return
 	}
+	reviews, err := m.store.ListReviews(ctx, task.Id)
+	if err != nil {
+		slog.Error("list reviews to review", "task", task.Id, "err", err)
+		return
+	}
+	for _, r := range reviews {
+		if r.SnapshotN > snap.N {
+			slog.Info("review not started, a newer snapshot has one", "task", task.Id, "snapshot", snap.N, "newer", r.SnapshotN)
+			return
+		}
+	}
 	err = m.startReviewer(ctx, task, *snap)
 	if err != nil {
 		slog.Error("start review", "task", task.Id, "snapshot", snap.N, "err", err)
@@ -157,6 +169,7 @@ func (m *Manager) startReviewer(ctx context.Context, task api.Task, snap api.Sna
 			return err
 		}
 		slog.Info("review skipped, snapshot has no diff", "task", task.Id, "snapshot", snap.N)
+		m.failRunningReviews(ctx, task.Id, project, snap.N, fmt.Sprintf("superseded by snapshot %d", snap.N))
 		return nil
 	}
 
@@ -211,6 +224,7 @@ func (m *Manager) startReviewer(ctx context.Context, task api.Task, snap api.Sna
 		return err
 	}
 	slog.Info("review started", "review", reviewID, "task", task.Id, "snapshot", snap.N, "vendor", vendor, "model", model)
+	m.failRunningReviews(ctx, task.Id, project, snap.N, fmt.Sprintf("superseded by snapshot %d", snap.N))
 	return nil
 }
 
@@ -496,17 +510,21 @@ func (m *Manager) failReview(ctx context.Context, reviewID, reason string) {
 func (m *Manager) cancelReviews(ctx context.Context, task api.Task, project api.Project) {
 	m.reviewMu.Lock()
 	defer m.reviewMu.Unlock()
-	reviews, err := m.store.ListReviews(ctx, task.Id)
+	m.failRunningReviews(ctx, task.Id, project, math.MaxInt, "the task was closed")
+}
+
+func (m *Manager) failRunningReviews(ctx context.Context, taskID string, project api.Project, before int, reason string) {
+	reviews, err := m.store.ListReviews(ctx, taskID)
 	if err != nil {
-		slog.Error("list reviews to cancel", "task", task.Id, "err", err)
+		slog.Error("list reviews to cancel", "task", taskID, "err", err)
 		return
 	}
 	for _, r := range reviews {
-		if r.State != api.ReviewStateRunning {
+		if r.State != api.ReviewStateRunning || r.SnapshotN >= before {
 			continue
 		}
 		m.endReviewer(ctx, r.AgentSessionId, project, r.Id)
-		m.failReview(ctx, r.Id, "the task was closed")
+		m.failReview(ctx, r.Id, reason)
 	}
 }