Copilot OAuth credential fallback and auth pool resolution

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #16901 fix(copilot): fall back to credential_pool OAuth access_token for /model picker (#16708)

  merged to main

  files: hermes_cli/models.py, tests/hermes_cli/test_copilot_catalog_oauth_fallback.py

  Show the diff

  • hermes_cli/models.py

    @@ -1700,14 +1700,61 @@ def resolve_fast_mode_overrides(model_id: Optional[str]) -> dict[str, Any] | Non
     
     
     def _resolve_copilot_catalog_api_key() -> str:
    -    """Best-effort GitHub token for fetching the Copilot model catalog."""
    +    """Best-effort GitHub token for fetching the Copilot model catalog.
    +
    +    Resolution order:
    +      1. ``resolve_api_key_provider_credentials("copilot")`` — env vars
    +         (``COPILOT_GITHUB_TOKEN`` / ``GH_TOKEN`` / ``GITHUB_TOKEN``) plus
    +         the ``gh auth token`` CLI fallback.
    +      2. ``read_credential_pool("copilot")`` — a token (typically a
    +         ``gho_*`` from device-code login, or a fine-grained PAT) stored in
    +         ``auth.json`` under ``credential_pool.copilot[]``. The pool is
    +         populated by ``hermes auth add copilot`` and by ``_seed_from_env``
    +         when the env var is set in ``~/.hermes/.env``.
    +
    +    Without (2), users whose only Copilot credential is in the pool see
    +    the ``/model`` picker fall back to a stale hardcoded list because the
    +    live catalog fetch silently 401s. To avoid wedging on a malformed pool
    +    entry, each candidate is exchanged via ``exchange_copilot_token`` —
    +    only entries that actually exchange successfully are returned, so a
    +    later valid entry is reachable when an earlier one is unsupported.
    +    """
         try:
             from hermes_cli.auth import resolve_api_key_provider_credentials
     
             creds = resolve_api_key_provider_credentials("copilot")
    -        return str(creds.get("api_key") or "").strip()
    +        api_key = [redacted:secret-assignment]"api_key") or "").strip()
    +        if api_key:
    +            return api_key
         except Exception:
    -        return ""
    +        pass
    +
    +    try:
    +        from hermes_cli.auth import read_credential_pool
    +        from hermes_cli.copilot_auth import (
    +            exchange_copilot_token,
    +            validate_copilot_token,
    +        )
    +
    +        for entry in read_credential_pool("copilot"):
    +            if not isinstance(entry, dict):
    +                continue
    +            raw = str(entry.get("access_token") or "").strip()
    +            if not raw:
    +                continue
    +            valid, _ = validate_copilot_token(raw)
    +            if not valid:
    +                continue
    +            try:
    +                api_token, _expires_at = exchange_copilot_token(raw)
    +            except Exception:
    +                continue
    +            if api_token:
    +                return api_token
    +    except Exception:
    +        pass
    +
    +    return ""
     
     
     # Providers where models.dev is treated as authoritative: curated static

  • tests/hermes_cli/test_copilot_catalog_oauth_fallback.py

    @@ -0,0 +1,157 @@
    +"""Catalog-API-key fallback for the Copilot ``/model`` picker.
    +
    +Regression for #16708: when the user's only Copilot credential is a
    +``gho_*`` token (typically obtained via device-code login) stored in
    +``auth.json`` under ``credential_pool.copilot[]`` — placed there by
    +``hermes auth add copilot`` or by ``_seed_from_env`` when the env var
    +is set in ``~/.hermes/.env`` — the picker was silently dropping back to
    +a stale hardcoded list because ``_resolve_copilot_catalog_api_key``
    +only consulted env vars / ``gh auth token`` and never read the
    +credential pool.
    +"""
    +
    +from unittest.mock import patch
    +
    +from hermes_cli.models import _resolve_copilot_catalog_api_key
    +
    +
    +class TestCopilotCatalogApiKeyResolution:
    +    def test_env_var_token_wins_over_pool(self):
    +        """Env-resolved token still short-circuits the pool fallback."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": "[redacted:secret-assignment]"},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +        ) as mock_pool:
    +            assert _resolve_copilot_catalog_api_key() == "env-token"
    +            mock_pool.assert_not_called()
    +
    +    def test_falls_back_to_pool_oauth_token(self):
    +        """Empty env → walk credential_pool.copilot[] for an OAuth access_token."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[{"access_token": "[redacted:secret-assignment]"}],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +            return_value=("tid_exchanged_xyz", 1234567890.0),
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == "tid_exchanged_xyz"
    +
    +    def test_falls_back_when_env_resolution_raises(self):
    +        """Env path raising an exception still falls through to the pool."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            side_effect=RuntimeError("auth.json corrupt"),
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[{"access_token": "[redacted:secret-assignment]"}],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +            return_value=("tid_exchanged_xyz", 1234567890.0),
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == "tid_exchanged_xyz"
    +
    +    def test_skips_classic_pat_in_pool(self):
    +        """Classic PATs (``ghp_…``) are unsupported by the Copilot API — skip them."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[{"access_token": "[redacted:secret-assignment]"}],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +        ) as mock_exchange:
    +            assert _resolve_copilot_catalog_api_key() == ""
    +            mock_exchange.assert_not_called()
    +
    +    def test_skips_invalid_pool_entries_until_first_exchangeable(self):
    +        """Non-dict entries and entries without an ``access_token`` are skipped."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[
    +                "not-a-dict",
    +                {"label": "no-token-here"},
    +                {"access_token": ""},
    +                {"access_token": "[redacted:secret-assignment]"},
    +                {"access_token": "[redacted:secret-assignment]"},
    +            ],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +            return_value=("tid_from_first", 1234567890.0),
    +        ) as mock_exchange:
    +            assert _resolve_copilot_catalog_api_key() == "tid_from_first"
    +            mock_exchange.assert_called_once_with("gho_first_real_token")
    +
    +    def test_skips_pool_entry_that_fails_to_exchange(self):
    +        """If the first entry won't exchange, try the next — an unsupported pool[0]
    +        must not wedge a later valid entry (Copilot review #16868 finding)."""
    +        attempts: list[str] = []
    +
    +        def fake_exchange(raw_token: str):
    +            attempts.append(raw_token)
    +            if raw_token == "gho_unsupported_account":
    +                raise ValueError("Copilot token exchange failed: HTTP 401")
    +            return ("tid_from_second", 1234567890.0)
    +
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[
    +                {"access_token": "[redacted:secret-assignment]"},
    +                {"access_token": "[redacted:secret-assignment]"},
    +            ],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +            side_effect=fake_exchange,
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == "tid_from_second"
    +            assert attempts == ["gho_unsupported_account", "gho_valid_token"]
    +
    +    def test_all_pool_entries_fail_exchange_returns_empty(self):
    +        """All exchanges fail → return "" so the caller falls back to curated."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[
    +                {"access_token": "[redacted:secret-assignment]"},
    +                {"access_token": "[redacted:secret-assignment]"},
    +            ],
    +        ), patch(
    +            "hermes_cli.copilot_auth.exchange_copilot_token",
    +            side_effect=ValueError("Copilot token exchange failed"),
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == ""
    +
    +    def test_returns_empty_string_when_no_credentials_anywhere(self):
    +        """No env, no pool → empty string (caller falls back to curated list)."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            return_value=[],
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == ""
    +
    +    def test_pool_failure_returns_empty_string(self):
    +        """If the pool read itself raises, swallow and return ""."""
    +        with patch(
    +            "hermes_cli.auth.resolve_api_key_provider_credentials",
    +            return_value={"api_key": ""},
    +        ), patch(
    +            "hermes_cli.auth.read_credential_pool",
    +            side_effect=RuntimeError("auth.json locked"),
    +        ):
    +            assert _resolve_copilot_catalog_api_key() == ""