Skip to content

fix(copilot): honor token precedence and derive api_mode per target model - #17622

Open
patrickchin wants to merge 1 commit into
NousResearch:mainfrom
patrickchin:fix/copilot-pool-priority-and-api-mode
Open

fix(copilot): honor token precedence and derive api_mode per target model#17622
patrickchin wants to merge 1 commit into
NousResearch:mainfrom
patrickchin:fix/copilot-pool-priority-and-api-mode

Conversation

@patrickchin

@patrickchin patrickchin commented Apr 29, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes two compounding bugs that surface as HTTP 400 model_not_supported on Copilot for most non-default-model requests, even when the user's GitHub account does have access to the model in question.

Bug 1 — Credential pool ignores documented Copilot token precedence

hermes_cli/copilot_auth.py:resolve_copilot_token documents the precedence COPILOT_GITHUB_TOKEN > GH_TOKEN > GITHUB_TOKEN > gh auth token. That precedence is honored on the first load_pool() because _seed_from_singletons calls resolve_copilot_token() directly and seeds a single entry.

But the pool is persisted to ~/.hermes/auth.json. On subsequent loads, the previously seeded entry (e.g. gh_cli) is read back from disk before re-seeding runs. If the user later adds a higher-precedence source — for example, by running hermes model to log into Copilot via device-code flow, which writes COPILOT_GITHUB_TOKEN to ~/.hermes/.env — the new entry is upserted at the next free priority, behind the stale gh_cli entry. Subsequent requests authenticate as the wrong GitHub account, and Copilot returns model_not_supported for any model the wrong account can't reach.

_normalize_pool_priorities is the function that re-ranks pool entries after the load/upsert cycle — but it only special-cased anthropic. PR #2647 introduced the function specifically to solve this same bug class (multi-source providers where insertion order doesn't match the user's intended precedence). Copilot acquired multiple credential sources in commit 0bd3f521 (April 2026) without _normalize_pool_priorities being extended to match.

Bug 2 — Copilot api_mode not recomputed per target model

Copilot serves different model families through different endpoints under https://api.githubcopilot.com:

  • /responses (codex_responses): GPT-5.x non-mini, GPT-5.x-codex
  • /chat/completions: GPT-5-mini, GPT-4.1, GPT-4o, Gemini
  • /v1/messages (anthropic_messages): Claude

hermes_cli/models.copilot_model_api_mode() already encodes this mapping correctly. But hermes_cli/runtime_provider.py:_copilot_runtime_api_mode short-circuited on the persisted model.api_mode when the configured provider was Copilot. Config written while model.default = gpt-5.4 records api_mode: codex_responses; a later /model claude-sonnet-4.6 (or any other non-codex Copilot model) was then dispatched to /responses and rejected.

PR #15106 fixed the same root cause for opencode-zen / opencode-go by threading target_model through resolve_runtime_provider and _resolve_runtime_from_pool_entry. PR #9033 fixed the equivalent in the cron path. The runtime resolver path for Copilot was missed.

hermes_cli/model_switch.py:919 already calls copilot_model_api_mode() at config-write time, so this fix is defense-in-depth — it covers any path that bypasses the model-switch helper (gateway dispatch, runtime overrides, fresh sessions on stale config).

Related Issues

No existing issue covers this exact pair. Closely related context:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

agent/credential_pool.py — refactor _normalize_pool_priorities to select a per-provider source_rank table via if/elif/else instead of an early-out on non-anthropic. Adds a copilot branch mirroring the precedence in hermes_cli/copilot_auth.py:resolve_copilot_token. The existing manual/seeded split, sort, and priority rewrite logic is unchanged and now shared between both providers. Net +13 lines, zero behavior change for anthropic.

hermes_cli/runtime_provider.py

  • _copilot_runtime_api_mode now reads target_model first, falls back to model.default, and always invokes copilot_model_api_mode() to re-derive from the actual model being dispatched.
  • The two pool-resolver call sites (_resolve_runtime_from_pool_entry and the explicit-runtime fallback in resolve_runtime_provider) thread target_model into the cfg dict so mid-session /model switches see the new model.

tests/agent/test_credential_pool.pytest_copilot_pool_prioritizes_env_token_over_gh_cli. Placed next to the existing test_load_pool_seeds_copilot_via_gh_auth_token.

tests/hermes_cli/test_runtime_provider_resolution.pytest_copilot_api_mode_recomputed_for_non_codex_default_model, test_copilot_api_mode_follows_target_model_on_runtime_switch. Placed next to the existing test_opencode_go_model_derivation_beats_stale_persisted_api_mode which uses the same precedent pattern.

How to Test

