Skip to content

fix(*): resolve releases without spending the github api quota - #295

Open
arelchan wants to merge 1 commit into
mainfrom
fix/release_lookup_without_api_quota
Open

fix(*): resolve releases without spending the github api quota#295
arelchan wants to merge 1 commit into
mainfrom
fix/release_lookup_without_api_quota

Conversation

@arelchan

Copy link
Copy Markdown
Contributor

Summary

raven upgrade and both installers resolved the latest release through api.github.com
with no credentials. GitHub meters unauthenticated API requests at 60 per hour per IP
address
, so every machine behind one office NAT draws down a single shared counter. Once
it is spent, upgrading and installing both stop working:

Unable to upgrade Raven: Client error '403 rate limit exceeded' for url
'https://api.github.com/repos/EverMind-AI/Raven/releases/latest'
... Check your network and try again

The network was fine. The advice sent the user to diagnose the wrong thing, and the same
quota takes down install.sh, which dies with "Could not resolve the latest raven release
wheel". The pending startup update check (24h TTL) adds one request per user per day to the
same shared counter, so this gets worse, not better, as adoption grows inside a company.

This resolves the release from the plain github.com redirect instead:

GET https://github.com/EverMind-AI/Raven/releases/latest
-> 302 location: https://github.com/EverMind-AI/Raven/releases/tag/v0.1.11

That is not the REST API and carries no per-IP API quota. Asset names are deterministic, and
_parse_release_payload already required the wheel URL to be exactly the one the tag implies,
so nothing is lost by not reading the API payload. Downloads never counted against the API
quota, so the whole install and upgrade path can now run without spending any of it.

The API stays as a fallback and now sends GITHUB_TOKEN / GH_TOKEN when either is set,
which moves a developer or CI run onto its own 5000-per-hour quota. An exhausted quota is
recognised from x-ratelimit-remaining and reported with the reset time from
x-ratelimit-reset; UpgradeError grew an optional hint so that message replaces the
generic network advice rather than being appended to it.

Not in this PR: publishing a latest.json next to the hosted installer, which would also
allow staged rollouts and a fast rollback of a bad release. That needs a change to the
release workflow and to what the site serves, so it is worth doing separately.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

uv run pytest tests/test_cli_upgrade_commands.py -> 109 passed. New cases cover: the
redirect is preferred and the API is not called at all; the redirect is never followed even
on a client configured to follow redirects; three malformed redirect shapes each fall back to
the API; GITHUB_TOKEN and GH_TOKEN each produce an Authorization header (and no header
is sent when neither is set); an exhausted quota raises with the limit, the reset time and a
token hint; the hint drops the token advice when a token is already in use; a 403 that is not
a quota failure stays an httpx.HTTPStatusError; and _release_from_tag produces exactly
what _parse_release_payload accepts. One CLI-level case asserts the hint replaces the
"Check your network" line.

install.sh was run end to end against stub uv / node / npm with a curl wrapper that
blocks one endpoint at a time, from a directory that is not a checkout:

curl allowed resolved wheel API calls attempted
everything .../download/v0.1.11/raven-0.1.11-py3-none-any.whl 0
api.github.com blocked same URL 0
redirect blocked same URL (via the API) 1

The Python redirect path was also run against the live endpoint:
_fetch_latest_release_via_redirect returned
ReleaseInfo(version='0.1.11', wheel_url='https://github.com/EverMind-AI/Raven/releases/download/v0.1.11/raven-0.1.11-py3-none-any.whl').

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally (ruff check, ruff format)
  • User-facing docs or screenshots are updated when needed

install.ps1 was reviewed by reading, not executed: this was verified on macOS. Its redirect
probe is wrapped so that any failure, including the PowerShell 5.1 behaviour of raising on a
3xx, returns $null and falls through to the previous API path. The worst case is therefore
today's behaviour, not a broken Windows install.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Notes: the wheel URL is still constrained to github.com/EverMind-AI/Raven/releases/download/
by _parse_release_payload, and the tag pattern only accepts vX.Y.Z, so the redirect cannot
point the installer at another host. A token is read from the environment and sent only to
api.github.com. releases/latest excludes drafts and prereleases in both the web and API
forms, so the stability check is preserved. Rollback is a revert of this commit.

Related Issues

N/A

`raven upgrade` and both installers resolved the latest release through
api.github.com with no credentials. GitHub meters unauthenticated API
requests at 60 per hour per IP address, so every machine behind one office
NAT draws down a single shared counter: once it is spent, upgrading and
installing both fail with a 403 that the CLI reported as "Check your
network and try again", sending users to diagnose the wrong thing.

Resolve the release from the plain github.com redirect instead
(releases/latest -> releases/tag/vX.Y.Z), which is not the REST API and has
no such per-IP quota. Asset names follow from the tag, and the payload
parser already required the wheel URL to be exactly the one the tag
implies, so nothing is lost. The API stays as a fallback and now sends
GITHUB_TOKEN / GH_TOKEN when either is set, which moves a developer or CI
run onto its own 5000-per-hour quota.

An exhausted quota is now identified from x-ratelimit-remaining and
reported with the reset time from x-ratelimit-reset, and UpgradeError
carries an optional hint so that message replaces the generic network
advice instead of being appended to it. Both installers say the same thing
and point at GITHUB_TOKEN and RAVEN_WHEEL_URL.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this is superseded. #299 landed the same fix on main on 2026-08-13 and all four files here conflict, so it cannot merge as it stands -- but two ideas in this branch are genuinely not on main, and closing this without carrying them forward would lose them.

