fix(cli): honor --resume/--continue in oneshot (-z) — hydrate and chain the session - #57859
nathansmithopenclaw-alt wants to merge 1 commit into
Conversation
hermes -z parsed --resume but silently dropped it: both dispatch sites called run_oneshot() without the flag, and the agent was built with no session id and no history — every one-shot turn was stateless. Now: - run_oneshot(resume=...) loads the session transcript as conversation_history (walking compression chains via resolve_resume_session_id, dropping session_meta rows) and pins the agent to the SAME session id, mirroring the interactive resume path. - Create-on-first-use: an id Hermes hasn't seen is used as-is with no history, so scripted callers (gateways, cron workers) can mint stable session keys up front and pass them on every turn — no output parsing. - --continue resolves by name / most-recent exactly like interactive chat (_resolve_oneshot_resume). - Message flush dedup is inherited: history dicts seeded via conversation_history are skipped by identity, so resumed transcripts are never re-written to the store. - Best-effort: a broken session store degrades to a stateless turn. Verified live: ZEBRA planted and recalled across two -z invocations on a caller-minted id. tests/hermes_cli/test_oneshot_resume.py (7 new); tests/hermes_cli 7804 passed — failure set byte-identical to the pre-change baseline (259 pre-existing, attributed by clean-tree diff). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Related: fixes #49195. Carries forward the hydration approach of closed #40333 (independently derived) and is the deferred-hydration change #49204 explicitly punted on. Competing with open #49204, which takes the inverse minimal path (reject |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying forward the session-hydration approach. The current-main premise is confirmed: both oneshot dispatches call run_oneshot() before resume routing and pass no resume value (hermes_cli/main.py:12670-12685, hermes_cli/main.py:14826-14843). The implementation direction matches the existing interactive lineage and history flow (hermes_cli/cli_agent_setup_mixin.py:272-292) and the flusher’s identity-based seeded-history deduplication (run_agent.py:1885-1911).
Problems
- The new tests are mock-only (
tests/hermes_cli/test_oneshot_resume.py:63-81), so they do not verify real SQLite create-on-first-use, a second-process reload, or no duplicate transcript rows. The contribution rubric requires an E2E path for session-resolution and file-I/O changes.
Suggested changes
- Add a temp-
HERMES_HOMEintegration regression covering two caller-minted resumed oneshot turns and durable transcript assertions. - Salvage onto current main while preserving
usage_file, added by7dfd5077cetohermes_cli/oneshot.py:174and both dispatch calls.
Automated hermes-sweeper review.
| db.reopen_session.side_effect = RuntimeError("db locked") | ||
| sid, hist = _load_resume_history(db, "sid") | ||
| assert sid == "sid" | ||
| assert hist is None |
There was a problem hiding this comment.
This harness mocks SessionDB and AIAgent, so it cannot prove create-on-first-use or a second independent -z --resume invocation reloads and appends the real SQLite transcript. Please add a temp-HERMES_HOME integration test for that two-invocation path.
Rebases nathansmithopenclaw-alt's NousResearch#57859 onto current main (which had since landed the MCP-discovery wait and requested_provider/usage_file params in _run_agent — this carries both forward alongside the resume hydration) and addresses the two points from @teknium1's review: - The existing test_oneshot_resume.py coverage is entirely mock-based (SessionDB, AIAgent, and _load_resume_history's own dependencies are all MagicMocks) — real create-on-first-use behavior, a second-process reload, and duplicate-row risk on the resume-and-reflush cycle were never actually exercised against disk. - Adds TestOneshotResumeIntegration: two _run_agent() calls against a real, temp-path-backed SessionDB (only AIAgent is replaced, with a fake that performs the SAME real create_session/append_messages_batch calls a genuine turn's flush does), then a THIRD read through a brand-new SessionDB instance — the same shape a second `hermes -z` process invocation would see. Asserts the full two-turn transcript loads in order with exactly 4 rows, not 8: turn 1's messages are seeded as conversation_history and never re-appended by the flush's identity dedup. tests/hermes_cli/test_oneshot_resume.py: 9 passed (7 existing + 2 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Closing in favor of #80799, which carries this fix rebased onto current main with real-SQLite coverage. |
What
hermes --resume <id> -z "<prompt>"(and--continue [name]) now actually chains: the session's prior transcript is loaded as conversation history and the turn is appended to the SAME session id. Ids that don't exist yet are created on first use, so scripted callers can mint a stable session key per conversation up front and pass it on every turn — no output parsing, no discovery step.Why
The oneshot (
-z) dispatch inhermes_cli/main.pycalledrun_oneshot()andsys.exit()ed before the--resume/--continue→ chat routing, andrun_oneshot()never hydrated anything — the flag was accepted (exit 0, no warning) and silently dropped. Every-zturn was stateless and wrote a new throwaway session.This is #49195. It bites any frontend driving oneshot with a stable per-conversation id — our concrete case is a gateway that runs a Hermes chat seat over
-zone-shots; before this fix the agent was amnesiac on every turn.Relationship to existing PRs
fix(oneshot): honor --resume/--continue so -z can be session-aware #40333 implemented essentially this hydration and was closed by its author without review — this PR carries that approach forward (same core mechanism, independently derived) and adds create-on-first-use ids plus best-effort degradation.
fix(cli): reject --resume/--continue in oneshot instead of dropping it #49204 takes the inverse, minimal path (reject the flags in oneshot) and explicitly defers "full resume hydration" to a later change, citing exact-output replay and compression-lineage concerns. This PR is that later change, and both concerns are addressed:
SessionDB.resolve_resume_session_idbefore loading, exactly like interactive resume.run_conversation(conversation_history=...);_flush_messages_to_session_dbskips seeded history dicts by identity, so resumed transcripts are never re-written to the store (same mechanism the gateway relies on).If maintainers prefer this hydration path, fix(cli): reject --resume/--continue in oneshot instead of dropping it #49204's guard becomes unnecessary; if you'd rather land the guard first, this rebases trivially on top of it (the guard's two dispatch sites are the ones wired here).
Change
hermes_cli/oneshot.py—run_oneshot(resume=...)→_run_agent(resume=...)→ new_load_resume_history(session_db, resume): resolve the compression chain, load messages in conversation format (droppingsession_metarows), best-effortreopen_session, and pinAIAgent(session_id=...). Unknown ids return as-is with no history (create-on-first-use; the agent's existingcreate_sessionupsert makes the row on first persist). A broken session store degrades to a stateless turn —-znever gets less reliable than before.hermes_cli/main.py— both oneshot dispatch sites passresume=_resolve_oneshot_resume(args).--resumeis passed through verbatim (no existence check — that's what enables caller-minted ids);--continueresolves by name / most-recent exactly like interactive chat, and an unresolvable--continueerrors (nothing sensible to chain onto).hermes_cli/_parser.py—-zhelp documents the chaining semantics.--resume/--continue, behavior is byte-for-byte unchanged (session id stays agent-generated).Tests
tests/hermes_cli/test_oneshot_resume.py(7 tests): history load +session_metafiltering + lineage resolution, create-on-first-use, broken-store degradation, and wiring assertions thatAIAgentgetssession_idandrun_conversationgetsconversation_history(and that the no-resume path is untouched).tests/hermes_cli/test_tui_resume_flow.py— two kwarg-capture assertions extended with the newresumekey.tests/hermes_clifull run: failure set byte-identical to a clean-tree baseline run (pre-existing failures only; +7 passing).Manual verification (live CLI):
Closes #49195
🤖 Generated with Claude Code