diff --git a/gateway/platforms/whatsapp_common.py b/gateway/platforms/whatsapp_common.py index 09e7b0b64f886..0ebe6133d2f2c 100644 --- a/gateway/platforms/whatsapp_common.py +++ b/gateway/platforms/whatsapp_common.py @@ -35,6 +35,7 @@ import json import logging import os import re +from pathlib import Path from typing import Any, Dict, Optional from agent.secret_scope import UnscopedSecretError as _UnscopedSecretError @@ -503,50 +504,19 @@ class WhatsAppBehaviorMixin: # Shared bridge directory resolution for CLI and adapter # --------------------------------------------------------------------------- +SOURCE_BRIDGE_DIR = Path(__file__).resolve().parents[2] / "scripts" / "whatsapp-bridge" + + def resolve_whatsapp_bridge_dir() -> Path: - """Resolve the WhatsApp bridge directory, mirroring to HERMES_HOME if needed. + """Return the directory the WhatsApp bridge should run from. - When the install tree is read-only (e.g., Docker /opt/hermes), this function - mirrors the bridge source to a writable HERMES_HOME location and returns that - path. This ensures npm install works in Docker environments. + gateway.sidecar_runtime holds the rungs and the reason a read-only tree + has to copy the bridge somewhere writable. - Returns the resolved bridge directory path. + The bridge keeps its Baileys credentials in ``$HERMES_HOME/whatsapp/session``, + which is a separate directory, so where the bridge itself runs from does + not affect a paired account. """ - import shutil - from pathlib import Path as _Path + from gateway.sidecar_runtime import resolve_sidecar - # Default location in install tree (may be read-only) - from hermes_constants import get_hermes_home - install_bridge = _Path(__file__).resolve().parents[2] / "scripts" / "whatsapp-bridge" - - # Try HERMES_HOME location first - hermes_home = get_hermes_home() - hermes_home_bridge = hermes_home / "scripts" / "whatsapp-bridge" - - # Check if install dir is writable - try: - test_file = install_bridge / ".write_test" - test_file.touch() - test_file.unlink() - install_writable = True - except (OSError, PermissionError): - install_writable = False - - if install_writable: - return install_bridge - - # Install dir is read-only, mirror to HERMES_HOME if needed - if hermes_home_bridge.exists(): - return hermes_home_bridge - - # Mirror the bridge source to HERMES_HOME - try: - hermes_home_bridge.parent.mkdir(parents=True, exist_ok=True) - shutil.copytree( - install_bridge, - hermes_home_bridge, - dirs_exist_ok=False, - ) - return hermes_home_bridge - except Exception: - return install_bridge + return resolve_sidecar("whatsapp", SOURCE_BRIDGE_DIR) diff --git a/gateway/sidecar_runtime.py b/gateway/sidecar_runtime.py new file mode 100644 index 0000000000000..38b3128e2a808 --- /dev/null +++ b/gateway/sidecar_runtime.py @@ -0,0 +1,157 @@ +"""Resolve the directory a Node sidecar runs from. + +Hermes ships two Node sidecars: the Photon iMessage bridge +(``plugins/platforms/photon/sidecar``) and the WhatsApp Baileys bridge +(``scripts/whatsapp-bridge``). Both need a ``node_modules`` beside their entry +file, and some installs put the source tree somewhere nothing can write. + +Node decides the shape of the answer. Its ESM resolver reads ``node_modules`` +only from directories above the importing file. ``NODE_PATH`` applies to +CommonJS and not to ESM, and both sidecars declare ``"type": "module"``, so +there is no way to leave the code in a read-only tree and point Node at +packages held elsewhere. The entry file and the packages have to share a tree. +That is why a read-only install with no usable deps copies the sidecar to a +writable directory: it is the only arrangement Node accepts, not a preference. + +Order: + +1. ``HERMES__SIDECAR_DIR`` — an operator override, used as given. +2. Source writable → run in place. Dev checkouts, and any plugin a user + installs under ``$HERMES_HOME/plugins``. +3. Source read-only, deps present and matching the lockfile → run in place. + The Nix store and the container image both arrive this way, and the + sidecar never writes inside its own directory. +4. Source read-only, deps missing or stale → copy the sidecar to + ``$HERMES_HOME/sidecars/`` and hand back that path, where the + caller's usual npm install can work. + +Rung 4 copies the whole tree apart from ``node_modules``. It does not consult +a list of files to copy: such a list has to name every module the entry file +imports, and the one this replaces was wrong twice, each time in a way that +only appears on a read-only install. +""" +from __future__ import annotations + +import filecmp +import logging +import os +import shutil +from pathlib import Path + +logger = logging.getLogger(__name__) + +_DEPS_DIR = "node_modules" +# npm writes this inside node_modules to record the state of the last install. +# Comparing it with the committed lockfile is the check `npm ci` runs. +_INSTALL_MARKER = ".package-lock.json" + + +def override_env_var(name: str) -> str: + """Return the override variable for sidecar ``name``.""" + return f"HERMES_{name.upper().replace('-', '_')}_SIDECAR_DIR" + + +def dir_writable(path: Path) -> bool: + """Can Hermes create a file in ``path``? + + Probe with a real create and delete. A stat of the mode bits gives the + wrong answer under root-squash and on a read-only bind mount, which are + the cases this function exists to detect. + """ + probe = path / ".hermes-write-probe" + try: + probe.touch() + probe.unlink() + return True + except OSError: + return False + + +def deps_are_current(sidecar_dir: Path) -> bool: + """Does the install in ``sidecar_dir`` match its lockfile? + + False when either file is absent or unreadable, so a first run and an odd + filesystem both resolve to "install needed" rather than to an error. + + ``plugins.platforms.photon.adapter._sidecar_deps_stale`` reads the same + two files with the opposite missing-file answer, on purpose: there the + missing case belongs to ``sidecar_deps_installed``. + """ + lockfile = sidecar_dir / "package-lock.json" + marker = sidecar_dir / _DEPS_DIR / _INSTALL_MARKER + try: + return marker.stat().st_mtime >= lockfile.stat().st_mtime + except OSError: + return False + + +def _refresh_mirror(source: Path, mirror: Path) -> None: + """Copy ``source`` into ``mirror``, without ``node_modules``. + + Runs on each resolve, so an image update reaches a mirror that already + exists. Compares content rather than mtime, because a copy has the mtime + of the copy. ``node_modules`` stays out: npm owns the mirror's copy, and + replacing it would make each update a fresh install. + """ + mirror.mkdir(parents=True, exist_ok=True) + shutil.copytree( + source, + mirror, + ignore=shutil.ignore_patterns(_DEPS_DIR), + copy_function=_copy_if_changed, + dirs_exist_ok=True, + ) + + +def _copy_if_changed(src: str, dst: str) -> None: + """Copy, skipped when the destination already holds the same bytes. + + ``shutil.copy``, not ``copy2``. copy2 gives the destination the mtime + of the SOURCE, and a Nix store source has mtime = epoch. A refreshed + mirror lockfile with an epoch mtime always looks older than npm's + install marker, so ``deps_are_current`` keeps stale node_modules + through every upgrade. A plain copy stamps the file with the copy + time, so a content change always postdates the previous install. + + npm's ``node_modules/.package-lock.json`` cannot replace this content + comparison. It is a different document, and npm matches it against the + committed lockfile semantically, not byte for byte. + """ + if os.path.exists(dst) and filecmp.cmp(src, dst, shallow=False): + return + shutil.copy(src, dst) + + +def resolve_sidecar(name: str, source_dir: Path) -> Path: + """Return the directory sidecar ``name`` should run from. + + ``source_dir`` is where the sidecar ships in the install tree. + """ + source = Path(source_dir) + + override = os.getenv(override_env_var(name)) + if override: + return Path(override) + + if dir_writable(source): + return source + + if (source / _DEPS_DIR).exists() and deps_are_current(source): + return source + + from hermes_constants import get_hermes_home + + mirror = get_hermes_home() / "sidecars" / name + try: + _refresh_mirror(source, mirror) + return mirror + except OSError as exc: + logger.warning( + "[%s] the install tree is read-only and the copy to %s failed " + "(%s). Falling back to the read-only source directory, where a " + "dependency install cannot run.", + name, + mirror, + exc, + ) + return source diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 88ce3ecba73bf..bf1ea69ddf461 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -1639,6 +1639,36 @@ def run_doctor(args): fixed_count += 1 else: check_warn(f"{_DHH}/{subdir_name}/ not found", "(will be created on first use)") + + # Orphaned sidecar mirrors. The shared resolver (gateway/sidecar_runtime) + # mirrors the Node sidecars to $HERMES_HOME/sidecars/. The old + # per-sidecar resolvers used $HERMES_HOME/photon/sidecar and + # $HERMES_HOME/scripts/whatsapp-bridge. Nothing reads those paths now, + # and each one can hold a node_modules of some hundred MB. + legacy_mirrors = [ + hermes_home / "photon" / "sidecar", + hermes_home / "scripts" / "whatsapp-bridge", + ] + for legacy in legacy_mirrors: + if not legacy.is_dir() or legacy.is_symlink(): + continue + rel = legacy.relative_to(hermes_home) + if should_fix: + import shutil as _shutil + + _shutil.rmtree(legacy, ignore_errors=True) + # Drop the parent too when the mirror was its only content. + try: + legacy.parent.rmdir() + except OSError: + pass + check_ok(f"Removed orphaned sidecar mirror {_DHH}/{rel}/") + fixed_count += 1 + else: + check_warn( + f"{_DHH}/{rel}/ is an orphaned sidecar mirror", + "(replaced by ~/.hermes/sidecars/ — run `hermes doctor --fix` to remove)", + ) # Check for SOUL.md persona file soul_path = hermes_home / "SOUL.md" diff --git a/plugins/platforms/photon/adapter.py b/plugins/platforms/photon/adapter.py index 662e8ced6c599..418703648a200 100644 --- a/plugins/platforms/photon/adapter.py +++ b/plugins/platforms/photon/adapter.py @@ -464,6 +464,12 @@ def _sidecar_deps_stale() -> bool: 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. + + ``gateway.sidecar_runtime.deps_are_current`` reads the same two files with + the OPPOSITE missing-file answer: the resolver treats a missing marker as + "install needed" and mirrors, this adapter treats it as "nothing to do" + because ``sidecar_deps_installed`` owns the missing case. Keep that + difference if you merge them. """ lockfile = _sidecar_dir() / "package-lock.json" marker = _sidecar_dir() / "node_modules" / ".package-lock.json" diff --git a/plugins/platforms/photon/sidecar_paths.py b/plugins/platforms/photon/sidecar_paths.py index 69f86a145846c..2ff972fc8307a 100644 --- a/plugins/platforms/photon/sidecar_paths.py +++ b/plugins/platforms/photon/sidecar_paths.py @@ -1,32 +1,10 @@ """ Resolve where the Photon sidecar runs from and where its Node deps live. -The sidecar source ships inside the installed plugin tree -(``plugins/platforms/photon/sidecar/``). On dev/source installs that tree is -writable and everything — ``npm ci``, the spectrum patch, the sidecar itself — -happens in place. Hosted/managed images instead keep the whole install tree -under an immutable ``/opt/hermes`` (read-only for the hermes user), which -broke every install/self-heal path with EROFS (NS-606). - -Resolution order (mirrors ``resolve_whatsapp_bridge_dir`` for the Baileys -bridge, which hit the same wall): - -1. ``PHOTON_SIDECAR_DIR`` env override — operator escape hatch, used as-is. -2. Source dir writable → run in place (dev installs, unchanged behavior). -3. Source dir read-only but ``node_modules`` is baked and current → run in - place. This is the managed-image happy path: the Dockerfile bakes the - sidecar deps with ``npm ci`` at build time (deterministic installs, - NS-559), so no runtime install is ever needed. -4. Source dir read-only and deps missing or stale → mirror the sidecar - source files to ``$HERMES_HOME/photon/sidecar`` (the durable data volume, - e.g. ``/opt/data`` on hosted) and return that. The caller's normal - install/self-heal machinery then works there because it is writable. - -The mirror is refreshed on every resolve: when an image update changes a -sidecar source file, the changed file is re-copied (content compare, not -mtime) while ``node_modules`` is left in place — the adapter's existing -lockfile-vs-install-marker staleness check then triggers the ``npm ci`` -self-heal inside the mirror. +The rungs and the reason a read-only tree has to copy the sidecar are in +``gateway.sidecar_runtime``, which this module wraps. What belongs here is +the Photon-specific part: where the sidecar ships, and the +``PHOTON_SIDECAR_DIR`` override name that predates the shared resolver. This module is import-light on purpose: both ``adapter.py`` (gateway) and ``cli.py`` (``hermes photon ...``) use it. @@ -34,108 +12,42 @@ This module is import-light on purpose: both ``adapter.py`` (gateway) and from __future__ import annotations -import filecmp import logging import os -import shutil from pathlib import Path from typing import Optional +from gateway.sidecar_runtime import ( + dir_writable, + override_env_var, + resolve_sidecar, +) + logger = logging.getLogger(__name__) SOURCE_SIDECAR_DIR = Path(__file__).parent / "sidecar" -# The files that define the sidecar. Mirrored into the writable runtime dir -# when the install tree is read-only. node_modules is deliberately absent — -# it is either baked (managed image) or installed by npm in the mirror. -_MIRROR_FILES = ( - "index.mjs", - "package.json", - "package-lock.json", - "patch-spectrum-mixed-attachments.mjs", -) - - -def dir_writable(path: Path) -> bool: - """True when we can create files in ``path`` (probe-based, not stat). - - A stat-mode check lies on containers (root-squash, read-only bind - mounts), so probe with a real create+unlink like the WhatsApp bridge - resolver does. - """ - probe = path / ".hermes-write-probe" - try: - probe.touch() - probe.unlink() - return True - except OSError: - return False - +_SIDECAR_NAME = "photon" # Backwards-friendly private alias for module-internal use. _dir_writable = dir_writable -def _lock_newer_than_install(sidecar_dir: Path) -> bool: - """True when the committed lockfile postdates npm's install marker. - - Same signal as ``adapter._sidecar_deps_stale`` — duplicated here (three - lines) rather than imported so this module stays import-light for the - CLI. Returns False on any stat failure so an odd filesystem never forces - the mirror path. - """ - 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 resolve_sidecar_dir(source_dir: Optional[Path] = None) -> Path: - """Return the directory the sidecar should run from (see module doc). + """Return the directory the sidecar should run from. ``source_dir`` defaults to the installed plugin tree; tests and callers that monkeypatch the adapter's ``_SIDECAR_DIR`` pass it through so the override keeps working. + + ``PHOTON_SIDECAR_DIR`` is read as well as the shared + ``HERMES_PHOTON_SIDECAR_DIR``. Operators set the short name before the + resolver was shared, and it costs one line to keep working. """ source = Path(source_dir) if source_dir is not None else SOURCE_SIDECAR_DIR - override = os.getenv("PHOTON_SIDECAR_DIR") - if override: - return Path(override) + legacy = os.getenv("PHOTON_SIDECAR_DIR") + if legacy and not os.getenv(override_env_var(_SIDECAR_NAME)): + return Path(legacy) - if _dir_writable(source): - return source - - # Read-only install tree (hosted/managed image). If the image baked the - # deps at build time and they match the lockfile, run in place — the - # sidecar itself never writes inside its own directory. - if (source / "node_modules").exists() and not _lock_newer_than_install(source): - return source - - # Deps missing or stale inside a read-only tree: mirror to the durable - # data volume so the normal install/self-heal machinery has somewhere - # writable to work. - from hermes_constants import get_hermes_home - - mirror = get_hermes_home() / "photon" / "sidecar" - try: - mirror.mkdir(parents=True, exist_ok=True) - for name in _MIRROR_FILES: - src = source / name - if not src.exists(): - continue - dst = mirror / name - if not dst.exists() or not filecmp.cmp(str(src), str(dst), shallow=False): - shutil.copy2(str(src), str(dst)) - return mirror - except OSError as exc: - logger.warning( - "[photon] install tree is read-only and mirroring the sidecar " - "to %s failed (%s) — falling back to the read-only source dir; " - "dependency installs will not be possible", - mirror, - exc, - ) - return source + return resolve_sidecar(_SIDECAR_NAME, source) diff --git a/tests/gateway/test_sidecar_runtime.py b/tests/gateway/test_sidecar_runtime.py new file mode 100644 index 0000000000000..f6b4639fba085 --- /dev/null +++ b/tests/gateway/test_sidecar_runtime.py @@ -0,0 +1,269 @@ +"""Behaviour of the shared Node sidecar resolver. + +The resolver answers one question: which directory does a Node sidecar run +from? A sidecar needs its node_modules, and Node's ESM resolver only looks in +ancestor directories of the importing file, so the entry file and the deps +must live in one tree. NODE_PATH does not work for ESM, and both in-tree +sidecars are "type": "module". + +That constraint is why a read-only install tree with no baked deps has to copy +the sidecar somewhere writable. Nothing else Node offers gets the two into the +same tree. +""" +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from gateway.sidecar_runtime import resolve_sidecar + + +def _make_sidecar(root: Path, *, files=("index.mjs", "helper.mjs")) -> Path: + """A sidecar source dir with a lockfile and a couple of ESM helpers.""" + src = root / "src" + src.mkdir(parents=True) + (src / "package.json").write_text(json.dumps({"name": "sc", "type": "module"})) + (src / "package-lock.json").write_text(json.dumps({"lockfileVersion": 3})) + for name in files: + (src / name).write_text(f"// {name}\n") + return src + + +def _install_deps(src: Path, *, current: bool = True) -> None: + """Give the sidecar a node_modules with npm's install marker.""" + nm = src / "node_modules" + (nm / "somepkg").mkdir(parents=True) + (nm / "somepkg" / "index.js").write_text("module.exports = 1;\n") + marker = nm / ".package-lock.json" + marker.write_text("{}") + lock = src / "package-lock.json" + if current: + # Marker newer than the lockfile: the install matches. + os.utime(marker, (lock.stat().st_atime + 10, lock.stat().st_mtime + 10)) + else: + os.utime(marker, (lock.stat().st_atime - 10, lock.stat().st_mtime - 10)) + + +class TestRungOrder: + """The four rungs, in the order the resolver must try them.""" + + def test_env_override_is_used_as_is(self, tmp_path, monkeypatch): + """An operator override wins over every other rung. + + It is the escape hatch for a layout Hermes cannot predict, so the + resolver must not second-guess it. + """ + src = _make_sidecar(tmp_path) + override = tmp_path / "elsewhere" + override.mkdir() + monkeypatch.setenv("HERMES_TESTSC_SIDECAR_DIR", str(override)) + assert resolve_sidecar("testsc", src) == override + + def test_writable_source_runs_in_place(self, tmp_path, monkeypatch): + """A dev checkout installs and runs in the source tree.""" + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + assert resolve_sidecar("testsc", src) == src + + def test_readonly_source_with_current_deps_runs_in_place( + self, tmp_path, monkeypatch + ): + """Baked deps that match the lockfile need no writable directory. + + This is the Nix store and the built container image. The sidecar + never writes inside its own directory, so read-only is fine. + """ + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + _install_deps(src, current=True) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + assert resolve_sidecar("testsc", src) == src + + def test_readonly_source_with_stale_deps_mirrors(self, tmp_path, monkeypatch): + """A lockfile newer than the install means the deps must be rebuilt. + + The source tree cannot take the write, so the sidecar moves to the + durable directory where npm can run. + """ + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + _install_deps(src, current=False) + home = tmp_path / "home" + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + got = resolve_sidecar("testsc", src) + assert got != src + assert got == home / "sidecars" / "testsc" + + def test_readonly_source_with_no_deps_mirrors(self, tmp_path, monkeypatch): + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + assert resolve_sidecar("testsc", src) != src + + +class TestMirrorCarriesTheWholeSidecar: + """The mirror must be complete. A manifest of files is not enough. + + The Photon resolver kept a list of files to copy. The list drifted twice: + it named a file that had been deleted, and it omitted two helpers that + index.mjs imports. Both faults appear only on a read-only install, which + is the one place nobody runs by hand. + """ + + @pytest.fixture + def mirrored(self, tmp_path, monkeypatch): + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar( + tmp_path, files=("index.mjs", "helper.mjs", "deep-helper.mjs") + ) + (src / "sub").mkdir() + (src / "sub" / "nested.mjs").write_text("// nested\n") + _install_deps(src, current=False) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + return src, resolve_sidecar("testsc", src) + + def test_every_source_file_reaches_the_mirror(self, mirrored): + """Whatever the source holds, the mirror holds. + + Asserted against the source tree itself, not against a second list, + so a new helper cannot be forgotten. + """ + src, mirror = mirrored + want = { + p.relative_to(src) + for p in src.rglob("*") + if p.is_file() and "node_modules" not in p.parts + } + got = { + p.relative_to(mirror) + for p in mirror.rglob("*") + if p.is_file() and "node_modules" not in p.parts + } + assert want <= got, f"missing from the mirror: {sorted(want - got)}" + + def test_node_modules_is_not_copied(self, mirrored): + """npm owns the mirror's deps. Copying them wastes time and space.""" + _src, mirror = mirrored + assert not (mirror / "node_modules" / "somepkg").exists() + + +class TestRefresh: + """An image update changes a source file. The mirror must follow.""" + + def _mirror_twice(self, tmp_path, monkeypatch, mutate): + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + mirror = resolve_sidecar("testsc", src) + # Deps installed in the mirror by npm — these must survive a refresh. + (mirror / "node_modules" / "somepkg").mkdir(parents=True) + (mirror / "node_modules" / "somepkg" / "index.js").write_text("1") + mutate(src) + return src, resolve_sidecar("testsc", src) + + def test_a_changed_source_file_is_recopied(self, tmp_path, monkeypatch): + def mutate(src): + (src / "index.mjs").write_text("// version two\n") + + _src, mirror = self._mirror_twice(tmp_path, monkeypatch, mutate) + assert (mirror / "index.mjs").read_text() == "// version two\n" + + def test_a_new_source_file_appears(self, tmp_path, monkeypatch): + def mutate(src): + (src / "brand-new.mjs").write_text("// new helper\n") + + _src, mirror = self._mirror_twice(tmp_path, monkeypatch, mutate) + assert (mirror / "brand-new.mjs").exists() + + def test_the_mirrors_node_modules_survives(self, tmp_path, monkeypatch): + """A refresh must not delete the deps npm installed in the mirror. + + Wiping them turns every image update into a reinstall, on the + installs least able to afford one. + """ + def mutate(src): + (src / "index.mjs").write_text("// version two\n") + + _src, mirror = self._mirror_twice(tmp_path, monkeypatch, mutate) + assert (mirror / "node_modules" / "somepkg" / "index.js").exists() + + +class TestFailureIsNotFatal: + def test_an_unwritable_mirror_falls_back_to_the_source( + self, tmp_path, monkeypatch + ): + """Returning the read-only source loses installs, not the process. + + The caller's own readiness check then reports the real error, which + is more use than a traceback out of a path resolver. + """ + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + + def _boom(*a, **k): + raise OSError("read-only file system") + + monkeypatch.setattr("gateway.sidecar_runtime.shutil.copytree", _boom) + assert resolve_sidecar("testsc", src) == src + + +class TestEpochMtimeSources: + """A Nix store source has mtime = epoch on every file. + + copy2 would stamp the mirror's lockfile with that epoch mtime, so npm's + install marker (stamped at install time) always postdates it and + deps_are_current() never reports stale — the exact WhatsApp keep-old- + node_modules bug this resolver exists to fix. The copy must stamp "now". + """ + + def test_a_content_change_with_epoch_mtimes_marks_deps_stale( + self, tmp_path, monkeypatch + ): + monkeypatch.delenv("HERMES_TESTSC_SIDECAR_DIR", raising=False) + src = _make_sidecar(tmp_path) + lock = src / "package-lock.json" + for p in src.rglob("*"): + os.utime(p, (1, 1)) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + + mirror = resolve_sidecar("testsc", src) + # npm install in the mirror: marker stamped now. + nm = mirror / "node_modules" + nm.mkdir() + (nm / ".package-lock.json").write_text("{}") + + # Upgrade: lockfile CONTENT changes, mtime stays epoch (new store path). + lock.write_text(json.dumps({"lockfileVersion": 3, "v": 2})) + os.utime(lock, (2, 2)) + + mirror = resolve_sidecar("testsc", src) + from gateway.sidecar_runtime import deps_are_current + + assert deps_are_current(mirror) is False, ( + "a refreshed lockfile must postdate the previous npm install, " + "or an upgrade keeps stale node_modules forever" + ) diff --git a/tests/gateway/test_whatsapp_bridge_dir.py b/tests/gateway/test_whatsapp_bridge_dir.py new file mode 100644 index 0000000000000..0a73c71d958a6 --- /dev/null +++ b/tests/gateway/test_whatsapp_bridge_dir.py @@ -0,0 +1,110 @@ +"""WhatsApp's part of the sidecar resolver. + +The rungs are covered by tests/gateway/test_sidecar_runtime.py. What matters +here: the bridge resolves through the shared resolver, under its own name, so +it gains the staleness check and the refresh that its own resolver never had. +""" +from __future__ import annotations + +from pathlib import Path + +from gateway.platforms.whatsapp_common import ( + SOURCE_BRIDGE_DIR, + resolve_whatsapp_bridge_dir, +) + + +def test_override_is_honoured(tmp_path, monkeypatch) -> None: + """An operator can point the gateway at a bridge checkout of their own.""" + override = tmp_path / "bridge" + override.mkdir() + monkeypatch.setenv("HERMES_WHATSAPP_SIDECAR_DIR", str(override)) + assert resolve_whatsapp_bridge_dir() == override + + +def test_it_resolves_under_the_whatsapp_name(tmp_path, monkeypatch) -> None: + """The name picks the mirror directory and the override variable. + + A collision with another sidecar's name would put two sidecars in one + directory. + """ + seen = {} + + def _record(name, source): + seen["name"] = name + seen["source"] = source + return source + + monkeypatch.setattr( + "gateway.sidecar_runtime.resolve_sidecar", _record, raising=True + ) + resolve_whatsapp_bridge_dir() + assert seen["name"] == "whatsapp" + assert seen["source"] == SOURCE_BRIDGE_DIR + + +def test_the_source_dir_holds_the_bridge(monkeypatch) -> None: + """SOURCE_BRIDGE_DIR must point at the shipped bridge, not near it.""" + assert SOURCE_BRIDGE_DIR.name == "whatsapp-bridge" + assert (SOURCE_BRIDGE_DIR / "package.json").is_file() + assert (SOURCE_BRIDGE_DIR / "bridge.js").is_file() + + +def test_a_writable_checkout_runs_in_place(monkeypatch) -> None: + """A source install must keep running the bridge where it ships.""" + monkeypatch.delenv("HERMES_WHATSAPP_SIDECAR_DIR", raising=False) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: True + ) + assert resolve_whatsapp_bridge_dir() == SOURCE_BRIDGE_DIR + + +def test_a_readonly_tree_moves_to_hermes_home(tmp_path, monkeypatch) -> None: + """A read-only install tree must not be where npm runs (#49561). + + In the container image /opt/hermes/scripts/whatsapp-bridge is read-only, + so an install there fails with EACCES. The bridge moves to HERMES_HOME, + which is writable. + + The bridge also gains a staleness check that its own resolver lacked: + the old code returned any existing mirror without comparing it against + the lockfile, so a bridge upgrade kept running the old node_modules. + """ + monkeypatch.delenv("HERMES_WHATSAPP_SIDECAR_DIR", raising=False) + home = tmp_path / "home" + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + resolved = resolve_whatsapp_bridge_dir() + assert resolved == home / "sidecars" / "whatsapp" + assert (resolved / "bridge.js").is_file() + assert not (resolved / "node_modules").exists() + + +def test_the_bridge_source_is_copied_whole(tmp_path, monkeypatch) -> None: + """Every file the bridge ships must reach the mirror. + + bridge.js imports allowlist.js and other siblings, and Node's ESM + resolver reads them from beside the entry file. A partial copy fails at + import, on read-only installs only. + """ + monkeypatch.delenv("HERMES_WHATSAPP_SIDECAR_DIR", raising=False) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "gateway.sidecar_runtime.dir_writable", lambda p: False + ) + mirror = resolve_whatsapp_bridge_dir() + + want = { + p.relative_to(SOURCE_BRIDGE_DIR) + for p in SOURCE_BRIDGE_DIR.rglob("*") + if p.is_file() and "node_modules" not in p.parts + } + got = { + p.relative_to(mirror) + for p in mirror.rglob("*") + if p.is_file() and "node_modules" not in p.parts + } + assert want, "no bridge sources found — the fixture is wrong, not the code" + assert want <= got, f"missing from the mirror: {sorted(want - got)}" diff --git a/tests/gateway/test_whatsapp_bridge_dir_resolution.py b/tests/gateway/test_whatsapp_bridge_dir_resolution.py deleted file mode 100644 index b473b731d9f37..0000000000000 --- a/tests/gateway/test_whatsapp_bridge_dir_resolution.py +++ /dev/null @@ -1,60 +0,0 @@ -"""Tests for resolve_whatsapp_bridge_dir() — read-only install tree handling. - -Regression coverage for #49561: in the Docker image the install tree -(/opt/hermes/scripts/whatsapp-bridge) is read-only, so `npm install` fails -with EACCES. The resolver must detect the read-only install dir and mirror the -bridge source into a writable HERMES_HOME location instead. -""" -import importlib -from pathlib import Path - -import pytest - -from gateway.platforms import whatsapp_common - - -def _seed_install_tree(install_bridge: Path) -> None: - """Create a minimal fake bridge source tree.""" - install_bridge.mkdir(parents=True, exist_ok=True) - (install_bridge / "bridge.js").write_text("// bridge\n") - (install_bridge / "package.json").write_text('{"name": "whatsapp-bridge"}\n') - - -def test_readonly_install_mirrors_to_hermes_home(tmp_path, monkeypatch): - """A read-only install tree is mirrored into a writable HERMES_HOME.""" - install_root = tmp_path / "install" - install_bridge = install_root / "scripts" / "whatsapp-bridge" - _seed_install_tree(install_bridge) - - hermes_home = tmp_path / "hermes_home" - hermes_home.mkdir() - - monkeypatch.setattr( - whatsapp_common, "__file__", - str(install_root / "gateway" / "platforms" / "whatsapp_common.py"), - ) - monkeypatch.setattr( - "hermes_constants.get_hermes_home", lambda: hermes_home - ) - - # Simulate a read-only install tree. chmod(0o555) is unreliable under - # root (CI/Docker bypass permission bits), so force the write probe to - # fail by raising on the .write_test touch for the install dir only. - _real_touch = Path.touch - - def _fake_touch(self, *a, **kw): - if self.name == ".write_test" and install_bridge in self.parents: - raise PermissionError("read-only install tree") - return _real_touch(self, *a, **kw) - - monkeypatch.setattr(Path, "touch", _fake_touch) - - resolved = whatsapp_common.resolve_whatsapp_bridge_dir() - - expected = hermes_home / "scripts" / "whatsapp-bridge" - assert resolved == expected - # Source was mirrored, not symlinked. - assert (expected / "bridge.js").read_text() == "// bridge\n" - assert (expected / "package.json").exists() - - diff --git a/tests/plugins/platforms/photon/test_sidecar_paths.py b/tests/plugins/platforms/photon/test_sidecar_paths.py index 66ee07feb6d63..c2ee2fcf99be6 100644 --- a/tests/plugins/platforms/photon/test_sidecar_paths.py +++ b/tests/plugins/platforms/photon/test_sidecar_paths.py @@ -1,9 +1,8 @@ -"""Tests for the Photon sidecar directory resolver (NS-606). +"""Photon's part of the sidecar resolver. -Hosted/managed images keep the plugin tree under an immutable -``/opt/hermes``; ``resolve_sidecar_dir`` must run in place when the deps are -baked and current, and mirror the sidecar to the writable ``HERMES_HOME`` -volume when a runtime install is unavoidable. +The rungs themselves are covered by tests/gateway/test_sidecar_runtime.py. +What is Photon-specific, and tested here: the legacy ``PHOTON_SIDECAR_DIR`` +override, and that neither the adapter nor the CLI resolves at import time. """ from __future__ import annotations @@ -16,93 +15,46 @@ import pytest import plugins.platforms.photon.sidecar_paths as sidecar_paths -def _seed_source(source: Path, *, with_node_modules: bool = False) -> None: - source.mkdir(parents=True, exist_ok=True) - for name in sidecar_paths._MIRROR_FILES: - (source / name).write_text(f"// {name}\n", encoding="utf-8") - if with_node_modules: - (source / "node_modules").mkdir() - (source / "node_modules" / ".package-lock.json").write_text( - "{}", encoding="utf-8" - ) +def test_legacy_override_still_works(tmp_path, monkeypatch) -> None: + """``PHOTON_SIDECAR_DIR`` predates the shared resolver. - -def _freeze_writability(monkeypatch, *, writable: bool) -> None: - monkeypatch.setattr(sidecar_paths, "_dir_writable", lambda _p: writable) - - -def test_env_override_wins(tmp_path, monkeypatch) -> None: + Operators set it, so it keeps working alongside the shared + ``HERMES_PHOTON_SIDECAR_DIR``. + """ override = tmp_path / "custom" + monkeypatch.delenv("HERMES_PHOTON_SIDECAR_DIR", raising=False) monkeypatch.setenv("PHOTON_SIDECAR_DIR", str(override)) assert sidecar_paths.resolve_sidecar_dir(tmp_path / "src") == override -def test_writable_source_runs_in_place(tmp_path, monkeypatch) -> None: - """Dev installs: writable tree keeps today's behavior exactly.""" +def test_shared_override_wins_over_the_legacy_one(tmp_path, monkeypatch) -> None: + """Both set means the operator moved to the shared name. Honour it.""" + shared = tmp_path / "shared" + monkeypatch.setenv("PHOTON_SIDECAR_DIR", str(tmp_path / "legacy")) + monkeypatch.setenv("HERMES_PHOTON_SIDECAR_DIR", str(shared)) + assert sidecar_paths.resolve_sidecar_dir(tmp_path / "src") == shared + + +def test_the_source_dir_is_the_shipped_sidecar(monkeypatch) -> None: + """A resolve with no argument reads the sidecar inside the plugin.""" monkeypatch.delenv("PHOTON_SIDECAR_DIR", raising=False) - source = tmp_path / "src" - _seed_source(source) - _freeze_writability(monkeypatch, writable=True) - assert sidecar_paths.resolve_sidecar_dir(source) == source + monkeypatch.delenv("HERMES_PHOTON_SIDECAR_DIR", raising=False) + seen = {} + def _record(name, source): + seen["name"] = name + seen["source"] = source + return source -def test_readonly_source_with_baked_fresh_deps_runs_in_place( - tmp_path, monkeypatch -) -> None: - """Managed-image happy path: deps baked at build time, no mirror needed.""" - monkeypatch.delenv("PHOTON_SIDECAR_DIR", raising=False) - source = tmp_path / "src" - _seed_source(source, with_node_modules=True) - # Marker newer than lockfile == fresh install. - lock = source / "package-lock.json" - marker = source / "node_modules" / ".package-lock.json" - os.utime(lock, (1000.0, 1000.0)) - os.utime(marker, (2000.0, 2000.0)) - _freeze_writability(monkeypatch, writable=False) - assert sidecar_paths.resolve_sidecar_dir(source) == source - - -def test_mirror_refresh_updates_changed_files_and_keeps_node_modules( - tmp_path, monkeypatch -) -> None: - """Image update changes index.mjs → re-copied; installed deps survive.""" - monkeypatch.delenv("PHOTON_SIDECAR_DIR", raising=False) - home = tmp_path / "home" - monkeypatch.setenv("HERMES_HOME", str(home)) - source = tmp_path / "src" - _seed_source(source) - _freeze_writability(monkeypatch, writable=False) - - mirror = sidecar_paths.resolve_sidecar_dir(source) - # Simulate a completed npm install in the mirror. - (mirror / "node_modules").mkdir() - (mirror / "node_modules" / "installed.txt").write_text("x", encoding="utf-8") - - # Image update rewrites a source file. - (source / "index.mjs").write_text("// index.mjs v2\n", encoding="utf-8") - - resolved = sidecar_paths.resolve_sidecar_dir(source) - - assert resolved == mirror - assert (mirror / "index.mjs").read_text(encoding="utf-8") == "// index.mjs v2\n" - assert (mirror / "node_modules" / "installed.txt").exists() - - -def test_dir_writable_probe(tmp_path) -> None: - assert sidecar_paths.dir_writable(tmp_path) is True - ro = tmp_path / "ro" - ro.mkdir() - ro.chmod(0o555) - try: - if os.geteuid() == 0: # pragma: no cover - root ignores perms - pytest.skip("root bypasses directory permissions") - assert sidecar_paths.dir_writable(ro) is False - finally: - ro.chmod(0o755) + monkeypatch.setattr(sidecar_paths, "resolve_sidecar", _record) + sidecar_paths.resolve_sidecar_dir() + assert seen["name"] == "photon" + assert seen["source"] == sidecar_paths.SOURCE_SIDECAR_DIR + assert seen["source"].name == "sidecar" def test_adapter_import_does_not_resolve_sidecar_dir(monkeypatch) -> None: - """Importing the adapter must not probe the filesystem or mirror files. + """Importing the adapter must not probe the filesystem or copy files. resolve_sidecar_dir() touch/unlink-probes the source tree and may copy files to HERMES_HOME; the adapter and CLI resolve lazily on first use so @@ -137,3 +89,17 @@ def test_adapter_import_does_not_resolve_sidecar_dir(monkeypatch) -> None: monkeypatch.undo() importlib.reload(photon_adapter) importlib.reload(photon_cli) + + +def test_dir_writable_probe(tmp_path) -> None: + """Re-exported for the adapter, which gates npm on it.""" + assert sidecar_paths.dir_writable(tmp_path) is True + ro = tmp_path / "ro" + ro.mkdir() + ro.chmod(0o555) + try: + if os.geteuid() == 0: # pragma: no cover - root ignores perms + pytest.skip("root bypasses directory permissions") + assert sidecar_paths.dir_writable(ro) is False + finally: + ro.chmod(0o755)