Skip to content

fix(desktop): keep model picker switches session-scoped - #59493

Closed
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/desktop-model-picker-session-scope-59480
Closed

fix(desktop): keep model picker switches session-scoped#59493
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/desktop-model-picker-session-scope-59480

Conversation

@izumi0uu

@izumi0uu izumi0uu commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the desktop model picker path for live sessions so selecting a model, including a MoA preset, stays session-scoped instead of falling through to the global config.yaml persist path.

The desktop hook was already sending session_id to config.set, but the gateway decides global persistence from the parsed model flags. Without --session, resolve_persist_behavior(False, False) can still persist the switch globally when model.persist_switch_by_default is enabled. This PR appends --session for active desktop sessions and adds coverage for both normal provider selections and MoA presets.

This intentionally does not change Telegram sticky-session migration or MoA fallback behavior; it closes the desktop-to-global bleed entry point described in the issue.

Related Issue

Fixes #59480

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • apps/desktop/src/app/session/hooks/use-model-controls.ts
    • Append --session when the desktop picker applies a model switch to an active session.
  • apps/desktop/src/app/session/hooks/use-model-controls.test.tsx
    • Assert active-session provider switches include --session.
    • Add a MoA preset regression test proving BeastMode --provider moa --session is sent.
  • apps/desktop/src/app/shell/model-menu-panel.tsx
    • Update the MoA preset comment to reflect the live-session scoped path.
  • apps/desktop/src/app/shell/model-menu-panel.test.tsx
    • Clarify the existing MoA picker routing assertion.

How to Test

  1. cd apps/desktop && npm run test:ui -- src/app/session/hooks/use-model-controls.test.tsx src/app/shell/model-menu-panel.test.tsx
  2. cd apps/desktop && npm run typecheck
  3. git diff --check

Additional source-level probe used during development:

BeastMode --provider moa
  parsed= BeastMode moa global= False session= False
  persist_global= True
BeastMode --provider moa --session
  parsed= BeastMode moa global= False session= True
  persist_global= False

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/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS Darwin 24.6.0 arm64

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

Desktop active-session picker calls already pass a session_id, but the gateway's model switch persistence is controlled by parsed model flags. Add --session so the shared parser keeps live-session selections, including MoA virtual provider presets, out of profile config.yaml.

Constraint: config.set model values are parsed by hermes_cli.model_switch before persistence is decided.

Rejected: backend special-case for desktop session_id | it would duplicate existing --session semantics and widen the gateway surface.

Confidence: high

Scope-risk: narrow

Directive: Keep desktop model picker active-session switches explicit with --session; do not rely on session_id alone for persistence.

Tested: npm run test:ui -- src/app/session/hooks/use-model-controls.test.tsx src/app/shell/model-menu-panel.test.tsx

Tested: npm run typecheck

Tested: git diff --check

Not-tested: full pytest suite; change is desktop TypeScript/UI routing only.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 6, 2026
@linfeng961

Copy link
Copy Markdown
Contributor

Thanks for the PR

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #62694. Your Desktop and MoA commit was cherry-picked onto current main with authorship preserved, then widened to every TUI picker path. Thank you for the focused fix.

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/*) P3 Low — cosmetic, nice to have 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]: Desktop MoA selection cascades to Telegram via global config persist + session state retains stale provider

4 participants