hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(whatsapp): body-read timeouts, Windows and failed /health keep ownership

Browse Source 
Gaps in the unanswered-/health handling, found in a second review:

- _probe_bridge_health swallowed a timeout while reading the JSON body
  and reported an unparseable 200, so a bridge that sent headers but
  stalled on the body still reached _kill_port_process. Timeouts now
  propagate from either phase.
- The guard listed listeners with the POSIX-only lsof/ss scan, which
  finds nothing on Windows, so Windows fell through to the netstat-based
  kill. Both now use _port_listener_pids(); if listing fails, nothing
  is killed.
- _standalone_send treated a non-200 /health as an older bridge and
  sent anyway. It now sends nothing.
- Docs named a nonexistent `hermes whatsapp send`; the command is
  `hermes send --to whatsapp:<chat_id>`.

Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
This commit is contained in:
unsupportedpastels
2026-10-04 02:38:25 +00:00
committed by Mark S.
parent e1cc967c29
commit 20bd00439c
4 changed files with 48 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
plugins/platforms/whatsapp/adapter.py
View File

@@ -72,6 +72,11 @@ def _windows_listener_pids(port: int) -> list:
return _safe_ints(p[4] for p in rows if len(p) >= 5 and p[3] == "LISTENING" and p[1].endswith(f":{port}"))
def _port_listener_pids(port: int) -> list:
"""Listening PIDs on ``port`` for this platform: netstat on Windows, lsof/ss elsewhere."""
return _windows_listener_pids(port) if _IS_WINDOWS else _listener_pids_on_port(port)
def _pid_looks_like_node_bridge(pid: int) -> bool:
"""Fail-closed: the live process must be a ``node`` executable (a scan-time PID can be a stranger by kill time).
@@ -92,7 +97,7 @@ def _pid_looks_like_node_bridge(pid: int) -> bool:
def _kill_port_process(port: int) -> None:
"""Kill any node bridge *listening* on the given TCP port (never a client); SIGTERM on POSIX, taskkill /F on Windows."""
with suppress(Exception):
for pid in (_windows_listener_pids(port) if _IS_WINDOWS else _listener_pids_on_port(port)):
for pid in _port_listener_pids(port):
# Killing a mistyped or recycled PID is unrecoverable — verify first.
if pid <= 0 or not _pid_looks_like_node_bridge(pid):
logger.warning("[whatsapp] Not killing PID %s on port %d: process is not a node bridge (or identity unverifiable)", pid, port)
@@ -350,13 +355,15 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
return getattr(self._http_session, method)(self._bridge_url(path), **kwargs, timeout=aiohttp.ClientTimeout(total=timeout))
async def _probe_bridge_health(self) -> tuple[bool, Any]:
"""GET /health with a fresh session → ``(http_200, json)``; unparseable 200 body → ``(True, None)``; connection errors propagate."""
"""GET /health with a fresh session → ``(http_200, json)``; unparseable 200 body → ``(True, None)``; connection errors and timeouts propagate."""
import aiohttp
async with aiohttp.ClientSession() as session, session.get(self._bridge_url("health"), timeout=aiohttp.ClientTimeout(total=2)) as resp:
if resp.status != 200:
return False, None
try:
return True, await resp.json()
except asyncio.TimeoutError:
raise # a body that never arrives is no answer, the same as headers that never arrive
except Exception:
return True, None
@@ -600,7 +607,11 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
# No /health answer is no ownership evidence (another profile's busy bridge looks the same), and
# our own stale bridge was just reaped by its pidfile: a holder that remains is left running.
await asyncio.sleep(1)
if _listener_pids_on_port(self._bridge_port):
try:
held = bool(_port_listener_pids(self._bridge_port))
except Exception:
held = True # listeners could not be listed: still no evidence to kill on
if held:
self._set_fatal_error(
"whatsapp_bridge_unresponsive",
f"Port {self._bridge_port} is held by a process that did not answer /health in time, so "
@@ -1059,7 +1070,10 @@ async def _standalone_send(pconfig, chat_id, message, *, thread_id=None, media_f
f"http://localhost:{bridge_port}/health",
timeout=aiohttp.ClientTimeout(total=5),
) as resp:
health = await resp.json() if resp.status == 200 else {}
if resp.status != 200:
return send_error(
f"WhatsApp bridge on port {bridge_port} answered /health with HTTP {resp.status}; nothing was sent.")
health = await resp.json()
reported_session = health.get("session")
if reported_session and reported_session != own_session:
return send_error(

28
tests/gateway/test_whatsapp_stale_bridge.py
View File

@@ -18,7 +18,6 @@ package.json changes, not only when node_modules is missing.
import asyncio
import os
from contextlib import nullcontext
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch
@@ -83,13 +82,14 @@ def _make_adapter(bridge_script: str = "/tmp/test-bridge.js",
return adapter
def _mock_health(json_data):
"""Mock aiohttp.ClientSession whose GET returns 200 + *json_data*."""
def _mock_health(json_data, raise_on=None):
"""Mock aiohttp.ClientSession whose GET returns 200 + *json_data*; ``raise_on`` "headers"/"body" times out that phase."""
mock_resp = MagicMock()
mock_resp.status = 200
mock_resp.json = AsyncMock(return_value=json_data)
mock_resp.json = AsyncMock(return_value=json_data, side_effect=asyncio.TimeoutError if raise_on == "body" else None)
mock_session = MagicMock()
mock_session.get = MagicMock(return_value=_AsyncCM(mock_resp))
mock_session.get = MagicMock(
return_value=_AsyncCM(mock_resp), side_effect=asyncio.TimeoutError if raise_on == "headers" else None)
mock_session.close = AsyncMock()
return MagicMock(return_value=_AsyncCM(mock_session))
@@ -162,8 +162,11 @@ class TestStaleBridgeHandshake:
("own", "connected", None),
("other", "connected", "whatsapp_bridge_foreign_session"),
("other", "disconnected", "whatsapp_bridge_foreign_session"),
("unanswered", None, "whatsapp_bridge_unresponsive"),
], ids=["own-session-adopted", "other-session-connected", "other-session-starting", "no-health-answer"])
("headers-timeout", None, "whatsapp_bridge_unresponsive"),
("body-timeout", None, "whatsapp_bridge_unresponsive"),
("body-timeout-windows", None, "whatsapp_bridge_unresponsive"),
], ids=["own-session-adopted", "other-session-connected", "other-session-starting",
"health-headers-timeout", "health-body-timeout", "health-body-timeout-windows"])
async def test_bridge_is_adopted_or_left_running_by_its_session(self, tmp_path, holder, status, fatal):
"""Two profiles default to one bridge_port; only this profile's own session may be adopted. Another profile's
bridge is never killed, even while it reports ``disconnected`` (startup, reconnect, QR wait), and neither is a
@@ -183,19 +186,20 @@ class TestStaleBridgeHandshake:
"scriptHash": _file_content_hash(bridge_dir / "bridge.js"),
"sendReadReceipts": False,
"session": str(reported),
}
},
raise_on=holder.split("-")[0] if "timeout" in holder else None,
)
unanswered = (patch.object(adapter, "_probe_bridge_health", AsyncMock(side_effect=asyncio.TimeoutError))
if holder == "unanswered" else nullcontext())
windows = holder.endswith("-windows") # the listener scan must be the platform's own, or Windows still kills
with (
unanswered,
patch("plugins.platforms.whatsapp.adapter.check_whatsapp_requirements", return_value=True),
patch("aiohttp.ClientSession", mock_client),
patch("plugins.platforms.whatsapp.adapter.asyncio.sleep", new_callable=AsyncMock),
patch("plugins.platforms.whatsapp.adapter._kill_stale_bridge_by_pidfile") as mock_kill_pidfile,
patch("plugins.platforms.whatsapp.adapter._kill_port_process") as mock_kill_port,
patch("plugins.platforms.whatsapp.adapter._listener_pids_on_port", return_value=[4242]),
patch("plugins.platforms.whatsapp.adapter._IS_WINDOWS", windows),
patch("plugins.platforms.whatsapp.adapter._listener_pids_on_port", return_value=[] if windows else [4242]),
patch("plugins.platforms.whatsapp.adapter._windows_listener_pids", return_value=[4242] if windows else []),
patch("subprocess.Popen", return_value=MagicMock()) as mock_popen,
patch.object(adapter, "_acquire_platform_lock", return_value=True, create=True),
):

