Skip to content

fix: desktop model picker hid Anthropic despite valid Claude Code credentials - #72155

Closed
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-model-picker-anthropic
Closed

fix: desktop model picker hid Anthropic despite valid Claude Code credentials#72155
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-model-picker-anthropic

Conversation

@adurham

@adurham adurham commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the desktop model picker hiding Anthropic even when the user has
valid Claude Code CLI or Hermes-managed OAuth credentials — while the CLI
/model picker shows it correctly on the exact same credentials.

Root Cause

_filter_explicit_provider_rows() in hermes_cli/inventory.py (the
desktop-only explicit_only=True picker filter) re-checks every row
against is_provider_explicitly_configured(), which deliberately
excludes CLAUDE_CODE_OAUTH_TOKEN / external Claude Code credential files
(#4210 — prevents auxiliary tasks from silently burning the user's Claude
Code subscription tokens without an explicit choice). That gate is
correct for aux-task consumption, but this desktop-only filter was also
using it to decide what to display, silencing a provider row that
list_authenticated_providers() (used by both the CLI and desktop
pickers, upstream of this filter) already surfaces.

Confirmed the premise directly by reading the code: list_authenticated_providers()
in hermes_cli/model_switch.py already has this exact fallback (~line
2126: if not has_creds and hermes_slug == "anthropic":, checking
read_hermes_oauth_credentials()/read_claude_code_credentials() from
agent/anthropic_adapter.py, with a comment citing the same #4210
rationale). This PR extends an already-established, already-documented
upstream pattern to the desktop-only filter that wasn't updated to match
it — not new credential-detection logic.

Changes Made

hermes_cli/inventory.py: when is_provider_explicitly_configured("anthropic")
is False, also check for valid external Claude Code / Hermes-PKCE
credentials before dropping the row from the desktop picker's display.
is_provider_explicitly_configured() itself is untouched, so aux-task
gating is unaffected — this only widens what the desktop picker shows.

Related Issue

No existing issue found (searched gh search issues for "desktop model
picker missing anthropic", "Claude Code credentials not shown picker",
"explicit_only provider filter" — no hits).

Type of Change

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

How to Test

  1. Authenticate via Claude Code CLI or Hermes-managed OAuth (no
    ANTHROPIC_API_KEY set, no explicit Hermes-side Anthropic
    configuration).
  2. Open the CLI /model picker — Anthropic/Claude models appear.
  3. Open the Desktop model picker on the same credentials.
  4. Before this fix: Anthropic is missing from the desktop picker.
  5. After this fix: Anthropic appears, matching the CLI.
  6. pytest tests/hermes_cli/test_inventory.py -q — 46 passed.
  7. Also ran adjacent credential-detection test files to check for
    regressions: test_inventory_pricing.py,
    test_model_switch_custom_providers.py,
    test_model_switch_configured_provider_routing.py — 117 passed total,
    0 failed.

Checklist

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs/issues (none found)
  • My PR contains only changes related to this fix
  • I've run the relevant test suite and all tests pass
  • I've added tests for my changes
  • Tested on macOS

adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 26, 2026
…tems

Documents outcomes for the 6 desktop-category Bucket A items:

Submitted (4):
- PR NousResearch#72151 -- RAF-throttle blank transcript on session switch
- PR NousResearch#72152 -- profile deletion zombie backend + cross-window rail staleness
- PR NousResearch#72153 -- Nerd Font terminal fallback
- PR NousResearch#72155 -- desktop model picker hiding Anthropic

Deprioritized, needs hand-reconciliation (3): drag-to-reorder, workspace
tab close button, queued composer wrong-session delivery. All three
rejected 1+ files on git apply --check with real semantic drift (not
line-offset noise) against the fast-churning desktop session/composer
code. Flagged for revisit rather than force-applying a stale patch.

The profile-deletion and model-picker items both got real scrutiny beyond
"does it apply cleanly": profile-deletion required two rounds of external
consult after search-first turned up 2 already-merged PRs on the same
headline symptom -- verified directly (not assumed) that this fix's two
pieces are genuinely non-overlapping gaps, not stale duplicates. Model
picker's fix was confirmed to extend an already-established upstream
credential-detection pattern rather than introduce new CC-mimicry
plumbing, addressing the specific caution flagged in the original audit.

Saved verified patches to .upstream-candidates/ for reference.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard provider/anthropic Anthropic native Messages API labels Jul 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Current main still has the reported mismatch: hermes_cli/model_switch.py:2298-2308 includes Anthropic when external Claude Code/Hermes OAuth credentials provide an access token, while hermes_cli/inventory.py:581-613 subsequently removes that same non-explicit row for Desktop’s explicit_only response. Desktop defaults to that response mode at apps/desktop/src/hermes.ts:1448-1449.

The proposed display-only carve-out matches the existing CLI discovery behavior without changing the intentional auxiliary-task gate in hermes_cli/auth.py:1661-1731. The positive and negative inventory tests cover the new filter branch. GitHub currently reports the branch as conflicting with main, but the verified production change is narrow and mechanically salvageable.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 30, 2026
…dentials

