Skip to content

fix(model): default --provider switches to session-only persistence - #58371

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-58290-model-provider-persist
Closed

fix(model): default --provider switches to session-only persistence#58371
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-58290-model-provider-persist

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When /model is called with --provider but without --global or --session, the switch now defaults to session-only instead of persisting to config.yaml.

Before this change, /model sonnet --provider anthropic would overwrite model.default and model.provider in config.yaml, which is unexpected when the user is just trying a different provider for the current conversation. The --global flag still forces persistence when explicitly desired.

Related Issue

Fixes #58290

Type of Change

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

Changes Made

  • hermes_cli/model_switch.py: Added explicit_provider parameter to resolve_persist_behavior(). When a provider is explicitly specified without --global/--session, returns False (session-only).
  • gateway/slash_commands.py: Pass explicit_provider to resolve_persist_behavior() in the gateway /model handler.
  • tui_gateway/server.py: Pass explicit_provider to resolve_persist_behavior() in the TUI /model handler.
  • cli.py: Pass explicit_provider to resolve_persist_behavior() in the CLI /model handler.
  • tests/hermes_cli/test_model_switch_persist_default.py: Added 4 test cases covering --provider flag behavior with and without --global/--session.

How to Test

  1. Run pytest tests/hermes_cli/test_model_switch_persist_default.py -v — all 17 tests should pass.
  2. In a gateway session, send /model sonnet --provider anthropic — the model should switch for this session only, and config.yaml should remain unchanged.
  3. Send /model sonnet --provider anthropic --global — this should persist to config.yaml as before.
  4. Send /model sonnet (no --provider) — this should persist to config.yaml (existing behavior unchanged).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/hermes_cli/test_model_switch_persist_default.py -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When /model is called with --provider but without --global or --session,
the switch now defaults to session-only instead of persisting to
config.yaml. Provider switches are typically exploratory — the user is
trying a different backend for this conversation, not reconfiguring the
default. --global can still force persist when desired.

This addresses a regression from fad4b40 where /model switched to
persist-by-default, causing /model xxx --provider xxx to overwrite the
global config when the user only intended a temporary switch.

Fixes NousResearch#58290
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/config Config system, migrations, profiles labels Jul 4, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the behavior across CLI, gateway, and TUI.

Problems

  • Current main intentionally treats /model as persist-by-default across all three surfaces: fad4b40d9d38573641c7f5de29fa4fc6f66e6d16 introduced --session as the opt-out, and hermes_cli/model_switch.py:416-443 still implements that shared contract. This PR makes an explicit provider a new exception, so the semantic change needs maintainer confirmation rather than being a mechanical bug fix.
  • The new tests cover the resolver only. They do not prove the modified callers in cli.py:8069, gateway/slash_commands.py:1453, and tui_gateway/server.py:2882-2886 pass the provider value through.

Suggested changes

  • Confirm the intended persistence contract for provider switches; existing --session and model.persist_switch_by_default already support explicit session-only operation.
  • If the exception is approved, add caller-level coverage for CLI, gateway, and TUI/desktop, including --global precedence.

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 area/sessions Session lifecycle, resume, persistence, history labels Jul 15, 2026
teknium1 added a commit that referenced this pull request Jul 20, 2026
Flip the resolve_persist_behavior() fallback from persist-to-config to
session-only. A plain /model <name> (typed or via any picker — CLI,
TUI/Desktop, gateway) now affects only the current session; --global
persists explicitly, and model.persist_switch_by_default: true restores
the old opt-out behavior for users who want switches to stick.

This is the root cause behind the recurring 'session switch applied
globally' bug class (#61458, #63083, #58290, #61190): every surface
funnels its no-flag default through this one function, so per-surface
patches kept missing paths. Fixing the default fixes all surfaces at
once: CLI typed + picker, TUI/Desktop config.set + slash, gateway typed
+ inline picker.

Builds on liuhao1024's #58371 (--provider session scoping, cherry-picked
as the previous commit) and supersedes the per-surface #61488.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #67946 — your commit from #58371 was cherry-picked onto current main with your authorship preserved in git log (rebase merge). The session-only default you built for --provider switches is now the default for ALL /model switches on every surface, so this PR's gateway-specific half is covered by the same root fix. Thanks for the clean fix and the tests — they shipped with your name on them.

@teknium1 teknium1 closed this Jul 20, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Flip the resolve_persist_behavior() fallback from persist-to-config to
session-only. A plain /model <name> (typed or via any picker — CLI,
TUI/Desktop, gateway) now affects only the current session; --global
persists explicitly, and model.persist_switch_by_default: true restores
the old opt-out behavior for users who want switches to stick.

This is the root cause behind the recurring 'session switch applied
globally' bug class (NousResearch#61458, NousResearch#63083, NousResearch#58290, NousResearch#61190): every surface
funnels its no-flag default through this one function, so per-surface
patches kept missing paths. Fixing the default fixes all surfaces at
once: CLI typed + picker, TUI/Desktop config.set + slash, gateway typed
+ inline picker.

Builds on liuhao1024's NousResearch#58371 (--provider session scoping, cherry-picked
as the previous commit) and supersedes the per-surface NousResearch#61488.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists 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.

[Bug]: using /model xxx --provider xxx will automaticlly change the config.yaml, and I never input the --global

3 participants