fix(cli): honor --resume in one-shot mode (#105892)
The -z exit path accepted --resume/-c in the parser but never forwarded args.resume: every resumed one-shot turn silently started a fresh session, so each wire request carried only [system, current user] and the model lost all prior context (reported against Ollama/custom OpenAI-compatible endpoints, but provider-independent). Normalize session args (latest/title/--continue/--in + cwd restore) via the chat path's _resolve_chat_session_args before the oneshot exit path takes over, then load the resumed transcript in _run_agent through the same contract the interactive CLI uses (compression-chain redirect, safe-resume guard, session_meta filtering) and continue the existing session id instead of creating a new one. An explicit --resume of an unknown session now fails loudly instead of starting fresh.
This commit is contained in:
@@ -140,6 +140,7 @@ def _run_and_exit_oneshot(
|
||||
toolsets: object = None,
|
||||
skills: object = None,
|
||||
usage_file: object = None,
|
||||
resume: object = None,
|
||||
) -> None:
|
||||
try:
|
||||
from hermes_cli.oneshot import run_oneshot
|
||||
@@ -151,6 +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,
|
||||
)
|
||||
except KeyboardInterrupt:
|
||||
rc = 130
|
||||
@@ -2879,6 +2881,10 @@ def _run_oneshot_from_args(args) -> None:
|
||||
Bypasses cli.py entirely; _run_and_exit_oneshot never returns.
|
||||
"""
|
||||
_confirm_startup_expensive_model_override(args)
|
||||
# -z honors --resume/-c/--in exactly like chat (#105892): normalize BEFORE the
|
||||
# oneshot exit path takes over, else the flags parse fine but silently do nothing
|
||||
# and the turn starts a fresh session (every wire request loses all history).
|
||||
_resolve_chat_session_args(args, use_tui=False)
|
||||
_run_and_exit_oneshot(
|
||||
args.oneshot,
|
||||
model=getattr(args, "model", None),
|
||||
@@ -2886,6 +2892,7 @@ def _run_oneshot_from_args(args) -> None:
|
||||
toolsets=getattr(args, "toolsets", None),
|
||||
skills=getattr(args, "skills", None),
|
||||
usage_file=getattr(args, "usage_file", None),
|
||||
resume=getattr(args, "resume", None),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -167,12 +167,14 @@ def run_oneshot(
|
||||
toolsets: object = None,
|
||||
skills: object = None,
|
||||
usage_file: Optional[str] = None,
|
||||
resume: Optional[str] = None,
|
||||
) -> int:
|
||||
"""Execute a single prompt and print only the final content block.
|
||||
|
||||
Model/provider fall back to ``HERMES_INFERENCE_MODEL`` and config.yaml. ``usage_file`` gets a
|
||||
JSON usage report even when the run fails. Returns the exit code; the caller owns process
|
||||
termination.
|
||||
JSON usage report even when the run fails. ``resume`` is a session id (already normalized by
|
||||
the CLI layer: latest/title/--continue resolution) whose transcript is loaded and continued
|
||||
by this turn. Returns the exit code; the caller owns process termination.
|
||||
"""
|
||||
# Silence every stdlib logger: AIAgent, tools and provider adapters log to stderr through the
|
||||
# root logger. File handlers from setup_logging() keep working (level-independent).
|
||||
@@ -220,6 +222,7 @@ def run_oneshot(
|
||||
toolsets=explicit_toolsets,
|
||||
use_config_toolsets=use_config_toolsets,
|
||||
skills=skills,
|
||||
resume=resume,
|
||||
)
|
||||
except BaseException as exc: # noqa: BLE001
|
||||
# Capture anything escaping the agent (OSError from prompt_toolkit on a non-TTY pipe,
|
||||
@@ -340,6 +343,28 @@ def _resolve_model_and_provider(cfg: dict, model: Optional[str], provider: Optio
|
||||
return choice
|
||||
|
||||
|
||||
def _load_resume_target(session_db, resume: Optional[str]) -> tuple[Optional[str], list]:
|
||||
"""Resolve ``resume`` to ``(session_id, conversation_history)`` for a oneshot turn.
|
||||
|
||||
Follows the same contract as the interactive CLI resume: compression-chain redirect via
|
||||
``resolve_resume_session_id``, safe-resume guard, model-projection history with
|
||||
``session_meta`` rows dropped. An unknown session raises (the user passed an explicit id;
|
||||
silently starting a fresh session is the resume-dropped failure mode this exists to fix —
|
||||
see #105892). An empty stored transcript keeps the turn as a fresh session.
|
||||
"""
|
||||
if not resume:
|
||||
return None, []
|
||||
if session_db is None:
|
||||
raise RuntimeError(f"cannot resume session {resume}: session store unavailable")
|
||||
resolved = session_db.resolve_resume_session_id(resume) or resume
|
||||
if not session_db.get_session(resolved):
|
||||
raise RuntimeError(f"session not found: {resume}")
|
||||
session_db.assert_resume_safe(resolved, tip_only=True)
|
||||
restored, _display = session_db.get_resume_conversations(resolved)
|
||||
history = [m for m in restored if m.get("role") != "session_meta"]
|
||||
return (resolved if history else None), history
|
||||
|
||||
|
||||
def _run_agent(
|
||||
prompt: str,
|
||||
model: Optional[str] = None,
|
||||
@@ -347,6 +372,7 @@ def _run_agent(
|
||||
toolsets: object = None,
|
||||
use_config_toolsets: bool = True,
|
||||
skills: object = None,
|
||||
resume: Optional[str] = None,
|
||||
) -> tuple[str, dict]:
|
||||
"""Build an AIAgent exactly like a normal CLI chat turn, run one conversation, and return
|
||||
``(final_response, run_result)``. Imports are local to keep CLI startup cheap."""
|
||||
@@ -382,6 +408,7 @@ def _run_agent(
|
||||
skills_prompt = _build_preloaded_skills_prompt(skills)
|
||||
|
||||
session_db = _create_session_db_for_oneshot()
|
||||
resume_sid, conversation_history = _load_resume_target(session_db, resume)
|
||||
# The try spans agent construction (not just ``chat``) so the store is always closed, even when
|
||||
# ``AIAgent(...)`` raises — the one-shot exit path hard-exits via os._exit and skips finalizers.
|
||||
agent = None
|
||||
@@ -397,6 +424,7 @@ def _run_agent(
|
||||
quiet_mode=True,
|
||||
platform="cli",
|
||||
session_db=session_db,
|
||||
session_id=resume_sid,
|
||||
credential_pool=runtime.get("credential_pool"),
|
||||
fallback_model=get_fallback_chain(cfg) or None,
|
||||
ephemeral_system_prompt=skills_prompt,
|
||||
@@ -410,7 +438,7 @@ def _run_agent(
|
||||
agent.stream_delta_callback = None
|
||||
agent.tool_gen_callback = None
|
||||
|
||||
result = agent.run_conversation(prompt)
|
||||
result = agent.run_conversation(prompt, conversation_history=conversation_history or None)
|
||||
return (result.get("final_response") or "", result)
|
||||
finally:
|
||||
_close_agent(agent, session_db)
|
||||
|
||||
124
tests/hermes_cli/test_oneshot_resume.py
Normal file
124
tests/hermes_cli/test_oneshot_resume.py
Normal file
@@ -0,0 +1,124 @@
|
||||
"""Tests for `hermes -z --resume <session>` (#105892).
|
||||
|
||||
The oneshot path used to accept ``--resume``/``-c`` in the parser but silently drop
|
||||
them: ``_run_oneshot_from_args`` ran before any session-arg normalization and never
|
||||
forwarded ``args.resume``, so every resumed one-shot turn started a FRESH session —
|
||||
the wire carried only ``[system, current user]`` and the model "forgot" everything.
|
||||
These tests pin the loader contract (chain redirect, unknown-session error,
|
||||
session_meta filtering, empty-session fresh start) and the resume kwarg wiring.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_state import SessionDB
|
||||
from hermes_cli.oneshot import _load_resume_target, run_oneshot
|
||||
|
||||
|
||||
def _db_with_session(tmp_path, sid, *, messages=()):
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session(session_id=sid, source="cli")
|
||||
for role, content in messages:
|
||||
db.append_message(sid, role, content=content)
|
||||
return db
|
||||
|
||||
|
||||
class TestLoadResumeTarget:
|
||||
def test_no_resume_is_a_noop(self, tmp_path):
|
||||
db = _db_with_session(tmp_path, "s1")
|
||||
try:
|
||||
assert _load_resume_target(db, None) == (None, [])
|
||||
assert _load_resume_target(db, "") == (None, [])
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_missing_store_raises_rather_than_silent_fresh(self):
|
||||
# An explicit --resume with no session store must fail loudly; starting a
|
||||
# fresh session here is exactly the history-dropping bug this fixes.
|
||||
with pytest.raises(RuntimeError, match="session store unavailable"):
|
||||
_load_resume_target(None, "s1")
|
||||
|
||||
def test_unknown_session_raises(self, tmp_path):
|
||||
db = _db_with_session(tmp_path, "s1")
|
||||
try:
|
||||
with pytest.raises(RuntimeError, match="session not found: missing-sid"):
|
||||
_load_resume_target(db, "missing-sid")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_returns_history_for_stored_session(self, tmp_path):
|
||||
db = _db_with_session(
|
||||
tmp_path, "s1",
|
||||
messages=[("user", "Remember the secret word ZEBRA42"),
|
||||
("assistant", "I've noted the secret word: ZEBRA42.")],
|
||||
)
|
||||
try:
|
||||
sid, history = _load_resume_target(db, "s1")
|
||||
assert sid == "s1"
|
||||
assert [m["role"] for m in history] == ["user", "assistant"]
|
||||
assert history[0]["content"] == "Remember the secret word ZEBRA42"
|
||||
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 = _load_resume_target(db, "s1")
|
||||
assert sid == "s1"
|
||||
assert [m["role"] for m in history] == ["user"]
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_empty_session_starts_fresh(self, tmp_path):
|
||||
# Chat's contract: a resumed session with no messages starts fresh (same session
|
||||
# id, no history rows to replay) — oneshot must not crash or resurrect the id.
|
||||
db = _db_with_session(tmp_path, "s1")
|
||||
try:
|
||||
assert _load_resume_target(db, "s1") == (None, [])
|
||||
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).
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session(session_id="parent", source="cli")
|
||||
db.create_session(session_id="child", source="cli", parent_session_id="parent")
|
||||
db.append_message("child", "user", content="hi")
|
||||
try:
|
||||
sid, history = _load_resume_target(db, "parent")
|
||||
assert sid == "child"
|
||||
assert [m["role"] for m in history] == ["user"]
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
class TestRunOneshotForwardsResume:
|
||||
def test_resume_kwarg_reaches_run_agent(self, monkeypatch):
|
||||
captured = {}
|
||||
|
||||
def _fake_run_agent(prompt, **kwargs):
|
||||
captured.update(kwargs, prompt=prompt)
|
||||
return "ok", {"final_response": "ok"}
|
||||
|
||||
monkeypatch.setattr("hermes_cli.oneshot._run_agent", _fake_run_agent)
|
||||
rc = run_oneshot("hello", model="m", provider="custom", resume="sess-1")
|
||||
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
|
||||
Reference in New Issue
Block a user