Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion agent/conversation_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -4357,7 +4357,11 @@ def _perform_api_call(next_api_kwargs):
"as final response"
)
final_response = _recovered
agent._response_was_previewed = True
# Streaming delivered a fragment, not a confirmed
# final preview. Leave response_previewed false so
# gateway fallback delivery can send the recovered
# text plus the abnormal-turn explanation.
agent._response_was_previewed = False
break

# If the previous turn already delivered real content alongside
Expand Down
9 changes: 8 additions & 1 deletion agent/turn_finalizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,14 @@ def finalize_turn(
and len(_stripped) <= 24
and _stripped[-1:] not in {".", "!", "?", "。", "!", "?", "`", ")"}
)
if _is_empty_terminal or _is_partial_fragment:
_is_partial_stream_recovery = (
str(_turn_exit_reason) == "partial_stream_recovery"
)
if (
_is_empty_terminal
or _is_partial_fragment
or _is_partial_stream_recovery
):
_explanation = agent._format_turn_completion_explanation(
_turn_exit_reason
)
Expand Down
21 changes: 17 additions & 4 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2558,6 +2558,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
_restart_task_started: bool = False
_restart_detached: bool = False
_restart_via_service: bool = False
_detached_restart_helper_started: bool = False
_restart_command_source: Optional[SessionSource] = None
_stop_task: Optional[asyncio.Task] = None
_restart_task: Optional[asyncio.Task] = None
Expand Down Expand Up @@ -2639,6 +2640,7 @@ def __init__(self, config: Optional[GatewayConfig] = None):
self._restart_task_started = False
self._restart_detached = False
self._restart_via_service = False
self._detached_restart_helper_started = False
self._restart_command_source: Optional[SessionSource] = None
self._stop_task: Optional[asyncio.Task] = None
self._restart_task: Optional[asyncio.Task] = None
Expand Down Expand Up @@ -5325,8 +5327,12 @@ async def _launch_detached_restart_command(self) -> None:
if not hermes_cmd:
logger.error("Could not locate hermes binary for detached /restart")
return
if self._detached_restart_helper_started:
return
self._detached_restart_helper_started = True

current_pid = os.getpid()
restart_after_s = max(float(getattr(self, "_restart_drain_timeout", 0.0) or 0.0) + 5.0, 5.0)

# On Windows there's no bash/setsid chain — spawn a tiny Python
# watcher directly via sys.executable instead. The watcher polls
Expand All @@ -5343,8 +5349,9 @@ async def _launch_detached_restart_command(self) -> None:
import os, subprocess, sys, time
from hermes_cli._subprocess_compat import windows_detach_flags_without_breakaway
pid = int(sys.argv[1])
cmd = sys.argv[2:]
deadline = time.monotonic() + 120
restart_after_s = float(sys.argv[2])
cmd = sys.argv[3:]
deadline = time.monotonic() + restart_after_s

