fix(github): reject empty or repeated review-thread cursors

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## 0.9.5 - Unreleased
 
+- Stop review-thread GraphQL pagination when GitHub returns an empty or repeated endCursor. Thanks @SebTardif.
+
 ## 0.9.4 - 2026-08-30
 
 - Report the actual reset failure when portable-store initialization cannot recover from a dirty merge. Thanks @SebTardif.

docs/sync.md

@@ -86,6 +86,12 @@ issue or pull request URLs.
 
 PR details land in `pr_files`, `pr_commits`, `pr_checks`, and `pr_runs` tables for local review, search, clustering, and TUI workflows.
 
+Review-thread and nested-comment pagination fails if GitHub claims another page
+but returns an empty or previously followed `endCursor`. Sync reports a
+`missing endCursor` or `repeated endCursor` error instead of repeatedly fetching
+the same page. Retry after the GitHub API or proxy returns advancing cursors;
+the incomplete review-thread response is not saved as complete evidence.
+
 Use `gitcrawl coverage [owner/repo] --json` to inspect archive completeness after a sync. It reports issue, PR, comment, and review counts alongside hydrated PR detail rows, missing PR details, known failed hydrations, and detail-table row counts per repository. The additive `enrichment` object exposes supported, eligible, covered, fresh, missing, stale, completeness, ratios, and latest timestamps for revisions, fingerprints, key summaries, clusters, and PR details. Use `--repos owner/a,owner/b` to compare selected repositories and `--min-missing-pr-details N` to focus backfill work on repositories with gaps.
 
 `gitcrawl sync-failures owner/repo --json` lists unresolved PR hydration failures with their operation, error class and message, timestamps, and retry count. Add `--include-resolved` to inspect failures cleared by a later successful hydration. This operational ledger stays local when `portable prune` runs unless the publisher explicitly passes `--include-sync-failures`, which retains the ledger only after replacing every error message with a redaction marker.

internal/github/client_test.go

@@ -439,6 +439,175 @@ func TestListPullReviewThreadsPaginatesReviewThreadComments(t *testing.T) {
 	}
 }
 
