fix(credential_pool): check copilot suppression before token exchange

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.
2026-08-02 02:16:17 +08:00
parent c062fde8b1
commit 0a2a69d80b
2 changed files with 74 additions and 21 deletions
@@ -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)

@@ -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):