Skip to content

fix(server): PR comments load on GitHub Enterprise without rate limits - #14472

Closed
wumphlett wants to merge 1 commit into
pingdotgg:mainfrom
wumphlett:fix/ghes-rate-limit-disabled
Closed

wumphlett wants to merge 1 commit into
pingdotgg:mainfrom
wumphlett:fix/ghes-rate-limit-disabled

Conversation

@wumphlett

@wumphlett wumphlett commented Sep 30, 2026 •

Copy link
Copy Markdown

On GitHub Enterprise servers with rate limiting disabled, the PR sidebar's Comments/Activity and stack views fail with "GitHub CLI command failed." while Checks still works. Before every gh pr view, gh pr list, and gh repo view, T3 probes gh api rate_limit --hostname <host>. On those servers the probe returns HTTP 404: Rate limiting is not enabled., and T3 treats that as fatal, so the real command never runs. Checks go straight through GraphQL and skip the probe, which is why they work.

The probe only feeds the GraphQL budget, so it should never be the thing that fails a read. The quota lookup in GitHubCli.ts now treats a generic GitHubCliCommandError from the probe as "no quota information": the read runs, and that result is cached for the usual 30s so we don't re-probe on every call. Authentication, rate-limit, and missing-gh errors from the probe still come through as before. If the read itself fails, it reports its own error.

Verification: a new test in GitHubCli.test.ts makes the probe fail with the GHES 404. Reads succeed, and the probe runs once across two reads. The test fails without the fix. I didn't test against a real GHES instance.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 30, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 30, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e188cfd

Macroscope's review found this PR approvable — This is a narrowly scoped bug fix that makes the advisory rate-limit probe non-blocking for GitHub Enterprise while preserving the actual read and existing specialized error paths. The accompanying test verifies both successful reads and probe caching, with no product-default or static-analysis changes.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 404d4dd6-ea5d-4b77-b727-3a3fb897c9cf

📥 Commits

Reviewing files that changed from the base of the PR and between 35be904 and e188cfd.

📒 Files selected for processing (2)
  • apps/server/src/sourceControl/GitHubCli.test.ts
  • apps/server/src/sourceControl/GitHubCli.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 quota probe now suppresses GitHubCliCommandError failures. A test verifies that pull-request reads succeed after a rate-limit-disabled HTTP 404 and that repeated reads do not repeat the probe.

Changes

GitHub CLI quota probe

Layer / File(s) Summary
Suppress quota probe command errors
apps/server/src/sourceControl/GitHubCli.ts, apps/server/src/sourceControl/GitHubCli.test.ts
The quota probe suppresses GitHubCliCommandError. A test confirms that two reads succeed after a rate-limit-disabled HTTP 404 and that the probe runs once.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e188c

The change allows reads to proceed when Enterprise rate-limit probing is unavailable while preserving other error handling. It is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the server fix for PR comments on GitHub Enterprise without rate limiting.
Description check ✅ Passed The description clearly explains the problem, the quota-probe change, preserved error behavior, caching, test coverage, and the limitation that real GHES testing was not performed. It does not use the…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 1, 2026 18:36

Dismissing prior approval to re-evaluate e188cfd

@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Superseded by #14673, which removed the REST gh api rate_limit preflight probe that GHES 404'd when rate limiting is disabled. Budget readings now use GraphQL rateLimit and a failed reading never blocks the read. Closing this PR as obsolete.

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants