mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
feat(tui): reach the model picker without wrecking your draft, and switch mid-turn (#74756)
* feat(tui): Ctrl+O opens the model picker without clearing your draft Reaching the model picker meant typing /model, which forces you to wipe whatever you'd already drafted. Bind Ctrl+O to open the same picker overlay directly, leaving the composer untouched. Ctrl+O is added to the textInput pass-through allowlist so the composer doesn't swallow it, mirroring the existing Ctrl+X session-switcher path. * feat(tui): apply a mid-turn model switch at the next turn instead of rejecting it Picking a model while a turn was streaming hit a 4009 'session busy' reject: switch_model() mutates the agent's model/provider/base_url/client in place and the worker thread reads those every iteration. Now config.set queues the pick in session[pending_model_switch] and _apply_pending_model_switch applies it on the turn thread at the next turn start, before any model call — no race, no interrupt, no waiting on the client rebuild. The TUI paints the pick optimistically and notes '(applies next turn)'.
This commit is contained in:
parent
dd4eadcf79
commit
f27d45e288
8 changed files with 139 additions and 29 deletions
|
|
@ -9598,14 +9598,16 @@ def test_respond_unpacks_sid_tuple_correctly():
|
|||
# /model switch and other agent-mutating commands must reject while the
|
||||
# session is running. agent.switch_model() mutates self.model, self.provider,
|
||||
# self.base_url, self.client etc. in place — the worker thread running
|
||||
# agent.run_conversation is reading those on every iteration. Same class of
|
||||
# bug as the session.undo / session.compress mid-run silent-drop; same fix
|
||||
# pattern: reject with 4009 while running.
|
||||
# agent.run_conversation is reading those on every iteration. So a mid-turn
|
||||
# config.set model must NOT switch in place; instead it queues the pick
|
||||
# (session["pending_model_switch"]) and _apply_pending_model_switch applies it
|
||||
# on the turn thread at the next turn start, where nothing is in flight.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_config_set_model_rejects_while_running(monkeypatch):
|
||||
"""/model via config.set must reject during an in-flight turn."""
|
||||
def test_config_set_model_defers_while_running(monkeypatch):
|
||||
"""/model via config.set queues the pick during an in-flight turn instead
|
||||
of rejecting or racing the worker thread."""
|
||||
seen = {"called": False}
|
||||
|
||||
def _fake_apply(sid, session, raw, **_kwargs):
|
||||
|
|
@ -9627,17 +9629,47 @@ def test_config_set_model_rejects_while_running(monkeypatch):
|
|||
},
|
||||
}
|
||||
)
|
||||
assert resp.get("error")
|
||||
assert resp["error"]["code"] == 4009
|
||||
assert "session busy" in resp["error"]["message"]
|
||||
assert not resp.get("error")
|
||||
result = resp["result"]
|
||||
assert result["deferred"] is True
|
||||
assert result["value"] == "anthropic/claude-sonnet-4.6"
|
||||
assert not seen["called"], (
|
||||
"_apply_model_switch was called mid-turn — would race with "
|
||||
"the worker thread reading agent.model / agent.client"
|
||||
"_apply_model_switch ran mid-turn — would race the worker thread "
|
||||
"reading agent.model / agent.client; it must defer to turn start"
|
||||
)
|
||||
pending = server._sessions["sid"].get("pending_model_switch")
|
||||
assert pending and pending["raw"] == "anthropic/claude-sonnet-4.6"
|
||||
finally:
|
||||
server._sessions.pop("sid", None)
|
||||
|
||||
|
||||
def test_apply_pending_model_switch_runs_queued_pick(monkeypatch):
|
||||
"""The queued pick is consumed once, on the turn thread, via
|
||||
_apply_model_switch — and cleared so it can't re-fire next turn."""
|
||||
calls = []
|
||||
|
||||
def _fake_apply(sid, session, raw, **kwargs):
|
||||
calls.append(raw)
|
||||
return {"value": raw, "warning": "", "confirm_required": False}
|
||||
|
||||
monkeypatch.setattr(server, "_apply_model_switch", _fake_apply)
|
||||
|
||||
session = _session(running=False)
|
||||
session["agent"] = object()
|
||||
session["pending_model_switch"] = {
|
||||
"raw": "anthropic/claude-sonnet-4.6",
|
||||
"confirm_expensive_model": False,
|
||||
}
|
||||
|
||||
server._apply_pending_model_switch("sid", session)
|
||||
assert calls == ["anthropic/claude-sonnet-4.6"]
|
||||
assert "pending_model_switch" not in session
|
||||
|
||||
# Idempotent: a second turn start with nothing queued is a no-op.
|
||||
server._apply_pending_model_switch("sid", session)
|
||||
assert calls == ["anthropic/claude-sonnet-4.6"]
|
||||
|
||||
|
||||
def test_config_set_model_allowed_when_idle(monkeypatch):
|
||||
"""Regression guard: idle sessions can still switch models."""
|
||||
seen = {"called": False}
|
||||
|
|
|
|||
|
|
@ -4294,6 +4294,44 @@ def _sync_agent_model_with_config(sid: str, session: dict) -> None:
|
|||
)
|
||||
|
||||
|
||||
def _apply_pending_model_switch(sid: str, session: dict) -> None:
|
||||
"""Apply a model switch queued while a turn was running.
|
||||
|
||||
``config.set model`` on a busy session doesn't mutate the live agent (the
|
||||
worker thread is reading model/client mid-request); it stashes the pick in
|
||||
``session["pending_model_switch"]``. This runs on the TURN thread at turn
|
||||
start — before the first model call, nothing in flight — so the in-place
|
||||
swap (client rebuild, the slow part) is safe here. A failed switch keeps
|
||||
the current model and never blocks the turn, matching
|
||||
``_sync_agent_model_with_config``.
|
||||
"""
|
||||
pending = session.pop("pending_model_switch", None)
|
||||
if not pending or session.get("agent") is None:
|
||||
return
|
||||
try:
|
||||
result = _apply_model_switch(
|
||||
sid,
|
||||
session,
|
||||
pending["raw"],
|
||||
confirm_expensive_model=bool(pending.get("confirm_expensive_model")),
|
||||
)
|
||||
# A queued pick is a deliberate user action; honour the expensive-model
|
||||
# confirm by NOT applying it silently — surface the warning and drop the
|
||||
# switch rather than spend on a pricey model the user never confirmed.
|
||||
if result.get("confirm_required"):
|
||||
_emit(
|
||||
"error",
|
||||
sid,
|
||||
{"message": result.get("confirm_message") or result.get("warning") or ""},
|
||||
)
|
||||
except Exception as e:
|
||||
_emit(
|
||||
"error",
|
||||
sid,
|
||||
{"message": f"Could not switch model: {e}"},
|
||||
)
|
||||
|
||||
|
||||
class CompressionLockHeld(Exception):
|
||||
"""Raised by _compress_session_history when compression skipped due
|
||||
to a concurrent lock on the session's compression_locks row."""
|
||||
|
|
@ -8989,6 +9027,11 @@ def _run_prompt_submit(
|
|||
# (#29923 review defect). Any config.yaml change is adopted on
|
||||
# the NEXT turn, after the finally-restore below.
|
||||
if not one_turn_restore:
|
||||
# A model picked mid-turn was queued (not applied in-place) —
|
||||
# apply it now, on the turn thread before the first model call,
|
||||
# so this turn runs on the model the user chose. Runs before the
|
||||
# config sync so an explicit pick wins over a config.yaml change.
|
||||
_apply_pending_model_switch(sid, session)
|
||||
_sync_agent_model_with_config(sid, session)
|
||||
cwd = _session_cwd(session)
|
||||
_register_session_cwd(session)
|
||||
|
|
@ -9956,22 +9999,41 @@ def _(rid, params: dict) -> dict:
|
|||
if not value:
|
||||
return _err(rid, 4002, "model value required")
|
||||
if session:
|
||||
# Reject during an in-flight turn. agent.switch_model()
|
||||
# mutates self.model / self.provider / self.base_url /
|
||||
# self.client in place; the worker thread running
|
||||
# agent.run_conversation is reading those on every
|
||||
# iteration. A mid-turn swap can send an HTTP request
|
||||
# with the new base_url but old model (or vice versa),
|
||||
# producing 400/404s the user never asked for. Parity
|
||||
# with the gateway's running-agent /model guard.
|
||||
if session.get("running"):
|
||||
return _err(
|
||||
rid,
|
||||
4009,
|
||||
"session busy — /interrupt the current turn before switching models",
|
||||
)
|
||||
from hermes_cli.model_switch import parse_model_switch_args
|
||||
|
||||
# A live swap can't run in-place while a turn streams:
|
||||
# agent.switch_model() mutates self.model / self.provider /
|
||||
# self.base_url / self.client, and the worker thread running
|
||||
# agent.run_conversation reads those every iteration — a
|
||||
# mid-turn swap can fire an HTTP request with the new base_url
|
||||
# but old model (400/404s). So instead of rejecting the pick
|
||||
# (the old 4009), stash it and apply it at the NEXT turn start
|
||||
# (_apply_pending_model_switch), where nothing is in flight.
|
||||
# The user gets to pick, keep typing, and send the next turn on
|
||||
# the new model without waiting for the swap or interrupting.
|
||||
if session.get("running"):
|
||||
try:
|
||||
pending_model = parse_model_switch_args(value).model_input
|
||||
except Exception:
|
||||
pending_model = str(value)
|
||||
session["pending_model_switch"] = {
|
||||
"raw": value,
|
||||
"confirm_expensive_model": bool(
|
||||
params.get("confirm_expensive_model", False)
|
||||
),
|
||||
}
|
||||
return _ok(
|
||||
rid,
|
||||
{
|
||||
"key": key,
|
||||
"value": pending_model,
|
||||
"warning": "",
|
||||
"confirm_required": False,
|
||||
"confirm_message": "",
|
||||
"scope": "session",
|
||||
"deferred": True,
|
||||
},
|
||||
)
|
||||
parsed_flags = parse_model_switch_args(value)
|
||||
explicit_provider = parsed_flags.explicit_provider
|
||||
if session.get("agent") is None and not explicit_provider.strip():
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ describe('shouldPassThroughToGlobalHandler', () => {
|
|||
it('always passes through non-voice global control keys', () => {
|
||||
expect(shouldPassThroughToGlobalHandler('c', key({ ctrl: true }))).toBe(true)
|
||||
expect(shouldPassThroughToGlobalHandler('x', key({ ctrl: true }))).toBe(true)
|
||||
expect(shouldPassThroughToGlobalHandler('o', key({ ctrl: true }))).toBe(true)
|
||||
expect(shouldPassThroughToGlobalHandler('', key({ escape: true }))).toBe(true)
|
||||
expect(shouldPassThroughToGlobalHandler('', key({ tab: true }))).toBe(true)
|
||||
expect(shouldPassThroughToGlobalHandler('', key({ pageUp: true }))).toBe(true)
|
||||
|
|
|
|||
|
|
@ -104,10 +104,11 @@ export const sessionCommands: SlashCommand[] = [
|
|||
help: 'change or show model',
|
||||
name: 'model',
|
||||
run: (arg, ctx) => {
|
||||
if (ctx.session.guardBusySessionSwitch('change models')) {
|
||||
return
|
||||
}
|
||||
|
||||
// No busy guard here (unlike session switching). A model change is a
|
||||
// session-scoped config.set: idle it switches immediately; mid-turn the
|
||||
// gateway QUEUES it and applies it at the next turn start (returning
|
||||
// deferred:true) instead of rejecting. Either way the pick sticks without
|
||||
// interrupting the stream or waiting on the swap.
|
||||
if (!arg.trim()) {
|
||||
return patchOverlayState({ modelPicker: true })
|
||||
}
|
||||
|
|
@ -145,7 +146,7 @@ export const sessionCommands: SlashCommand[] = [
|
|||
return ctx.transcript.sys('error: invalid response: model switch')
|
||||
}
|
||||
|
||||
ctx.transcript.sys(`model → ${r.value}`)
|
||||
ctx.transcript.sys(r.deferred ? `model → ${r.value} (applies next turn)` : `model → ${r.value}`)
|
||||
ctx.local.maybeWarn(r)
|
||||
|
||||
patchUiState(state => ({
|
||||
|
|
|
|||
|
|
@ -585,6 +585,15 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult {
|
|||
return patchOverlayState({ sessions: true })
|
||||
}
|
||||
|
||||
// Ctrl+O opens the model picker without disturbing a typed draft — the
|
||||
// same overlay `/model` opens, but reachable without clearing what you've
|
||||
// typed to run the command. Works mid-stream: picking a model writes the
|
||||
// session model (config.set), which the next turn reads while the in-flight
|
||||
// turn keeps streaming.
|
||||
if (isCtrl(key, ch, 'o')) {
|
||||
return patchOverlayState({ modelPicker: true })
|
||||
}
|
||||
|
||||
if (key.ctrl && ch.toLowerCase() === 'c') {
|
||||
if (live.busy && live.sid) {
|
||||
return turnController.interruptTurn({
|
||||
|
|
|
|||
|
|
@ -1540,6 +1540,7 @@ export const shouldPassThroughToGlobalHandler = (
|
|||
): boolean =>
|
||||
(key.ctrl && input === 'c') ||
|
||||
(key.ctrl && input === 'x') ||
|
||||
(key.ctrl && input === 'o') ||
|
||||
key.tab ||
|
||||
(key.shift && key.tab) ||
|
||||
key.pageUp ||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ export const HOTKEYS: [string, string][] = [
|
|||
['Tab', 'apply completion'],
|
||||
['↑/↓', 'completions / queue edit / history'],
|
||||
['Ctrl+X', 'open live session switcher (deletes queued message while editing)'],
|
||||
['Ctrl+O', 'open model picker (keeps your draft; applies to next turn mid-stream)'],
|
||||
[action + '+A/E', 'home / end of line'],
|
||||
[action + '+Z / ' + action + '+Y', 'undo / redo input edits'],
|
||||
[action + '+W', 'delete word'],
|
||||
|
|
|
|||
|
|
@ -144,6 +144,9 @@ export interface ConfigSetResponse {
|
|||
confirm_message?: string
|
||||
confirm_required?: boolean
|
||||
credential_warning?: string
|
||||
// A model pick made mid-turn is queued and applied at the next turn start,
|
||||
// not live yet — the handler says "next turn" instead of "model → X".
|
||||
deferred?: boolean
|
||||
history_reset?: boolean
|
||||
info?: SessionInfo
|
||||
value?: string
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue