Skip to content

fix(gateway): default /model to session scope on messaging platforms - #61488

Closed
liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-61458-model-scope
Closed

fix(gateway): default /model to session scope on messaging platforms#61488
liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-61458-model-scope

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When users pick models via Telegram/Discord/Slack picker menus, the switch now defaults to session-scoped unless --global is explicitly provided. This prevents accidental global config changes from platform picker interactions, which cannot send --global/--session flags.

Previously, platform picker taps silently persisted to config.yaml because resolve_persist_behavior() fell back to model.persist_switch_by_default=True. This UX failure meant a user selecting a model from a Telegram menu unintentionally made a permanent global configuration change — the opposite of CLI users, who can explicitly opt into persistence with --global.

Related Issue

Fixes #61458

Type of Change

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

Changes Made

  • gateway/slash_commands.py: Added context-aware default for messaging platforms. If source.platform != Platform.LOCAL and no explicit --global/--session flags are present, treat as if --session was passed (session-scoped by default).
  • tests/gateway/test_model_picker_persist.py: Updated test_picker_tap_persists_by_default to test_picker_tap_is_session_scoped_by_default_on_messaging_platforms to reflect the new expected behavior — picker taps on messaging platforms now apply session overrides in-memory but do not persist to config.yaml.

How to Test

  1. Start a Hermes gateway with Telegram adapter enabled.
  2. In a Telegram DM, send /model to open the inline keyboard picker.
  3. Select a model (e.g., "gpt-5.5") by tapping it.
  4. Observe that:
    • The model switch works (confirmation message shows the new model).
    • The session override is applied in-memory (subsequent messages use the new model).
    • config.yaml is NOT modified (the default model remains unchanged).
  5. Test with explicit flags:
    • /model gpt-5.5 --session → session-scoped (same as default on messaging platforms).
    • /model gpt-5.5 --global → persists to config.yaml (explicit opt-in).
  6. Run the updated test: pytest tests/gateway/test_model_picker_persist.py -v — all three tests pass.

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/gateway/test_model_picker_persist.py -v and all tests pass
  • I've added tests for my changes (updated existing test to reflect new behavior)
  • I've tested on my platform: macOS (verified test suite passes)

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) — affects all messaging platforms equally (Telegram, Discord, Slack, etc.)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When users pick models via Telegram/Discord/Slack menus (which cannot
send --global/--session flags), the switch now defaults to session-scoped
unless --global is explicitly provided. This prevents accidental global
config changes from platform picker interactions.

Fixes NousResearch#61458
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists labels Jul 9, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: LGTM

What the PR Does

Default /model picker usage on messaging platforms (Telegram, Discord, Slack, etc.) to session scope, preventing accidental global config changes when users pick models via platform menus.

Assessment

  • Correctness: The heuristic correctly identifies messaging platforms by checking source.platform is not None and source.platform != Platform.LOCAL. The session-scoped override is applied in-memory while leaving config.yaml untouched.
  • Testing: New test test_picker_tap_is_session_scoped_by_default_on_messaging_platforms verifies both that the session override IS set and that config.yaml remains unchanged.
  • Security: No security concerns. This reduces accidental config changes — a safety improvement.

Note: PR was previously unreviewed.


Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Comment

Overview

  • Defaults /model to session scope on messaging platforms
  • Small fix (+41/-13)

Looks Good

  • Clean, targeted fix
  • No security concerns

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused gateway fix. Current main still resolves a no-flag /model command through resolve_persist_behavior(False, False) (gateway/slash_commands.py:1420-1430), whose default is persistence (hermes_cli/model_switch.py:416-443). That value controls both the picker config write (gateway/slash_commands.py:1647-1674) and typed-command config write (gateway/slash_commands.py:1893-1923).

The PR changes the common decision before those paths, preserves explicit --global, and adds regression assertions that the in-session override is set while config.yaml remains unchanged. The target region has not changed since the PR base (git log -L 1420,1432:gateway/slash_commands.py), so salvage should be mechanical.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 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/gateway Gateway runner, session dispatch, delivery 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /model command defaults to global config in messaging gateways

4 participants