def _alive(p):
# On Windows, os.kill(pid, 0) is NOT a no-op — it maps to
Expand Down Expand Up @@ -5400,7 +5407,7 @@ def _alive(p):
pythonpath.append(watcher_env["PYTHONPATH"])
watcher_env["PYTHONPATH"] = os.pathsep.join(dict.fromkeys(pythonpath))
subprocess.Popen(
[sys.executable, "-c", watcher, str(current_pid), *cmd_argv],
[sys.executable, "-c", watcher, str(current_pid), str(restart_after_s), *cmd_argv],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
env=watcher_env,
Expand All @@ -5410,7 +5417,8 @@ def _alive(p):

cmd = " ".join(shlex.quote(part) for part in hermes_cmd)
shell_cmd = (
f"while kill -0 {current_pid} 2>/dev/null; do sleep 0.2; done; "
f"deadline=$(( $(date +%s) + {int(restart_after_s)} )); "
f"while kill -0 {current_pid} 2>/dev/null && [ $(date +%s) -lt $deadline ]; do sleep 0.2; done; "
f"{cmd} gateway restart"
)
# Same marker scrub as the Windows watcher above: this watcher runs
Expand Down Expand Up @@ -5524,6 +5532,11 @@ def request_restart(self, *, detached: bool = False, via_service: bool = False)
self._restart_task_started = True

async def _run_restart() -> None:
if detached:
try:
await self._launch_detached_restart_command()
except Exception as e:
logger.error("Failed to launch detached gateway restart helper: %s", e)
await asyncio.sleep(0.05)
await self.stop(restart=True, detached_restart=detached, service_restart=via_service)

Expand Down
1 change: 1 addition & 0 deletions tests/gateway/restart_test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def make_restart_runner(
runner._restart_task_started = False
runner._restart_detached = False
runner._restart_via_service = False
runner._detached_restart_helper_started = False
runner._restart_command_source = None
runner._restart_drain_timeout = DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT
runner._stop_task = None
Expand Down
19 changes: 19 additions & 0 deletions tests/gateway/test_restart_drain.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ def test_load_restart_drain_timeout_prefers_env_then_config_then_default(
async def test_request_restart_is_idempotent():
runner, _adapter = make_restart_runner()
runner.stop = AsyncMock()
runner._launch_detached_restart_command = AsyncMock()

# _run_restart is held on self._restart_task and is intentionally NOT in
# _background_tasks, so _stop_impl's cancel loop can't abort it mid-await
Expand All @@ -191,6 +192,7 @@ async def test_request_restart_is_idempotent():

await runner._restart_task

runner._launch_detached_restart_command.assert_awaited_once_with()
runner.stop.assert_awaited_once_with(
restart=True, detached_restart=True, service_restart=False
)
Expand Down Expand Up @@ -263,6 +265,7 @@ def fake_popen(cmd, **kwargs):
assert cmd[:2] == ["/usr/bin/setsid", "bash"]
assert "gateway restart" in cmd[-1]
assert "kill -0 321" in cmd[-1]
assert "deadline=$(( $(date +%s) +" in cmd[-1]
assert kwargs["start_new_session"] is True
assert kwargs["stdout"] is subprocess.DEVNULL
assert kwargs["stderr"] is subprocess.DEVNULL
Expand All @@ -271,6 +274,22 @@ def fake_popen(cmd, **kwargs):
assert kwargs["env"].get("_HERMES_GATEWAY") is None


@pytest.mark.asyncio
async def test_detached_restart_helper_is_idempotent(monkeypatch):
runner, _adapter = make_restart_runner()
popen_calls = []

monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["/usr/bin/hermes"])
monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321)
monkeypatch.setattr(shutil, "which", lambda cmd: None)
monkeypatch.setattr(subprocess, "Popen", lambda *a, **k: popen_calls.append((a, k)))

await runner._launch_detached_restart_command()
await runner._launch_detached_restart_command()

assert len(popen_calls) == 1


def test_windows_gateway_venv_imports_add_site_packages(monkeypatch, tmp_path):
venv_dir = tmp_path / "venv"
site_packages = venv_dir / "Lib" / "site-packages"
Expand Down
8 changes: 6 additions & 2 deletions tests/run_agent/test_run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -4092,7 +4092,9 @@ def _capture_status(msg):
result = agent.run_conversation("ask me")
# Should recover partial streamed content, not fall through to (empty)
assert result["completed"] is True
assert result["final_response"] == "The answer to your question is that"
assert result["final_response"].startswith("The answer to your question is that")
assert "No reply:" in result["final_response"]
assert result["response_previewed"] is False
assert result["api_calls"] == 1 # No wasted retries
# Should emit the stream-interrupted status, NOT the empty-retry status
recovery_msgs = [m for m in status_messages if "stream interrupted" in m.lower()]
Expand Down Expand Up @@ -4122,7 +4124,9 @@ def _fake_api_call(api_kwargs):
):
result = agent.run_conversation("question")
# Should use the streamed content, not the old prior-turn fallback
assert result["final_response"] == "Fresh partial content from this turn"
assert result["final_response"].startswith("Fresh partial content from this turn")
assert "No reply:" in result["final_response"]
assert result["response_previewed"] is False
assert result["api_calls"] == 1

def test_interrupt_during_stream_preserves_partial_assistant_text(self, agent):
Expand Down
31 changes: 31 additions & 0 deletions tests/run_agent/test_turn_completion_explainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,37 @@ def test_run_conversation_empty_exhausted_surfaces_explanation():
assert "No reply:" in result["final_response"]


def test_run_conversation_partial_stream_recovery_surfaces_explanation():
"""A long recovered partial stream still needs the visible footer.

Without this, the gateway marks the turn as previewed and suppresses
the final send, leaving messaging users with a fragment and no reason.
"""
agent = _make_agent(max_iterations=10)
empty_stub = _mock_response(content=None, finish_reason="stop")
recovered = (
"I inspected the running gateway and found that the current turn "
"stopped after the provider stream timed out."
)

def _fake_api_call(_api_kwargs):
agent._current_streamed_assistant_text = recovered
return empty_stub

with (
patch.object(agent, "_interruptible_api_call", side_effect=_fake_api_call),
patch.object(agent, "_persist_session"),
patch.object(agent, "_save_trajectory"),
patch.object(agent, "_cleanup_task_resources"),
):
result = agent.run_conversation("do something")

assert result["turn_exit_reason"] == "partial_stream_recovery"
assert result["final_response"].startswith(recovered)
assert "No reply:" in result["final_response"]
assert result["response_previewed"] is False


def test_run_conversation_normal_reply_stays_quiet():
"""A normal short reply like 'Done.' must NOT get an explainer footer."""
agent = _make_agent(max_iterations=10)
Expand Down
Loading