From 0a2a69d80b7d2d1ff8e40ee570e1b5c51c747443 Mon Sep 17 00:00:00 2001 From: wangyunyou Date: Sun, 2 Aug 2026 02:16:17 +0800 Subject: [PATCH] fix(credential_pool): check copilot suppression before token exchange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The copilot branch of _seed_from_singletons ran the suppression gate _after get_copilot_api_token(), which retries the network exchange 3x with backoff (~13s worst case). A source the user already suppressed (hermes auth remove copilot gh_cli) still burned the full exchange dead time on every pool load — model picker open, /model, agent startup — only to have the entry discarded afterwards. Move the _is_suppressed() gate ahead of the network call, matching the early-gate pattern every other singleton branch uses. Suppressed copilot sources now skip the exchange entirely. Measured: model.options payload build drops from ~13s to ~0.2-0.4s for a user with copilot suppressed. Add regression test test_load_pool_skips_exchange_for_suppressed_copilot asserting the exchange is never invoked for a suppressed source. --- agent/credential_pool.py | 50 +++++++++++++++++------------ tests/agent/test_credential_pool.py | 45 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index adb24f75a6..4ccf67fe5a 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -2390,6 +2390,16 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup from hermes_cli.copilot_auth import resolve_copilot_token, get_copilot_api_token token, source = resolve_copilot_token() if token: + source_name = "gh_cli" if "gh" in source.lower() else f"env:{source}" + # Suppression gate BEFORE the network exchange. The + # exchange retries 3x with backoff (~13s worst case), so a + # source the user already suppressed (hermes auth remove + # copilot gh_cli) must not burn that dead time on every pool + # load (model picker open, /model, agent startup) just to + # have the entry discarded afterwards. This is the same + # early-gate pattern every other singleton branch uses. + if _is_suppressed(provider, source_name): + return changed, active_sources api_token, enterprise_base_url = get_copilot_api_token(token) # Observability: get_copilot_api_token falls back to returning # the RAW token when the exchange fails. A raw ~40-char token @@ -2405,27 +2415,25 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup "unavailable); enterprise-only models may 400 with " "model_not_available_for_integrator until exchange recovers." ) - source_name = "gh_cli" if "gh" in source.lower() else f"env:{source}" - if not _is_suppressed(provider, source_name): - active_sources.add(source_name) - pconfig = PROVIDER_REGISTRY.get(provider) - # Use enterprise base URL from token exchange if available, - # otherwise fall back to the provider's default. - effective_base_url = enterprise_base_url or ( - pconfig.inference_base_url if pconfig else "" - ) - changed |= _upsert_entry( - entries, - provider, - source_name, - { - "source": source_name, - "auth_type": AUTH_TYPE_API_KEY, - "access_token": api_token, - "base_url": effective_base_url, - "label": source, - }, - ) + active_sources.add(source_name) + pconfig = PROVIDER_REGISTRY.get(provider) + # Use enterprise base URL from token exchange if available, + # otherwise fall back to the provider's default. + effective_base_url = enterprise_base_url or ( + pconfig.inference_base_url if pconfig else "" + ) + changed |= _upsert_entry( + entries, + provider, + source_name, + { + "source": source_name, + "auth_type": AUTH_TYPE_API_KEY, + "access_token": api_token, + "base_url": effective_base_url, + "label": source, + }, + ) except Exception as exc: logger.debug("Copilot token seed failed: %s", exc) diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index fa3d2b2111..0fb56e7616 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -1352,6 +1352,51 @@ def test_load_pool_seeds_copilot_via_gh_auth_token(tmp_path, monkeypatch): assert entries[0].base_url == "https://api.githubcopilot.com" +def test_load_pool_skips_exchange_for_suppressed_copilot(tmp_path, monkeypatch): + """A suppressed copilot source must NOT run the token exchange. + + Regression test: the suppression gate used to sit AFTER + ``get_copilot_api_token`` (which retries 3x with backoff, ~13s worst + case), so every pool load — model picker open, /model, agent startup — + burned the full exchange dead time for a source the user had already + removed with ``hermes auth remove copilot gh_cli``. The gate must run + BEFORE the network call. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store( + tmp_path, + { + "version": 1, + "credential_pool": {}, + "suppressed_sources": {"copilot": ["gh_cli"]}, + }, + ) + + monkeypatch.setattr( + "hermes_cli.copilot_auth.resolve_copilot_token", + lambda: ("gho_fake_token_abc123", "gh auth token"), + ) + + exchange_called = False + + def _boom(token): + nonlocal exchange_called + exchange_called = True + raise AssertionError("exchange must not run for a suppressed source") + + monkeypatch.setattr( + "hermes_cli.copilot_auth.get_copilot_api_token", + _boom, + ) + + from agent.credential_pool import load_pool + pool = load_pool("copilot") + + assert not exchange_called + assert not pool.has_credentials() + assert pool.entries() == [] + + def test_load_pool_seeds_qwen_oauth_via_cli_tokens(tmp_path, monkeypatch):