Skip to content

fix(desktop): don't overwrite sticky $currentProvider with backend billing-class "custom" or "custom:<name>" slugs - #50151

Closed
lost9999 wants to merge 1 commit into
NousResearch:mainfrom
lost9999:fix/desktop-skip-custom-provider-info
Closed

fix(desktop): don't overwrite sticky $currentProvider with backend billing-class "custom" or "custom:<name>" slugs#50151
lost9999 wants to merge 1 commit into
NousResearch:mainfrom
lost9999:fix/desktop-skip-custom-provider-info

Conversation

@lost9999

Copy link
Copy Markdown
Contributor

fix(desktop): don't overwrite sticky $currentProvider with backend's billing-class "custom"

What does this PR do?

Fixes a Desktop regression where every new chat fails with HTTP 400: ... GenerateContentRequest.model: unexpected model name format until the user manually re-picks a model in the picker — even though the chat was started on a fully configured custom_providers: entry (e.g. tokenrouter).

A new helper isStickySafeProvider() is introduced in use-message-stream.ts. The session.info handler now uses it to refuse the literal billing-class string "custom" (which backend writes into agent.provider for every named providers: / custom_providers: entry), keeping the existing sticky $currentProvider atom value instead.

Related Issue

This addresses a recurrence of the cross-surface staleness pattern already covered by PR #40163 / #43702 ("keep model runtime state per session"). The previous fixes bail-when-active-session; this fix is the missing sibling: bail-when-payload-provider-is-not-user-facing.

Root cause

  1. The desktop composer ships its model + provider UI state to every session.create call (use-session-actions.ts:462).
  2. Backend's tui_gateway/server.py:_session_info (line 2642) sets info["provider"] = getattr(agent, "provider", ""), and agent.provider is the resolved billing classruntime_provider.py:_resolve_named_custom_runtime returns {"provider": "custom", ...} for every named custom_providers: entry.
  3. Desktop's use-message-stream.ts:744 then wrote that literal "custom" into localStorage via setCurrentProvider("custom").
  4. On every subsequent session.create, the desktop sent {model: "MiniMax-M3", provider: "custom"}. Backend's resolve_runtime_provider("custom") could not match custom_providers:tokenrouter from "custom", fell through to the OpenAI-compat fallback chain, hit https://generativelanguage.googleapis.com/v1beta, and Gemini returned 400 unexpected model name format for MiniMax-M3.
  5. The cb6b4127e refactor(desktop): make composer model picker sticky session state (June 16) makes this persistent: once the invalid value is in localStorage, refreshCurrentModel() bails (!force && $currentModel.get() short-circuit), so the desktop never re-syncs from the gateway until the user manually picks a model in the UI again.

backend.log evidence from a reproducer on Windows 11 / hermes-agent main @ fcdefb418:

[hermes] ⚠️  API call failed (attempt 1/3): BadRequestError [HTTP 400]
[hermes]    🔌 Provider: custom  Model: MiniMax-M3
[hermes]    🌐 Endpoint: https://generativelanguage.googleapis.com/v1beta
[hermes]    📝 Error: ... GenerateContentRequest.model: unexpected model name format

desktop Local Storage/leveldb evidence — two different shapes observed across reproducer runs (provider atom written verbatim by the buggy handler):

# Run A — pre-fix desktop sticky atom after a Telegram /model + picker round-trip:
hermes.desktop.composer.provider = "custom"
hermes.desktop.composer.model    = "MiniMax-M3"

# Run B — after the picker was re-opened and the model re-picked:
hermes.desktop.composer.provider = "custom:minimax m3"      # ← the canonical_custom_identity recovery path
hermes.desktop.composer.model    = "MiniMax-M3"

Either shape in localStorage causes the next session.create to send an unresolvable provider and surface as the same 400 error. The fix therefore rejects both "custom" and "custom:<x>" (any case, with surrounding whitespace).

Type of Change

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

Changes Made

  • apps/desktop/src/app/session/hooks/use-message-stream.ts — add isStickySafeProvider() exported helper; replace the unguarded setCurrentProvider(payload!.provider || '') with the helper-gated call. Net +38 / -1.
  • apps/desktop/src/app/session/hooks/use-message-stream.test.ts — new vitest suite, 6 cases covering the literal "custom" rejection, named custom_providers: acceptance, built-in provider acceptance, canonical custom:<name> acceptance, and the empty/whitespace/null/undefined guards.

How to Test

Automated (the regression guard):

cd apps/desktop
npx vitest run --environment jsdom src/app/session/hooks/use-message-stream.test.ts
# Expected: 6 passed

