Skip to content

feat(forge): read PR review threads and required checks from GitHub GraphQL (RIG-1728) - #1354

Open
rigel-mintaka wants to merge 3 commits into
mainfrom
compass-server/rig-1728-pr-threads-required
Open

rigel-mintaka wants to merge 3 commits into
mainfrom
compass-server/rig-1728-pr-threads-required

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Refs RIG-1728 (forge read half; PR ingestion and projection wait on RIG-4034).

What

GitHub.GetPullRequest and GitHub.Checks add one paginated GraphQL leg (go/internal/forge/github_graphql.go):

  • PullRequest.Threads is filled from reviewThreads, with resolution state and paged comments. A bot author gets the REST [bot] suffix. A null author gives an empty author.
  • Check.Required comes from isRequired(pullRequestNumber:). The repo uses rulesets, so the REST branch-protection endpoint returns 404.
  • Contexts are read through pullRequest.commits(last:1), so the App needs only Pull requests: read. The commit oid is pinned to the REST head SHA on every page. A push mid-read fails with errHeadMoved, never a silent Required:false.
  • The fail-fast rate gate is keyed per x-ratelimit-resource (core vs graphql). A drained GraphQL bucket no longer blocks REST calls. RATE_LIMITED on HTTP 200 maps to *RateLimitError.
  • A GraphQL errors array is always an error. A cursor that is missing or repeated is an error.

ChecksConditional (the notify poller) is unchanged and stays on REST core.

Tests

  • github_graphql_test.go: paging (threads, contexts, both together), error branches, head moved, per-resource gate, RATE_LIMITED.
  • The happy test and the golden fixture are updated. endCursor and oid are canonicalized in wireVolatile.
  • go test ./internal/forge/... ./internal/ingest/... ./internal/board/... passes. golangci-lint ./internal/forge/: 0 issues.
  • Checked live against compass docs(design): record the unclipped IssueToken handle lookup (RIG-4019) #1348 (rollup required) and cli/cli bot threads.

…raphQL (RIG-1728)

GetPullRequest and Checks now add one paginated GraphQL leg. It fills
PullRequest.Threads (with resolution and bot-suffixed authors) and sets
Check.Required from isRequired(pullRequestNumber:). Rulesets make the REST
branch-protection endpoint 404. Contexts are read through the PR's last
commit, so the App needs only Pull requests: read. The fail-fast rate gate
is now keyed per x-ratelimit-resource, so a drained GraphQL bucket no longer
blocks REST calls, and RATE_LIMITED on HTTP 200 maps to RateLimitError.

Refs RIG-1728

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@trunk-io

trunk-io Bot commented Sep 26, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@linear-code

linear-code Bot commented Sep 26, 2026

Copy link
Copy Markdown

RIG-1728

rigel-mintaka and others added 2 commits September 26, 2026 07:16
The contexts read through the PR's last commit now select its oid. Each page
fails with errHeadMoved when that oid differs from the REST head SHA. Before
this, a push between the reads could clear a failing required check's flag.

Refs RIG-1728

Co-authored-by: Matt Wilkinson <matt@rigel.build>
No caller retries errHeadMoved, so the comment now says only that a fresh
read is consistent.

Refs RIG-1728

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@github-actions

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-server-rig-1728-pr-t.compass-eng-docs.pages.dev

Deployed from compass-server/rig-1728-pr-threads-required at 6f17841.

@rigel-mintaka
rigel-mintaka marked this pull request as ready for review September 26, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant