Skip to content

fix(server): keep stack badges on the per-repo pull request list - #16035

Closed
saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:stack/fix-pr-badge-decode
Closed

saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:stack/fix-pr-badge-decode

Conversation

@saphid

@saphid saphid commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Rebased 2026-10-10 onto main b1ec4b3687; head 3ec4980aeb. Main has since fixed the underlying bug a different way: #16986 (d720210) stopped adding a rateLimit selection to GraphQL documents, so the stack-membership answer no longer carries that key and main's decoder works on real responses. This rebase only moves the same patch into packages/source-control-github; what remains is a defensive decoder change, and this PR can be closed if maintainers prefer main's fix alone. At this head these pass: focused tests (2 files, 228 tests), @t3tools/source-control-github typecheck, lint and fmt on the changed files, and knip.

Fixes #16034.

Problem

The per-repository pull request listing (GitHubPullRequestCli.listPullRequests) never shows stack badges. Every GraphQL document goes through the GraphQL budget, which appends a rateLimit reading, so the stack-membership answer has a data.rateLimit key beside its s<n> aliases. RawStackMembershipsSchema required pullRequest on every key in data, so the whole decode failed. The enrichment swallowed the failure, logged Pull request stack membership enrichment failed, and returned the rows without stack.

Why this qualifies