Manual (full reproducer on Windows + Telegram gateway + Desktop):

  1. Configure a named custom_providers: entry in ~/.hermes/config.yaml (e.g. tokenrouter pointing at a working endpoint with a valid TOKENROUTER_API_KEY).
  2. Start Hermes Desktop. Open a session on that custom provider; verify the picker shows tokenrouter:MiniMax-M3 (or equivalent).
  3. In Telegram, run /model grok-build-0.1 (any model switch via /model will do).
  4. Return to Desktop. Type any short message and send. Without the fix: error banner reads HTTP 400 ... GenerateContentRequest.model: unexpected model name format. With the fix: the chat starts normally on the previously-picked custom provider, exactly as it did before the Telegram switch.
  5. Restart Desktop. The composer picker still shows the named custom provider (not custom). Subsequent new chats continue to work.

Why this scope and not "fix the backend instead"

I considered two alternative fix sites, both deliberately not in this PR:

  1. Backend _session_info returns the user-facing name instead of agent.provider. Cleaner in principle, but _session_info is also the canonical source for resume, footer labels, and the model_config DB row; changing its shape touches PR fix(desktop): keep model runtime state per session #40163 / fix(desktop): keep model runtime state per session #43702 semantics and the _runtime_model_config write-back path. Out of scope for a one-file Desktop fix.
  2. Fix the canonical_custom_identity path to emit custom:<name> from _resolve_named_custom_runtime consistently (so the same shape lands in both agent.provider and _runtime_model_config recovery). This would let Desktop accept custom:<name> rather than reject it. Out of scope because it changes the wire format mid-stream and would break existing sticky-state values for every user already on the buggy code path.
  3. Desktop selectModel stops calling config.set when there is an active session. Removes the trigger but not the symptom — any other surface that pushes session.info would still brick the sticky atom.

The Desktop-side guard is the minimum-diff, minimum-blast-radius fix that closes the user-visible regression without changing the wire format. Follow-up PRs can move the trust boundary further upstream.

Checklist

  • Tests pass locally (vitest run --environment jsdom src/app/session/hooks/ → 74/74)
  • Conventional Commit format (fix(desktop): ...)
  • Pre-existing test failures in unrelated files (src/app/right-sidebar/files/ipc.test.ts etc.) verified not introduced by this change (git stash round-trip confirmed)
  • No Python / gateway / backend changes
  • No new dependencies
  • One logical change per PR
  • Reproduction steps + evidence captured above

Platforms tested

  • Windows 11 / WSL2, hermes-agent @ fcdefb418 (June 20), Electron desktop release built from same source.

Out of scope

  • The complementary design oversight that model.persist_switch_by_default: True lets /model on any surface (Telegram, CLI, Web UI) rewrite the global config.yaml model block and silently leak into other surfaces — that's a separate cross-surface stale-state class (tracked locally; will be filed as a separate issue).
  • A more comprehensive refresh-on-externally-changed-config flow that could replace the localStorage sticky model with a per-profile cached value — bigger refactor, not needed to close this bug.

…lling-class "custom" or "custom:<name>" slugs

The sticky  atom (localStorage key
hermes.desktop.composer.provider) is the next session.create's
provider field. Backend's session.info event pushes agent.provider,
which is the resolved billing class:

  - _resolve_named_custom_runtime writes bare "custom" for every
    named providers:/custom_providers: entry
  - _runtime_model_config recovers "custom:<name>" via
    canonical_custom_identity

Neither shape is a routable user-facing slug; either one written
to localStorage bricks every subsequent session.create round-trip
until the user manually re-picks a model in the picker (Desktop 400
"unexpected model name format", caused by the Gemini fallback
endpoint rejecting the upstream model's name).

Gate the setCurrentProvider call in use-message-stream.ts on a new
isStickySafeProvider() helper that rejects both shapes (any case,
any whitespace). Keeps the existing sticky atom value, which is the
right behavior until the desktop receives a real user-facing slug.

Vitest: 6/6 pass in src/app/session/hooks/use-message-stream.test.ts.
Full session/hooks suite: 74/74 pass. Pre-existing failures in 39
unrelated test files verified not introduced by this commit (git
stash round-trip). No Python / gateway / backend changes; minimal
diff (+92/-1 across 2 files).
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists labels Jun 21, 2026
@lost9999
lost9999 marked this pull request as ready for review June 21, 2026 13:13
@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) and removed comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as wrong-direction on current main. Canonical custom:<name> is now the durable, routable identity used by session persistence and resume; rejecting every custom:<name> payload in Desktop would discard the correct provider selection. Bare custom is already healed upstream through canonical_custom_identity before persistence. The right boundary is to emit canonical identity from the backend, not suppress valid identities in the client. Thanks for tracing the stale-state symptom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants