ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 12 Packages Projects Releases Wiki Activity

refactor(cli): one stored_session_route for interactive and one-shot resume

Browse Source 
_apply_stored_session_runtime was a line-for-line copy of the first half of
_restore_session_model (stored-model guard, session_gateway_runtime, bare-custom heal,
model/provider-changed check). Extract that pure decision into
cli_model_switch_mixin.stored_session_route and have both resume paths call it; the
one-shot keeps only the _ModelChoice mapping and the drop-ambient-key rule.

main.py stops re-normalising `resume` — _resolve_chat_session_args already did.
Tests trimmed from 20 to 13: near-duplicate unit tests of the private helpers go, the
end-to-end _run_agent contracts (stored runtime + reopen; explicit --model wins) and the
empty-session-keeps-id case stay.
This commit is contained in:
kshitijk4poor
2026-09-09 11:44:01 +05:30
committed by kshitij
parent 8aa773af89
commit 32273b8118
4 changed files with 37 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/cli_model_switch_mixin.py
+23 -14
View File
@@ -28,6 +28,25 @@ def _runtime_fields(cli) -> dict:
return {key: getattr(cli, key, None) for key in _RUNTIME_FIELDS}
def stored_session_route(session_meta, *, current_model, current_provider):
"""The route a resumed session should run on, or ``None`` when the stored one is absent or
already current. Returns ``(model, provider, base_url, api_mode, provider_changed)``; the
canonical row reader is ``SessionDB.session_gateway_runtime`` (``model_config.gateway_runtime``,
else the TUI's top-level keys). Bare ``custom`` is healed because the CLI resolve path
hard-fails on it (the TUI gateway keeps it when a base_url exists)."""
stored_model = str((session_meta or {}).get("model") or "").strip()
if not stored_model:
return None
from hermes_state import SessionDB as _SessionDB
runtime = _SessionDB.session_gateway_runtime(session_meta)
base_url = runtime.get("base_url") or None
provider = _heal_bare_custom_provider(runtime.get("provider") or None, base_url=base_url, model=stored_model)
provider_changed = bool(provider) and provider != current_provider
if stored_model == current_model and not provider_changed:
return None
return stored_model, provider, base_url, (runtime.get("api_mode") or None), provider_changed
def _heal_bare_custom_provider(provider, *, base_url, model):
"""Bare ``custom`` is a billing class, not a routable identity: persisting/restoring it makes a
later resume hard-fail once the config default leaves the custom endpoint. Recover the durable
@@ -348,22 +367,12 @@ class CLIModelSwitchMixin:
so the session still opens (the first turn surfaces the auth error).
"""
from cli import logger
stored_model = (session_meta or {}).get("model")
if not stored_model or getattr(self, "_explicit_model_override", False):
if getattr(self, "_explicit_model_override", False):
return
# Canonical row reader: model_config.gateway_runtime, else the TUI's top-level keys.
from hermes_state import SessionDB as _SessionDB
_stored_runtime = _SessionDB.session_gateway_runtime(session_meta)
stored_base_url = _stored_runtime.get("base_url") or None
stored_api_mode = _stored_runtime.get("api_mode") or None
# Stricter than the TUI gateway's recovery (which keeps bare "custom" when a
# base_url exists) — the CLI's resolve path would hard-fail on it.
stored_provider = _heal_bare_custom_provider(
_stored_runtime.get("provider") or None, base_url=stored_base_url, model=stored_model)
model_changed = stored_model != self.model
provider_changed = bool(stored_provider) and stored_provider != self.provider
if not model_changed and not provider_changed:
route = stored_session_route(session_meta, current_model=self.model, current_provider=self.provider)
if route is None:
return
stored_model, stored_provider, stored_base_url, stored_api_mode, provider_changed = route
self.model = stored_model
if stored_provider:
self.provider = stored_provider
hermes_cli/main.py
+1 -1
View File
@@ -152,7 +152,7 @@ def _run_and_exit_oneshot(
toolsets=toolsets,
skills=skills,
usage_file=usage_file,
resume=resume if isinstance(resume, str) and resume.strip() else None,
resume=resume,
)
except KeyboardInterrupt:
rc = 130
hermes_cli/oneshot.py
+13 -24
View File
@@ -383,36 +383,25 @@ def _load_resume_target(session_db, resume: Optional[str]) -> tuple[Optional[str
def _apply_stored_session_runtime(
choice: _ModelChoice, session_meta: Optional[dict], *, explicit_model: bool,
) -> _ModelChoice:
"""Run a resumed one-shot on the session's stored runtime, not the ambient config.
Same contract as the interactive ``_restore_session_model`` (cli_model_switch_mixin): no
stored model, or an explicit ``--model``, keeps the resolved ambient choice; otherwise the
stored model/provider/base_url/api_mode replace it (a resumed transcript must not be sent
to whatever the ambient config happens to point at). A changed provider drops the resolved
``api_key`` — it belongs to the ambient/alias endpoint, and api_key is never persisted to
the session DB, so runtime resolution re-fetches credentials for the restored provider.
No-op when the stored route already matches the resolved one.
"""
stored_model = str((session_meta or {}).get("model") or "").strip()
if not stored_model or explicit_model:
"""Run a resumed one-shot on the session's stored runtime, not the ambient config — the same
contract as the interactive ``_restore_session_model``, via the shared ``stored_session_route``.
An explicit ``--model`` keeps the ambient choice. A changed provider drops the resolved
``api_key``: it belongs to the ambient endpoint and is never persisted, so runtime resolution
re-fetches credentials for the restored provider."""
if explicit_model:
return choice
from hermes_state import SessionDB as _SessionDB
from hermes_cli.cli_model_switch_mixin import _heal_bare_custom_provider
from hermes_cli.cli_model_switch_mixin import stored_session_route
stored_runtime = _SessionDB.session_gateway_runtime(session_meta)
stored_base_url = stored_runtime.get("base_url") or None
# Stricter than the TUI gateway's recovery — the CLI's resolve path hard-fails on bare "custom".
stored_provider = _heal_bare_custom_provider(
stored_runtime.get("provider") or None, base_url=stored_base_url, model=stored_model)
if stored_model == choice.model and (not stored_provider or stored_provider == choice.provider):
route = stored_session_route(session_meta, current_model=choice.model, current_provider=choice.provider)
if route is None:
return choice
choice.model = stored_model
if stored_provider and stored_provider != choice.provider:
choice.model, stored_provider, stored_base_url, stored_api_mode, provider_changed = route
if provider_changed:
choice.provider = stored_provider
choice.base_url = stored_base_url
choice.api_key = None
if stored_runtime.get("api_mode"):
choice.api_mode = str(stored_runtime["api_mode"])
if stored_api_mode:
choice.api_mode = str(stored_api_mode)
return choice
tests/hermes_cli/test_oneshot_resume.py
-73
View File
@@ -69,18 +69,6 @@ class TestLoadResumeTarget:
finally:
db.close()
def test_session_meta_rows_are_dropped(self, tmp_path):
db = _db_with_session(
tmp_path, "s1",
messages=[("session_meta", "model switch"), ("user", "hello")],
)
try:
sid, history, _meta = _load_resume_target(db, "s1")
assert sid == "s1"
assert [m["role"] for m in history] == ["user"]
finally:
db.close()
def test_empty_session_keeps_resolved_id(self, tmp_path):
# Chat's contract: a resumed session with no messages starts fresh (no rows to
# replay) but the turn is recorded under the SELECTED id. Dropping the id here
@@ -95,20 +83,6 @@ class TestLoadResumeTarget:
finally:
db.close()
def test_empty_compression_head_without_rows_keeps_head_id(self, tmp_path):
# A chain head with no descendant rows must not fall through to a fresh id either:
# the turn stays anchored to the resolved head (redirect only happens when the
# descendant actually holds messages).
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session(session_id="head", source="cli")
db.create_session(session_id="child", source="cli", parent_session_id="head")
try:
sid, history, _meta = _load_resume_target(db, "head")
assert sid == "head"
assert history == []
finally:
db.close()
def test_compression_chain_redirects_to_child_with_messages(self, tmp_path):
# Compression ends a session and forks a child that holds the rows; the loader
# must land on the child, not the empty parent (resolve_resume_session_id).
@@ -140,19 +114,6 @@ class TestLoadResumeTarget:
finally:
db.close()
def test_reopen_failure_is_best_effort(self, tmp_path, monkeypatch):
db = _db_with_session(tmp_path, "s1", messages=[("user", "hi")])
try:
def _boom(_sid):
raise RuntimeError("reopen unavailable")
monkeypatch.setattr(db, "reopen_session", _boom)
sid, history, _meta = _load_resume_target(db, "s1")
assert sid == "s1"
assert [m["role"] for m in history] == ["user"]
finally:
db.close()
class TestApplyStoredSessionRuntime:
"""A resumed one-shot must run on the session's stored runtime, not the ambient
config (review on #105957): with ambient ``openrouter/ambient-model`` and a stored
@@ -196,14 +157,6 @@ class TestApplyStoredSessionRuntime:
assert choice.provider == "openrouter"
assert choice.api_key is self._AMBIENT_KEY
def test_no_stored_model_keeps_ambient_choice(self, tmp_path):
meta = self._stored_db(tmp_path, model=None)
choice = _apply_stored_session_runtime(
_ModelChoice("ambient-model", "openrouter", api_key=self._AMBIENT_KEY), meta, explicit_model=False)
assert choice.model == "ambient-model"
assert choice.provider == "openrouter"
assert choice.api_key is self._AMBIENT_KEY
def test_matching_route_is_noop_and_keeps_credentials(self, tmp_path):
meta = self._stored_db(tmp_path, route={"provider": "openrouter"})
choice = _apply_stored_session_runtime(
@@ -212,20 +165,6 @@ class TestApplyStoredSessionRuntime:
assert choice.api_key is self._AMBIENT_KEY
assert choice.api_mode is None
def test_none_meta_is_a_noop(self):
choice = _ModelChoice("ambient-model", "openrouter")
assert _apply_stored_session_runtime(choice, None, explicit_model=False) == choice
def test_bare_custom_provider_is_healed_or_dropped(self, tmp_path):
# Bare "custom" persisted by older builds is not a routable identity; the CLI's
# resolve path would hard-fail on it, so it is healed via the base_url or dropped.
meta = self._stored_db(tmp_path, route={"provider": "custom"})
choice = _apply_stored_session_runtime(
_ModelChoice("ambient-model", "openrouter"), meta, explicit_model=False)
assert choice.model == "stored-model"
assert choice.provider in (None, "openrouter") or choice.provider.startswith("custom:")
class TestRunAgentResumeRuntime:
"""End-to-end wiring: ``_run_agent`` must hand AIAgent the session's stored runtime
and a reopened session row (both regressions from the review on #105957)."""
@@ -327,15 +266,3 @@ class TestRunOneshotForwardsResume:
assert rc == 0
assert captured["prompt"] == "hello"
assert captured["resume"] == "sess-1"
def test_no_resume_forwards_none(self, monkeypatch):
captured = {}
def _fake_run_agent(prompt, **kwargs):
captured.update(kwargs)
return "ok", {"final_response": "ok"}
monkeypatch.setattr("hermes_cli.oneshot._run_agent", _fake_run_agent)
rc = run_oneshot("hello", model="m", provider="custom")
assert rc == 0
assert captured.get("resume") is None