diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index 64247673cc..d0e3e6f1c1 100644 --- a/apps/backend/internal/github/service.go +++ b/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 diff --git a/apps/backend/internal/github/service_test.go b/apps/backend/internal/github/service_test.go index 2d26372b2b..ae9f72a416 100644 --- a/apps/backend/internal/github/service_test.go +++ b/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)) + } +}