+func TestListPullReviewThreadsRejectsEmptyEndCursor(t *testing.T) {
+	var calls atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if calls.Add(1) > 8 {
+			http.Error(w, "stuck pagination", http.StatusInternalServerError)
+			return
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"repository": map[string]any{"pullRequest": map[string]any{
+			"reviewThreads": map[string]any{
+				"nodes":    []map[string]any{{"id": "PRRT_1"}},
+				"pageInfo": map[string]any{"hasNextPage": true, "endCursor": ""},
+			},
+		}}}})
+	}))
+	defer server.Close()
+
+	client := New(Options{BaseURL: server.URL, PageDelay: -1})
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	_, err := client.ListPullReviewThreads(ctx, "openclaw", "gitcrawl", 8, nil)
+	if err == nil {
+		t.Fatal("expected empty endCursor error")
+	}
+	if !strings.Contains(err.Error(), "missing endCursor") {
+		t.Fatalf("error = %v", err)
+	}
+	if got := calls.Load(); got != 1 {
+		t.Fatalf("calls = %d, want 1", got)
+	}
+}
+
+func TestListPullReviewThreadsRejectsRepeatedEndCursor(t *testing.T) {
+	var calls atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if calls.Add(1) > 8 {
+			http.Error(w, "stuck pagination", http.StatusInternalServerError)
+			return
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"repository": map[string]any{"pullRequest": map[string]any{
+			"reviewThreads": map[string]any{
+				"nodes":    []map[string]any{{"id": "PRRT_1"}},
+				"pageInfo": map[string]any{"hasNextPage": true, "endCursor": "thread-cursor-1"},
+			},
+		}}}})
+	}))
+	defer server.Close()
+
+	client := New(Options{BaseURL: server.URL, PageDelay: -1})
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	_, err := client.ListPullReviewThreads(ctx, "openclaw", "gitcrawl", 8, nil)
+	if err == nil {
+		t.Fatal("expected repeated endCursor error")
+	}
+	if !strings.Contains(err.Error(), "repeated endCursor") {
+		t.Fatalf("error = %v", err)
+	}
+	if got := calls.Load(); got != 2 {
+		t.Fatalf("calls = %d, want 2", got)
+	}
+}
+
+func TestListPullReviewThreadsPaginatesReviewThreads(t *testing.T) {
+	var calls int
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
+		var body graphqlEnvelope
+		if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+			t.Fatalf("decode request: %v", err)
+		}
+		switch calls {
+		case 1:
+			if body.Variables["cursor"] != nil {
+				t.Fatalf("first request should omit cursor, variables=%+v", body.Variables)
+			}
+			_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"repository": map[string]any{"pullRequest": map[string]any{
+				"reviewThreads": map[string]any{
+					"nodes":    []map[string]any{{"id": "PRRT_1"}},
+					"pageInfo": map[string]any{"hasNextPage": true, "endCursor": "thread-cursor-1"},
+				},
+			}}}})
+		case 2:
+			if body.Variables["cursor"] != "thread-cursor-1" {
+				t.Fatalf("second request cursor = %+v", body.Variables)
+			}
+			_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"repository": map[string]any{"pullRequest": map[string]any{
+				"reviewThreads": map[string]any{
+					"nodes":    []map[string]any{{"id": "PRRT_2"}},
+					"pageInfo": map[string]any{"hasNextPage": false, "endCursor": "thread-cursor-2"},
+				},
+			}}}})
+		default:
+			t.Fatalf("unexpected graphql call %d", calls)
+		}
+	}))
+	defer server.Close()
+
+	client := New(Options{BaseURL: server.URL, PageDelay: -1})
+	rows, err := client.ListPullReviewThreads(context.Background(), "openclaw", "gitcrawl", 8, nil)
+	if err != nil {
+		t.Fatalf("list review threads: %v", err)
+	}
+	if calls != 2 {
+		t.Fatalf("calls = %d", calls)
+	}
+	if len(rows) != 2 || rows[0]["id"] != "PRRT_1" || rows[1]["id"] != "PRRT_2" {
+		t.Fatalf("rows = %#v", rows)
+	}
+}
+
+func TestListPullReviewThreadsRejectsRepeatedCommentEndCursor(t *testing.T) {
+	var calls atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		call := calls.Add(1)
+		if call > 8 {
+			http.Error(w, "stuck pagination", http.StatusInternalServerError)
+			return
+		}
+		var body graphqlEnvelope
+		if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+			t.Fatalf("decode request: %v", err)
+		}
+		if call == 1 {
+			if body.Variables["threadID"] != nil {
+				t.Fatalf("first request should fetch review threads, variables=%+v", body.Variables)
+			}
+			_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"repository": map[string]any{"pullRequest": map[string]any{
+				"reviewThreads": map[string]any{
+					"nodes": []map[string]any{{
+						"id": "PRRT_1",
+						"comments": map[string]any{
+							"nodes":    []map[string]any{{"id": "PRRC_1"}},
+							"pageInfo": map[string]any{"hasNextPage": true, "endCursor": "comment-cursor-1"},
+						},
+					}},
+					"pageInfo": map[string]any{"hasNextPage": false, "endCursor": ""},
+				},
+			}}}})
+			return
+		}
+		if body.Variables["threadID"] != "PRRT_1" || body.Variables["cursor"] != "comment-cursor-1" {
+			t.Fatalf("comment page variables = %+v", body.Variables)
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{
+			"node": map[string]any{
+				"comments": map[string]any{
+					"nodes":    []map[string]any{{"id": "PRRC_2"}},
+					"pageInfo": map[string]any{"hasNextPage": true, "endCursor": "comment-cursor-1"},
+				},
+			},
+		}})
+	}))
+	defer server.Close()
+
+	client := New(Options{BaseURL: server.URL, PageDelay: -1})
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	_, err := client.ListPullReviewThreads(ctx, "openclaw", "gitcrawl", 8, nil)
+	if err == nil {
+		t.Fatal("expected repeated comment endCursor error")
+	}
+	if !strings.Contains(err.Error(), "repeated endCursor") {
+		t.Fatalf("error = %v", err)
+	}
+	if got := calls.Load(); got != 2 {
+		t.Fatalf("calls = %d, want 2", got)
+	}
+}
+
 func TestNextPageAndReporterBranches(t *testing.T) {
 	header := `<https://api.github.test/repos/o/r/issues?page=2&state=open>; rel="next", <https://api.github.test/repos/o/r/issues?page=9>; rel="last"`
 	if got := nextPage(header, "https://api.github.test"); got != "/repos/o/r/issues?page=2&state=open" {

internal/github/review_threads.go

@@ -122,6 +122,7 @@ type graphqlResponseEnvelope struct {
 func (c *Client) ListPullReviewThreads(ctx context.Context, owner, repo string, number int, reporter Reporter) ([]map[string]any, error) {
 	var out []map[string]any
 	var cursor string
+	seen := make(map[string]struct{})
 	for {
 		vars := map[string]any{
 			"owner": owner,
@@ -148,7 +149,15 @@ func (c *Client) ListPullReviewThreads(ctx context.Context, owner, repo string,
 		if !page.PageInfo.HasNextPage {
 			break
 		}
-		cursor = page.PageInfo.EndCursor
+		next := page.PageInfo.EndCursor
+		if next == "" {
+			return nil, fmt.Errorf("review threads page missing endCursor")
+		}
+		if _, ok := seen[next]; ok {
+			return nil, fmt.Errorf("review threads page repeated endCursor %q", next)
+		}
+		seen[next] = struct{}{}
+		cursor = next
 	}
 	return out, nil
 }
@@ -162,11 +171,16 @@ func (c *Client) completeReviewThreadComments(ctx context.Context, thread map[st
 	if !comments.PageInfo.HasNextPage {
 		return nil
 	}
+	seen := make(map[string]struct{})
 	for comments.PageInfo.HasNextPage {
 		cursor := comments.PageInfo.EndCursor
 		if cursor == "" {
 			return fmt.Errorf("review thread %s comments page missing endCursor", threadID)
 		}
+		if _, ok := seen[cursor]; ok {
+			return fmt.Errorf("review thread %s comments page repeated endCursor %q", threadID, cursor)
+		}
+		seen[cursor] = struct{}{}
 		vars := map[string]any{"threadID": threadID, "cursor": cursor}
 		var resp pullReviewThreadCommentsResponse
 		if err := c.doGraphQL(ctx, pullReviewThreadCommentsQuery, vars, reporter, &resp); err != nil {