fix(github): clean up orphaned review PR dedup records when task is already deleted (#551)

apps/backend/internal/github/service.go

@@ -969,6 +969,12 @@ func (s *Service) CleanupMergedReviewTasks(ctx context.Context, watch *ReviewWat
 			continue
 		}
 		if err := s.taskDeleter.DeleteTask(ctx, rpt.TaskID); err != nil {
+			if strings.Contains(err.Error(), "not found") {
+				// Task already deleted; clean up the orphaned dedup record.
+				_ = s.store.DeleteReviewPRTask(ctx, rpt.ID)
+				deleted++
+				continue
+			}
 			s.logger.Warn("failed to delete review PR task",
 				zap.String("task_id", rpt.TaskID), zap.Error(err))
 			continue

apps/backend/internal/github/service_test.go

@@ -3,6 +3,7 @@ package github
 import (
 	"context"
 	"database/sql"
+	"fmt"
 	"strings"
 	"sync"
 	"testing"
@@ -746,3 +747,70 @@ func TestTriggerPRSync_NoWatch(t *testing.T) {
 		t.Errorf("expected nil TaskPR for task without watch, got %+v", result)
 	}
 }
+
+// stubTaskDeleter implements TaskDeleter for testing.
+type stubTaskDeleter struct {
+	err error
+}
+
+func (s *stubTaskDeleter) DeleteTask(_ context.Context, _ string) error {
+	return s.err
+}
+
+// TestCleanupMergedReviewTasks_TaskAlreadyDeleted verifies that when DeleteTask
+// returns "not found" the orphaned dedup record is still removed, preventing the
+// 5-minute poller from logging the same warning indefinitely.
+func TestCleanupMergedReviewTasks_TaskAlreadyDeleted(t *testing.T) {
+	_, svc, mockClient, store := setupPollerTest(t)
+	ctx := context.Background()
+
+	// Create a review watch.
+	watch := &ReviewWatch{WorkspaceID: "ws-1", Enabled: true}
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("CreateReviewWatch: %v", err)
+	}
+
+	// Create a dedup record pointing to an already-deleted task.
+	taskID := "task-already-gone"
+	rpt := &ReviewPRTask{
+		ReviewWatchID: watch.ID,
+		RepoOwner:     "acme",
+		RepoName:      "widget",
+		PRNumber:      42,
+		PRURL:         "https://github.com/acme/widget/pull/42",
+		TaskID:        taskID,
+	}
+	if err := store.CreateReviewPRTask(ctx, rpt); err != nil {
+		t.Fatalf("CreateReviewPRTask: %v", err)
+	}
+
+	// Mock: PR is merged so shouldDeleteReviewTask returns true.
+	mockClient.AddPR(&PR{
+		Number:    42,
+		State:     prStateMerged,
+		RepoOwner: "acme",
+		RepoName:  "widget",
+	})
+
+	// Stub: DeleteTask returns "not found" as the real service does.
+	svc.SetTaskDeleter(&stubTaskDeleter{
+		err: fmt.Errorf("task not found: %s", taskID),
+	})
+
+	deleted, err := svc.CleanupMergedReviewTasks(ctx, watch)
+	if err != nil {
+		t.Fatalf("CleanupMergedReviewTasks returned error: %v", err)
+	}
+	if deleted != 1 {
+		t.Errorf("expected 1 deleted, got %d", deleted)
+	}
+
+	// The orphaned dedup record must be gone.
+	remaining, err := store.ListReviewPRTasksByWatch(ctx, watch.ID)
+	if err != nil {
+		t.Fatalf("ListReviewPRTasksByWatch: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("expected 0 remaining dedup records, got %d", len(remaining))
+	}
+}