Manual reproduction (requires two GitHub accounts with different Copilot entitlements):

  1. gh auth login as account A (limited Copilot access — e.g. Individual plan with no Claude/Gemini access).
  2. Run hermes model and complete OAuth device-code login as account B (Copilot Business / Enterprise with full model access). Verify ~/.hermes/.env now contains COPILOT_GITHUB_TOKEN=gho_….
  3. During interactive setup, set model.default: gpt-5.4 so ~/.hermes/config.yaml records model.api_mode: codex_responses.
  4. Before this PR: hermes chat -q "hi" against any non-GPT-5 Copilot model (claude-sonnet-4.6, gemini-3.1-pro-preview, gpt-5-mini) returns 400 model_not_supported. Inspect ~/.hermes/auth.json and confirm gh_cli outranks env:COPILOT_GITHUB_TOKEN.
  5. After this PR: the same calls succeed. Account B's identity is used (Bug 1 fixed) and the request is dispatched to the correct endpoint for the target model (Bug 2 fixed).

Automated:

scripts/run_tests.sh tests/agent/test_credential_pool.py \
                     tests/agent/test_credential_pool_routing.py \
                     tests/hermes_cli/test_runtime_provider_resolution.py \
                     tests/hermes_cli/test_copilot_auth.py \
                     tests/hermes_cli/test_model_switch_copilot_api_mode.py

→ 187 passed.

Checklist

Code

Documentation & Housekeeping

  • N/A — no doc changes needed; the precedence is already documented in hermes_cli/copilot_auth.py:resolve_copilot_token and the fix makes the runtime match the doc
  • N/A — no config keys added or changed
  • N/A — no architecture or workflow changes
  • Cross-platform: pure Python logic, no OS-specific calls
  • N/A — no tool schema changes

…odel

Two compounding bugs that surface as HTTP 400 model_not_supported on
Copilot for most non-default-model requests, even when the user's
GitHub account has access to the model.

1. _normalize_pool_priorities only special-cased anthropic, so a
   Copilot pool with both env:COPILOT_GITHUB_TOKEN (from `hermes model`
   device login) and gh_cli (from a pre-existing `gh auth login` for a
   different account) could let the gh CLI entry win after the
   persist/reload cycle. Tuple-order in api_key_env_vars only protects
   the first load; once the pool is persisted to ~/.hermes/auth.json,
   subsequent loads can keep a stale gh_cli ranked above a
   higher-precedence env token added later. Extends the existing
   anthropic precedence pattern (PR NousResearch#2647) to copilot, mirroring the
   precedence documented in
   hermes_cli/copilot_auth.py:resolve_copilot_token.

2. _copilot_runtime_api_mode short-circuited on the persisted
   model.api_mode without consulting the actual model being dispatched.
   Config written while default=gpt-5.4 persists api_mode=codex_responses,
   so a later /model claude-sonnet-4.6 (or gemini-3.1-pro-preview, or
   gpt-5-mini) was sent to /responses and rejected. Threads
   target_model into the resolver and always re-derives api_mode via
   copilot_model_api_mode(). Same fix pattern as PR NousResearch#15106 (opencode)
   and PR NousResearch#9033 (cron).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard provider/copilot GitHub Copilot (ACP + Chat) labels Apr 29, 2026
@lucvan

lucvan commented May 12, 2026

Copy link
Copy Markdown

Confirming this fix is correct: applied the equivalent change locally and verified via httpx wire-level tracing that without it, is what reaches the agent constructor when the configured model.default is a GPT-5 variant and a Copilot/Claude entry is selected from fallback_providers:. Claude traffic then gets sent to /v1/responses with the documented unsupported_api_for_model 400. This PR's approach (threading via _model_cfg["target_model"]) cleanly addresses it. Adding +1 from a separate downstream user.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating both the persisted-pool precedence issue and the stale Copilot transport selection.

Problems

  • The target-model fix is incomplete against current main. hermes_cli/runtime_provider.py:1483 is a separate explicit-runtime Copilot path that still calls _copilot_runtime_api_mode(model_cfg, api_key); _resolve_explicit_runtime does not receive target_model. An explicit credential resolution can therefore still select the persisted model.api_mode / model.default rather than the requested target.

Suggested changes

  • Thread target_model through _resolve_explicit_runtime from resolve_runtime_provider, pass it into the Copilot mode resolver, and add an explicit-credential regression test. The existing pool-path test covers hermes_cli/runtime_provider.py:446, but not the path at :1483.

Automated hermes-sweeper review.

@@ -219,7 +230,9 @@ def _resolve_runtime_from_pool_entry(
elif provider == "nous":
api_mode = "chat_completions"
elif provider == "copilot":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please cover the sibling explicit-runtime Copilot path as well. On current main, _resolve_explicit_runtime still calls _copilot_runtime_api_mode(model_cfg, api_key) without a target model (hermes_cli/runtime_provider.py:1483), so explicit-key/base-URL resolution retains the stale configured transport after this pool-path fix.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/copilot GitHub Copilot (ACP + Chat) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants