mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
joaomarcos
Teknium
parent
703de9bacb
commit
de5c39c033
5 changed files with 541 additions and 4 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
2
plugins/platforms/photon/sidecar/.gitignore
vendored
Normal file
2
plugins/platforms/photon/sidecar/.gitignore
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
node_modules/
|
||||
.photon-npm-error.log
|
||||
248
tests/plugins/platforms/photon/test_check_requirements_risks.py
Normal file
248
tests/plugins/platforms/photon/test_check_requirements_risks.py
Normal file
|
|
@ -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}"
|
||||
)
|
||||
231
tests/plugins/platforms/photon/test_npm_error_log_regression.py
Normal file
231
tests/plugins/platforms/photon/test_npm_error_log_regression.py
Normal file
|
|
@ -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}"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue