mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(photon): auto-reinstall stale sidecar deps before start
A `hermes update` that bumps the spectrum-ts pin rewrites the Photon sidecar's package-lock.json but never reinstalls node_modules. The sidecar then spawns against the old install and the v8 postinstall patch throws "@spectrum-ts/imessage dist not found", so the gateway retries the photon platform every 300s forever without ever repairing the deps. Observed in the wild: a June pin bump to spectrum-ts 8.0.0 left node_modules at 3.1.0, and inbound/outbound iMessage stayed dead for days with the reconnect loop faithfully restarting into the identical broken state. _start_sidecar only checked that node_modules exists, not that it matches the lockfile, so restart never became repair. Detect the skew with the same signal npm ci uses: the top-level package-lock.json being newer than npm's node_modules/.package-lock.json install marker. When stale, reinstall (npm ci, falling back to npm install) before spawning. The reinstall runs via asyncio.to_thread so a cold install can't block the event loop and stall every other platform's traffic; worst case it heals on the next reconnect tick instead. First-run "deps not installed" behavior is unchanged, and a missing/unreadable marker fails safe to "not stale" so start is never blocked. Reuses the existing npm ci -> npm install fallback from `hermes photon install-sidecar`. Adds unit tests for the staleness signal (stale / fresh / missing-marker).
This commit is contained in:
parent
ede7e31636
commit
3cd93f6aa8
2 changed files with 122 additions and 0 deletions
|
|
@ -137,6 +137,64 @@ def check_requirements() -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def _sidecar_deps_stale() -> bool:
|
||||
"""True when node_modules exists but is older than the committed lockfile.
|
||||
|
||||
`hermes update` rewrites ``package-lock.json`` when the spectrum-ts pin is
|
||||
bumped, but does not reinstall ``node_modules``. npm records the state of
|
||||
the last install in ``node_modules/.package-lock.json``; when the top-level
|
||||
lockfile is newer than that marker, the install is out of date. This is the
|
||||
same signal ``npm ci`` uses. Returns False (do nothing) if either file is
|
||||
missing or unreadable, so a first-run or odd filesystem never blocks start.
|
||||
"""
|
||||
lockfile = _SIDECAR_DIR / "package-lock.json"
|
||||
marker = _SIDECAR_DIR / "node_modules" / ".package-lock.json"
|
||||
try:
|
||||
return lockfile.stat().st_mtime > marker.stat().st_mtime
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _reinstall_sidecar_deps() -> None:
|
||||
"""Reinstall the sidecar's node_modules from the lockfile (blocking).
|
||||
|
||||
Mirrors ``hermes photon install-sidecar``: ``npm ci`` for an exact,
|
||||
reproducible install, falling back to ``npm install`` if the lockfile is
|
||||
missing or drifted. Runs the postinstall patch as part of the install.
|
||||
Best-effort — a failure here just leaves the (stale) deps in place and the
|
||||
normal ``_start_sidecar`` readiness check reports the real error.
|
||||
"""
|
||||
npm = shutil.which("npm")
|
||||
if not npm:
|
||||
logger.warning("[photon] cannot reinstall stale sidecar deps: npm not on PATH")
|
||||
return
|
||||
result = subprocess.run( # noqa: S603
|
||||
[npm, "ci"],
|
||||
cwd=str(_SIDECAR_DIR),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
logger.warning(
|
||||
"[photon] sidecar `npm ci` failed; falling back to `npm install`"
|
||||
)
|
||||
result = subprocess.run( # noqa: S603
|
||||
[npm, "install"],
|
||||
cwd=str(_SIDECAR_DIR),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
logger.error(
|
||||
"[photon] sidecar dependency reinstall failed: %s",
|
||||
(result.stderr or result.stdout or "").strip(),
|
||||
)
|
||||
else:
|
||||
logger.info("[photon] sidecar dependencies reinstalled from lockfile")
|
||||
|
||||
|
||||
def validate_config(cfg: PlatformConfig) -> bool:
|
||||
extra = cfg.extra or {}
|
||||
project_id = extra.get("project_id") or os.getenv("PHOTON_PROJECT_ID")
|
||||
|
|
@ -841,6 +899,19 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
f"Photon sidecar deps not installed. Run: "
|
||||
f"cd {_SIDECAR_DIR} && npm install (or `hermes photon setup`)"
|
||||
)
|
||||
# A `hermes update` that bumps the spectrum-ts pin rewrites
|
||||
# package-lock.json but never reinstalls node_modules, so the sidecar
|
||||
# spawns against stale deps and dies on every reconnect (the v8 patch
|
||||
# script can't find @spectrum-ts/imessage/dist that only v8 ships).
|
||||
# Self-heal by reinstalling when the lockfile is newer than npm's
|
||||
# install marker. Runs off the event loop so a cold install can't
|
||||
# freeze every other platform's traffic.
|
||||
if _sidecar_deps_stale():
|
||||
logger.warning(
|
||||
"[photon] sidecar deps are stale (lockfile newer than install); "
|
||||
"reinstalling before start"
|
||||
)
|
||||
await asyncio.to_thread(_reinstall_sidecar_deps)
|
||||
await self._reap_stale_sidecar()
|
||||
|
||||
env = os.environ.copy()
|
||||
|
|
|
|||
51
tests/plugins/platforms/photon/test_sidecar_deps_stale.py
Normal file
51
tests/plugins/platforms/photon/test_sidecar_deps_stale.py
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
"""Regression tests for the Photon sidecar stale-dependency self-heal.
|
||||
|
||||
A `hermes update` that bumps the spectrum-ts pin rewrites the sidecar's
|
||||
``package-lock.json`` but never reinstalls ``node_modules``, so the sidecar
|
||||
spawns against stale deps and dies on every reconnect. ``_sidecar_deps_stale``
|
||||
detects that skew (lockfile newer than npm's install marker) so
|
||||
``_start_sidecar`` can reinstall before spawning.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import plugins.platforms.photon.adapter as photon_adapter
|
||||
|
||||
|
||||
def _seed(sidecar: Path, *, lock_mtime: float, marker_mtime: float | None) -> None:
|
||||
"""Create a fake sidecar dir with a lockfile and (optionally) npm's marker."""
|
||||
(sidecar / "node_modules").mkdir(parents=True)
|
||||
lock = sidecar / "package-lock.json"
|
||||
lock.write_text("{}", encoding="utf-8")
|
||||
os.utime(lock, (lock_mtime, lock_mtime))
|
||||
if marker_mtime is not None:
|
||||
marker = sidecar / "node_modules" / ".package-lock.json"
|
||||
marker.write_text("{}", encoding="utf-8")
|
||||
os.utime(marker, (marker_mtime, marker_mtime))
|
||||
|
||||
|
||||
def test_stale_when_lockfile_newer_than_marker(tmp_path, monkeypatch) -> None:
|
||||
"""The update-rewrites-lockfile-but-skips-install case must reinstall."""
|
||||
sidecar = tmp_path / "sidecar"
|
||||
_seed(sidecar, lock_mtime=2000.0, marker_mtime=1000.0)
|
||||
monkeypatch.setattr(photon_adapter, "_SIDECAR_DIR", sidecar)
|
||||
assert photon_adapter._sidecar_deps_stale() is True
|
||||
|
||||
|
||||
def test_fresh_when_marker_newer_than_lockfile(tmp_path, monkeypatch) -> None:
|
||||
"""A normal install (marker at/after lockfile) must NOT trigger a reinstall."""
|
||||
sidecar = tmp_path / "sidecar"
|
||||
_seed(sidecar, lock_mtime=1000.0, marker_mtime=2000.0)
|
||||
monkeypatch.setattr(photon_adapter, "_SIDECAR_DIR", sidecar)
|
||||
assert photon_adapter._sidecar_deps_stale() is False
|
||||
|
||||
|
||||
def test_not_stale_when_marker_missing(tmp_path, monkeypatch) -> None:
|
||||
"""No marker (first run / unreadable) must fail safe to False, never block start."""
|
||||
sidecar = tmp_path / "sidecar"
|
||||
_seed(sidecar, lock_mtime=2000.0, marker_mtime=None)
|
||||
monkeypatch.setattr(photon_adapter, "_SIDECAR_DIR", sidecar)
|
||||
assert photon_adapter._sidecar_deps_stale() is False
|
||||
Loading…
Add table
Add a link
Reference in a new issue