From 1647d030cf1002dd3728c2d6c7fd402c8cb12470 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:23:05 -0700 Subject: [PATCH] fix(update): call out GitHub rate limiting/outage on fetch 429 instead of a generic failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A GitHub-side HTTP 429 during 'hermes update' printed only 'Failed to fetch updates from origin.' — and the curl 'unable to access ... returned error: 429' shape even matched the network-error branch, blaming the user's connection for a GitHub outage. - new _classify_fetch_failure(): 429/rate-limit -> 'GitHub is rate limiting requests or having an outage — try again in 5 minutes'; 5xx -> outage message with githubstatus.com; ordered BEFORE the generic 'unable to access' network check - both fetch-failure sites (update apply + --check) now share the classifier via _print_fetch_failure(), and both always print the first raw stderr line so the wire error stays diagnosable - tests: classifier matrix + E2E against a live local HTTP server returning 429 through real git Fixes #89287 --- hermes_cli/update_cmd.py | 68 ++++++++++------ .../test_update_fetch_failure_classifier.py | 77 +++++++++++++++++++ 2 files changed, 122 insertions(+), 23 deletions(-) create mode 100644 tests/hermes_cli/test_update_fetch_failure_classifier.py diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index a39420e3ac..a7b9b7c594 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -2714,6 +2714,49 @@ def _run_logged_subprocess(cmd, *, cwd=None, env=None): _log_only_write(result.stdout or "") return result +def _classify_fetch_failure(stderr: str) -> str: + """Map git-fetch stderr to a one-line, user-facing diagnosis. + + Order matters: curl surfaces HTTP failures as + ``fatal: unable to access '': The requested URL returned error: 429``, + so the rate-limit/outage checks must run BEFORE the generic + "unable to access" network check or a GitHub 429/5xx gets misreported as a + local network problem. The caller always prints the first raw stderr line + alongside this diagnosis — the friendly message adds guidance, it never + replaces the wire error. + """ + + def _has_http_code(*codes: str) -> bool: + return any( + f"HTTP {code}" in stderr or f"returned error: {code}" in stderr + for code in codes + ) + + if _has_http_code("429") or "rate limit" in stderr.lower(): + return ( + "✗ GitHub is rate limiting requests or having an outage (HTTP 429)" + " — try again in 5 minutes." + ) + if _has_http_code("500", "502", "503", "504"): + return ( + "✗ GitHub appears to be having an outage — try again in a few" + " minutes (https://www.githubstatus.com)." + ) + if "Could not resolve host" in stderr or "unable to access" in stderr: + return "✗ Network error — cannot reach the remote repository." + if "Authentication failed" in stderr or "could not read Username" in stderr: + return "✗ Authentication failed — check your git credentials or SSH key." + return "✗ Failed to fetch updates from origin." + + +def _print_fetch_failure(stderr: str) -> None: + """Print the classified diagnosis plus the first raw stderr line.""" + stderr = (stderr or "").strip() + print(_classify_fetch_failure(stderr)) + if stderr: + print(f" {stderr.splitlines()[0]}") + + def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): """Implement ``hermes update --check``: fetch and report without installing. @@ -2835,15 +2878,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): compare_branch = f"origin/{branch}" if fetch_result.returncode != 0: - stderr = fetch_result.stderr.strip() - if "Could not resolve host" in stderr or "unable to access" in stderr: - print("✗ Network error — cannot reach the remote repository.") - elif "Authentication failed" in stderr or "could not read Username" in stderr: - print("✗ Authentication failed — check your git credentials or SSH key.") - else: - print("✗ Failed to fetch.") - if stderr: - print(f" {stderr.splitlines()[0]}") + _print_fetch_failure(fetch_result.stderr) sys.exit(1) # Verify the compare ref actually exists before asking rev-list about it. @@ -4896,20 +4931,7 @@ def _cmd_update_impl(args, gateway_mode: bool): text=True, encoding="utf-8", errors="replace", ) if fetch_result.returncode != 0: - stderr = fetch_result.stderr.strip() - if "Could not resolve host" in stderr or "unable to access" in stderr: - print("✗ Network error — cannot reach the remote repository.") - print(f" {stderr.splitlines()[0]}" if stderr else "") - elif ( - "Authentication failed" in stderr or "could not read Username" in stderr - ): - print( - "✗ Authentication failed — check your git credentials or SSH key." - ) - else: - print("✗ Failed to fetch updates from origin.") - if stderr: - print(f" {stderr.splitlines()[0]}") + _print_fetch_failure(fetch_result.stderr) sys.exit(1) # Get current branch (returns literal "HEAD" when detached) diff --git a/tests/hermes_cli/test_update_fetch_failure_classifier.py b/tests/hermes_cli/test_update_fetch_failure_classifier.py new file mode 100644 index 0000000000..f5e270c82c --- /dev/null +++ b/tests/hermes_cli/test_update_fetch_failure_classifier.py @@ -0,0 +1,77 @@ +"""Fetch-failure classification for `hermes update` / `hermes update --check`. + +A GitHub-side HTTP 429 (rate limit / outage) used to be reported as the +generic "Failed to fetch updates from origin." — or worse, matched the +"unable to access" branch and got called a local network error. The +classifier must call out rate limiting / outages explicitly, and the raw +stderr line must always be printed alongside the diagnosis. +""" + +from hermes_cli import update_cmd + + +RATE_LIMIT_STDERR = ( + "error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429\n" + "fatal: expected flush after ref listing" +) +CURL_429_STDERR = ( + "fatal: unable to access 'https://github.com/NousResearch/hermes-agent.git/':" + " The requested URL returned error: 429" +) + + +class TestClassifyFetchFailure: + def test_http_429_rpc_failure_reports_rate_limit(self): + msg = update_cmd._classify_fetch_failure(RATE_LIMIT_STDERR) + assert "rate limiting" in msg + assert "try again in 5 minutes" in msg + + def test_curl_unable_to_access_429_is_rate_limit_not_network(self): + # "unable to access" also appears here — 429 must win. + msg = update_cmd._classify_fetch_failure(CURL_429_STDERR) + assert "rate limiting" in msg + assert "Network error" not in msg + + def test_rate_limit_phrase_without_code(self): + msg = update_cmd._classify_fetch_failure("fatal: GitHub rate limit exceeded") + assert "rate limiting" in msg + + def test_5xx_reports_outage(self): + msg = update_cmd._classify_fetch_failure( + "fatal: unable to access 'https://github.com/x.git/':" + " The requested URL returned error: 503" + ) + assert "outage" in msg + assert "githubstatus.com" in msg + + def test_dns_failure_reports_network_error(self): + msg = update_cmd._classify_fetch_failure( + "fatal: unable to access 'https://github.com/x.git/':" + " Could not resolve host: github.com" + ) + assert msg.startswith("✗ Network error") + + def test_auth_failure(self): + msg = update_cmd._classify_fetch_failure( + "fatal: Authentication failed for 'https://github.com/x.git/'" + ) + assert "Authentication failed" in msg + + def test_unknown_falls_back_to_generic(self): + msg = update_cmd._classify_fetch_failure("fatal: something novel") + assert msg == "✗ Failed to fetch updates from origin." + + +class TestPrintFetchFailure: + def test_prints_diagnosis_and_first_raw_line(self, capsys): + update_cmd._print_fetch_failure(RATE_LIMIT_STDERR) + out = capsys.readouterr().out + assert "rate limiting" in out + assert "HTTP 429" in out + # raw first stderr line preserved for diagnosability + assert "error: RPC failed" in out + + def test_empty_stderr_prints_only_diagnosis(self, capsys): + update_cmd._print_fetch_failure("") + out = capsys.readouterr().out.strip().splitlines() + assert out == ["✗ Failed to fetch updates from origin."]