fix(cron): bot-chat delivery cap bounds the bot's turn, not its exit linger
A cron job delivering to Bot Chat booked "timed out after 600s" for turns that finished in seconds: cron/scheduler_delivery.py::_deliver_to_bot_chat waited for the `hermes chat -Q` child to EXIT, but when the bot's turn had messaged a teammate (message_agent -> notify_on_complete runner) the child then runs the one-shot exit linger, bounded by terminal.oneshot_completion_wait_seconds (default 600) — the same default as cron.bot_chat_delivery_timeout_seconds. The cap counts from the claim, the linger only starts when the turn ends, so the cap expired first on every such delivery, booked a completed turn as a timeout, held the job's fire fence for the full cap, and killed the child mid-linger — tearing down the reply the linger exists to protect (#90879). Ordering the two bounds cannot fix it (the turn has no duration bound; the linger has its own contract), so the cap stops competing with the linger: - hermes_cli/quiet_single_query.py: the -Q child accepts a per-process report path (HERMES_QUIET_TURN_REPORT_FILE), popped before the turn like HERMES_TURN_AUTHOR so nothing the turn spawns inherits it; cli.py writes {pid, exit_code, error} there the moment the turn ends, BEFORE the linger. - cron: _run_bot_chat_turn polls the child and that report under the cap. Report present -> the delivery is booked from it (real exit code and stream tails when the child exits within a short grace) and the still- lingering child is left running, drained and reaped by a daemon thread. No report by the cap -> the turn never ended: killed and booked as a timeout, exactly as before. The linger itself is untouched. Live repro (real _deliver_to_bot_chat, real `hermes chat -Q` against a loopback provider whose turn spawns `sleep 90` with notify_on_complete, cap 45s): base books the timeout at 45.7s for a turn that ended at +5s and kills the child; fixed head books success at 11.2s, the child lingers, the teammate follow-up turn runs at +97s and the child exits on its own. Supersedes #113649's cap = delivery + linger (the thread shows a headroom only moves the race and lengthens the fence hold); analysis credit to the reporter and the thread's independent verification. Fixes #113608 Co-authored-by: KoNit-K <konit.block@protonmail.com>
This commit is contained in:
teknium1
11
cli.py
11
cli.py
@@ -4153,10 +4153,13 @@ def _run_quiet_single_query(cli, effective_query, emitter=None):
|
||||
from agent.turn_author import take_turn_author_from_env
|
||||
from hermes_cli.quiet_single_query import (
|
||||
adopt_unanswered_turn, bind_quiet_session_key, continue_quiet_notify_completions,
|
||||
quiet_notify_linger_seconds,
|
||||
quiet_notify_linger_seconds, take_turn_report_path, write_turn_report,
|
||||
)
|
||||
|
||||
author = take_turn_author_from_env()
|
||||
# A spawner that bounds only the turn (cron Bot Chat lane) learns the outcome from this
|
||||
# report, written before the linger below; popped so tool subprocesses do not inherit it.
|
||||
turn_report_path = take_turn_report_path()
|
||||
# A dispatcher's re-run of a failed bot delivery resumes the DM row its first attempt persisted.
|
||||
adopt_unanswered_turn(cli, effective_query)
|
||||
author_kwargs = {"turn_author": author} if author is not None and _accepts_keyword(cli.agent.run_conversation, "turn_author") else {}
|
||||
@@ -4174,6 +4177,12 @@ def _run_quiet_single_query(cli, effective_query, emitter=None):
|
||||
# The exit line below reports session_id to stderr for automation wrappers;
|
||||
# without this sync it would point at the ended parent after compression.
|
||||
_sync_cli_session_id_from_agent(cli)
|
||||
# The turn is over and persisted: the one-shot exit linger that follows protects nested
|
||||
# notify_on_complete replies and is NOT part of the spawner's delivery (#113608).
|
||||
write_turn_report(
|
||||
turn_report_path, exit_code=_single_query_exit_code(result),
|
||||
error=str(result.get("error") or "") if isinstance(result, dict) else "agent turn did not run",
|
||||
)
|
||||
if isinstance(result, dict) and not result.get("failed"):
|
||||
history = result.get("messages") or cli.conversation_history
|
||||
|
||||
|
||||
@@ -17,6 +17,8 @@ import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, List, Optional
|
||||
|
||||
@@ -694,6 +696,48 @@ _BOT_CHAT_STDERR_TAIL = 500
|
||||
# stdout is the model's answer; only a short tail is persisted (jobs.json / ledger).
|
||||
_BOT_CHAT_STDOUT_TAIL = 200
|
||||
_BOT_CHAT_BANNER_PREFIXES = ("Resumed session", "session_id:")
|
||||
# After the child reports its turn, a child with nothing to linger for exits at once; give it
|
||||
# that long so its real exit code and stream tails are booked instead of the report's summary.
|
||||
_BOT_CHAT_EXIT_GRACE_SECONDS = 2.0
|
||||
|
||||
|
||||
def _run_bot_chat_turn(argv: list, env: dict, report_path: str, timeout: float) -> subprocess.CompletedProcess:
|
||||
"""Run one ``hermes chat -Q`` delivery child; the cap bounds the TURN, not the process.
|
||||
|
||||
The child records its turn outcome at *report_path* (``hermes_cli.quiet_single_query``)
|
||||
the moment the turn ends, then runs the one-shot exit linger for nested
|
||||
``notify_on_complete`` replies — bounded by ``terminal.oneshot_completion_wait_seconds``,
|
||||
whose default equals this lane's cap, so waiting for process exit booked every delivered
|
||||
turn that left a reply pending as a timeout and killed the linger (#113608). Once the
|
||||
report exists the delivery is booked from it and the still-lingering child is left
|
||||
running (a daemon thread drains and reaps it); only a turn that never ends is killed.
|
||||
"""
|
||||
from hermes_cli.quiet_single_query import read_turn_report
|
||||
|
||||
proc = subprocess.Popen(
|
||||
argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
|
||||
env=env, creationflags=windows_hide_flags())
|
||||
streams: dict = {}
|
||||
|
||||
def _drain() -> None:
|
||||
streams["out"], streams["err"] = proc.communicate()
|
||||
|
||||
drain = threading.Thread(target=_drain, name=f"bot-chat-delivery-{proc.pid}", daemon=True)
|
||||
drain.start()
|
||||
deadline = time.monotonic() + timeout
|
||||
report = None
|
||||
while True:
|
||||
drain.join(timeout=0.25 if report is None else _BOT_CHAT_EXIT_GRACE_SECONDS)
|
||||
if not drain.is_alive():
|
||||
return subprocess.CompletedProcess(argv, proc.returncode, streams.get("out", ""), streams.get("err", ""))
|
||||
if report is not None:
|
||||
# Turn over, child still lingering for a nested reply: not this lane's wait.
|
||||
return subprocess.CompletedProcess(argv, int(report["exit_code"]), "", report.get("error") or "")
|
||||
report = read_turn_report(report_path, proc.pid)
|
||||
if report is None and time.monotonic() >= deadline:
|
||||
proc.kill()
|
||||
drain.join(timeout=5.0)
|
||||
raise subprocess.TimeoutExpired(argv, timeout)
|
||||
|
||||
|
||||
def _format_failure_streams(result) -> str:
|
||||
@@ -870,9 +914,10 @@ def _deliver_to_bot_chat(job: dict, content: str, profile: str, *, deferred: Opt
|
||||
"chat", "--in", "~", "-c", "Bot Chat", "--create-if-missing",
|
||||
"-Q", "--query-file", query_file,
|
||||
]
|
||||
result = subprocess.run(
|
||||
argv, capture_output=True, text=True, timeout=_get_bot_chat_delivery_timeout(), env=env,
|
||||
creationflags=windows_hide_flags())
|
||||
from hermes_cli.quiet_single_query import TURN_REPORT_FILE_ENV
|
||||
report_file = f"{query_file}.turn.json"
|
||||
env[TURN_REPORT_FILE_ENV] = report_file
|
||||
result = _run_bot_chat_turn(argv, env, report_file, _get_bot_chat_delivery_timeout())
|
||||
if result.returncode != 0:
|
||||
tail = _format_failure_streams(result)
|
||||
logger.warning(
|
||||
@@ -899,8 +944,9 @@ def _deliver_to_bot_chat(job: dict, content: str, profile: str, *, deferred: Opt
|
||||
"The result is saved; run `hermes cron runs` to see it, or `hermes doctor` if this keeps happening")
|
||||
finally:
|
||||
if query_file:
|
||||
with contextlib.suppress(OSError):
|
||||
os.unlink(query_file)
|
||||
for path in (query_file, f"{query_file}.turn.json"):
|
||||
with contextlib.suppress(OSError):
|
||||
os.unlink(path)
|
||||
|
||||
|
||||
def _normalize_deliver_value(deliver) -> str:
|
||||
|
||||
@@ -11,6 +11,7 @@ was never injected as a follow-up.
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import json
|
||||
import os
|
||||
import time
|
||||
from typing import Any, Callable, MutableMapping
|
||||
@@ -18,6 +19,44 @@ from typing import Any, Callable, MutableMapping
|
||||
# Nested A→B→C is one extra turn; this caps a runaway message_agent chain.
|
||||
_MAX_QUIET_NOTIFY_ROUNDS = 8
|
||||
|
||||
# A spawner that bounds only the TURN (the cron Bot Chat lane) hands the quiet child a report
|
||||
# path here. The child records the turn's outcome there the moment the turn ends, BEFORE the
|
||||
# one-shot exit linger, so the spawner can book the delivery and stop waiting while the linger
|
||||
# keeps protecting nested ``notify_on_complete`` replies. Popped before the turn runs (same
|
||||
# contract as HERMES_TURN_AUTHOR): nothing the turn spawns inherits it, and a nested one-shot
|
||||
# never writes over its host's report — the record also carries the writer's pid.
|
||||
TURN_REPORT_FILE_ENV = "HERMES_QUIET_TURN_REPORT_FILE"
|
||||
|
||||
|
||||
def take_turn_report_path(environ: MutableMapping[str, str] = os.environ) -> str | None:
|
||||
"""Read and remove the spawner's turn-report path so subprocesses started during the turn do not inherit it."""
|
||||
return environ.pop(TURN_REPORT_FILE_ENV, None) or None
|
||||
|
||||
|
||||
def write_turn_report(path: str | None, *, exit_code: int, error: str = "") -> None:
|
||||
"""Atomically record ``{pid, exit_code, error}`` at *path*; a no-op without a path. Never raises:
|
||||
the report is the spawner's convenience, the turn itself is already persisted."""
|
||||
if not path:
|
||||
return
|
||||
record = {"pid": os.getpid(), "exit_code": int(exit_code), "error": str(error or "")}
|
||||
with contextlib.suppress(Exception):
|
||||
tmp = f"{path}.{os.getpid()}.tmp"
|
||||
with open(tmp, "w", encoding="utf-8") as fh:
|
||||
json.dump(record, fh)
|
||||
os.replace(tmp, path)
|
||||
|
||||
|
||||
def read_turn_report(path: str, pid: int) -> dict | None:
|
||||
"""The child's turn report, or None while absent, unreadable, or written by another process."""
|
||||
try:
|
||||
with open(path, encoding="utf-8") as fh:
|
||||
record = json.load(fh)
|
||||
except (OSError, ValueError):
|
||||
return None
|
||||
if not isinstance(record, dict) or record.get("pid") != pid:
|
||||
return None
|
||||
return record
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def bind_quiet_session_key(session_id: str):
|
||||
|
||||
@@ -29,7 +29,7 @@ def test_cli_keeps_discovered_home_when_launch_selection_changes(tmp_path, monke
|
||||
(root / "active_profile").write_text("other", encoding="utf-8")
|
||||
return None
|
||||
|
||||
def run(argv, **kwargs):
|
||||
def run(argv, env, report_path, timeout):
|
||||
# Exercise the actual startup resolver with the production child env/flags.
|
||||
code = ('import json,sys; sys.argv=["hermes"]+json.loads(sys.argv[1]); '
|
||||
'import hermes_cli.main; from hermes_constants import get_hermes_home; '
|
||||
@@ -37,13 +37,13 @@ def test_cli_keeps_discovered_home_when_launch_selection_changes(tmp_path, monke
|
||||
# Everything after the launcher (binary or ``python -m hermes_cli.main``) is the CLI argv.
|
||||
cli_argv = argv[3:] if argv[1:3] == ["-m", "hermes_cli.main"] else argv[1:]
|
||||
result = real_run([sys.executable, "-c", code, json.dumps(cli_argv)],
|
||||
env=kwargs["env"], capture_output=True, text=True, timeout=30)
|
||||
env=env, capture_output=True, text=True, timeout=30)
|
||||
assert result.returncode == 0, result.stderr
|
||||
seen.append(Path(json.loads(result.stdout.strip().splitlines()[-1])))
|
||||
return subprocess.CompletedProcess(argv, 0, "", "")
|
||||
|
||||
monkeypatch.setattr("tools.bot_live_delivery.find_canonical_live_owner", discover)
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
try:
|
||||
assert delivery._deliver_to_bot_chat({"id": "job"}, "output", profile) is None
|
||||
assert seen == [home]
|
||||
@@ -69,7 +69,7 @@ def test_missing_destination_never_launches_or_recreates(tmp_path, monkeypatch,
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("tools.bot_live_delivery.find_canonical_live_owner", discover)
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
try:
|
||||
error = delivery._deliver_to_bot_chat({"id": "job"}, "output", "beta")
|
||||
assert error is not None and str(home) in error
|
||||
|
||||
@@ -21,7 +21,7 @@ def test_cli_owner_deferral_and_attempt_fence(tmp_path, monkeypatch, error):
|
||||
lease, refusal = try_acquire_active_session(session_id="chat", surface="cli", config={}, registry_home=tmp_path)
|
||||
assert refusal is None and lease is not None
|
||||
run = Mock(side_effect=error, return_value=subprocess.CompletedProcess([], 0, "", ""))
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
monkeypatch.setattr(delivery.shutil, "which", lambda _: "/bin/hermes")
|
||||
job = {"id": "job", "execution_id": "execution"}
|
||||
try:
|
||||
@@ -69,12 +69,12 @@ def test_delivery_exception_retains_attempt_and_continues_siblings(tmp_path, mon
|
||||
raise PermissionError("target traversal denied after discovery")
|
||||
return original_is_dir(self)
|
||||
|
||||
def run(*args, **kwargs):
|
||||
calls.append(kwargs["env"]["HERMES_HOME"])
|
||||
def run(argv, env, report_path, timeout):
|
||||
calls.append(env["HERMES_HOME"])
|
||||
return subprocess.CompletedProcess([], 0, "", "")
|
||||
|
||||
monkeypatch.setattr(importlib.util, "find_spec", resolve_cli)
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
monkeypatch.setattr(Path, "is_dir", is_dir)
|
||||
queue.drain()
|
||||
assert queue.read_pending("b" * 64)["status"] == "ambiguous"
|
||||
@@ -114,7 +114,7 @@ def test_policy_change_settles_diagnostic_without_waiting_for_cli_owner(tmp_path
|
||||
lease, refusal = try_acquire_active_session(session_id="chat", surface="cli", config={}, registry_home=tmp_path)
|
||||
assert refusal is None
|
||||
run = Mock()
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
job = {"id": "failure", "execution_id": "run"}
|
||||
try:
|
||||
assert "queued" in delivery._deliver_to_bot_chat(job, "diagnostic", "", for_failure=True)
|
||||
|
||||
@@ -36,7 +36,7 @@ def test_deferred_destination_does_not_follow_root_changes(tmp_path, monkeypatch
|
||||
db.close()
|
||||
monkeypatch.setattr("hermes_cli.profiles.get_profile_dir", lambda _: other)
|
||||
run = Mock(return_value=subprocess.CompletedProcess([], 0, "", ""))
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
if recipient == "desktop":
|
||||
lease, refusal = try_acquire_active_session(
|
||||
session_id="chat", surface="desktop", config={}, registry_home=home,
|
||||
@@ -53,7 +53,7 @@ def test_deferred_destination_does_not_follow_root_changes(tmp_path, monkeypatch
|
||||
assert run.call_count == 1
|
||||
argv = run.call_args.args[0]
|
||||
assert "-p" not in argv
|
||||
assert Path(run.call_args.kwargs["env"]["HERMES_HOME"]) == home
|
||||
assert Path(run.call_args.args[1]["HERMES_HOME"]) == home
|
||||
elif recipient == "desktop":
|
||||
run.assert_not_called()
|
||||
receipt = read_delivery_result(home, key)
|
||||
|
||||
@@ -19,7 +19,7 @@ def owner(tmp_path, monkeypatch):
|
||||
db.set_session_title("chat", "Bot Chat")
|
||||
leases = []
|
||||
run = Mock(side_effect=AssertionError("must not launch a second owner"))
|
||||
monkeypatch.setattr(delivery.subprocess, "run", run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", run)
|
||||
|
||||
def acquire(live):
|
||||
lease, refusal = try_acquire_active_session(
|
||||
|
||||
@@ -6,8 +6,11 @@ preflight exemption, create-time validation, the subprocess delivery lane,
|
||||
and the delivery-targets listing used by UI pickers.
|
||||
"""
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import textwrap
|
||||
import time
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
@@ -22,6 +25,7 @@ from cron.scheduler_delivery import (
|
||||
parse_bot_chat_deliver_token,
|
||||
)
|
||||
from cron.scheduler_preflight import _preflight_check_delivery
|
||||
from hermes_cli.quiet_single_query import TURN_REPORT_FILE_ENV, write_turn_report
|
||||
|
||||
|
||||
# ── token parsing ────────────────────────────────────────────────────────────
|
||||
@@ -124,12 +128,11 @@ def test_deliver_runs_canonical_bot_chat_lane():
|
||||
chat --in ~ -c "Bot Chat" --create-if-missing -Q --query-file <tmp>."""
|
||||
calls = {}
|
||||
|
||||
def fake_run(argv, **kwargs):
|
||||
calls["argv"] = argv
|
||||
calls["kwargs"] = kwargs
|
||||
def fake_run(argv, env, report_path, timeout):
|
||||
calls["argv"], calls["env"], calls["report_path"] = argv, env, report_path
|
||||
return _completed()
|
||||
|
||||
with mock.patch.object(sched.subprocess, "run", side_effect=fake_run), \
|
||||
with mock.patch.object(sched_delivery, "_run_bot_chat_turn", side_effect=fake_run), \
|
||||
mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "Daily digest"}, "the output", "")
|
||||
|
||||
@@ -145,11 +148,13 @@ def test_deliver_runs_canonical_bot_chat_lane():
|
||||
assert "--query-file" in argv
|
||||
# Message rides a temp file, never inline argv (quote/expansion safety).
|
||||
assert not any("the output" in str(a) for a in argv)
|
||||
# The child reports its turn outcome here so the cap bounds the turn, not the exit linger.
|
||||
assert calls["env"][TURN_REPORT_FILE_ENV] == calls["report_path"]
|
||||
|
||||
|
||||
def test_deliver_failure_returns_error_string():
|
||||
with mock.patch.object(
|
||||
sched.subprocess, "run", return_value=_completed(returncode=1, stderr="boom")
|
||||
sched_delivery, "_run_bot_chat_turn", return_value=_completed(returncode=1, stderr="boom")
|
||||
), mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "n"}, "out", "")
|
||||
assert err is not None
|
||||
@@ -160,7 +165,7 @@ def test_deliver_failure_reports_both_streams_labeled():
|
||||
"""A failed turn must keep stderr AND stdout, labeled — ``stderr or
|
||||
stdout`` discarded half the signal (#104056)."""
|
||||
with mock.patch.object(
|
||||
sched.subprocess, "run",
|
||||
sched_delivery, "_run_bot_chat_turn",
|
||||
return_value=_completed(returncode=1, stdout="banner out", stderr="boom-err"),
|
||||
), mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "n"}, "out", "")
|
||||
@@ -176,7 +181,7 @@ def test_deliver_failure_banner_only_stdout_names_exit_code_not_banner():
|
||||
banner = ('↻ Resumed session 20260905_121420_8084c7 "Bot Chat" (1 user message, 1 total messages)'
|
||||
'\n\nsession_id: 20260905_121420_8084c7')
|
||||
with mock.patch.object(
|
||||
sched.subprocess, "run",
|
||||
sched_delivery, "_run_bot_chat_turn",
|
||||
return_value=_completed(returncode=1, stdout=banner, stderr=""),
|
||||
), mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "n"}, "out", "")
|
||||
@@ -192,7 +197,7 @@ def test_deliver_failure_persisted_stdout_tail_is_short_and_redacted():
|
||||
answer on stdout is capped to a short tail and secrets are scrubbed."""
|
||||
answer = "x" * 5000 + "\nToken: sk-ant-api03-" + "A" * 80 + " done"
|
||||
with mock.patch.object(
|
||||
sched.subprocess, "run",
|
||||
sched_delivery, "_run_bot_chat_turn",
|
||||
return_value=_completed(returncode=1, stdout=answer, stderr="boom-err"),
|
||||
), mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "n"}, "out", "")
|
||||
@@ -204,7 +209,7 @@ def test_deliver_failure_persisted_stdout_tail_is_short_and_redacted():
|
||||
|
||||
def test_deliver_timeout_returns_error_string():
|
||||
with mock.patch.object(
|
||||
sched.subprocess, "run",
|
||||
sched_delivery, "_run_bot_chat_turn",
|
||||
side_effect=subprocess.TimeoutExpired(cmd="hermes", timeout=600),
|
||||
), mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
err = _deliver_to_bot_chat({"id": "j1", "name": "n"}, "out", "")
|
||||
@@ -216,13 +221,13 @@ def test_deliver_message_carries_cron_attribution(tmp_path):
|
||||
"""The injected turn must self-identify as scheduled output, not the user."""
|
||||
captured = {}
|
||||
|
||||
def fake_run(argv, **kwargs):
|
||||
def fake_run(argv, env, report_path, timeout):
|
||||
qf = argv[argv.index("--query-file") + 1]
|
||||
with open(qf, encoding="utf-8") as fh:
|
||||
captured["message"] = fh.read()
|
||||
return _completed()
|
||||
|
||||
with mock.patch.object(sched.subprocess, "run", side_effect=fake_run), \
|
||||
with mock.patch.object(sched_delivery, "_run_bot_chat_turn", side_effect=fake_run), \
|
||||
mock.patch.object(sched_delivery.shutil, "which", return_value="/usr/bin/hermes"):
|
||||
_deliver_to_bot_chat({"id": "j1", "name": "Daily digest"}, "the payload", "")
|
||||
|
||||
@@ -231,6 +236,55 @@ def test_deliver_message_carries_cron_attribution(tmp_path):
|
||||
assert "the payload" in captured["message"]
|
||||
|
||||
|
||||
_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(sched_delivery.__file__)))
|
||||
|
||||
|
||||
def _child_env() -> dict:
|
||||
"""The stand-in child imports ``hermes_cli`` from this checkout, like the real ``-m hermes_cli.main``."""
|
||||
return {**os.environ, "PYTHONPATH": os.pathsep.join(p for p in (_REPO_ROOT, os.environ.get("PYTHONPATH")) if p)}
|
||||
|
||||
|
||||
def test_turn_report_books_the_delivery_while_the_child_still_lingers(tmp_path):
|
||||
"""The cap bounds the TURN: a child that reported its turn and then lingers for a nested
|
||||
notify_on_complete reply (bounded by oneshot_completion_wait_seconds, default == the cap) is
|
||||
booked from the report promptly and is NOT killed (#113608)."""
|
||||
report = tmp_path / "turn.json"
|
||||
child = textwrap.dedent("""
|
||||
import os, time
|
||||
from hermes_cli.quiet_single_query import TURN_REPORT_FILE_ENV, write_turn_report
|
||||
write_turn_report(os.environ.pop(TURN_REPORT_FILE_ENV), exit_code=0)
|
||||
time.sleep(30)
|
||||
""")
|
||||
procs, real_popen = [], subprocess.Popen
|
||||
|
||||
def spy(*args, **kwargs):
|
||||
procs.append(real_popen(*args, **kwargs))
|
||||
return procs[-1]
|
||||
|
||||
started = time.monotonic()
|
||||
try:
|
||||
with mock.patch.object(sched_delivery.subprocess, "Popen", side_effect=spy):
|
||||
result = sched_delivery._run_bot_chat_turn(
|
||||
[sys.executable, "-c", child], {**_child_env(), TURN_REPORT_FILE_ENV: str(report)}, str(report), timeout=10)
|
||||
elapsed = time.monotonic() - started
|
||||
assert result.returncode == 0
|
||||
assert elapsed < 8, elapsed
|
||||
assert procs[0].poll() is None, "the lingering child must survive the booking"
|
||||
finally:
|
||||
for proc in procs:
|
||||
proc.kill()
|
||||
proc.wait(timeout=10)
|
||||
|
||||
|
||||
def test_turn_that_never_ends_is_still_killed_at_the_cap(tmp_path):
|
||||
"""Control: with no turn report the cap stays the guard it always was."""
|
||||
started = time.monotonic()
|
||||
with pytest.raises(subprocess.TimeoutExpired):
|
||||
sched_delivery._run_bot_chat_turn(
|
||||
[sys.executable, "-c", "import time; time.sleep(30)"], _child_env(), str(tmp_path / "turn.json"), timeout=1)
|
||||
assert time.monotonic() - started < 8
|
||||
|
||||
|
||||
# ── delivery-targets listing (UI pickers) ────────────────────────────────────
|
||||
|
||||
def test_delivery_targets_include_local_profiles():
|
||||
|
||||
@@ -11,7 +11,7 @@ def test_live_delivery_retry_keeps_receipt_across_owner_loss(tmp_path, monkeypat
|
||||
source = tmp_path / "custom-home"
|
||||
monkeypatch.setenv("HERMES_HOME", str(source))
|
||||
subprocess_run = Mock(side_effect=AssertionError("live owner must not spawn CLI"))
|
||||
monkeypatch.setattr(delivery.subprocess, "run", subprocess_run)
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", subprocess_run)
|
||||
from hermes_cli.profiles import get_profile_dir
|
||||
|
||||
for profile, home in [("", source), ("research", get_profile_dir("research"))]:
|
||||
@@ -52,7 +52,7 @@ def test_result_records_pending_until_terminal_receipt(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(mailbox, "find_canonical_live_owner", lambda home: owner)
|
||||
monkeypatch.setattr(delivery._sched, "load_config", lambda: {})
|
||||
monkeypatch.setattr(config, "load_gateway_config", lambda: None)
|
||||
monkeypatch.setattr(delivery.subprocess, "run", Mock(side_effect=AssertionError("CLI")))
|
||||
monkeypatch.setattr(delivery, "_run_bot_chat_turn", Mock(side_effect=AssertionError("CLI")))
|
||||
updates = []
|
||||
monkeypatch.setattr(jobs, "update_job", lambda key, values: updates.append(values))
|
||||
job = dict(id="digest", execution_id="run", deliver="bot-chat")
|
||||
|
||||
@@ -76,3 +76,36 @@ def test_rerun_adopts_the_dm_behind_the_failed_attempts_tool_scaffolding(monkeyp
|
||||
agent, seen = _quiet_turn(monkeypatch, [{"role": "assistant", "content": "earlier"}, tail, *scaffolding], "1")
|
||||
assert seen["history"] == [{"role": "assistant", "content": "earlier"}]
|
||||
assert agent._pending_cli_user_message is tail and tail[_DB_PERSISTED_MARKER] is True
|
||||
|
||||
|
||||
def test_turn_report_is_written_before_the_exit_linger_and_the_path_is_not_inherited(monkeypatch, tmp_path):
|
||||
"""A spawner that bounds only the turn (cron Bot Chat lane, #113608) reads the outcome from
|
||||
HERMES_QUIET_TURN_REPORT_FILE: written the moment the turn ends — before the one-shot exit
|
||||
linger — stamped with this pid, and the variable is popped before the turn spawns anything."""
|
||||
from hermes_cli import quiet_single_query as qsq
|
||||
|
||||
monkeypatch.delenv("HERMES_KANBAN_GOAL_MODE", raising=False)
|
||||
monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False)
|
||||
report = tmp_path / "turn.json"
|
||||
monkeypatch.setenv(qsq.TURN_REPORT_FILE_ENV, str(report))
|
||||
seen = {}
|
||||
|
||||
def run_conversation(**kwargs):
|
||||
seen["env_during_turn"] = os.environ.get(qsq.TURN_REPORT_FILE_ENV)
|
||||
seen["report_during_turn"] = report.exists()
|
||||
return {"final_response": "ok"}
|
||||
|
||||
def linger(*args, **kwargs):
|
||||
seen["report_at_linger"] = qsq.read_turn_report(str(report), os.getpid())
|
||||
return {"waited": [], "completed": [], "timed_out": []}
|
||||
|
||||
monkeypatch.setattr("tools.process_registry.process_registry.wait_for_pending_completions", linger)
|
||||
agent = SimpleNamespace(run_conversation=run_conversation, session_id="s-1")
|
||||
try:
|
||||
cli._run_quiet_single_query(SimpleNamespace(agent=agent, conversation_history=[], session_id="s-1"), "hello")
|
||||
except SystemExit as exc:
|
||||
assert exc.code == 0
|
||||
assert seen["env_during_turn"] is None and seen["report_during_turn"] is False
|
||||
assert seen["report_at_linger"] == {"pid": os.getpid(), "exit_code": 0, "error": ""}
|
||||
# Another process's record is not this child's report.
|
||||
assert qsq.read_turn_report(str(report), os.getpid() + 1) is None
|
||||
|
||||
@@ -846,6 +846,8 @@ cron:
|
||||
|
||||
A timed-out delivery is recorded in `last_delivery_error`; the bot's turn may still complete on its own.
|
||||
|
||||
The cap bounds the bot's **turn** only. When that turn messages a teammate (`message_agent`), the delivery process stays alive afterwards — bounded by `terminal.oneshot_completion_wait_seconds` — so the teammate's reply can land in the Bot Chat; that wait is not part of the delivery and is never counted against, or cut short by, this cap.
|
||||
|
||||
## No-agent mode (script-only jobs)
|
||||
|
||||
For recurring jobs that don't need LLM reasoning — classic watchdogs, disk/memory alerts, heartbeats, CI pings — pass `no_agent=True` at creation time. The scheduler runs your script on schedule and delivers its stdout directly, skipping the agent entirely:
|
||||
|
||||
Reference in New Issue
Block a user