fix: surface GitHub rate limits from find_pr_by_branch

[mattmillerai]

ELI-5

When you ask the CLI to install a PR by user:branch, it asks GitHub "which PR is this?". If GitHub is rate-limiting you, it answers with a 403 error. The code caught that error and quietly turned it into "no PR found" — so you'd get told "PR not found" and go hunting for a typo in a branch name that was perfectly fine. Now it tells you the truth: you're rate-limited, here's when to retry.

The bug

find_pr_by_branch was missing the 403/429 → handle_github_rate_limit check that its two sibling functions (get_latest_release, fetch_pr_info) both had:

• a rate-limited 403 → raise_for_status() → requests.HTTPError
• → the function's own except requests.RequestException: return None
• → callers see None → print PR not found / Frontend PR not found.

What changed

Two module-private helpers in install.py, no new public surface:

• _github_get(url, *, params=None, timeout) — one place that does the GITHUB_TOKEN auth, the 403/429 rate-limit check, and raise_for_status(). The three duplicated token blocks are gone; os.getenv("GITHUB_TOKEN") now appears exactly once in the file (was 3×).
• _pr_info_from_github(data) — the 8-field PRInfo construction that was duplicated byte-for-byte between fetch_pr_info and find_pr_by_branch.

The three functions keep their own try/except and their divergent error semantics exactly (get_latest_release → None + message; fetch_pr_info → re-raise wrapped; find_pr_by_branch → None).

Behavior change (intended, the point of the PR)

GitHubRateLimitError is a plain Exception, not a RequestException, so it now propagates out of find_pr_by_branch instead of being swallowed. Both call sites already wrap the call in except Exception as e and print e, so they render the rate-limit message correctly with no call-site changes. I grepped for other callers — there are none outside this file and its tests.

A genuine network error still returns None (test_find_pr_by_branch_error, unmodified, still passes).

Security note

_github_get attaches the user's GITHUB_TOKEN as a Bearer header, so it rejects any URL outside https://api.github.com/. The trailing slash is load-bearing: it rejects https://api.github.com.evil.com/. It's kept module-private so the guard can't be bypassed by a new caller elsewhere.

Tests

Two added, both to the existing file in its existing Mock/patch style:

• test_find_pr_by_branch_rate_limit — 403 + exhausted-limit headers now raises GitHubRateLimitError (previously returned None; this test fails on main).
• test_github_get_rejects_non_github_urls — a non-GitHub URL raises ValueError and asserts requests.get was never called, i.e. the token never leaves the process.

All pre-existing tests pass unmodified — the test-file diff is 21 insertions, 0 deletions, so nothing existing was touched or re-asserted.

Verification