It is a small fix for an obvious bug, with an issue (#16034) that describes the root cause.

Fix

pullRequest is now optional in RawStackMembershipsSchema, as it already is in the sibling stats schema. The decoder already reads only the s<n> aliases, so the rateLimit entry is ignored instead of failing the decode. That is the whole production change: one schema field in apps/server/src/pullRequest/gitHubPullRequestJson.ts.

Tests:

  • gitHubPullRequestJson.test.ts: decodes a membership answer that has the rateLimit key beside the aliases.
  • GitHubPullRequestCli.test.ts: the existing fallback-enrichment test now returns the rateLimit key that the real budget-wrapped document gets.

Both tests fail with main's decoder (2 failed / 216 passed) and pass with this change (218/218).

Evidence

This is a server-only change, so the proof is a server-level trace rather than screenshots. For each revision, a small harness imported that revision's real GitHubPullRequestCli, GraphQL budget and process runner. It called listPullRequests against an offline fake GitHub, using the real gh CLI routed to a local socket, no credentials and no network. The fake repository has four open PRs; #101 and #102 form native stack 7. Before is main 7812230572; after is this PR's head.

Before (main) After (this PR)
listPullRequests → #101 no stack stack 7, position 1 of 2, base main
listPullRequests → #102 no stack stack 7, position 2 of 2, base main
Server log WARN Pull request stack membership enrichment failed { operation: 'listPullRequestStackMemberships', rows: 4 } no warning
Membership document sent PullRequestStackMemberships, with the budget's rateLimit selection same
searchPullRequests (control) #101/#102 in stack 7 same

The fake GitHub's answer to the membership document contained data.rateLimit beside s0–s3, exactly the shape that failed the decode.

Checks at this head (CI=true): the two test files above (218 passed), and vp lint / vp fmt --check / vp run knip:check on the changed files. GPT-6.1 Sol (high) reviewed the change: SHIP, no findings. This head is that reviewed commit rebased onto current main with an identical patch.

Surfaces

  • Server: fixed. Both listing paths now enrich the same way.
  • Web / desktop / mobile: no client change. Clients already render stack when the server sends it, and now they get it from the per-repository path too.
  • Remote / relay / tunnel: no wire or contract change.
  • Providers: not involved.

Not verified

  • The badge rendering itself was not captured in a client. The UI has no control that selects the per-repository backend: the app calls it only when the cross-repository search fails or leaves out a repository, and the fake GitHub has no way to simulate either. A screenshot would therefore show badges on both revisions and prove nothing.
  • Not run against live GitHub.

Coordination and the fix by Claude Opus 5.5; review by GPT-6.1 Sol (high); the before/after trace by GPT-6 Astra, all through T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 5, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 8244cf0

Macroscope's review found this PR approvable — This is a narrowly scoped server decoder fix with regression tests, allowing stack-membership results to coexist with the existing GraphQL rate-limit field. It does not change product defaults, production infrastructure, APIs, or static-analysis configuration.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 830096c1-1776-418a-a874-0de74388ec71

📥 Commits

Reviewing files that changed from the base of the PR and between 8244cf0 and 3ec4980.


📒 Files selected for processing (3)
  • packages/source-control-github/src/server/GitHubPullRequestApi.test.ts
  • packages/source-control-github/src/server/gitHubPullRequestJson.test.ts
  • packages/source-control-github/src/server/gitHubPullRequestJson.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The stack membership response decoder now accepts rate-limit data alongside aliased membership data. Tests cover the response shape and verify that membership data still decodes.

Changes

Stack membership response decoding

Layer / File(s) Summary
Decode rate-limited membership responses
packages/source-control-github/src/server/gitHubPullRequestJson.ts, packages/source-control-github/src/server/gitHubPullRequestJson.test.ts, packages/source-control-github/src/server/GitHubPullRequestApi.test.ts
The schema allows an alias value or its pullRequest field to be null or omitted. Decoder and API tests include rate-limit fields in the mocked response.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk: ⚪ Minimal · up to 3ec49

Valid stack memberships continue to decode alongside quota data, and tests cover that response shape. No material current-head risk remains.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 3ec49

The change tolerates missing pull-request fields in response entries while preserving validation of actual stack memberships. Its effect is limited to optional metadata on already-listed pull requests; no material security risk was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The inspected consumer enriches only already-selected rows on github.com, querying the input repository and those rows’ pull-request numbers. Extra decoded indices cannot create additional listing rows.

Trust Boundaries and Controls

  • observed — Response JSON still passes through schema decoding before membership extraction. Only aliases matching s followed by digits with a non-null pullRequest can produce memberships. Query construction remains a read operation with repository-part and positive safe-integer validation; these controls are unchanged by the PR.

Resilience and Maintainability Implications

  • observed — Existing failure containment is preserved: an invalid query document leaves its chunk unchanged, missing memberships leave individual rows unchanged, and enrichment errors return the original chunk rather than failing the successful listing.


Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #16034 requires stack membership on per-repository pull-request rows without the Pull request stack membership enrichment failed warning. The reviewed change makes RawStackMembershipsSchema …
Out of Scope Changes check Passed The schema change and the two test changes directly support issue #16034. The available change summary identifies no unrelated production or test changes. No ignored files are listed.
Title check Passed The title clearly and concisely describes the primary change: preserving stack badges on the per-repository pull request list.
Description check Passed The description is comprehensive. It explains the problem, root cause, production change, issue scope, tests, verification results, limitations, affected surfaces, and agent usage. It uses alternative…


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@saphid

saphid commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Review requested

Date (UTC) Reviewer Where
2026-10-06 Julius Discord DM

Logged so this PR shows when a maintainer was asked to review it.

The GraphQL budget appends a `rateLimit` reading to every document it
sends, so the stack-membership answer carries a `rateLimit` key beside the
`s<n>` aliases. The membership schema required `pullRequest` on every key,
so the whole decode failed, enrichment was dropped with a warning, and the
per-repository listing showed no stack badges. Make `pullRequest` optional;
the decoder already reads only `s<n>` aliases.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the stack/fix-pr-badge-decode branch from 8244cf0 to 3ec4980 Compare October 10, 2026 00:16
@saphid

saphid commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing: #16986 (merged 2026-10-08) stopped adding the rateLimit field to GitHub GraphQL queries and reads rate limits from response headers instead. That extra key was what broke the stack-membership decode (#16034), so the badges no longer drop on current main. Nothing in main's server code adds rateLimit to a query any more. Reopen if the badges disappear again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-repository pull request list never shows stack badges

1 participant