12
tests/tools/test_whatsapp_send_message_media.py
View File

@@ -233,23 +233,25 @@ def test_secondary_standalone_sends_use_active_profile_port_for_text_media_and_m
assert record.read_text(encoding="utf-8") == str(persisted_port)
@pytest.mark.parametrize("reported", ["own", "other", None], ids=["own-session", "other-profile-session", "pre-session-bridge"])
@pytest.mark.parametrize("reported", ["own", "other", None, "unhealthy"],
ids=["own-session", "other-profile-session", "pre-session-bridge", "health-503"])
def test_standalone_send_posts_only_through_this_profiles_bridge(tmp_path, reported):
"""Profiles sharing a bridge_port must not send from each other's WhatsApp account, text or media."""
own = tmp_path / "b" / "session"
media = tmp_path / "image.png"
media.write_bytes(b"image")
health = {"session": str(own if reported == "own" else tmp_path / "a" / "session")} if reported else {}
sessions = {"own": own, "other": tmp_path / "a" / "session"}
health = _resp(503) if reported == "unhealthy" else _resp(200, {"session": str(sessions[reported])} if reported else {})
session_ctx, calls = _session_with(
[_resp(200, {"messageId": "text"}), _resp(200, {"messageId": "media"})], health=_resp(200, health))
[_resp(200, {"messageId": "text"}), _resp(200, {"messageId": "media"})], health=health)
with patch("aiohttp.ClientSession", return_value=session_ctx):
result = asyncio.run(_standalone_send(
SimpleNamespace(token="", extra={"bridge_port": 3000, "session_path": str(own)}), "12345", "hello",
media_files=[(str(media), False)],
))
posted = [url for url, _ in calls if not url.endswith("/health")]
if reported == "other":
assert "another profile's session" in result["error"]
if reported in ("other", "unhealthy"):
assert "nothing was sent" in result["error"]
assert posted == []
else:
assert result.get("success") is True, result

13
website/docs/user-guide/messaging/whatsapp.md
View File

@@ -44,7 +44,7 @@ record claims, and saves it in its own `platforms/whatsapp/bridge_port` file for
subsequent starts. Operators can pre-create that file with a port number; delete
it to have a new port allocated. The launch profile uses port 3000 unless it
already has that file (from serving as a secondary), in which case its gateway
and `hermes whatsapp send` both keep using the recorded port.
and `hermes send --to whatsapp:<chat_id>` both keep using the recorded port.
A secondary adopts a bridge already running on its port only when its own
session pidfile identifies that process (pid, kernel start time, and the port it
@@ -70,11 +70,12 @@ platforms:
A gateway identifies a running bridge by the session directory the bridge
reports in `/health`. A bridge serving another profile's session is never
adopted and never stopped: the second profile's WhatsApp fails to start with
`whatsapp_bridge_foreign_session`, naming the port and the other session. On
Linux and macOS, a process that holds the port but does not answer `/health` in
time is left running too, and WhatsApp fails with the retryable
`whatsapp_bridge_unresponsive`. `hermes whatsapp send` and cron delivery check
the same field and refuse to send through another profile's bridge. If you
`whatsapp_bridge_foreign_session`, naming the port and the other session. A
process that holds the port but does not answer `/health` in time is left
running too, and WhatsApp fails with the retryable
`whatsapp_bridge_unresponsive`. `hermes send --to whatsapp:<chat_id>` and cron
delivery check the same field and send nothing through another profile's bridge
or through one whose `/health` fails. If you
override `session_path`, keep it distinct per profile, or the profiles share one
WhatsApp login. Bridges started by an older Hermes report no session directory
and are restarted once, as after a bridge update.