fix(cli): honor --resume/--continue in oneshot (-z) — rebase + real-SQLite coverage

[blairjordan]

What

Carries forward #57859 (fix/oneshot-resume-hydration, @nathansmithopenclaw-alt): hermes --resume <id> -z "<prompt>" (and --continue [name]) chains — the session's prior transcript loads as conversation history and the turn appends 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.

This closes #49195, and addresses everything from @teknium1's review of #57859:

1. Rebased onto current main. fix(cli): honor --resume/--continue in oneshot (-z) — hydrate and chain the session #57859 predates the MCP-discovery-wait (ensure_mcp_discovery_before_agent_build) and requested_provider/usage_file additions to _run_agent/run_oneshot — this carries the resume hydration forward alongside all three.
2. Real-SQLite integration coverage, not mock-only: two/three-turn _run_agent() calls against a real, temp-path-backed SessionDB (only the LLM call is faked — the stand-in drives the genuine AIAgent._flush_messages_to_session_db against real disk), then a reload through a brand-new SessionDB instance — the shape a second hermes -z process invocation would see. The row-count/dedup assertion is verified non-vacuous by mutation (breaking the real identity-dedup makes it fail).

Update: this PR went through two independent adversarial reviews after opening (see below) that found and fixed three additional real issues beyond what's described above. Net diff vs. the originally-opened version: a hardened exit path for -z --resume/--continue failures, a path-traversal guard on caller-minted ids, and ID-or-title resolution for --resume matching its own documented contract.

Why this needs to exist

hermes -z (oneshot) is the interface a piped/scripted caller uses precisely to avoid chat's banner/box/session chrome on stdout. Every such caller that wants multi-turn continuity — a gateway running a chat seat over -z one-shots, a cron worker, a CI agent pipeline driving Hatchet-style multi-step workflows — currently gets an amnesiac agent: --resume/--continue are accepted (exit 0, no warning) and silently dropped, so every turn starts from zero. This is the third independent fix attempt at this exact gap (#40333 closed in error unrelated to merit, #49204 takes the inverse "reject the flag" minimal path) — the bug report, the review signal, and the number of independent contributors who've hit this all say it's a real, wanted capability.

Relationship to existing PRs/issues

• Closes hermes -z (oneshot) silently ignores --resume / --continue (session never hydrated) #49195 (the bug report).
• Carries forward fix(cli): honor --resume/--continue in oneshot (-z) — hydrate and chain the session #57859's approach (same author's hydration mechanism, independently reviewed as salvageability=high) — rebased + tests added per that review.
• fix(oneshot): honor --resume/--continue so -z can be session-aware #40333: an earlier independent attempt at the same hydration approach, closed by its author unrelated to merit.
• fix(cli): reject --resume/--continue in oneshot instead of dropping it #49204: the inverse minimal "fail loud" guard — not superseded by this PR; a maintainer call on which direction to take (guard vs. hydrate). If the guard lands first, this rebases trivially on top per fix(cli): honor --resume/--continue in oneshot (-z) — hydrate and chain the session #57859's own note.
• fix: preserve one-shot resume context #70136: a third independent hydration attempt, reviewed down for (a) using an ID-only resolver against a flag documented as "by ID or title", and (b) not forwarding --no-restore-cwd. This PR fixes (a) — see below — and does not touch (b); flagging that gap below rather than silently leaving it unaddressed.

Change

• hermes_cli/oneshot.py:
  • run_oneshot(resume=...) → _run_agent(resume=...) → _load_resume_history(session_db, resume): resolves the compression chain via resolve_resume_session_id, loads messages via get_messages_as_conversation (dropping session_meta rows), best-effort reopen_session, and pins AIAgent(session_id=...). A broken session store degrades to a stateless turn — -z never gets less reliable than before.
  • The resume load happens inside the try/finally that owns session_db, matching that block's own stated invariant that the connection is always closed (not reachable as a live bug today — _load_resume_history's own internals are individually guarded — but it contradicted the comment directly above it).
• hermes_cli/main.py:
  • Both oneshot dispatch sites (Termux fast-path and main) resolve resume=_resolve_oneshot_resume(args) and forward it through _run_and_exit_oneshot.
  • --resume resolves by ID or title, exactly like cmd_chat and the flag's own _parser.py help text ("Resume a previous session by ID or title"). Without this, --resume "my project" would silently mint an empty, wrongly-named session instead of resuming — the identical "documented behavior silently doesn't happen in -z" bug class as hermes -z (oneshot) silently ignores --resume / --continue (session never hydrated) #49195 itself, and the exact gap sibling PR fix: preserve one-shot resume context #70136 was reviewed down for. Resolution still falls through to create-on-first-use when nothing matches.
  • Path-traversal guard on caller-minted ids (_is_unsafe_new_session_id): a create-on-first-use id becomes a filename downstream (SessionDB's session-file removal path is unsanitized), so hermes --resume ../../x -z ... would mint a row that a later sessions delete/prune unlinks outside the sessions directory (CWE-22). Mirrors the gateway's own _is_path_unsafe guard at its equivalent entry boundary. Only applied to values about to become new ids — a title containing / that resolves to a real session is untouched.
  • Hardened exit path (_fail_and_exit_oneshot): all three _resolve_oneshot_resume failure branches (unmatched --continue by name, no prior session for bare --continue, path-unsafe --resume id) now exit via _cleanup_oneshot_runtime() + _exit_after_oneshot() instead of a bare sys.exit(2). By the time oneshot dispatch runs, _prepare_agent_startup() has already discovered plugins and can have started MCP server subprocesses — unwinding through normal interpreter finalization both orphans them and re-enters the native finalizer teardown _exit_after_oneshot exists to skip (Bug: hermes -z prints successful response then aborts during shutdown with exit 134 #30387/Bug: hermes -z aborts with SIGABRT during teardown on AL2023 #43055).
  • --continue resolves by name/most-recent exactly like interactive chat; an unresolvable --continue errors (nothing sensible to chain onto).
• hermes_cli/_parser.py — -z help documents ID-or-title + create-on-first-use + the --continue error semantics.
• With no --resume/--continue, behavior is byte-for-byte unchanged.

One open question for a maintainer, not resolved here

Whether --resume <title> should resolve at all in oneshot was reviewed both ways during this PR's own review cycle: one review argued for matching the documented ID-or-title contract (implemented, above); a second flagged a theoretical hijack risk — a caller-minted key that happens to collide with an existing session's human-readable title would land on the wrong session. I kept the resolution because it's the same resolution function and ordering (exact id first, title fallback) that interactive chat already uses today, so this isn't a new risk class introduced by this PR, and it matches what --parser.py promises and what #70136's review already asked for. Flagging explicitly in case a maintainer weighs this differently.

Tests

• tests/hermes_cli/test_oneshot_resume.py: 26 passed (7 original mock-based + 2 real-SQLite integration + 3 new integration cases — third-turn accumulation, compression-chain redirect, unknown-id create-on-first-use — + 14 in a new TestResolveOneshotResume covering title resolution, path-traversal rejection (5 shapes), the hardened-exit contract, and --continue resolution).
• tests/hermes_cli/ -k "resume or continue or oneshot or session": 203 passed, 5 failed — all 5 are pre-existing dashboard-auth 401s unrelated to this change (confirmed against a clean-main baseline in the same sandbox).
• Full tests/hermes_cli/ suite: no regressions vs. a clean-main baseline in the same sandbox (identical pre-existing failure count; the only pass-count delta matches this PR's own new tests).
• Manual verification, live, against a real deployed nousresearch/hermes-agent:latest container: planted a codeword on turn 1 under a caller-minted --resume id, recalled it correctly on turn 2 under the same id, confirmed a different --resume id sees no cross-session leakage.

Review history

This branch was reviewed twice after opening, independently and adversarially (one focused on correctness of the session/exit-path mechanics, one on completeness against this repo's own CONTRIBUTING.md/AGENTS.md contribution rubric and against gaps sibling PRs were reviewed down for). Both found real, non-overlapping issues; both are folded into the current diff, verified together (not just individually) before pushing. Happy to expand on either review's specific findings if useful for re-review.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Update: two more independent reviews (security, concurrency/production-robustness)

After the round above, this branch went through two more adversarial passes — one hunting security issues specifically, one focused on concurrency and production robustness given the actual motivating use case (a Hatchet-orchestrated pipeline running multiple concurrent multi-step chains against the same session store). Both found real, distinct, confirmed issues, now folded into the diff:

Security:

• SessionDB._remove_session_files (hermes_state.py) interpolated session_id raw into a glob pattern. An id of literally * — legal under the path-traversal guard, since * isn't a traversal character — expands request_dump_{id}_*.json into request_dump_*_*.json and deletes every other session's request dumps. --resume made this CLI-reachable for the first time (previously gateway-only). Fixed with glob.escape.
• _is_unsafe_new_session_id only ported one of the three checks the gateway's own equivalent entry boundary applies together. Added control-character rejection (CR/LF can forge log lines; ESC can inject terminal escapes into the rejection message itself) and the same 256-char length cap the gateway enforces (past NAME_MAX the session snapshot file becomes permanently uncreatable). Rejection messages now echo the offending value through a safe-echo helper so rejecting a hostile id can't itself be the injection vector.
• Investigated and found clean: prompt injection via replayed history (nothing re-dispatches replayed tool calls; approval state isn't restorable from transcript content), --yolo bleed across resumed turns, toolset/model scope leakage, unbounded-history growth (handled by existing compression machinery), torn reads.
• Flagged, not fixed (product decisions, not bugs): create-on-first-use resolving by title before falling back to "use as-is" (same tradeoff as the title-resolution decision above); reopen_session clearing end_reason unconditionally (matches interactive chat's existing behavior — not a regression, just an existing shared question); no single-writer enforcement on a caller-minted id (not a vulnerability, but worth a docs line).

Concurrency/production-robustness — this is the one I'd flag as most load-bearing given the actual use case:

• The two --resume reads had no lock-contention patience, unlike every write path in this codebase (which gets 20–60s of jittered retry). Off WAL — which is the default on any pre-3.51.3 SQLite (i.e. stock CPython 3.11/3.12/3.13) or NFS/SMB/ZFS — a sibling connection holding the write lock for even a few seconds (a large FTS append, an incremental merge, a checkpoint, VACUUM — all real, existing code paths) made the read raise database is locked, which the resume loader already swallowed to a stateless degrade. Concretely: a resumed turn would silently run with zero prior context, exit 0, and append its context-less answer into the middle of an otherwise-good transcript — full token cost paid, wrong answer produced, chain poisoned for every downstream step, with no error signal at all. Reproduced against a real SessionDB under a held write lock. Fixed by giving the read path the same lock-patience budget the write path already has, escalating to a new ResumeStoreContendedError (routed through the existing non-zero-exit error path, so an orchestrator can retry) only if the lock never actually clears — a genuinely broken store still degrades to stateless as before.
• Everything else probed held up under real, measured load: throughput overhead from resume chaining is ~1.7x DB-side cost at N=24 concurrent chains (well under 1% of a real LLM turn's wall-clock), retries never corrupt state (the healing path already handles an abandoned tool-call tail), concurrent same-id minting degrades gracefully via the existing upsert semantics, and multi-hop (3+ turn) chaining behaves identically to the tested 2-turn case.
• Flagged, not fixed (out of scope for this diff): a double SessionDB open per resume call (main.py resolves the id, then oneshot.py opens its own store — measurable but small, and fixing it means threading a connection through a wider call chain than this PR should touch); bare --continue under concurrency resolves to the workspace's most-recently-used session, which is correct per the flag's definition but means concurrent callers in the same repo must use --resume <stable-id>, never bare --continue — worth a docs note for anyone building an orchestrator on this.

Tests: tests/hermes_cli/test_oneshot_resume.py now 40 passed (up from 26). tests/hermes_cli/ -k "resume or continue or oneshot or session": 217 passed, same 5 pre-existing unrelated dashboard-auth failures every prior review round hit.

[blairjordan]

This has been running in production for us since 2026-08-07, so I can speak to the two risk labels with real operational evidence rather than just code review.

Context: we hand-patched oneshot.py / main.py / hermes_state.py as a local override (pinned to a specific base image digest) ahead of this PR landing, because we have an internal Buildkite-triggered pipeline ("dev-loop") that chains 5–6 hermes -z --resume <id> calls into one continuous session per run — classify → investigate_bug/plan_change → implement_change → create_pr → address_review_feedback, each step resuming the session the previous step produced. It's the exact multi-turn --resume chaining this PR adds. That pipeline has since authored and merged several of its own PRs against its own repo, purely by chaining oneshot turns through the same session id.

On sweeper:risk-compatibility: worth being upfront that our first attempt at this override did break prod — we patched against an independently-resolved hermes-agent:latest pull from a dev laptop, and the box's own docker compose build --pull resolved :latest to a different digest at deploy time. The override's hermes_state.py ended up missing methods (flush_token_counts, append_messages_batch, preflight_db_writability) that run_agent.py/kanban_db.py call on every turn — every message failed to persist for a few minutes until we caught it (AttributeError: 'SessionDB' object has no attribute ... on every message) and hotfixed. Root cause was purely version drift between the patch and the actual running image, not the resume/continue logic itself. Fix was pinning the base image to an exact digest and diffing method signatures against that exact digest before redeploying — since then, zero session-related errors or SQLite lock-contention failures in production logs. If anything, this is an argument for landing the fix upstream: every downstream consumer who wants --resume in oneshot today has to hand-roll this same override and is exposed to the same drift risk we hit.

On sweeper:risk-session-state: before redeploying the corrected patch, we did a live correctness check — planted a codeword in one caller-minted --resume turn, recalled it correctly on a second turn against the same session id, and confirmed plain (non-resume) oneshot calls were unaffected. In production since, the chained dev-loop steps above have run repeatedly with no session corruption, no cross-session bleed, and no mis-associated context that we've observed.

Happy to point at specific commits/logs if it helps move this past needs-decision, or to expand test coverage further if there's a specific scenario the maintainers want covered beyond what's already in the PR.

[blairjordan]

Separately — CI hasn't run on this PR yet (no checks show up), which I think is the first-time-contributor workflow-approval gate. If a maintainer can approve the workflow run, that'd help get this off the "no CI signal" pile — happy to push a rebase if that's easier to trigger a fresh run.