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
9 changes: 8 additions & 1 deletion plugins/platforms/photon/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,14 @@ async def _start_sidecar(self) -> None:
from hermes_cli._subprocess_compat import windows_hide_flags

try:
patch = subprocess.run( # noqa: S603
# Off the event loop, for the same reason the dep reinstall above
# hops to a thread: this spawns node and *waits* for it (up to 10s).
# Run inline it holds the shared gateway loop for that whole window,
# so every other platform's traffic stalls β€” and _start_sidecar runs
# on every reconnect (connect(is_reconnect=True)), not just startup,
# so the stall recurs on a live gateway.
patch = await asyncio.to_thread(
subprocess.run, # noqa: S603
[
self._node_bin,
str(_SIDECAR_DIR / "patch-spectrum-mixed-attachments.mjs"),
Expand Down
67 changes: 67 additions & 0 deletions tests/plugins/platforms/photon/test_sidecar_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,70 @@ class _Resp:
assert kwargs["env"]["PHOTON_SIDECAR_WATCH_STDIN"] == "1"
assert spawned["patch_kwargs"]["creationflags"] == hidden_flags
assert kwargs["creationflags"] == hidden_flags


@pytest.mark.asyncio
async def test_spectrum_patch_runs_off_the_event_loop(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The node patch run must not block the shared gateway event loop.

``_start_sidecar`` spawns the Spectrum patch script and *waits* for it
(``timeout=10``). Run inline it holds the loop for that whole window, so
every other platform's traffic stalls β€” and ``_start_sidecar`` runs on
every reconnect (``connect(is_reconnect=True)``), not just startup, so the
stall recurs on a live gateway. The dep reinstall a few lines above already
hops to a thread for exactly this reason; the patch run must too.
"""
import threading

adapter = _make_adapter(monkeypatch)
main_thread = threading.current_thread()
seen: Dict[str, Any] = {}

# node_modules present + deps fresh, so we reach the patch run.
monkeypatch.setattr(photon_adapter.Path, "exists", lambda self: True)
monkeypatch.setattr(photon_adapter, "_sidecar_deps_stale", lambda: False)

async def _no_reap() -> None:
return None

monkeypatch.setattr(adapter, "_reap_stale_sidecar", _no_reap)

def _fake_run(*a: Any, **k: Any) -> Any:
seen["thread"] = threading.current_thread()

class _Done:
returncode = 0
stdout = ""
stderr = ""

return _Done()

monkeypatch.setattr(photon_adapter.subprocess, "run", _fake_run)

class _FakeProc:
pid = 4242
stdin = None
stdout = None

def poll(self) -> None:
return None

monkeypatch.setattr(
photon_adapter.subprocess, "Popen", lambda *a, **k: _FakeProc()
)

try:
await adapter._start_sidecar()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This awaits the full readiness path with an always-live fake process but no mocked httpx.AsyncClient, so it can probe localhost and sleep through the 15-second deadline before this broad handler hides the timeout. Please mock a 200 health response as the existing lifecycle test does above, then await normally.

except Exception:
# Readiness/handshake past the patch run may fail under the fakes β€”
# irrelevant here; we only assert where the patch run executed.
pass

assert seen.get("thread") is not None, "patch run never executed"
assert seen["thread"] is not main_thread, (
"Spectrum patch subprocess ran on the event-loop thread; it must be "
"dispatched via asyncio.to_thread so a 10s node spawn can't freeze "
"every other platform on the gateway loop"
)
Loading