_filter_explicit_provider_rows() in hermes_cli/inventory.py (the
desktop-only explicit_only=True picker filter) re-checked every row
against is_provider_explicitly_configured(), which deliberately excludes
CLAUDE_CODE_OAUTH_TOKEN / external Claude Code credential files (PR NousResearch#4210,
prevents aux tasks from silently burning subscription tokens). That gate
is correct for aux-task consumption, but was also silencing the picker's
display of a provider list_authenticated_providers() already surfaces for
both CLI and desktop via the exact same credential check -- so the CLI's
/model picker showed Anthropic while the desktop's did not, on identical
credentials.

Confirmed the premise directly: list_authenticated_providers() in
hermes_cli/model_switch.py already has this exact fallback (line ~2126,
`if not has_creds and hermes_slug == "anthropic":` checking
read_hermes_oauth_credentials()/read_claude_code_credentials() from
agent/anthropic_adapter.py, with a comment citing the same PR NousResearch#4210
rationale) -- this fix extends an already-established, already-documented
upstream pattern to the desktop-only filter that wasn't updated to match,
rather than inventing new credential-detection logic.

Add a narrow carve-out: when is_provider_explicitly_configured("anthropic")
is False, also check for valid external Claude Code / Hermes-PKCE
credentials before dropping the row. is_provider_explicitly_configured()
itself is untouched, so aux-task gating is unaffected -- this only widens
what the desktop picker displays.

Tests: tests/hermes_cli/test_inventory.py -- 46 passed. Also ran
test_inventory_pricing.py, test_model_switch_custom_providers.py,
test_model_switch_configured_provider_routing.py to check for regressions
on adjacent credential-detection paths: 117 passed total, 0 failed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adurham
adurham force-pushed the upstream-pr/desktop-model-picker-anthropic branch from efcd994 to 71346fa Compare July 31, 2026 18:55
@adurham

adurham commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (was showing as conflicting). The only real conflict was in tests/hermes_cli/test_inventory.py, where an unrelated test-pruning pass upstream had removed 3 pre-existing tests my diff's context lines touched (test_include_unconfigured_keeps_current_provider_visible_without_credentials, test_include_unconfigured_does_not_duplicate_configured_current_row, test_explicit_only_keeps_moa_when_raw_config_has_enabled_preset) -- resolved by not resurrecting them, just cleanly inserting this PR's 2 new tests. hermes_cli/inventory.py itself auto-merged with zero conflicts against the intervening refactor(cli): route every aux picker through one provider-inventory seam work.

Verified after rebase: tests/hermes_cli/test_inventory.py 16/16 passed, plus ran the adjacent inventory/aux-picker/model-switch suites for regressions (59/59 passed, 0 failed) given the seam refactor landed in between.

adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 31, 2026
…s on all 7 PRs

Documents the real external engagement on the 7 upstream PRs filed
2026-07-26, and the 3 substantive follow-up fixes pushed in response:

- NousResearch#72054 closed as superseded, but merged anyway via NousResearch#74139 (contributor
  CrowLoki's reconciliation with NousResearch#62026, credited via Co-authored-by).
- NousResearch#72087, NousResearch#72151, NousResearch#72152, NousResearch#72153, NousResearch#72155, NousResearch#72164 all reviewed by the
  repo's automated sweeper -- keep_open/high on all 6.
- Fixed NousResearch#72087 (payload-proportional test assertions, catching a future
  allowlist-regression risk flagged by both the sweeper and an
  independent contributor who measured it precisely on their own fork).
- Fixed NousResearch#72152 (extracted ProfileRail's focus/visibilitychange wiring
  into a tested hook, matching the directory's own established
  use-profile-prewarm.ts pattern).
- Rebased NousResearch#72155 past a real merge conflict (an unrelated upstream
  test-pruning pass removed 3 tests my diff's context touched).

All fixes verified by simulating the exact regression each review was
warning about and confirming the new tests catch it, then restoring the
real fix.

Also noted a real environment issue found this session: the `upstream`
remote's SSH URL intermittently fails to connect from this network; a
one-off HTTPS fetch into a separate ref works around it without touching
the configured remote.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@teknium1

Copy link
Copy Markdown
Contributor

Closing with a credit correction: you were the first to submit this fix — filed July 26, nearly a month before the equivalent #92391 (Aug 22). Our post-merge duplicate sweep surfaced yours only after #92702 had already merged the later PR's commit, which independently re-derived the same root cause and the same carve-out approach (display-only filter fix, is_provider_explicitly_configured() / #4210 aux-task gating untouched).

The fix that landed on main matches your diagnosis exactly, plus one addition neither PR had: pool-only OAuth entries in auth.json credential_pool.anthropic (the canonical wired device-flow token location) are also accepted now.

Apologies for the credit miss — your PR was the earliest correct diagnosis of this bug, and this comment is the record of that. Thanks for the clean work.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have provider/anthropic Anthropic native Messages API 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants