Skip to content

fix(model): detect auth-store credential_pool for mapped providers#5753

Closed
jakubkrcmar wants to merge 1 commit into
NousResearch:mainfrom
jakubkrcmar:fix/interactive-model-picker
Closed

fix(model): detect auth-store credential_pool for mapped providers#5753
jakubkrcmar wants to merge 1 commit into
NousResearch:mainfrom
jakubkrcmar:fix/interactive-model-picker

Conversation

@jakubkrcmar

@jakubkrcmar jakubkrcmar commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • treat Hermes auth-store credential_pool entries as valid credentials during /model provider listing for Hermes-mapped providers
  • preserve provider visibility when credentials exist in Hermes auth storage but are not exported as environment variables in the current process
  • add a regression test covering the mapped-provider credential_pool path

Repro

With credentials present only in Hermes auth storage under credential_pool and no matching provider env var exported:

  • clean upstream omits gemini from /model
  • this patch makes gemini appear correctly with its curated models

Scope

This PR only fixes the mapped-provider credential_pool discovery gap in list_authenticated_providers().

It does not attempt broader slug-normalization cleanup, and it no longer includes the earlier openai-codex curated-model delta because current upstream already has that behavior.

Validation

  • smoke-tested list_authenticated_providers() against clean origin/main
  • verified the difference again with a real local auth-store containing gemini only in credential_pool
  • ran:
    • ./.venv/bin/python -m pytest tests/hermes_cli/test_overlay_slug_resolution.py -q
    • ./.venv/bin/python -m pytest tests/hermes_cli/test_model_switch_custom_providers.py -q

@jakubkrcmar jakubkrcmar force-pushed the fix/interactive-model-picker branch from 01aae9a to 325f4c0 Compare April 8, 2026 14:20
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

Refreshed this PR on top of current upstream v0.8.0 tip. The branch now contains only the still-needed delta: credential_pool detection for mapped providers like copilot plus curated gpt-5.4/gpt-5.4-mini entries for openai-codex.

@walkerfeng

Copy link
Copy Markdown

I hit a closely related case on the gateway /model flow.

In my repro, the problem was not only Copilot model visibility, but also provider visibility when credentials existed in Hermes auth storage / credential pool but were not exported as environment variables in the current process.

What I observed:

  • copilot was present in Hermes auth storage, but /model could show it incorrectly or with missing/empty model entries
  • gemini was also present in auth storage, but /model did not show it at all
  • the behavior depended too heavily on env-var detection during provider listing

It also looks like there is a provider-ID mismatch aspect for Copilot:

  • Hermes slug: copilot
  • models.dev provider id: github-copilot

So provider detection and curated model lookup can disagree unless both the Hermes slug and the mapped models.dev id are checked consistently.

A similar mapping issue may also affect Gemini / Google:

  • Hermes slug: gemini
  • models.dev provider id: google

In short, /model provider listing seems more reliable if it treats these as valid credential sources:

  • environment variables
  • auth store providers
  • auth store credential_pool

And then normalizes provider slug ↔ models.dev id before duplicate suppression / curated model lookup.

I’m mentioning this because the fix here may want to cover not just Copilot model enumeration, but the broader “authenticated in Hermes but not exported in env vars” case too.

@jakubkrcmar jakubkrcmar force-pushed the fix/interactive-model-picker branch from 325f4c0 to e82b456 Compare April 10, 2026 07:51
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

Thanks — I reproduced the broader case locally and refreshed this PR branch on top of current upstream.

Verified behavior difference against clean upstream:

  • providers authenticated in Hermes auth storage but not exported as env vars can be missing from /model
  • in my repro this affected not just copilot, but also gemini
  • clean upstream showed only openai-codex
  • local patched behavior showed copilot, gemini, and openai-codex

So I agree the fix should be framed as broader provider visibility from Hermes auth storage, not just Copilot model enumeration.

This PR still does two concrete things:

  • treat credential_pool as a valid credential source for mapped providers during provider listing
  • add curated gpt-5.4 / gpt-5.4-mini entries for openai-codex

It does not try to fully rewrite all slug <-> models.dev normalization paths, but it does address the verified "authenticated in Hermes, not exported in env vars" failure mode that affected both copilot and gemini in my local tests.

@jakubkrcmar jakubkrcmar changed the title fix(model_picker): resolve auth store properly and add gpt-5.4 fix(model_picker): detect auth-store credentials for mapped providers Apr 10, 2026
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

Quick note: GitHub Actions on this fork PR currently show action_required with 0s runtime (Tests / Nix / Docker / Supply Chain Audit), which looks like maintainer workflow approval is still needed before checks will actually run.

@jakubkrcmar jakubkrcmar force-pushed the fix/interactive-model-picker branch from e82b456 to 6e353aa Compare April 12, 2026 19:13
@jakubkrcmar jakubkrcmar changed the title fix(model_picker): detect auth-store credentials for mapped providers fix(model_picker): detect mapped-provider auth-store credentials Apr 12, 2026
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

Verified again against current origin/main after rebasing.\n\nCurrent upstream still misses Hermes-mapped providers when credentials exist only in auth-store credential_pool and are not exported as env vars. In a local smoke test, clean upstream omitted gemini in that scenario, while the reduced patch makes gemini appear with its curated models.\n\nI also trimmed this PR to only that surviving fix. The earlier openai-codex curated-model addition is no longer included because current upstream already has that behavior.

@jakubkrcmar jakubkrcmar force-pushed the fix/interactive-model-picker branch from 6e353aa to 4f227f1 Compare April 13, 2026 06:41
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

Refreshed this branch on top of current origin/main and re-verified the behavior against clean upstream.

What I checked locally:

  • clean upstream: mapped provider gemini does not appear when credentials exist only in Hermes auth-store credential_pool
  • patched branch: gemini appears correctly
  • real local auth-store also reproduces this difference (credential_pool currently contains gemini)

So the fix is still needed; this PR now contains only the surviving delta on top of current upstream.

@jakubkrcmar jakubkrcmar changed the title fix(model_picker): detect mapped-provider auth-store credentials fix(model): detect auth-store credential_pool for mapped providers Apr 13, 2026
@jakubkrcmar

Copy link
Copy Markdown
Contributor Author

This branch has been refreshed on top of current origin/main and reduced to the still-needed delta only.

Re-verified locally against clean upstream:

  • clean upstream omits gemini when credentials exist only in Hermes auth-store credential_pool
  • this patch makes the mapped provider appear correctly in /model

Would appreciate maintainer review when convenient.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful, well-maintained contribution @jakubkrcmar! This fix has already been merged into main.


Automated hermes-sweeper review.

The credential_pool detection logic and the regression test from this PR were salvaged into main as commit 1af44a13c (fix(model_picker): detect mapped-provider auth-store credentials), which has the identical diff (+8 lines to hermes_cli/model_switch.py, +19 lines to tests/hermes_cli/test_overlay_slug_resolution.py).

  • hermes_cli/model_switch.py line 1058–1065: credential_pool check is present verbatim
  • tests/hermes_cli/test_overlay_slug_resolution.py line 87: test_mapped_provider_credential_pool_visibility is present verbatim
  • 483 subsequent commits have landed on top of this fix

Closing as implemented on main.

@teknium1 teknium1 closed this Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants