fix(copilot): fall back to credential_pool OAuth access_token for /model picker (#16708)
Users whose only Copilot credential is the OAuth `access_token` saved by
`hermes auth add copilot` (device-code flow) saw the `/model` picker drop
back to a stale hardcoded list. Reason: `_resolve_copilot_catalog_api_key`
only consulted env vars (`COPILOT_GITHUB_TOKEN` / `GH_TOKEN` /
`GITHUB_TOKEN`) and the `gh auth token` CLI fallback, never the credential
pool that Hermes's own login flow writes into `auth.json`. With no token,
the live catalog fetch silently 401s and the picker hides current models
(claude-opus-4.7, claude-sonnet-4.6, gpt-5.5, grok-code-fast-1) — even
though `/model <id>` works fine because runtime inference reads the pool
through a different code path.
Mirror the Codex catalog resolver pattern: env-var first (unchanged), then
walk `read_credential_pool("copilot")` for the first entry with a
supported `access_token` (`gho_*` / `github_pat_*` / `ghu_*`). Run it
through `get_copilot_api_token()` so the catalog request uses the same
exchanged token the runtime path uses. Classic PATs (`ghp_*`) are still
rejected up-front via `validate_copilot_token` since the Copilot API
doesn't accept them.
Strictly additive: env still wins, and a missing/locked auth.json (or any
exception during pool read) still returns "" so the caller falls through
to the curated catalog.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dd789a4fdf
commit
fdfe40a48b
@ -1700,14 +1700,50 @@ 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")`` — OAuth ``access_token`` saved
|
||||
in ``auth.json`` by ``hermes auth add copilot`` (device-code flow).
|
||||
|
||||
Without (2), users whose only Copilot credential is the OAuth token
|
||||
Hermes itself stores see the ``/model`` picker fall back to a stale
|
||||
hardcoded list because the live catalog fetch silently 401s.
|
||||
"""
|
||||
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 = str(creds.get("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 (
|
||||
get_copilot_api_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
|
||||
return get_copilot_api_token(raw)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return ""
|
||||
|
||||
|
||||
# Providers where models.dev is treated as authoritative: curated static
|
||||
|
||||
111
tests/hermes_cli/test_copilot_catalog_oauth_fallback.py
Normal file
111
tests/hermes_cli/test_copilot_catalog_oauth_fallback.py
Normal file
@ -0,0 +1,111 @@
|
||||
"""Catalog-API-key fallback for the Copilot ``/model`` picker.
|
||||
|
||||
Regression for #16708: when the user's only Copilot credential is the
|
||||
OAuth ``access_token`` saved in ``auth.json`` (the device-code flow that
|
||||
``hermes auth add copilot`` itself produces), 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": "env-token"},
|
||||
), 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 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": "gho_abc123"}],
|
||||
), patch(
|
||||
"hermes_cli.copilot_auth.get_copilot_api_token",
|
||||
return_value="exchanged-tid_xyz",
|
||||
):
|
||||
assert _resolve_copilot_catalog_api_key() == "exchanged-tid_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": "gho_xyz"}],
|
||||
), patch(
|
||||
"hermes_cli.copilot_auth.get_copilot_api_token",
|
||||
return_value="exchanged-tid_xyz",
|
||||
):
|
||||
assert _resolve_copilot_catalog_api_key() == "exchanged-tid_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": "ghp_classic_pat"}],
|
||||
), patch(
|
||||
"hermes_cli.copilot_auth.get_copilot_api_token",
|
||||
) as mock_exchange:
|
||||
assert _resolve_copilot_catalog_api_key() == ""
|
||||
mock_exchange.assert_not_called()
|
||||
|
||||
def test_skips_invalid_pool_entries(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": "gho_first_real_token"},
|
||||
{"access_token": "gho_should_not_reach"},
|
||||
],
|
||||
), patch(
|
||||
"hermes_cli.copilot_auth.get_copilot_api_token",
|
||||
return_value="exchanged-from-first",
|
||||
) as mock_exchange:
|
||||
assert _resolve_copilot_catalog_api_key() == "exchanged-from-first"
|
||||
mock_exchange.assert_called_once_with("gho_first_real_token")
|
||||
|
||||
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() == ""
|
||||
Loading…
Reference in New Issue
Block a user