Run in a dedicated venv inside the worktree (verified comfy_cli.command.install.__file__ resolves to the worktree copy, so this isn't a false pass against the main checkout):

• pytest tests/comfy_cli/command/github/test_pr.py → 80 passed (78 collected on main + my 2).
• ruff check . → clean. ruff format --check . → clean.
• Full suite: 2566 passed, 9 failed. I diffed the failure set against a clean origin/main worktree: identical, 9 on both sides, zero unique to this branch. All pre-existing and unrelated (registry/test_config_parser.py + test_node_init_strips_credentials).

Judgment calls / notes for review

• _github_get returns dict | list (the pulls-list endpoint returns a list, the others a dict) and _pr_info_from_github takes dict, so a strict type checker would want a narrow/cast at the fetch_pr_info call. I kept the prescribed signature: there is no pyright/mypy gate in CI (gates are pytest + ruff, both green) and adding a cast seemed like more noise than the looseness costs. Happy to narrow it if reviewers prefer.
• The dead if response is None check in fetch_pr_info is gone — requests.get never returns None, so it was unreachable; it disappears naturally with the rewrite.
• Placement: _github_get sits next to handle_github_rate_limit; I put _pr_info_from_github next to its two callers rather than beside _github_get, which read better.
• Capability-denial check: this diff adds a raise ValueError deny path, so I checked whether it denies anything real. It doesn't. The ValueError guard is on a module-private helper reachable only from three hardcoded https://api.github.com/... URLs (verified every call site), so no user-reachable path hits it. And raising on a rate limit isn't denying the find-PR capability — the HTTP request had already failed; the change only stops the failure from being mislabeled "PR not found". Every success path still works, proven by test_find_pr_by_branch_success and the unmodified baseline suite.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e5b412fd-31f6-4325-b2dd-1fb3bacb2eb3

📥 Commits

Reviewing files that changed from the base of the PR and between a732f5c and 7990d0c.

📒 Files selected for processing (2)

• comfy_cli/command/install.py
• tests/comfy_cli/command/github/test_pr.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3276-github-get-rate-limit

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3276-github-get-rate-limit

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 2 finding(s).

Severity	Count
🟠 High	1
🟢 Low	1

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[mattmillerai]

Addressed both cursor-review findings in 7990d0c (_pr_info_from_github: guard null head.repo/base.repo from deleted forks; treat null mergeable as unknown rather than un-mergeable). Both threads replied to and resolved, two tests added.

On the two red checks — both are pre-existing and unrelated to this diff

This PR touches only comfy_cli/command/install.py and tests/comfy_cli/command/github/test_pr.py.

build (9 failures) — root-caused to a dependency drift, not this PR. CI resolves tomlkit-0.15.1 (visible in the build job's install log); 0.15.x made "comment contains a line break" a hard ValueError. Reproduced both directions locally:

uv run --with pytest --with jsonschema --with 'tomlkit==0.15.1' pytest tests/comfy_cli/registry/test_config_parser.py -q
# -> 8 failed, 73 passed   (matches CI exactly)

uv run --with pytest --with jsonschema pytest tests/comfy_cli/registry/test_config_parser.py -q
# (resolves tomlkit 0.13.3) -> 81 passed

The 9th (test_node_init_strips_credentials) is a knock-on of the same ValueError. test_config_parser.py imports only tomlkit and comfy_cli.registry.config_parser — there is no import path from it to install.py, so this diff cannot affect it. It blocks every open PR equally and needs its own fix (pin tomlkit<0.15, or stop handing multi-line strings to tomlkit comments in config_parser.py); a follow-up is filed rather than bolted onto a rate-limit PR.

test (windows) — ImportError: cannot import name '__version__' from 'pydantic_core' (unknown location), a broken venv on the runner. Environmental; unrelated to this diff. The Run Tests on Multiple Platforms (windows-latest, 3.10) job passed on the same commit.

Verified green locally on the code this PR actually changes: pytest tests/comfy_cli/command/github/test_pr.py tests/comfy_cli/command/test_launch_frontend_pr.py → 94 passed; ruff check + ruff format --diff clean under CI's pinned 0.15.15.

[mattmillerai]

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

• https://linear.app/comfyorg/issue/BE-3304|BE-3304 — Support installing PRs whose source fork was deleted via the base repo's refs/pull/N/head

[mattmillerai]

CI triage: both red checks are pre-existing repo-wide breakages, not from this diff

This PR touches only comfy_cli/command/install.py and tests/comfy_cli/command/github/test_pr.py. Neither red check is attributable to it, and both are already being fixed in other open PRs.

build — 9 failures, all ValueError: Comment cannot contain line breaks

Root cause is comfy_cli/registry/config_parser.py:75, which passes a multi-line string to tomlkit's .comment(). tomlkit is unpinned (pyproject.toml:49), so fresh resolutions pull 0.15.x, which rejects line breaks in comments. Verified locally in a clean worktree:

tomlkit	result
0.15.1	9 failed, 74 passed
0.13.2	83 passed

The same 9 failures reproduce on origin/main with zero local changes. Already tracked by #533 and #536 (which look like duplicates of each other, worth collapsing).

test (Windows Specific Commands) — pydantic_core ImportError

Real cause is earlier in the log: failed to remove file ...\pydantic_core\_pydantic_core.cp312-win_amd64.pyd: Access is denied. (os error 5). uv can't overwrite the DLL because the running comfy process has it loaded, leaving a half-installed pydantic_core. This workflow is currently failing on nearly every open branch. Already fixed by #535 (lazy telemetry import) — that branch's Windows run is the one green run in recent history.

This PR's own signal is green: pytest tests/comfy_cli/command/github/test_pr.py → 82 passed; ruff check . and ruff format --check . clean; GPU runners and all three platform matrix jobs (ubuntu/macos/windows) pass.

I'm deliberately not fixing either breakage here to avoid conflating unrelated concerns with a rate-limit fix. This should go green once #533/#536 and #535 land.

_{🤖 automated review-resolution pass}