From 3cd93f6aa8c9cd0bf0799cd0eb195c508420c1a8 Mon Sep 17 00:00:00 2001 From: Jash Lee Date: Fri, 3 Jul 2026 17:42:07 -0400 Subject: [PATCH] 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). --- plugins/platforms/photon/adapter.py | 71 +++++++++++++++++++ .../photon/test_sidecar_deps_stale.py | 51 +++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/plugins/platforms/photon/test_sidecar_deps_stale.py diff --git a/plugins/platforms/photon/adapter.py b/plugins/platforms/photon/adapter.py index d6e627f667b..d6b67eab102 100644 --- a/plugins/platforms/photon/adapter.py +++ b/plugins/platforms/photon/adapter.py @@ -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() diff --git a/tests/plugins/platforms/photon/test_sidecar_deps_stale.py b/tests/plugins/platforms/photon/test_sidecar_deps_stale.py new file mode 100644 index 00000000000..b52944959b2 --- /dev/null +++ b/tests/plugins/platforms/photon/test_sidecar_deps_stale.py @@ -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