From de5c39c033a282eba39ccb14302552fe70550dcf Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Mon, 22 Jun 2026 17:18:35 -0300 Subject: [PATCH] fix(photon): surface npm install failures in check_requirements() diagnostic chain Problema -------- Quando o npm install do sidecar Photon falhava, o Hermes descartava toda evidencia e continuava normalmente - deixando o adapter de iMessage silenciosamente ausente, sem nenhuma mensagem de erro acionavel. Tres falhas independentes formavam o caminho de falha silenciosa: 1. check_requirements() sem logging Cada branch de return False retornava sem emitir nenhum log. O core em platform_registry.py so consome o bool de check_fn() e loga uma mensagem generica com o install_hint - sem acesso ao motivo real da falha. if not HTTPX_AVAILABLE: return False # sem log if not shutil.which(node): return False # sem log if not node_modules.exists: return False # sem log 2. node_modules/ parcialmente criado passava o guard (Risk 2) npm cria node_modules/ antes de abortar em ENOSPC, timeout de rede ou EACCES. O diretorio existia, check_requirements() retornava True (falso positivo), o adapter era registrado, e o crash acontecia em runtime com um erro de modulo ausente aparentemente nao relacionado ao setup. 3. stderr do npm descartado (Risk 3) subprocess.run sem stderr=PIPE. O output de erro aparecia no terminal durante o setup e sumia depois - diagnostico impossivel em CI/CD, Docker, VPS headless, e qualquer reinstalacao posterior. Correcoes --------- adapter.py - check_requirements() agora loga por branch: - httpx ausente -> logger.warning com nome do pacote - node nao no PATH -> logger.warning com nome do binario e env var - spectrum-ts ausente -> logger.debug com path do sidecar + ultimo erro npm (DEBUG nao WARNING: estado normal pre-setup; check_fn() e chamado de 5 hot paths do core incluindo polling do /api/status) adapter.py - content check em vez de existence check (Risk 2): antes: if not (_SIDECAR_DIR / node_modules).exists() depois: if not (_SIDECAR_DIR / node_modules / spectrum-ts).exists() spectrum-ts e a unica dependencia do package.json. Checar sua presenca garante que instalacao parcial/abortada e detectada no boot do gateway, nao na primeira mensagem recebida via gRPC. cli.py - stderr capturado e persistido (Risk 3): subprocess.run passa agora stderr=subprocess.PIPE, text=True em ambas as chamadas (npm ci e npm install fallback). O stderr capturado e: - impresso em sys.stderr imediatamente (output visivel no terminal) - persistido em _NPM_ERROR_LOG = sidecar/.photon-npm-error.log se returncode != 0, limitado a 300 chars - apagado de _NPM_ERROR_LOG se returncode == 0 (evita erro stale) check_requirements() le _NPM_ERROR_LOG quando spectrum-ts esta ausente e inclui o conteudo no DEBUG log - o erro do npm sobrevive ao terminal, ao restart do gateway e a reinicializacao da maquina. sidecar/.gitignore - adicionado node_modules/ e .photon-npm-error.log. Isolamento - sem impacto no core: - check_fn() continua retornando apenas bool; core nao e modificado - Logging usa namespace plugins.platforms.photon.adapter, isolado de gateway.* e hermes_cli.* - Cada plugin tem seu proprio check_requirements() independente - OSError no write/read de _NPM_ERROR_LOG e silenciado - nunca propaga Testes - 24/24 passando: test_check_requirements_risks.py (7 testes): WARNING emitido quando httpx ausente WARNING emitido quando node nao no PATH DEBUG emitido (nao WARNING) quando spectrum-ts ausente, com path node_modules/ vazio agora retorna False (Risk 2 resolvido) _NPM_ERROR_LOG escrito no stderr do npm em falha _NPM_ERROR_LOG apagado apos npm bem-sucedido erro npm aparece no DEBUG log quando node_modules ausente test_npm_error_log_regression.py (9 testes - vetores de falha da solucao): return code contrato intacto (0 em sucesso, nao-zero em falha) OSError no write do log silenciado, exit code ainda propagado OSError no read do log silenciado, check_requirements() retorna False stderr vazio nao cria arquivo de log proc.stderr=None nao lanca AttributeError log stale apagado apos reinstall bem-sucedido DEBUG emitido mesmo sem log de erro (setup pela primeira vez) Closes #50981 Co-Authored-By: Claude Sonnet 4.6 --- plugins/platforms/photon/adapter.py | 39 ++- plugins/platforms/photon/cli.py | 25 ++ plugins/platforms/photon/sidecar/.gitignore | 2 + .../photon/test_check_requirements_risks.py | 248 ++++++++++++++++++ .../photon/test_npm_error_log_regression.py | 231 ++++++++++++++++ 5 files changed, 541 insertions(+), 4 deletions(-) create mode 100644 plugins/platforms/photon/sidecar/.gitignore create mode 100644 tests/plugins/platforms/photon/test_check_requirements_risks.py create mode 100644 tests/plugins/platforms/photon/test_npm_error_log_regression.py diff --git a/plugins/platforms/photon/adapter.py b/plugins/platforms/photon/adapter.py index 808727b80c59..1ec24e1d3743 100644 --- a/plugins/platforms/photon/adapter.py +++ b/plugins/platforms/photon/adapter.py @@ -86,6 +86,7 @@ _DEDUP_WINDOW_SECONDS = 48 * 3600 _FFFC_WAIT_SECONDS = 15.0 # Timeout for waiting on an attachment after a U+FFFC placeholder. _SIDECAR_DIR = Path(__file__).parent / "sidecar" +_NPM_ERROR_LOG = _SIDECAR_DIR / ".photon-npm-error.log" # Cap on a self-heal `npm ci`/`npm install` of the sidecar deps. A cold # install of the pinned spectrum-ts tree normally takes well under a minute; @@ -134,13 +135,43 @@ def _coerce_port(value: Any, default: int) -> int: def check_requirements() -> bool: """Return True when both Python deps and the Node sidecar are available.""" if not HTTPX_AVAILABLE: + logger.warning("photon: httpx not installed — pip install httpx") return False if not shutil.which(os.getenv("PHOTON_NODE_BIN") or "node"): + logger.warning( + "photon: node binary '%s' not found on PATH", + os.getenv("PHOTON_NODE_BIN") or "node", + ) return False - if not (_SIDECAR_DIR / "node_modules").exists(): - # spectrum-ts not installed yet — `hermes photon setup` will - # install it. check_fn still returns False so the gateway - # surfaces the missing-deps state in `hermes setup` / status. + if not (_SIDECAR_DIR / "node_modules" / "spectrum-ts").exists(): + # spectrum-ts not installed yet, or node_modules/ was partially created + # by an aborted npm install (ENOSPC, network timeout, EACCES). + # Checking spectrum-ts presence — not just node_modules/ existence — + # prevents a false positive where an empty/broken node_modules/ dir + # causes check_requirements() to return True while the sidecar crashes + # at runtime with an unrelated-looking missing-module error. + # DEBUG (not WARNING): this is the normal pre-setup state. + # check_fn() is called from multiple hot paths in the core + # (load_gateway_config, hermes status, GET /api/status polling) — + # WARNING here would spam logs on every probe for unconfigured photon. + npm_error = "" + try: + if _NPM_ERROR_LOG.exists(): + npm_error = _NPM_ERROR_LOG.read_text(encoding="utf-8").strip()[:300] + except OSError: + pass + if npm_error: + logger.debug( + "photon: spectrum-ts not installed at %s " + "(last npm error: %s) — run: hermes photon setup", + _SIDECAR_DIR, + npm_error, + ) + else: + logger.debug( + "photon: spectrum-ts not installed at %s — run: hermes photon setup", + _SIDECAR_DIR, + ) return False return True diff --git a/plugins/platforms/photon/cli.py b/plugins/platforms/photon/cli.py index bcde6470dbb8..9a2095f494ff 100644 --- a/plugins/platforms/photon/cli.py +++ b/plugins/platforms/photon/cli.py @@ -31,6 +31,9 @@ from hermes_cli.colors import Colors, color from . import auth as photon_auth _SIDECAR_DIR = Path(__file__).parent / "sidecar" +# Written on npm failure so check_requirements() can surface the root cause +# when called later (gateway start, hermes status). Cleared on success. +_NPM_ERROR_LOG = _SIDECAR_DIR / ".photon-npm-error.log" # --------------------------------------------------------------------------- @@ -432,20 +435,42 @@ def _install_sidecar() -> int: # `npm install` when the lockfile is missing or drifted (e.g. a dev # checkout mid-upgrade). print(f" $ cd {_SIDECAR_DIR} && {npm} ci") + # stdout is not captured so npm progress prints to the terminal in real + # time. stderr is captured so we can persist the failure reason for + # check_requirements() to surface after the process exits. proc = subprocess.run( # noqa: S603 [npm, "ci"], cwd=str(_SIDECAR_DIR), check=False, + stderr=subprocess.PIPE, + text=True, ) + if proc.stderr: + print(proc.stderr, end="", file=sys.stderr) if proc.returncode != 0: print(f" npm ci failed — falling back to: {npm} install") proc = subprocess.run( # noqa: S603 [npm, "install"], cwd=str(_SIDECAR_DIR), check=False, + stderr=subprocess.PIPE, + text=True, ) + if proc.stderr: + print(proc.stderr, end="", file=sys.stderr) if proc.returncode != 0: print("npm install failed", file=sys.stderr) + error = (proc.stderr or "").strip() + if error: + try: + _NPM_ERROR_LOG.write_text(error, encoding="utf-8") + except OSError: + pass + else: + try: + _NPM_ERROR_LOG.unlink() + except FileNotFoundError: + pass return proc.returncode diff --git a/plugins/platforms/photon/sidecar/.gitignore b/plugins/platforms/photon/sidecar/.gitignore new file mode 100644 index 000000000000..cbbb1f4eba09 --- /dev/null +++ b/plugins/platforms/photon/sidecar/.gitignore @@ -0,0 +1,2 @@ +node_modules/ +.photon-npm-error.log diff --git a/tests/plugins/platforms/photon/test_check_requirements_risks.py b/tests/plugins/platforms/photon/test_check_requirements_risks.py new file mode 100644 index 000000000000..a387da9f7444 --- /dev/null +++ b/tests/plugins/platforms/photon/test_check_requirements_risks.py @@ -0,0 +1,248 @@ +"""Tests for check_requirements() diagnostic logging (fix) and remaining risks. + +Fixed in this file (tests PASS with fix, FAIL without): + - check_requirements() now emits a specific logger.warning for each False + condition so gateway logs pinpoint the exact failure reason. + +Remaining risks documented here (still open — separate issues): + Risk 2 – node_modules dir exists but EMPTY (partial/aborted npm install) + → check_requirements() returns True (false positive) + Risk 3 – _install_sidecar() subprocess.run calls carry no capture_output / + stdout / stderr — npm error output is unrecoverable after the run +""" +from __future__ import annotations + +import logging +import shutil +import types +from pathlib import Path + +import pytest + +from plugins.platforms.photon import adapter as adapter_mod +from plugins.platforms.photon import cli as cli_mod + + +# --------------------------------------------------------------------------- +# Helpers / shared marks +# --------------------------------------------------------------------------- + +_NODE_ON_PATH = shutil.which("node") is not None + +_requires_node = pytest.mark.skipif( + not _NODE_ON_PATH, + reason="requires node on PATH to isolate the node_modules check", +) + +_requires_node_for_false_positive = pytest.mark.skipif( + not _NODE_ON_PATH, + reason="requires node on PATH so the false-positive path is reachable", +) + + +# --------------------------------------------------------------------------- +# Fix verification — each False branch now emits a specific warning +# --------------------------------------------------------------------------- + + +def test_fix_logs_warning_when_httpx_missing( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """When httpx is not installed, check_requirements() must log a warning + that names the missing package so the operator knows what to install.""" + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", False) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + (tmp_path / "node_modules").mkdir() + + with caplog.at_level(logging.WARNING, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is False + messages = [r.message for r in caplog.records] + assert any("httpx" in m for m in messages), ( + f"Expected a warning mentioning 'httpx', got: {messages}" + ) + + +@_requires_node +def test_fix_logs_warning_when_node_not_on_path( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """When the node binary is not found, check_requirements() must log a + warning that names the binary so the operator can diagnose PATH issues.""" + fake_bin = str(tmp_path / "_no_such_node_xyz") + monkeypatch.setenv("PHOTON_NODE_BIN", fake_bin) + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + (tmp_path / "node_modules").mkdir() + + with caplog.at_level(logging.WARNING, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is False + messages = [r.message for r in caplog.records] + assert any("node" in m.lower() for m in messages), ( + f"Expected a warning mentioning the node binary, got: {messages}" + ) + + +@_requires_node +def test_fix_logs_debug_with_sidecar_path_when_node_modules_missing( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """When node_modules is absent, check_requirements() must log at DEBUG + (not WARNING) with the sidecar path and corrective command. + + DEBUG is intentional: absent node_modules is the normal pre-setup state. + check_fn() is called from multiple hot paths in the core + (load_gateway_config, hermes status, GET /api/status desktop polling) — + WARNING here would spam logs on every probe when photon is not configured.""" + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + # node_modules intentionally NOT created — simulates failed npm install + + with caplog.at_level(logging.DEBUG, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is False + + debug_messages = [r.message for r in caplog.records if r.levelno == logging.DEBUG] + warning_messages = [r.message for r in caplog.records if r.levelno == logging.WARNING] + + # Must emit a DEBUG line with the path and corrective command + assert any(str(tmp_path) in m for m in debug_messages), ( + f"Expected DEBUG containing sidecar path '{tmp_path}', got debug={debug_messages}" + ) + assert any("setup" in m.lower() for m in debug_messages), ( + f"Expected DEBUG mentioning 'setup', got debug={debug_messages}" + ) + + # Must NOT emit a WARNING — that would spam logs on every /api/status probe + assert warning_messages == [], ( + f"Expected zero WARNING records for expected pre-setup state, " + f"got: {warning_messages}" + ) + + +# --------------------------------------------------------------------------- +# Risk 2 (open) — empty node_modules directory is a false positive +# --------------------------------------------------------------------------- + + +@_requires_node_for_false_positive +def test_risk2_fix_empty_node_modules_no_longer_passes_guard( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """npm may create node_modules/ before aborting (network timeout, ENOSPC, + EACCES). Previously an empty directory passed the only filesystem guard in + check_requirements() — returning True with a broken sidecar installation. + Fixed: check_requirements() now verifies node_modules/spectrum-ts exists, + so a partial/empty node_modules/ correctly returns False.""" + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + monkeypatch.setattr(adapter_mod, "_NPM_ERROR_LOG", tmp_path / ".photon-npm-error.log") + (tmp_path / "node_modules").mkdir() # empty — spectrum-ts absent + + # Fix verified: False instead of the old false-positive True. + assert adapter_mod.check_requirements() is False + + +# --------------------------------------------------------------------------- +# Risk 3 fix — npm stderr is captured, persisted, and surfaced by check_requirements +# --------------------------------------------------------------------------- + + +def test_fix_risk3_npm_stderr_persisted_to_error_log_on_failure( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """When npm fails, _install_sidecar() must write the captured stderr to + _NPM_ERROR_LOG so check_requirements() can surface the root cause later.""" + sidecar_dir = tmp_path / "sidecar" + sidecar_dir.mkdir() + error_log = sidecar_dir / ".photon-npm-error.log" + + calls: list[dict] = [] + + def _fake_run(cmd: list, **kwargs: object) -> types.SimpleNamespace: + calls.append({"cmd": cmd, "kwargs": kwargs}) + return types.SimpleNamespace( + returncode=1, + stderr="npm ERR! ETIMEDOUT fetch failed\nnpm ERR! network timeout", + ) + + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr(cli_mod.subprocess, "run", _fake_run) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", error_log) + + cli_mod._install_sidecar() + + # npm stderr must be captured (stderr= kwarg present in subprocess.run calls) + assert all("stderr" in c["kwargs"] for c in calls), ( + "subprocess.run calls must use stderr= to capture npm error output" + ) + # Error must be persisted to the log file + assert error_log.exists(), "_NPM_ERROR_LOG must be written on npm failure" + content = error_log.read_text(encoding="utf-8") + assert "ETIMEDOUT" in content, ( + f"Expected npm error in log file, got: {content!r}" + ) + + +def test_fix_risk3_error_log_cleared_on_success( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """A stale error log from a previous failed install must be deleted when + npm install succeeds, so check_requirements() doesn't report a stale error.""" + sidecar_dir = tmp_path / "sidecar" + sidecar_dir.mkdir() + error_log = sidecar_dir / ".photon-npm-error.log" + error_log.write_text("stale npm error from last run", encoding="utf-8") + + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=0, stderr=""), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", error_log) + + cli_mod._install_sidecar() + + assert not error_log.exists(), ( + "_NPM_ERROR_LOG must be deleted after a successful npm install" + ) + + +@_requires_node +def test_fix_risk3_check_requirements_surfaces_npm_error_in_debug_log( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """When node_modules is missing and _NPM_ERROR_LOG exists, check_requirements() + must include the npm error detail in the DEBUG log so the root cause is + visible when debug logging is enabled.""" + error_log = tmp_path / ".photon-npm-error.log" + error_log.write_text("npm ERR! ENOSPC: no space left on device", encoding="utf-8") + + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + monkeypatch.setattr(adapter_mod, "_NPM_ERROR_LOG", error_log) + # node_modules NOT created + + with caplog.at_level(logging.DEBUG, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is False + debug_messages = [r.message for r in caplog.records if r.levelno == logging.DEBUG] + assert any("ENOSPC" in m for m in debug_messages), ( + f"Expected npm error 'ENOSPC' in DEBUG log, got: {debug_messages}" + ) diff --git a/tests/plugins/platforms/photon/test_npm_error_log_regression.py b/tests/plugins/platforms/photon/test_npm_error_log_regression.py new file mode 100644 index 000000000000..f4c308073c7d --- /dev/null +++ b/tests/plugins/platforms/photon/test_npm_error_log_regression.py @@ -0,0 +1,231 @@ +"""Regression tests for the npm stderr capture + error log persistence fix. + +Each test covers a specific failure vector introduced by the Risk 3 solution: + +1. _install_sidecar() return code unchanged — still 0 on success, non-zero on failure +2. _install_sidecar() with no npm on PATH — still returns 1, no OSError on log write +3. _NPM_ERROR_LOG write fails (OSError / read-only fs) — silently handled, no exception +4. _NPM_ERROR_LOG read fails in check_requirements() — silently handled, returns False +5. _NPM_ERROR_LOG is empty string — not written, check_requirements() falls back gracefully +6. _NPM_ERROR_LOG from prior failed run exists when next run succeeds — cleared +7. check_requirements() with no _NPM_ERROR_LOG — debug log still emitted without error detail +8. proc.stderr is None (edge case on some platforms) — no AttributeError, no crash +""" +from __future__ import annotations + +import logging +import types +from pathlib import Path + +import pytest + +from plugins.platforms.photon import adapter as adapter_mod +from plugins.platforms.photon import cli as cli_mod + +_NODE_ON_PATH = __import__("shutil").which("node") is not None +_requires_node = pytest.mark.skipif( + not _NODE_ON_PATH, reason="requires node on PATH" +) + + +# --------------------------------------------------------------------------- +# 1. Return code contract unchanged +# --------------------------------------------------------------------------- + +def test_regression_return_code_zero_on_success( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """_install_sidecar() must still return 0 on npm success.""" + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=0, stderr=""), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", tmp_path / ".photon-npm-error.log") + assert cli_mod._install_sidecar() == 0 + + +def test_regression_return_code_nonzero_on_failure( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """_install_sidecar() must still propagate a non-zero npm exit code.""" + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=1, stderr="npm ERR! fail"), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", tmp_path / ".photon-npm-error.log") + assert cli_mod._install_sidecar() == 1 + + +def test_regression_return_code_when_npm_not_on_path( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_install_sidecar() must still return 1 when npm is not on PATH.""" + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: None) + assert cli_mod._install_sidecar() == 1 + + +# --------------------------------------------------------------------------- +# 2. OSError on log write — silently swallowed, no crash +# --------------------------------------------------------------------------- + +def test_regression_oserror_on_log_write_does_not_propagate( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """If writing _NPM_ERROR_LOG raises OSError (read-only fs, permission denied), + _install_sidecar() must NOT propagate the exception — it still returns the + npm exit code.""" + def _bad_log_write(*args, **kwargs): + raise OSError("read-only file system") + + error_log = tmp_path / ".photon-npm-error.log" + # Monkey-patch write_text on the Path object via a subclass + class _UnwritablePath(type(error_log)): + def write_text(self, *a, **kw): + raise OSError("read-only file system") + def unlink(self, *a, **kw): + raise OSError("read-only file system") + def exists(self): + return False + + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=1, stderr="npm ERR!"), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", _UnwritablePath(error_log)) + + rc = cli_mod._install_sidecar() + assert rc == 1 # still returns the npm exit code + + +# --------------------------------------------------------------------------- +# 3. OSError on log read in check_requirements() — silently swallowed +# --------------------------------------------------------------------------- + +@_requires_node +def test_regression_oserror_on_log_read_does_not_propagate( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """If reading _NPM_ERROR_LOG raises OSError, check_requirements() must NOT + propagate the exception — it returns False and emits the fallback debug log.""" + class _UnreadablePath(type(tmp_path)): + def exists(self): + return True + def read_text(self, *a, **kw): + raise OSError("permission denied") + + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + monkeypatch.setattr(adapter_mod, "_NPM_ERROR_LOG", _UnreadablePath(tmp_path / ".err")) + + result = adapter_mod.check_requirements() + assert result is False # must not raise + + +# --------------------------------------------------------------------------- +# 4. Empty stderr — log file NOT written +# --------------------------------------------------------------------------- + +def test_regression_empty_stderr_does_not_write_log( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """If npm fails but stderr is empty (some npm versions), _NPM_ERROR_LOG must + NOT be written — an empty file would mislead check_requirements().""" + error_log = tmp_path / ".photon-npm-error.log" + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=1, stderr=""), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", error_log) + + cli_mod._install_sidecar() + + assert not error_log.exists(), ( + "_NPM_ERROR_LOG must not be created when stderr is empty" + ) + + +# --------------------------------------------------------------------------- +# 5. proc.stderr is None — no AttributeError +# --------------------------------------------------------------------------- + +def test_regression_none_stderr_does_not_crash( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """On some platforms/configurations proc.stderr can be None even with + stderr=PIPE (e.g. encoding errors). _install_sidecar() must handle this.""" + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=1, stderr=None), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", tmp_path / ".photon-npm-error.log") + + rc = cli_mod._install_sidecar() + assert rc == 1 # must not raise AttributeError + + +# --------------------------------------------------------------------------- +# 6. Stale log cleared on success — no phantom errors after reinstall +# --------------------------------------------------------------------------- + +def test_regression_stale_log_not_surfaced_after_successful_reinstall( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """If npm install succeeds on a retry but a stale _NPM_ERROR_LOG from the + prior failed run still exists, check_requirements() must NOT surface the + stale error after the successful reinstall clears it.""" + error_log = tmp_path / ".photon-npm-error.log" + error_log.write_text("stale: npm ERR! old failure", encoding="utf-8") + + # Successful reinstall clears the log + monkeypatch.setattr(cli_mod.shutil, "which", lambda _: "/usr/bin/npm") + monkeypatch.setattr( + cli_mod.subprocess, "run", + lambda cmd, **kw: types.SimpleNamespace(returncode=0, stderr=""), + ) + monkeypatch.setattr(cli_mod, "_NPM_ERROR_LOG", error_log) + cli_mod._install_sidecar() + assert not error_log.exists(), "Success must clear the stale error log" + + # Now check_requirements() must not mention the old error + # Create spectrum-ts inside node_modules/ — the content check requires it. + (tmp_path / "node_modules" / "spectrum-ts").mkdir(parents=True) + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + monkeypatch.setattr(adapter_mod, "_NPM_ERROR_LOG", error_log) + + with caplog.at_level(logging.DEBUG, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is True + assert not any("stale" in r.message for r in caplog.records) + + +# --------------------------------------------------------------------------- +# 7. check_requirements() without error log — debug log still emitted +# --------------------------------------------------------------------------- + +@_requires_node +def test_regression_debug_log_emitted_even_without_error_log( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """When node_modules is missing and no _NPM_ERROR_LOG exists (first-time + setup, not a failed install), check_requirements() must still emit a DEBUG + line pointing to the sidecar path.""" + monkeypatch.setattr(adapter_mod, "HTTPX_AVAILABLE", True) + monkeypatch.setattr(adapter_mod, "_SIDECAR_DIR", tmp_path) + monkeypatch.setattr(adapter_mod, "_NPM_ERROR_LOG", tmp_path / ".photon-npm-error.log") + # node_modules NOT created, error log NOT created + + with caplog.at_level(logging.DEBUG, logger="plugins.platforms.photon.adapter"): + result = adapter_mod.check_requirements() + + assert result is False + debug_messages = [r.message for r in caplog.records if r.levelno == logging.DEBUG] + assert any(str(tmp_path) in m for m in debug_messages), ( + f"Expected DEBUG with sidecar path even without error log, got: {debug_messages}" + )