fix(cli): avoid one-shot SIGABRT during teardown
This commit is contained in:
harjoth
kshitij
parent
113d9f63b5
commit
bfa7a794cb
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue