fix(copilot): recover from stale/degraded token 400 AND expired IDE-token 401 - #58743
fix(copilot): recover from stale/degraded token 400 AND expired IDE-token 401#58743dstkwll wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the 400 path. The premise is present on current main: agent/conversation_loop.py:2851-2859 only refreshes Copilot on 401, while hermes_cli/copilot_auth.py:415-434 falls back to the raw token when exchange fails.
Problems
hermes_cli/copilot_auth.py:338and:399parse the persisted JWT cache with unboundedread_text(). The proposed 1 MiB check exists only in_load_jwt_from_disk()at:371-374, so recovery eviction and a later save can still load an oversized cache.- The PR changes no test file (
gh api repos/NousResearch/hermes-agent/pulls/58743/files?per_page=100), leaving the new persistence and recovery paths without committed regression coverage.
Suggested changes
- Use one bounded cache-read helper for load, eviction, and save; discard oversized/malformed content before parsing or rewriting.
- Add focused network-free tests for retry/disk reuse, oversized-cache handling, 400 classification, and both recovery outcomes.
Automated hermes-sweeper review.
| if not path or not path.exists(): | ||
| return | ||
| try: | ||
| store = json.loads(path.read_text()) |
There was a problem hiding this comment.
The 1 MiB guard is only in _load_jwt_from_disk(). Recovery reaches this unbounded read_text() path, so an oversized cache can still be fully loaded; share a bounded-read helper with eviction and _save_jwt_to_disk().
…oken 401 Copilot degrades in two related ways that both abort a turn as non-retryable and only clear on a gateway restart (a cold process re-runs the token exchange): 1. HTTP 400 model_not_available_for_integrator / model_not_supported — a raw/degraded token routes to the restricted copilot-language-server integrator whose allowlist omits enterprise-only models (e.g. claude-opus-4.8). Because it is a 400 (not 401), the existing 401 refresh path never fired. Prevented (retry-with-backoff exchange + on-disk JWT persistence + header guard at the client chokepoint) and self-healed at runtime (single-shot forced re-exchange + client rebuild + retry before fallback). 2. HTTP 401 'IDE token expired: unauthorized: token expired' — the short-TTL *exchanged* IDE token expires mid-turn. The clean-401 path DID fire and call _try_refresh_copilot_client_credentials(), but that method only re-resolved the stable raw ghu_ token and rebuilt the client — it never evicted the cached exchanged JWT or forced a fresh exchange, so the retry put the SAME expired token back on the wire, 401'd again, and the single-shot guard aborted the turn. Fix: force a fresh IDE-token exchange (evict cached JWT via evict_cached_exchanged_token + re-mint via get_copilot_api_token) before the client rebuild, mirroring the merged auxiliary-path recovery (NousResearch#59837) and the 400 recovery in this same PR. Graceful fallback to the resolved token if the exchange endpoint is unreachable; picks up the enterprise base_url on re-exchange. Brings main-loop clean-401 recovery to parity with the merged auxiliary path (NousResearch#59837), using the newer on-disk-aware evict helper. Companion context: NousResearch#58743 (this PR, expanded), NousResearch#51313, NousResearch#63204 (which assumed the 401 path already recovered — it reached the method but the method was too weak). Tests: exchange retry/persist round-trip, restart-blip disk reuse, stale-cred 400 classifier, 400 recovery, and 3 new 401 cases (fresh exchanged token on the wire; network-blip fallback to resolved token). 58 copilot tests green on current main.
11b7c45 to
8d3c0d4
Compare
|
Rebased onto current The 401 path already reached |
…oken 401 Copilot degrades in two related ways that both abort a turn as non-retryable and only clear on a gateway restart (a cold process re-runs the token exchange): 1. HTTP 400 model_not_available_for_integrator / model_not_supported — a raw/degraded token routes to the restricted copilot-language-server integrator whose allowlist omits enterprise-only models (e.g. claude-opus-4.8). Because it is a 400 (not 401), the existing 401 refresh path never fired. Prevented (retry-with-backoff exchange + on-disk JWT persistence + header guard at the client chokepoint) and self-healed at runtime (single-shot forced re-exchange + client rebuild + retry before fallback). 2. HTTP 401 'IDE token expired: unauthorized: token expired' — the short-TTL *exchanged* IDE token expires mid-turn. The clean-401 path DID fire and call _try_refresh_copilot_client_credentials(), but that method only re-resolved the stable raw ghu_ token and rebuilt the client — it never evicted the cached exchanged JWT or forced a fresh exchange, so the retry put the SAME expired token back on the wire, 401'd again, and the single-shot guard aborted the turn. Fix: force a fresh IDE-token exchange (evict cached JWT via evict_cached_exchanged_token + re-mint via get_copilot_api_token) before the client rebuild, mirroring the merged auxiliary-path recovery (#59837) and the 400 recovery in this same PR. Graceful fallback to the resolved token if the exchange endpoint is unreachable; picks up the enterprise base_url on re-exchange. Brings main-loop clean-401 recovery to parity with the merged auxiliary path (#59837), using the newer on-disk-aware evict helper. Companion context: #58743 (this PR, expanded), #51313, #63204 (which assumed the 401 path already recovered — it reached the method but the method was too weak). Tests: exchange retry/persist round-trip, restart-blip disk reuse, stale-cred 400 classifier, 400 recovery, and 3 new 401 cases (fresh exchanged token on the wire; network-blip fallback to resolved token). 58 copilot tests green on current main.
- Bound ALL reads of the on-disk JWT store through one _read_jwt_store() helper (load, eviction, save-merge) — the 1 MiB cap previously only covered the load path; eviction and save could still parse an oversized/corrupt store and rewrite it back out (sweeper finding). - Fix the class, not the site: the recovery gates checked the literal provider == "copilot" while /model and profile configs can leave the alias spelling in place (the reporter's own log shows provider=copilot AND provider=github-copilot in one session — the aliased turns would have silently skipped recovery). Single owner: AIAgent._is_copilot_provider() (slug aliases + Copilot base-URL fallback), used by both run_agent recovery methods and both conversation_loop gates. - Update the salvaged 401 test to current main's client-retirement contract (release deferred to GC — no synchronous .close()). - Add copilot_stale_cred_retry_attempted to the TurnRetryState field contract test; add bounded-store and alias-gate regression tests.
|
Merged via PR #75864 (#75864) — your commit was cherry-picked onto current main with your authorship preserved in git log (7779409). Thanks for the deterministic truth-table investigation and the layered prevent+recover design; we added small follow-ups on top (one bounded reader for all JWT-store reads, alias-spelling coverage for the provider gates, UTF-8 encoding per a new repo-wide lint rule). This fixes the widely-reported "most Copilot models return HTTP 400 while gpt-4.1/gpt-4o work" failure. Great work! |
…oken 401 Copilot degrades in two related ways that both abort a turn as non-retryable and only clear on a gateway restart (a cold process re-runs the token exchange): 1. HTTP 400 model_not_available_for_integrator / model_not_supported — a raw/degraded token routes to the restricted copilot-language-server integrator whose allowlist omits enterprise-only models (e.g. claude-opus-4.8). Because it is a 400 (not 401), the existing 401 refresh path never fired. Prevented (retry-with-backoff exchange + on-disk JWT persistence + header guard at the client chokepoint) and self-healed at runtime (single-shot forced re-exchange + client rebuild + retry before fallback). 2. HTTP 401 'IDE token expired: unauthorized: token expired' — the short-TTL *exchanged* IDE token expires mid-turn. The clean-401 path DID fire and call _try_refresh_copilot_client_credentials(), but that method only re-resolved the stable raw ghu_ token and rebuilt the client — it never evicted the cached exchanged JWT or forced a fresh exchange, so the retry put the SAME expired token back on the wire, 401'd again, and the single-shot guard aborted the turn. Fix: force a fresh IDE-token exchange (evict cached JWT via evict_cached_exchanged_token + re-mint via get_copilot_api_token) before the client rebuild, mirroring the merged auxiliary-path recovery (NousResearch#59837) and the 400 recovery in this same PR. Graceful fallback to the resolved token if the exchange endpoint is unreachable; picks up the enterprise base_url on re-exchange. Brings main-loop clean-401 recovery to parity with the merged auxiliary path (NousResearch#59837), using the newer on-disk-aware evict helper. Companion context: NousResearch#58743 (this PR, expanded), NousResearch#51313, NousResearch#63204 (which assumed the 401 path already recovered — it reached the method but the method was too weak). Tests: exchange retry/persist round-trip, restart-blip disk reuse, stale-cred 400 classifier, 400 recovery, and 3 new 401 cases (fresh exchanged token on the wire; network-blip fallback to resolved token). 58 copilot tests green on current main.
- Bound ALL reads of the on-disk JWT store through one _read_jwt_store() helper (load, eviction, save-merge) — the 1 MiB cap previously only covered the load path; eviction and save could still parse an oversized/corrupt store and rewrite it back out (sweeper finding). - Fix the class, not the site: the recovery gates checked the literal provider == "copilot" while /model and profile configs can leave the alias spelling in place (the reporter's own log shows provider=copilot AND provider=github-copilot in one session — the aliased turns would have silently skipped recovery). Single owner: AIAgent._is_copilot_provider() (slug aliases + Copilot base-URL fallback), used by both run_agent recovery methods and both conversation_loop gates. - Update the salvaged 401 test to current main's client-retirement contract (release deferred to GC — no synchronous .close()). - Add copilot_stale_cred_retry_attempted to the TurnRetryState field contract test; add bounded-store and alias-gate regression tests.
Summary
GitHub Copilot degrades in two related ways that both abort a turn as a non-retryable error and only clear on a gateway restart (a cold process re-runs the token exchange). This PR prevents the degraded state and self-heals from both at runtime. It has been rebased onto current
mainand its scope expanded from the original 400-only fix to also cover the clean-401 case.Bug 1 — HTTP 400
model_not_available_for_integrator/model_not_supportedA raw/degraded token routes the request to the restricted
copilot-language-serverintegrator, whose allowlist omits Enterprise-only models (most visiblyclaude-opus-4.8). Because it is a 400, not a 401, the existing Copilot 401 credential-refresh path never fired and the turn aborted.Deterministic truth table (verified live against the API) — varying only the token on the wire and whether
Copilot-Integration-Id: vscode-chatis present:vscode-chatheaderghu_(40-char OAuth)ghu_(40-char OAuth)copilot-language-server400 ← the bugmissing Editor-Version(different error)The fallback integrator is only reached when a raw token and a missing integration header coincide. Two mechanisms produce that state: (1) the token exchange degrades silently to the raw token on any exception and — because the exchanged-JWT cache was in-process only — sticks for the whole process lifetime; (2) two client-rebuild paths (
primary_recovery,restore_primary) reconstruct the client from the_primary_runtimesnapshot without re-applying Copilot headers.Bug 2 — HTTP 401
IDE token expired: unauthorized: token expiredThe short-TTL exchanged IDE token (minted from the stable raw
ghu_token) expires mid-turn. A heavy/long turn whose request straddles that expiry gets a clean 401. The clean-401 path did fire and call_try_refresh_copilot_client_credentials()— but that method only re-resolved the stable raw token and rebuilt the client; it never evicted the cached exchanged JWT or forced a fresh exchange. So the retry put the same expired IDE token back on the wire, 401'd again, and the single-shot guard aborted the turn as non-retryable. Only a gateway restart helped, because a cold process re-runs the exchange.This is the same failure class the merged auxiliary-path fix (#59837, closing #20832/#20837/#23379) already solved for compression/title-generation — but the main conversation loop was left with the weaker refresh. Note #63204 was closed
implemented_on_mainon the assumption that "conversation_loop.pyroutes Copilot 401 recovery through_try_refresh_copilot_client_credentials()" — it reaches the method, but the method was too weak to actually refresh the expired exchanged JWT. This PR makes the method do what that review already assumed it did.The fix (layered — prevent + recover)
hermes_cli/copilot_auth.pyexchange_copilot_token(): retry-with-backoff (3 attempts) instead of failing on the first blip.~/.hermes/.copilot_jwt.json,0o600, profile-aware, expired entries pruned, read bounded to 1 MiB). A fresh process reuses the still-valid ~30-min token before any network call.evict_cached_exchanged_token(): drop both cache tiers so a recovery can force a fresh mint.agent/credential_pool.py— WARNING when the copilot seed degrades to the raw token, so a recurrence is visible instead of silent.agent/agent_runtime_helpers.py— defense-in-depth header guard atcreate_openai_client()(the documented single chokepoint every primary client passes through). Forgithubcopilot.comhosts it fills any missing Copilot headers (never overrides caller-set ones; operates on the local per-call dict copy, so it never mutates_client_kwargsand can't break prompt caching).agent/conversation_loop.py+agent/turn_retry_state.py+run_agent.pymodel_not_supported400 as a refreshable stale-credential error (narrow match: 400 and a specific body marker), then — copilot-scoped and single-shot — force a fresh exchange, rebuild the client, and retry once on the same provider before falling through to the fallback chain. Aborts cleanly if the exchange stays degraded, so a genuinely unavailable model can't loop._try_refresh_copilot_client_credentials()now evicts the cached exchanged JWT and forces a fresh exchange (viaevict_cached_exchanged_token+get_copilot_api_token) before rebuilding the client, so the retry carries a valid IDE token. Falls back to the resolved token if the exchange endpoint is unreachable; picks up the Enterprisebase_urlon re-exchange. The clean-401 path is already single-shot-guarded (copilot_auth_retry_attempted).Prior art appropriated (credit)
evict_cached_exchanged_tokenhelper the auxiliary path predates.model_not_available_for_integratormarker)._custom_headersvs_default_headers): fix(primary): preserve routed client headers on auto-routed provider init #9065 — our guard sidesteps it by reconstructing headers rather than reading them back off a live client.Testing
model_not_supported→ true; wrong-model 400, 401, 500 → false), 400 recovery (aborts when still degraded; rebuilds + re-exchanges on success), and 3 new 401 cases (fresh exchanged token — not the raw token — goes on the wire; network-blip fallback to the resolved token; token-unchanged still rebuilds).main; existing suites unaffected (test_copilot_token_exchange,test_copilot_auth,test_credential_pool,test_create_openai_client_reuse,test_turn_retry_state).No new
HERMES_*config env vars; no change-detector tests; prompt caching, role alternation, and the caller's_client_kwargsare all preserved.Related
HTTP 400: Bad Requestwith Github Copilot provider #45813 (400 bug report).