fix(agent_health): read cross-container gateway runtime status from gateway_state.json (#5065)
In cross-container Docker setups the agent-health check resolves the gateway's runtime status from a shared gateway_state.json. After the keyword-form read_runtime_status(pid_path=...) call fails against an older agent signature, _read_gateway_runtime_status fell back to read_runtime_status(pid_path) passing the sibling gateway.pid path positionally — so the reader parsed PID metadata as runtime metadata and gateway_state/updated_at never resolved, breaking the freshness check (issue #5030, #1879). The positional fallback now passes the correct gateway_state.json sibling path (pid_path.with_name(runtime_status_file), where runtime_status_file comes from the agent's _RUNTIME_STATUS_FILE attr, defaulting to gateway_state.json). The path computation is hoisted so both fallback branches use the correct file. The keyword happy-path is unchanged for agents that accept pid_path=..., and normal PID-based health is unaffected. Verified: the new test test_cross_container_runtime_status_reads_sibling_runtime_file fails on master without the fix and passes with it; #716 PID-health tests still pass. Co-authored-by: rodboev <rodboev@users.noreply.github.com>
This commit is contained in:
1 parent
16a6827f73
commit
3ad07ba363
3 files changed
+58
-5
No files matched your search
@@ -5,6 +5,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Cross-container Docker setups report gateway health correctly again.** When WebUI and the Hermes agent run in separate containers, the agent-health check resolves the gateway's runtime status from a shared `gateway_state.json`. After the keyword-form `read_runtime_status(pid_path=…)` call fails against an older agent signature, the positional fallback was handed the sibling `gateway.pid` path instead of the runtime-status file — so the reader parsed PID metadata as runtime metadata and `gateway_state`/`updated_at` never resolved, breaking the freshness check. The fallback now passes the correct `gateway_state.json` sibling path (`pid_path.with_name(_RUNTIME_STATUS_FILE)`), with `gateway_state.json` as the default when the agent doesn't expose the filename. The keyword happy-path and normal PID-based health are unchanged. Thanks @rodboev. (#5065, fixes #5030)
|
||||
|
||||
- **Consumed mid-turn steer markers no longer leak into the model's replayed history.** When you steer a running turn, the agent embeds an `[OUT-OF-BAND USER MESSAGE]…[/OUT-OF-BAND USER MESSAGE]` control block in the turn, and that block was being persisted into the session's `messages`/`context_messages` (appended onto a tool-result entry). On every subsequent turn WebUI replayed that stored context into the provider-facing conversation history, so the model kept re-seeing the internal control marker — wasted tokens plus stale control data that can confuse the agent. WebUI now strips consumed OOB blocks at the model-facing boundary (the `_sanitize_messages_for_api` / runs-API sanitizers, plus the reasoning-projection and compression paths that route through them), so the markers are removed on the way to the provider while staying intact in the persisted transcript for display/reload fidelity. The strip is non-mutating, idempotent, and only removes a complete open+close marker pair. Thanks @Stacey2911. (#4978)
|
||||
|
||||
- **A background skill/memory review no longer makes the open conversation briefly vanish and reappear.** The active-session external-refresh check force-reloaded the whole transcript whenever the session's `last_message_at`/`updated_at` advanced — but the post-turn background skill/memory review bumps those timestamps without adding any chat messages, so the destructive reload (`loadSession(force)` clears the message list and awaits a round-trip) made the entire conversation visibly disappear and reappear a moment later with no new content. The refresh now reloads only when the message *count* actually changed, and treats a timestamp-only bump as metadata — advancing the local last-seen marker and refreshing the lightweight sidebar list without touching the transcript. Crucially, the count check fires in **both directions**: a *lower* remote count (another tab/client truncated, undid, retried, or regenerated the transcript) still force-reloads, so this tab can never silently keep a stale conversation. Thanks @allenliang2022. (#5061)
|
||||
|
||||
+6
-5
@@ -217,17 +217,18 @@ def _read_gateway_runtime_status(gateway_status: Any, pid_path: Path | None) ->
|
||||
try:
|
||||
return read_runtime_status(pid_path=pid_path)
|
||||
except TypeError:
|
||||
runtime_status_file = str(
|
||||
getattr(gateway_status, "_RUNTIME_STATUS_FILE", _GATEWAY_RUNTIME_STATUS_FILE)
|
||||
)
|
||||
runtime_status_path = pid_path.with_name(runtime_status_file)
|
||||
try:
|
||||
return read_runtime_status(pid_path)
|
||||
return read_runtime_status(runtime_status_path)
|
||||
except TypeError:
|
||||
if getattr(gateway_status, "__name__", "") == "gateway.status" or hasattr(
|
||||
gateway_status,
|
||||
"_read_json_file",
|
||||
):
|
||||
runtime_status_file = str(
|
||||
getattr(gateway_status, "_RUNTIME_STATUS_FILE", _GATEWAY_RUNTIME_STATUS_FILE)
|
||||
)
|
||||
runtime_status = _read_runtime_status_path(pid_path.with_name(runtime_status_file))
|
||||
runtime_status = _read_runtime_status_path(runtime_status_path)
|
||||
if runtime_status is not None:
|
||||
return runtime_status
|
||||
return read_runtime_status()
|
||||
|
||||
@@ -28,6 +28,7 @@ These tests pin every behavior the fix promises:
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
import pytest
|
||||
@@ -46,6 +47,19 @@ class _FakeGatewayStatus:
|
||||
return self._running_pid
|
||||
|
||||
|
||||
class _PathReadingGatewayStatus:
|
||||
def __init__(self):
|
||||
self.read_paths = []
|
||||
|
||||
def read_runtime_status(self, path=None):
|
||||
self.read_paths.append(path)
|
||||
return json.loads(path.read_text(encoding="utf-8"))
|
||||
|
||||
def get_running_pid(self, cleanup_stale=False):
|
||||
assert cleanup_stale is False
|
||||
return None
|
||||
|
||||
|
||||
def _runtime_status(updated_at: str | None, **overrides):
|
||||
payload = {
|
||||
"gateway_state": "running",
|
||||
@@ -116,6 +130,42 @@ def test_cross_container_alive_path_does_not_leak_raw_process_fields(monkeypatch
|
||||
# -- Stale / missing / malformed timestamps -----------------------------------
|
||||
|
||||
|
||||
def test_cross_container_runtime_status_reads_sibling_runtime_file(monkeypatch, tmp_path):
|
||||
"""#5030: the positional fallback must open gateway_state.json, not gateway.pid."""
|
||||
from api import agent_health
|
||||
|
||||
pid_path = tmp_path / "gateway.pid"
|
||||
runtime_status_path = tmp_path / "gateway_state.json"
|
||||
fresh_ts = _iso(datetime.now(timezone.utc) - timedelta(seconds=30))
|
||||
|
||||
pid_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"pid": 2468,
|
||||
"command": "hermes-agent --gateway",
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
runtime_status_path.write_text(
|
||||
json.dumps(_runtime_status(fresh_ts)),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
gateway_status = _PathReadingGatewayStatus()
|
||||
monkeypatch.setattr(agent_health, "_gateway_status_module", lambda: gateway_status)
|
||||
monkeypatch.setattr(agent_health, "_gateway_root_pid_path", lambda: pid_path)
|
||||
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is True
|
||||
assert payload["details"]["state"] == "alive"
|
||||
assert payload["details"]["reason"] == "cross_container_freshness"
|
||||
assert payload["details"]["gateway_state"] == "running"
|
||||
assert payload["details"]["updated_at"] == fresh_ts
|
||||
assert gateway_status.read_paths == [runtime_status_path]
|
||||
|
||||
|
||||
def test_stale_updated_at_with_running_state_reports_unknown(monkeypatch):
|
||||
"""Older gateways may not refresh the file while still processing messages."""
|
||||
from api import agent_health
|
||||
|
||||
Reference in new issue
Block a user