Where it stands

main now carries 1cb604aa (fix(): resolve the latest release without the github api quota*, #299), which does the same redirect-first resolution and goes further in three places this branch does not: it confirms the derived wheel with a HEAD request (so a release published without its wheel fails with a message that names the problem, instead of dying inside the upgrade helper), it routes both paths through one _release_wheel_url() so the URL rule cannot drift, and it updates raven/cli/update_notice.py -- the daily background refresh, which is the caller that actually burns one API request per machine per day.

$ git merge-tree --write-tree HEAD github/main
CONFLICT (content): Merge conflict in install.ps1
CONFLICT (content): Merge conflict in install.sh
CONFLICT (content): Merge conflict in raven/cli/upgrade_commands.py
CONFLICT (content): Merge conflict in tests/test_cli_upgrade_commands.py

Worth salvaging into a follow-up (neither is on main)

1. The installers on main still call the API first. This branch tries the release-page redirect first in both install.sh and install.ps1; the merged #299 versions try api.github.com first and fall back to the release page only when it fails. Concretely, on main today every curl ... | sh / irm ... | iex install still spends one unauthenticated request against the shared 60-per-hour per-IP counter -- the exact counter an office NAT exhausts. It degrades rather than fails once spent, but the quota is still drawn down on the path with the most callers. The ordering in this branch is the right one; I ran its resolve_wheel_url against the live endpoint:

$ curl -s -o /dev/null -w '%{http_code} %{redirect_url}\n' https://github.com/EverMind-AI/Raven/releases/latest
302 https://github.com/EverMind-AI/Raven/releases/tag/v0.1.11
resolved: .../download/v0.1.11/raven-0.1.11-py3-none-any.whl   wheel http=200   constraints http=200

2. GITHUB_TOKEN / GH_TOKEN on the API fallback. This branch adds it in all three places (upgrade_commands.py, install.sh, install.ps1). main has no token support anywhere -- grepping GITHUB_TOKEN|GH_TOKEN|Authorization across all three files on main returns nothing. Moving a developer or a CI run onto its own quota is worth keeping. One inline note on the code that should travel with it.

What I actually covered

  • The diff, and both callers: raven upgrade, and update_notice._refresh() (the daemon-thread daily check that this branch leaves untouched and #299 updates).
  • AGENTS.md: branch name per 2.1; commit header 62 chars, lowercase, (*) scope, body pure ASCII per 3.1.1 (grep -nP "[^\x00-\x7F]" clean), Co-authored-by with a real model id; tests land in tests/test_cli_upgrade_commands.py per 5.1/5.4 with no new file; no assets.
  • Backward compatibility: UpgradeError("msg") still works positionally -- hint is keyword-only with a default, so no existing raise site changes.
  • Tests were not weakened. The old test_fetch_latest_release_uses_github_api_contract keeps every header assertion when it becomes ..._falls_back_to_the_api, and gains one; test_release_from_tag_builds_the_url_the_payload_parser_accepts pins the synthesized URL against the payload validator, which is the right invariant to hold. uv run pytest tests/test_cli_upgrade_commands.py -q -> 109 passed, 0 skipped. Full suite: 5904 passed, 1 failed, 43 skipped; the failure is tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare, which passes in isolation and still reproduces with every upgrade/update-notice test excluded (--ignore=tests/test_cli_upgrade_commands.py --ignore=tests/test_cli_update_notice.py). Pre-existing Rich colour-system pollution, unrelated to this branch.
  • Candidates I dropped because I could not make them fail: removing follow_redirects=True from the owned client (api.github.com/repos/EverMind-AI/Raven/releases/latest answers 200 with no redirect today, so the API fallback is unaffected); the redirect Location host not being validated (the wheel URL is rebuilt from the hardcoded prefix, so only the version number is server-supplied, and the existing downgrade guard covers that); $Matches after -notmatch in PowerShell (documented to populate on a match); and the sed tag pattern under set -eu, which I executed against the live endpoint rather than reasoning about.

"spends the same quota. Set GITHUB_TOKEN to use your own, or wait for the reset."
)
return UpgradeError(
f"GitHub API quota of {limit} requests per hour is exhausted for this IP address{when}",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this IP address is only true on the unauthenticated path -- and this same PR starts sending GITHUB_TOKEN / GH_TOKEN from _api_headers(), so the authenticated case reaches this line too. There the quota is metered per account (5000/hr), or per repository for an Actions GITHUB_TOKEN (1000/hr), never per IP.

Concrete failure: a CI job with GITHUB_TOKEN set exhausts the repository's 1000/hr and is told GitHub API quota of 1000 requests per hour is exhausted for this IP address, sending the operator to look at the runner's egress IP when the counter is the token's. That is the same diagnose-the-wrong-thing failure this PR exists to remove. The hint two lines up already gets it right by branching on _github_token(); the message just needs the same branch -- drop the for this IP address clause when a token is present.

Worth carrying into the rebase: main's _rate_limit_detail hardcodes the same claim (unauthenticated requests share 60 per hour per IP), which becomes wrong there the moment this branch's token support lands on top of it.

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.

2 participants