Skip to content

watch-pr: review threads are fetched without pagination (first 100 only) #471

Closed

Description

@Authentis

Observed at commit 2eb7ed4.

pstack/skills/poteto-mode/scripts/watch-pr/github.ts line 5 defines the review-thread query as:

reviewThreads(first: 100) {
  nodes { id isResolved comments(first: 10) { nodes { ... } } }
}

It has no pageInfo and no cursor. reviewThreads() at line 571 issues this single query and parseReviewThreads (line 360) reads only reviewThreads.nodes. By contrast, the check-rollup query in the same file (line 9) paginates with pageInfo { hasNextPage endCursor } and a loop (around line 616).

Impact

On a PR with more than 100 review threads, threads after the first 100 are never seen. policy.ts line 66 builds the PR snapshot from this list (line 66), so an unresolved thread beyond the first page is invisible to the readiness decision. Comments beyond the first 10 in a thread are likewise dropped, though that matters less because only thread-level resolution seems to be used.

Minimal reproduction

Run the watcher against a PR with 101+ review threads where only a thread past the first 100 is unresolved; the PR is reported as having no unresolved threads.

Suggested fix

Add pageInfo { hasNextPage endCursor } and an $after cursor to the reviewThreads query and loop until hasNextPage is false, as is already done for contexts. If a hard cap is wanted, report an explicit "truncated" state rather than silently using the first page.

Activity

  1. deanrie commented on Oct 3, 2026

    @deanrie

    Hey, thanks — this one's clear from the code alone. REVIEW_THREADS_QUERY asks for reviewThreads(first: 100) with no pageInfo, reviewThreads() fires it once, and parseReviewThreads only reads nodes, so anything past the first page never reaches readSnapshot. Ironically the contexts query right below it already paginates properly, so the pattern was there to copy.

    There are two PRs on this and I went through both. #460 is the smaller one: it loops the cursor inside github.ts, keeps the reviewThreads() signature, and fails closed if the cursor goes missing or repeats. #484 mirrors how resolveChecks already does it — the reader returns a page, policy.ts loops — and updates types, fakes and tests to match. Both fix the bug; I've recommended #484 internally since it keeps thread and check pagination on the same shape, with a note to borrow #460's stuck-cursor guard. Neither touches comments(first: 10), which as you say matters less since only thread-level resolution feeds the decision.

  2. added a commit that references this issue on Oct 12, 2026
    ff8f83d
  3. added 3 commits that reference this issue on Oct 17, 2026
    55049d9
    5ee64f3
    a01835b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions