diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 88f5fa375bf1b..e2c2808b7b91f 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -65,6 +65,110 @@ import os import sys +def _exit_after_oneshot(rc: object) -> None: + """Exit one-shot mode without letting late native finalizers change rc. + + Once ``run_oneshot`` has returned, it has emitted any response or diagnostic + and ``_run_agent`` has already run the stateful agent cleanup + (memory-provider shutdown, ``agent.close()``, recall-store close). The + SIGABRT this guards against (#43055) fires in a native-extension finalizer + during CPython's ``Py_FinalizeEx``, *after* the response has printed, so we + flush user-visible streams, shut down file logging, then ``os._exit`` past + the interpreter finalization that aborts. + + We deliberately do *not* drain the Python ``atexit`` chain here. The + aborting finalizer has not been confirmed on the reporter's AL2023 host, + and several registered handlers (browser/LSP emergency sweeps) re-enter + native code and subprocess teardown — the exact class of code that may be + the abort source — so running them just before the hard exit risks + re-arming the crash this routine exists to contain. The stateful cleanup + that actually matters for one-shot (the recall SQLite store) is closed + explicitly in ``_run_agent``; the only atexit-managed resource otherwise + skipped is the symlink-safe-skills ``mkdtemp`` reaper, which is not created + on the toolless health-check path this issue is about, and whose fds the OS + reclaims at process death regardless. + """ + for stream in (sys.stdout, sys.stderr): + try: + stream.flush() + except Exception: + pass + try: + import logging + logging.shutdown() + except Exception: + pass + if rc is None: + exit_code = 0 + elif isinstance(rc, int): + exit_code = rc + else: + exit_code = 1 + os._exit(exit_code) + + +def _cleanup_oneshot_runtime() -> None: + """Best-effort process-global cleanup before one-shot hard exit. + + ``run_oneshot`` owns the agent-local cleanup. This mirrors the lightweight, + process-global pieces from the interactive CLI shutdown path that would + otherwise be skipped by ``os._exit``. + """ + try: + from tools.mcp_tool import shutdown_mcp_servers + shutdown_mcp_servers() + except BaseException: + pass + try: + from agent.auxiliary_client import shutdown_cached_clients + shutdown_cached_clients() + except Exception: + pass + + +def _run_and_exit_oneshot( + prompt: str, + *, + model: object = None, + provider: object = None, + toolsets: object = None, + usage_file: object = None, +) -> None: + try: + from hermes_cli.oneshot import run_oneshot + + rc = run_oneshot( + prompt, + model=model, + provider=provider, + toolsets=toolsets, + usage_file=usage_file, + ) + except KeyboardInterrupt: + rc = 130 + except SystemExit as exc: + if exc.code is not None and not isinstance(exc.code, int): + print(exc.code, file=sys.stderr) + rc = 1 + else: + rc = exc.code + except BaseException: + # Defense-in-depth. ``run_oneshot`` already converts agent failures + # into an int return code and only re-raises KeyboardInterrupt / + # SystemExit (handled above). Anything still escaping here means + # ``run_oneshot`` itself malfunctioned — surface it on stderr but never + # fall through to normal interpreter teardown, which is the exact path + # that aborts with SIGABRT on AL2023 (the bug this routine fixes). + import traceback + try: + traceback.print_exc() + except Exception: + pass + rc = 1 + _cleanup_oneshot_runtime() + _exit_after_oneshot(rc) + + def _set_process_title() -> None: """Set the process title to 'hermes' so tools like 'ps', 'top', and 'htop' show the app name instead of 'python3.xx'. @@ -12973,16 +13077,12 @@ def _try_termux_fast_cli_launch() -> bool: if getattr(args, "oneshot", None): _prepare_agent_startup(args) - from hermes_cli.oneshot import run_oneshot - - sys.exit( - run_oneshot( - args.oneshot, - model=getattr(args, "model", None), - provider=getattr(args, "provider", None), - toolsets=getattr(args, "toolsets", None), - usage_file=getattr(args, "usage_file", None), - ) + _run_and_exit_oneshot( + args.oneshot, + model=getattr(args, "model", None), + provider=getattr(args, "provider", None), + toolsets=getattr(args, "toolsets", None), + usage_file=getattr(args, "usage_file", None), ) if (args.resume or args.continue_last) and args.command is None: @@ -15130,16 +15230,12 @@ def main(): # Handle top-level --oneshot / -z: single-shot mode, stdout = final # response only, nothing else. Bypasses cli.py entirely. if getattr(args, "oneshot", None): - from hermes_cli.oneshot import run_oneshot - - sys.exit( - run_oneshot( - args.oneshot, - model=getattr(args, "model", None), - provider=getattr(args, "provider", None), - toolsets=getattr(args, "toolsets", None), - usage_file=getattr(args, "usage_file", None), - ) + _run_and_exit_oneshot( + args.oneshot, + model=getattr(args, "model", None), + provider=getattr(args, "provider", None), + toolsets=getattr(args, "toolsets", None), + usage_file=getattr(args, "usage_file", None), ) # Handle top-level --resume / --continue as shortcut to chat diff --git a/hermes_cli/oneshot.py b/hermes_cli/oneshot.py index 0ed1dcd303a14..d600c0a9f2975 100644 --- a/hermes_cli/oneshot.py +++ b/hermes_cli/oneshot.py @@ -188,7 +188,7 @@ def run_oneshot( run — even when the run fails — so pipelines can account for spend per invocation. - Returns the exit code. Caller should sys.exit() with the return. + Returns the exit code. The caller owns process termination. """ # Silence every stdlib logger for the duration. AIAgent, tools, and # provider adapters all log to stderr through the root logger; file @@ -396,44 +396,75 @@ def _run_agent( toolsets_list = sorted(_get_platform_tools(cfg, "cli")) session_db = _create_session_db_for_oneshot() - # Read the effective fallback chain from profile config so oneshot workers - # honour the same merge semantics as interactive CLI and gateway sessions. - _fb = get_fallback_chain(cfg) + # The try spans agent construction (not just ``chat``) so the SQLite store + # opened above is always closed — including when ``AIAgent(...)`` itself + # raises on a provider/config error. The one-shot exit path hard-exits via + # os._exit and skips finalizers, so an un-closed connection here would leak. + agent = None + try: + # Read the effective fallback chain from profile config so oneshot + # workers honour the same merge semantics as interactive CLI and + # gateway sessions. + _fb = get_fallback_chain(cfg) - agent = AIAgent( - api_key=runtime.get("api_key"), - base_url=runtime.get("base_url"), - provider=runtime.get("provider"), - api_mode=runtime.get("api_mode"), - model=effective_model, - enabled_toolsets=toolsets_list, - quiet_mode=True, - platform="cli", - session_db=session_db, - credential_pool=runtime.get("credential_pool"), - fallback_model=_fb or None, - # Interactive callbacks are intentionally NOT wired beyond this - # one. In oneshot mode there's no user sitting at a terminal: - # - clarify → returns a synthetic "pick a default" instruction - # so the agent continues instead of stalling on - # the tool's built-in "not available" error - # - sudo password prompt → terminal_tool gates on - # HERMES_INTERACTIVE which we never set - # - shell-hook approval → auto-approved via HERMES_ACCEPT_HOOKS=1 - # (set above); also falls back to deny on non-tty - # - dangerous-command approval → bypassed via HERMES_YOLO_MODE=1 - # - skill secret capture → returns gracefully when no callback set - clarify_callback=_oneshot_clarify_callback, - ) + agent = AIAgent( + api_key=runtime.get("api_key"), + base_url=runtime.get("base_url"), + provider=runtime.get("provider"), + api_mode=runtime.get("api_mode"), + model=effective_model, + enabled_toolsets=toolsets_list, + quiet_mode=True, + platform="cli", + session_db=session_db, + credential_pool=runtime.get("credential_pool"), + fallback_model=_fb or None, + # Interactive callbacks are intentionally NOT wired beyond this + # one. In oneshot mode there's no user sitting at a terminal: + # - clarify → returns a synthetic "pick a default" instruction + # so the agent continues instead of stalling on + # the tool's built-in "not available" error + # - sudo password prompt → terminal_tool gates on + # HERMES_INTERACTIVE which we never set + # - shell-hook approval → auto-approved via HERMES_ACCEPT_HOOKS=1 + # (set above); also falls back to deny on non-tty + # - dangerous-command approval → bypassed via HERMES_YOLO_MODE=1 + # - skill secret capture → returns gracefully when no callback set + clarify_callback=_oneshot_clarify_callback, + ) - # Belt-and-braces: make sure AIAgent doesn't invoke any streaming - # display callbacks that would bypass our stdout capture. - agent.suppress_status_output = True - agent.stream_delta_callback = None - agent.tool_gen_callback = None + # Belt-and-braces: make sure AIAgent doesn't invoke any streaming + # display callbacks that would bypass our stdout capture. + agent.suppress_status_output = True + agent.stream_delta_callback = None + agent.tool_gen_callback = None - result = agent.run_conversation(prompt) - return (result.get("final_response") or "", result) + result = agent.run_conversation(prompt) + return (result.get("final_response") or "", result) + finally: + if agent is not None: + try: + session_messages = getattr(agent, "_session_messages", None) + if isinstance(session_messages, list): + agent.shutdown_memory_provider(session_messages) + else: + agent.shutdown_memory_provider() + except Exception: + logging.debug("oneshot memory/context cleanup failed", exc_info=True) + try: + agent.close() + except Exception: + logging.debug("oneshot agent cleanup failed", exc_info=True) + # Close the recall SQLite store we opened for this run. Message rows + # are committed synchronously during the turn, so nothing is lost, but + # the one-shot exit path hard-exits via os._exit and skips finalizers + # — close here so the connection (and its WAL) is checkpointed cleanly + # instead of relying on interpreter teardown. + if session_db is not None: + try: + session_db.close() + except Exception: + logging.debug("oneshot session store cleanup failed", exc_info=True) def _oneshot_clarify_callback(question: str, choices=None) -> str: diff --git a/tests/hermes_cli/test_tui_resume_flow.py b/tests/hermes_cli/test_tui_resume_flow.py index 87a1e61cd6885..3fa313234583f 100644 --- a/tests/hermes_cli/test_tui_resume_flow.py +++ b/tests/hermes_cli/test_tui_resume_flow.py @@ -1,7 +1,9 @@ from argparse import Namespace import os from pathlib import Path +import subprocess import sys +import textwrap import types import pytest @@ -21,6 +23,10 @@ def _args(**overrides): return Namespace(**base) +def _raise_exit(rc): + raise SystemExit(rc) + + @pytest.fixture def main_mod(monkeypatch): import hermes_cli.main as mod @@ -368,6 +374,11 @@ def test_termux_fast_cli_launch_oneshot_uses_light_parser(monkeypatch, main_mod) or 17 ), ) + monkeypatch.setattr( + main_mod, + "_exit_after_oneshot", + _raise_exit, + ) with pytest.raises(SystemExit) as exc: main_mod._try_termux_fast_cli_launch() @@ -608,6 +619,11 @@ def test_main_top_level_oneshot_accepts_toolsets(monkeypatch, main_mod): or 0 ), ) + monkeypatch.setattr( + main_mod, + "_exit_after_oneshot", + _raise_exit, + ) with pytest.raises(SystemExit) as exc: main_mod.main() @@ -622,6 +638,637 @@ def test_main_top_level_oneshot_accepts_toolsets(monkeypatch, main_mod): } +def test_exit_after_oneshot_flushes_stdio_and_calls_os_exit( + monkeypatch, main_mod +): + flushed = [] + exits = [] + + class FakeStream: + def __init__(self, name): + self.name = name + + def flush(self): + flushed.append(self.name) + + def fake_exit(rc): + exits.append(rc) + raise SystemExit(rc) + + monkeypatch.setattr(main_mod.sys, "stdout", FakeStream("stdout")) + monkeypatch.setattr(main_mod.sys, "stderr", FakeStream("stderr")) + monkeypatch.setattr(main_mod.os, "_exit", fake_exit) + monkeypatch.setattr("logging.shutdown", lambda: None) + + with pytest.raises(SystemExit) as exc: + main_mod._exit_after_oneshot(17) + + assert exc.value.code == 17 + assert exits == [17] + assert flushed == ["stdout", "stderr"] + + +def test_exit_after_oneshot_invokes_logging_shutdown_in_order( + monkeypatch, main_mod +): + events = [] + + class FakeStream: + def __init__(self, name): + self.name = name + + def flush(self): + events.append(f"flush:{self.name}") + + def fake_exit(rc): + events.append(f"exit:{rc}") + raise SystemExit(rc) + + monkeypatch.setattr(main_mod.sys, "stdout", FakeStream("stdout")) + monkeypatch.setattr(main_mod.sys, "stderr", FakeStream("stderr")) + monkeypatch.setattr("logging.shutdown", lambda: events.append("shutdown")) + monkeypatch.setattr(main_mod.os, "_exit", fake_exit) + + with pytest.raises(SystemExit) as exc: + main_mod._exit_after_oneshot(0) + + assert exc.value.code == 0 + assert events == ["flush:stdout", "flush:stderr", "shutdown", "exit:0"] + + +def test_exit_after_oneshot_exits_even_if_logging_shutdown_raises( + monkeypatch, main_mod +): + exits = [] + + def fake_exit(rc): + exits.append(rc) + raise SystemExit(rc) + + monkeypatch.setattr( + main_mod.sys, "stdout", types.SimpleNamespace(flush=lambda: None) + ) + monkeypatch.setattr( + main_mod.sys, "stderr", types.SimpleNamespace(flush=lambda: None) + ) + monkeypatch.setattr( + "logging.shutdown", + lambda: (_ for _ in ()).throw(RuntimeError("shutdown failed")), + ) + monkeypatch.setattr(main_mod.os, "_exit", fake_exit) + + with pytest.raises(SystemExit) as exc: + main_mod._exit_after_oneshot(1) + + assert exc.value.code == 1 + assert exits == [1] + + +def test_exit_after_oneshot_flushes_stderr_when_stdout_flush_fails( + monkeypatch, main_mod +): + flushed = [] + exits = [] + + class BadStdout: + def flush(self): + raise BrokenPipeError("pipe closed") + + class FakeStderr: + def flush(self): + flushed.append("stderr") + + def fake_exit(rc): + exits.append(rc) + raise SystemExit(rc) + + monkeypatch.setattr(main_mod.sys, "stdout", BadStdout()) + monkeypatch.setattr(main_mod.sys, "stderr", FakeStderr()) + monkeypatch.setattr(main_mod.os, "_exit", fake_exit) + monkeypatch.setattr("logging.shutdown", lambda: None) + + with pytest.raises(SystemExit) as exc: + main_mod._exit_after_oneshot(2) + + assert exc.value.code == 2 + assert exits == [2] + assert flushed == ["stderr"] + + +def test_exit_after_oneshot_normalizes_non_int_exit_code(monkeypatch, main_mod): + exits = [] + + def fake_exit(rc): + exits.append(rc) + raise SystemExit(rc) + + monkeypatch.setattr(main_mod.os, "_exit", fake_exit) + monkeypatch.setattr("logging.shutdown", lambda: None) + + with pytest.raises(SystemExit) as exc: + main_mod._exit_after_oneshot(None) + + assert exc.value.code == 0 + assert exits == [0] + + +def test_run_and_exit_oneshot_routes_system_exit_to_hard_exit(monkeypatch, main_mod): + exits = [] + + def fake_run_oneshot(*_args, **_kwargs): + raise SystemExit(2) + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=fake_run_oneshot), + ) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hello") + + assert exits == [2] + + +def test_run_and_exit_oneshot_prints_system_exit_message( + monkeypatch, capsys, main_mod +): + exits = [] + + def fake_run_oneshot(*_args, **_kwargs): + raise SystemExit("fatal") + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=fake_run_oneshot), + ) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hello") + + assert exits == [1] + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "fatal\n" + + +def test_run_and_exit_oneshot_cleans_global_runtime_before_hard_exit( + monkeypatch, main_mod +): + events = [] + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=lambda *_args, **_kwargs: events.append("run") or 0), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_tool", + types.SimpleNamespace(shutdown_mcp_servers=lambda: events.append("mcp")), + ) + monkeypatch.setitem( + sys.modules, + "agent.auxiliary_client", + types.SimpleNamespace(shutdown_cached_clients=lambda: events.append("aux")), + ) + monkeypatch.setattr( + main_mod, "_exit_after_oneshot", lambda rc: events.append(f"exit:{rc}") + ) + + main_mod._run_and_exit_oneshot("hello") + + assert events == ["run", "mcp", "aux", "exit:0"] + + +def test_run_and_exit_oneshot_still_exits_when_global_cleanup_raises( + monkeypatch, main_mod +): + events = [] + + def _raise_mcp(): + raise RuntimeError("mcp boom") + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=lambda *_args, **_kwargs: 0), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_tool", + types.SimpleNamespace(shutdown_mcp_servers=_raise_mcp), + ) + monkeypatch.setitem( + sys.modules, + "agent.auxiliary_client", + types.SimpleNamespace(shutdown_cached_clients=lambda: events.append("aux")), + ) + monkeypatch.setattr( + main_mod, "_exit_after_oneshot", lambda rc: events.append(f"exit:{rc}") + ) + + main_mod._run_and_exit_oneshot("hello") + + assert events == ["aux", "exit:0"] + + +def test_run_and_exit_oneshot_routes_keyboard_interrupt_to_130( + monkeypatch, main_mod +): + exits = [] + + def fake_run_oneshot(*_args, **_kwargs): + raise KeyboardInterrupt + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=fake_run_oneshot), + ) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hello") + + assert exits == [130] + + +def test_run_and_exit_oneshot_hard_exits_on_unexpected_exception( + monkeypatch, main_mod, capsys +): + # ``run_oneshot`` is contracted to convert agent failures into an int and + # only re-raise KeyboardInterrupt / SystemExit. If it ever malfunctions and + # lets another exception escape, the one-shot path must still hard-exit + # (rc 1) rather than fall through to interpreter teardown — the exact path + # that SIGABRTs on AL2023. + exits = [] + cleaned = [] + + def fake_run_oneshot(*_args, **_kwargs): + raise RuntimeError("run_oneshot itself blew up") + + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=fake_run_oneshot), + ) + monkeypatch.setattr( + main_mod, "_cleanup_oneshot_runtime", lambda: cleaned.append(True) + ) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hello") + + assert exits == [1] + # Global cleanup still runs on the defensive path — resources must not leak + # just because run_oneshot malfunctioned. + assert cleaned == [True] + # The failure is surfaced on stderr, never swallowed silently. + assert "run_oneshot itself blew up" in capsys.readouterr().err + + +def test_run_and_exit_oneshot_hard_exits_when_oneshot_import_fails( + monkeypatch, main_mod, capsys +): + import builtins + + exits = [] + cleaned = [] + real_import = builtins.__import__ + + def fake_import(name, globals=None, locals=None, fromlist=(), level=0): + if name == "hermes_cli.oneshot" and "run_oneshot" in (fromlist or ()): + raise RuntimeError("oneshot import blew up") + return real_import(name, globals, locals, fromlist, level) + + monkeypatch.setattr(builtins, "__import__", fake_import) + monkeypatch.setattr( + main_mod, "_cleanup_oneshot_runtime", lambda: cleaned.append(True) + ) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hello") + + assert exits == [1] + assert cleaned == [True] + assert "oneshot import blew up" in capsys.readouterr().err + + +def test_oneshot_subprocess_exits_without_teardown_abort(): + program = textwrap.dedent( + """ + import hermes_cli.oneshot as oneshot + from hermes_cli.main import _exit_after_oneshot + + oneshot._run_agent = lambda *args, **kwargs: ("ok", {"final_response": "ok"}) + _exit_after_oneshot(oneshot.run_oneshot("hello")) + """ + ) + + result = subprocess.run( + [sys.executable, "-c", program], + cwd=Path(__file__).resolve().parents[2], + capture_output=True, + timeout=10, + check=False, + ) + + assert result.returncode == 0 + assert result.stdout == b"ok\n" + # Don't demand byte-empty stderr — an import-time warning from the heavy + # CLI import chain shouldn't fail this. What matters is no crash traceback. + assert b"Traceback" not in result.stderr + + +def test_exit_after_oneshot_bypasses_late_atexit_abort(): + program = textwrap.dedent( + """ + import atexit + import os + import sys + from hermes_cli.main import _exit_after_oneshot + + atexit.register(os.abort) + sys.stdout.write("done\\n") + _exit_after_oneshot(0) + """ + ) + + result = subprocess.run( + [sys.executable, "-c", program], + cwd=Path(__file__).resolve().parents[2], + capture_output=True, + timeout=10, + check=False, + ) + + assert result.returncode == 0 + assert result.stdout == b"done\n" + + +def test_run_and_exit_oneshot_passes_through_nonzero_return(monkeypatch, main_mod): + # A non-zero rc from run_oneshot (e.g. provider-without-model → 2, or the + # empty-response guard → 1) must reach os._exit unchanged. + exits = [] + monkeypatch.setitem( + sys.modules, + "hermes_cli.oneshot", + types.SimpleNamespace(run_oneshot=lambda *a, **k: 2), + ) + monkeypatch.setattr(main_mod, "_cleanup_oneshot_runtime", lambda: None) + monkeypatch.setattr(main_mod, "_exit_after_oneshot", lambda rc: exits.append(rc)) + + main_mod._run_and_exit_oneshot("hi") + + assert exits == [2] + + +def test_main_oneshot_path_bypasses_late_atexit_abort(): + # End-to-end through the real top-level ``main()`` ``-z`` path: a valid + # response prints, then a late atexit handler that would abort is bypassed + # by the hard exit, so the process reports success (#43055). + program = textwrap.dedent( + """ + import atexit + import os + import sys + import types + + import hermes_cli.main as main_mod + + sys.argv = ["hermes", "-z", "hello"] + main_mod._prepare_agent_startup = lambda args: None + + def _fake_run_oneshot(prompt, **kwargs): + print("ok") + return 0 + + sys.modules["hermes_cli.oneshot"] = types.SimpleNamespace( + run_oneshot=_fake_run_oneshot + ) + atexit.register(os.abort) + main_mod.main() + """ + ) + + result = subprocess.run( + [sys.executable, "-c", program], + cwd=Path(__file__).resolve().parents[2], + capture_output=True, + timeout=30, + check=False, + ) + + assert result.returncode == 0 + assert result.stdout == b"ok\n" + assert b"Traceback" not in result.stderr + + +def test_oneshot_run_agent_closes_agent_after_chat(monkeypatch): + import hermes_cli.oneshot as oneshot_mod + + closed = [] + shutdown_messages = [] + + class FakeAgent: + def __init__(self, **_kwargs): + self.suppress_status_output = False + self.stream_delta_callback = object() + self.tool_gen_callback = object() + self._session_messages = [{"role": "user", "content": "hello"}] + + def run_conversation(self, prompt, **_kwargs): + assert prompt == "hello" + return {"final_response": "done"} + + def shutdown_memory_provider(self, messages=None): + shutdown_messages.append(messages) + + def close(self): + closed.append(True) + + monkeypatch.setitem( + sys.modules, "run_agent", types.SimpleNamespace(AIAgent=FakeAgent) + ) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"model": {"default": "gpt-test", "provider": "openai"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "api_key": "key", + "base_url": "https://example.invalid", + "provider": "openai", + "api_mode": "chat_completions", + "credential_pool": None, + }, + ) + monkeypatch.setattr(oneshot_mod, "_create_session_db_for_oneshot", lambda: None) + + assert ( + oneshot_mod._run_agent( + "hello", model="gpt-test", provider="openai", use_config_toolsets=False + ) + == ("done", {"final_response": "done"}) + ) + assert closed == [True] + assert shutdown_messages == [[{"role": "user", "content": "hello"}]] + + +def test_oneshot_run_agent_closes_agent_when_chat_raises(monkeypatch): + import hermes_cli.oneshot as oneshot_mod + + closed = [] + shutdowns = [] + + class FakeAgent: + def __init__(self, **_kwargs): + self.suppress_status_output = False + self.stream_delta_callback = object() + self.tool_gen_callback = object() + + def run_conversation(self, _prompt, **_kwargs): + raise RuntimeError("boom") + + def shutdown_memory_provider(self, messages=None): + shutdowns.append(messages) + + def close(self): + closed.append(True) + + monkeypatch.setitem( + sys.modules, "run_agent", types.SimpleNamespace(AIAgent=FakeAgent) + ) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"model": {"default": "gpt-test", "provider": "openai"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "api_key": "key", + "base_url": "https://example.invalid", + "provider": "openai", + "api_mode": "chat_completions", + "credential_pool": None, + }, + ) + monkeypatch.setattr(oneshot_mod, "_create_session_db_for_oneshot", lambda: None) + + with pytest.raises(RuntimeError, match="boom"): + oneshot_mod._run_agent( + "hello", model="gpt-test", provider="openai", use_config_toolsets=False + ) + assert closed == [True] + assert shutdowns == [None] + + +def test_oneshot_run_agent_closes_session_db(monkeypatch): + # The one-shot exit path hard-exits via os._exit and skips finalizers, so + # the recall SQLite store it opens must be closed explicitly (checkpointing + # its WAL) rather than left to interpreter teardown. + import hermes_cli.oneshot as oneshot_mod + + db_closed = [] + + class FakeAgent: + def __init__(self, **_kwargs): + self.suppress_status_output = False + self.stream_delta_callback = object() + self.tool_gen_callback = object() + self._session_messages = [] + + def run_conversation(self, _prompt, **_kwargs): + return {"final_response": "done"} + + def shutdown_memory_provider(self, messages=None): + pass + + def close(self): + pass + + class FakeSessionDB: + def close(self): + db_closed.append(True) + + monkeypatch.setitem( + sys.modules, "run_agent", types.SimpleNamespace(AIAgent=FakeAgent) + ) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"model": {"default": "gpt-test", "provider": "openai"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "api_key": "key", + "base_url": "https://example.invalid", + "provider": "openai", + "api_mode": "chat_completions", + "credential_pool": None, + }, + ) + monkeypatch.setattr( + oneshot_mod, "_create_session_db_for_oneshot", lambda: FakeSessionDB() + ) + + assert ( + oneshot_mod._run_agent( + "hello", model="gpt-test", provider="openai", use_config_toolsets=False + ) + == ("done", {"final_response": "done"}) + ) + assert db_closed == [True] + + +def test_oneshot_run_agent_closes_session_db_when_agent_init_raises(monkeypatch): + # The recall store is opened before AIAgent is constructed. If construction + # raises (bad provider/config/model), the store must still be closed — the + # one-shot exit hard-exits via os._exit and skips finalizers, so an + # un-closed connection would leave a stale WAL behind. + import hermes_cli.oneshot as oneshot_mod + + db_closed = [] + + class FakeSessionDB: + def close(self): + db_closed.append(True) + + class FakeAgent: + def __init__(self, **_kwargs): + raise RuntimeError("init boom") + + monkeypatch.setitem( + sys.modules, "run_agent", types.SimpleNamespace(AIAgent=FakeAgent) + ) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"model": {"default": "gpt-test", "provider": "openai"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "api_key": "key", + "base_url": "https://example.invalid", + "provider": "openai", + "api_mode": "chat_completions", + "credential_pool": None, + }, + ) + monkeypatch.setattr( + oneshot_mod, "_create_session_db_for_oneshot", lambda: FakeSessionDB() + ) + + with pytest.raises(RuntimeError, match="init boom"): + oneshot_mod._run_agent( + "hello", model="gpt-test", provider="openai", use_config_toolsets=False + ) + + assert db_closed == [True] + + def _stub_plugin_discovery(monkeypatch): monkeypatch.setitem( sys.modules,