no-mistakes(review): Harden health-check provenance, infisical verification, report-only JSON, tests
This commit is contained in:
+268
-56
@@ -7,30 +7,46 @@ stale expectations rather than live faults.
|
||||
abiba (pi-only since the harness purge) was tested as a Hermes host, koby
|
||||
(report-only per the captain's 2026-08-17 ruling) was counted as repairable,
|
||||
koby's CT 111 was probed on amdpve where it does not exist (it runs on
|
||||
storepve .6), and a .env-based hermes wrapper was FAILed for not mentioning
|
||||
infisical.
|
||||
storepve .6), and the wrapper infisical check had two bugs — it read only
|
||||
the first 20 lines, so koonimo's wrapper (which references /usr/bin/infisical
|
||||
past line 20) false-failed, and it treated koby's genuine no-infisical
|
||||
(~/.hermes/.env) wrapper as broken.
|
||||
* gpu-monitor emitted "DEGRADED — GPU-rtx3090 000, GPU-rtx5070 000" three
|
||||
times from probing bare port 80 on GPU hosts while :8080 answered 200.
|
||||
* infrastructure-monitoring probed CT 116 for the PVE API (no pveproxy ->
|
||||
000) instead of the five real cluster nodes, which answer 401 = alive.
|
||||
|
||||
These tests pin the corrections so the false alarms cannot return silently.
|
||||
They are static/structural over the contracts plus an inert import of the health
|
||||
script — no live network, vault, or SSH access is required.
|
||||
These tests execute the health script (with SSH/vault stubbed) and the real
|
||||
provenance consumer (scripts/prose-lint.sh), and parse the contracts' executable
|
||||
check-health probe blocks into normalized probe sets. No live network, vault, or
|
||||
SSH access is required.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import textwrap
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = pathlib.Path(__file__).resolve().parents[1]
|
||||
AHC = ROOT / "scripts" / "agent-health-check.py"
|
||||
LINT = ROOT / "scripts" / "prose-lint.sh"
|
||||
GPU = ROOT / "gpu-monitor.prose.md"
|
||||
INFRA = ROOT / "infrastructure-monitoring.prose.md"
|
||||
PROXMOX = ROOT / "proxmox-monitor.prose.md"
|
||||
|
||||
PVE_NODE_IPS = {
|
||||
"192.168.68.9",
|
||||
"192.168.68.5",
|
||||
"192.168.68.15",
|
||||
"192.168.68.6",
|
||||
"192.168.68.12",
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
@@ -43,6 +59,26 @@ def ahc():
|
||||
return module
|
||||
|
||||
|
||||
# ── helpers: execute the health script with SSH/vault stubbed ─────────
|
||||
|
||||
def _run_main(ahc, monkeypatch, capsys, argv, ssh_result=None):
|
||||
ahc.FAIL.clear()
|
||||
ahc.REPORT_ONLY.clear()
|
||||
monkeypatch.setattr(ahc, "load_agent_keys", lambda: None)
|
||||
monkeypatch.setattr(ahc, "ssh", lambda *a, **k: ssh_result)
|
||||
monkeypatch.setattr(sys, "argv", ["agent-health-check.py", "--no-deploy", *argv])
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
ahc.main()
|
||||
return exc.value.code, capsys.readouterr().out
|
||||
|
||||
|
||||
def _json_payload(out):
|
||||
for line in reversed(out.splitlines()):
|
||||
if line.startswith('{"timestamp"'):
|
||||
return json.loads(line)
|
||||
raise AssertionError(f"no JSON payload in output:\n{out}")
|
||||
|
||||
|
||||
# ── agent-health-check: stale-expectation legs ───────────────────────
|
||||
|
||||
def test_import_does_not_contact_vault(ahc):
|
||||
@@ -68,17 +104,19 @@ def test_koby_ct111_is_on_storepve(ahc):
|
||||
assert ahc.AGENTS["koby"]["pve"] == "storepve"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("agent", ["koby", "koonimo", "tanko"])
|
||||
def test_report_only_legs_never_count_as_failures(ahc, agent):
|
||||
ahc.FAIL.clear()
|
||||
try:
|
||||
def test_report_only_legs_never_count_as_failures(ahc):
|
||||
for agent, report_only in (("koby", True), ("koonimo", False), ("tanko", False)):
|
||||
ahc.FAIL.clear()
|
||||
ahc.REPORT_ONLY.clear()
|
||||
ahc._fail(f"probe:{agent}", agent)
|
||||
if ahc.AGENTS[agent].get("report_only"):
|
||||
if report_only:
|
||||
assert ahc.FAIL == []
|
||||
assert ahc.REPORT_ONLY == [f"probe:{agent}"]
|
||||
else:
|
||||
assert ahc.FAIL == [f"probe:{agent}"]
|
||||
finally:
|
||||
ahc.FAIL.clear()
|
||||
assert ahc.REPORT_ONLY == []
|
||||
ahc.FAIL.clear()
|
||||
ahc.REPORT_ONLY.clear()
|
||||
|
||||
|
||||
def test_failure_recording_accepts_agentless_keys(ahc):
|
||||
@@ -90,63 +128,237 @@ def test_failure_recording_accepts_agentless_keys(ahc):
|
||||
ahc.FAIL.clear()
|
||||
|
||||
|
||||
def test_script_reports_absolute_execution_provenance(ahc):
|
||||
src = AHC.read_text()
|
||||
assert "execution_path" in src
|
||||
assert "os.getcwd()" in src
|
||||
def test_json_reports_absolute_execution_provenance(ahc, monkeypatch, capsys):
|
||||
code, out = _run_main(ahc, monkeypatch, capsys, ["--json"])
|
||||
payload = _json_payload(out)
|
||||
assert payload["execution_path"] == os.path.abspath(str(AHC))
|
||||
assert payload["cwd"] == os.getcwd()
|
||||
assert code == 1 # stubbed SSH fails every leg, but provenance is still emitted
|
||||
|
||||
|
||||
def test_wrapper_check_has_no_bare_infisical_expectation(ahc):
|
||||
# A wrapper that never mentions infisical (koonimo sources ~/.hermes/.env)
|
||||
# must not be FAILed merely for lacking an infisical reference.
|
||||
assert "wrapper resolves creds without infisical" in AHC.read_text()
|
||||
def test_quiet_run_still_carries_provenance_on_the_alert_path(ahc, monkeypatch, capsys):
|
||||
# The production cron runs --quiet; a report without provenance is
|
||||
# unactionable (item 4). Both the header line and the ALERT line must carry it.
|
||||
code, out = _run_main(ahc, monkeypatch, capsys, ["--quiet"])
|
||||
assert code == 1
|
||||
assert "📍 executed from: script=" in out
|
||||
alerts = [ln for ln in out.splitlines() if ln.startswith("ALERT agent-health:")]
|
||||
assert alerts, out
|
||||
assert f"script={os.path.abspath(str(AHC))}" in alerts[0]
|
||||
assert f"cwd={os.getcwd()}" in alerts[0]
|
||||
|
||||
|
||||
# ── item 4: every contract report carries its execution path ──────────
|
||||
def test_json_surfaces_report_only_findings_separately(ahc, monkeypatch, capsys):
|
||||
# Koby's down legs are reported but must not count as fleet failures; the
|
||||
# --json payload exposes them in their own array (item 1 + f8).
|
||||
_, out = _run_main(ahc, monkeypatch, capsys, ["--json"])
|
||||
payload = _json_payload(out)
|
||||
assert isinstance(payload["report_only"], list)
|
||||
assert any(key.startswith(("gateway-down:koby", "ct-unreachable:koby"))
|
||||
for key in payload["report_only"])
|
||||
assert not any("koby" in key for key in payload["failures"])
|
||||
|
||||
@pytest.mark.parametrize("contract", [GPU, INFRA, PROXMOX], ids=lambda p: p.name)
|
||||
def test_report_format_requires_execution_provenance(contract):
|
||||
|
||||
# ── agent-health-check: wrapper infisical behavior (f3) ───────────────
|
||||
|
||||
def _stub_wrapper_ssh(ahc, monkeypatch, wrapper_body, test_x_result="OK", command_v="/usr/local/bin/infisical"):
|
||||
def fake_ssh(host, cmd, user="root"):
|
||||
if cmd.startswith("cat /root/.local/bin/hermes"):
|
||||
return wrapper_body
|
||||
if cmd.startswith("ls -la /root/.local/bin/hermes "):
|
||||
return "-rwxr-xr-x 1 root root 0 Jan 1 00:00 /root/.local/bin/hermes"
|
||||
if cmd.startswith("ls -la /root/.local/bin/hermes-real") or "venv/bin/hermes" in cmd:
|
||||
return "-rwxr-xr-x 1 root root 0 Jan 1 00:00 /root/.local/bin/hermes-real"
|
||||
if cmd.startswith("grep -c 'LITELLM_API_KEY'"):
|
||||
return "1"
|
||||
if cmd.startswith("test -x "):
|
||||
return test_x_result
|
||||
if cmd.startswith("command -v infisical"):
|
||||
return command_v
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(ahc, "ssh", fake_ssh)
|
||||
monkeypatch.setattr(ahc, "AGENTS", {"koonimo": dict(ahc.AGENTS["koonimo"])})
|
||||
ahc.FAIL.clear()
|
||||
ahc.REPORT_ONLY.clear()
|
||||
|
||||
|
||||
def test_env_based_wrapper_without_infisical_is_not_failed(ahc, monkeypatch, capsys):
|
||||
_stub_wrapper_ssh(ahc, monkeypatch,
|
||||
"#!/bin/bash\nsource ~/.hermes/.env\nexec hermes-real \"$@\"\n")
|
||||
ahc.check_wrapper_integrity()
|
||||
out = capsys.readouterr().out
|
||||
assert ahc.FAIL == []
|
||||
assert "wrapper resolves creds without infisical" in out
|
||||
|
||||
|
||||
def test_dangling_absolute_infisical_path_is_failed(ahc, monkeypatch, capsys):
|
||||
# Wrapper hardcodes /usr/bin/infisical, which is absent, while PATH resolves
|
||||
# infisical to /usr/local/bin/infisical. The literal path must be verified,
|
||||
# not inferred from PATH resolution.
|
||||
_stub_wrapper_ssh(ahc, monkeypatch,
|
||||
"#!/bin/bash\n/usr/bin/infisical run -- hermes-real \"$@\"\n",
|
||||
test_x_result="MISS", command_v="/usr/local/bin/infisical")
|
||||
ahc.check_wrapper_integrity()
|
||||
assert "wrapper-infisical-path:koonimo" in ahc.FAIL
|
||||
|
||||
|
||||
def test_existing_absolute_infisical_path_passes(ahc, monkeypatch, capsys):
|
||||
_stub_wrapper_ssh(ahc, monkeypatch,
|
||||
"#!/bin/bash\n/usr/bin/infisical run -- hermes-real \"$@\"\n",
|
||||
test_x_result="OK")
|
||||
ahc.check_wrapper_integrity()
|
||||
out = capsys.readouterr().out
|
||||
assert ahc.FAIL == []
|
||||
assert "wrapper infisical path OK" in out
|
||||
|
||||
|
||||
# ── item 4: prose-lint enforces report provenance (real consumer) ─────
|
||||
|
||||
GOOD_CONTRACT = textwrap.dedent("""\
|
||||
---
|
||||
kind: function
|
||||
name: good
|
||||
description: fixture with provenance
|
||||
---
|
||||
|
||||
## Parameters
|
||||
|
||||
- x: y
|
||||
|
||||
## Returns
|
||||
|
||||
ok
|
||||
|
||||
### check-health
|
||||
|
||||
```bash
|
||||
pwd -P
|
||||
```
|
||||
|
||||
**Report format**: Begin with the absolute path the probe executed from.
|
||||
""")
|
||||
|
||||
DECOY_CONTRACT = textwrap.dedent("""\
|
||||
---
|
||||
kind: function
|
||||
name: decoy
|
||||
description: fixture with provenance only outside the report format
|
||||
---
|
||||
|
||||
## Parameters
|
||||
|
||||
- x: y
|
||||
|
||||
## Returns
|
||||
|
||||
ok
|
||||
|
||||
The absolute path of the config is /etc/foo.
|
||||
|
||||
### check-health
|
||||
|
||||
```bash
|
||||
true
|
||||
```
|
||||
|
||||
**Report format**: Summarize actual results from each probe.
|
||||
""")
|
||||
|
||||
MISSING_CONTRACT = textwrap.dedent("""\
|
||||
---
|
||||
kind: function
|
||||
name: missing
|
||||
description: check-health contract with no report format
|
||||
---
|
||||
|
||||
## Parameters
|
||||
|
||||
- x: y
|
||||
|
||||
## Returns
|
||||
|
||||
ok
|
||||
|
||||
### check-health
|
||||
|
||||
```bash
|
||||
pwd -P
|
||||
```
|
||||
""")
|
||||
|
||||
|
||||
def _run_lint(tmp_path, text, name):
|
||||
(tmp_path / name).write_text(text)
|
||||
return subprocess.run(["bash", str(LINT)], cwd=tmp_path,
|
||||
capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_prose_lint_accepts_report_format_with_provenance(tmp_path):
|
||||
result = _run_lint(tmp_path, GOOD_CONTRACT, "good.prose.md")
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
|
||||
def test_prose_lint_rejects_report_format_without_provenance(tmp_path):
|
||||
result = _run_lint(tmp_path, DECOY_CONTRACT, "decoy.prose.md")
|
||||
assert result.returncode == 1, result.stdout
|
||||
assert "lacks execution provenance" in result.stdout
|
||||
|
||||
|
||||
def test_prose_lint_requires_report_format_on_check_health_contract(tmp_path):
|
||||
result = _run_lint(tmp_path, MISSING_CONTRACT, "missing.prose.md")
|
||||
assert result.returncode == 1, result.stdout
|
||||
assert "no **Report format** paragraph" in result.stdout
|
||||
|
||||
|
||||
# ── contracts: parse the executable check-health probe block ─────────
|
||||
|
||||
def _check_health_block(contract):
|
||||
"""Extract the bash probe block under ### check-health (the probe interface)."""
|
||||
text = contract.read_text()
|
||||
assert "**Report format**" in text
|
||||
assert re.search(r"absolute path|pwd -P|executed from", text)
|
||||
marker = "### check-health"
|
||||
assert marker in text, f"{contract.name} has no {marker}"
|
||||
after = text.split(marker, 1)[1]
|
||||
match = re.search(r"```bash\n(.*?)```", after, re.S)
|
||||
assert match, f"{contract.name} check-health has no bash probe block"
|
||||
return match.group(1)
|
||||
|
||||
|
||||
# ── item 3: gpu-monitor probes the real GPU endpoints ────────────────
|
||||
|
||||
def test_gpu_monitor_probes_gpu_health_on_8080():
|
||||
text = GPU.read_text()
|
||||
assert "http://192.168.68.8:8080/health" in text
|
||||
assert "http://192.168.68.110:8080/health" in text
|
||||
def _urls(block):
|
||||
# Comments document the forbidden/deprecated probes; only count real commands.
|
||||
code = "\n".join(ln for ln in block.splitlines()
|
||||
if not ln.lstrip().startswith("#"))
|
||||
return set(re.findall(r"https?://[^\s\"')]+", code))
|
||||
|
||||
|
||||
def test_gpu_monitor_forbids_bare_port_80_on_gpu_hosts():
|
||||
text = GPU.read_text()
|
||||
assert re.search(r"never .{0,20}bare port 80", text, re.I)
|
||||
# The false-DEGRADED symptom must be documented, not just implied.
|
||||
assert "DEGRADED" in text
|
||||
def test_gpu_monitor_probes_every_gpu_health_on_8080():
|
||||
block = _check_health_block(GPU)
|
||||
gpu_health = {u for u in _urls(block) if ":8080/health" in u}
|
||||
assert {"http://192.168.68.8:8080/health",
|
||||
"http://192.168.68.110:8080/health"} <= gpu_health
|
||||
|
||||
|
||||
def test_gpu_monitor_never_probes_bare_port_80_on_gpu_hosts():
|
||||
block = _check_health_block(GPU)
|
||||
bare = {u for u in _urls(block)
|
||||
if re.match(r"https?://192\.168\.68\.(8|110|15)/", u)}
|
||||
assert bare == set()
|
||||
assert re.search(r"never .{0,40}bare port 80", block, re.I)
|
||||
|
||||
|
||||
def test_gpu_monitor_documents_router_301_as_alive():
|
||||
text = GPU.read_text()
|
||||
assert "/gpu/gpu-data" in text
|
||||
assert "301" in text
|
||||
block = _check_health_block(GPU)
|
||||
assert "/health/unified" in block
|
||||
assert "301" in block
|
||||
|
||||
|
||||
# ── item 2: infrastructure-monitoring PVE API vantage ────────────────
|
||||
def test_infra_monitoring_probes_every_real_pve_node():
|
||||
block = _check_health_block(INFRA)
|
||||
match = re.search(r"for node in ([^\n;]+)", block)
|
||||
assert match, "PVE liveness loop not found in check-health"
|
||||
assert set(match.group(1).split()) == PVE_NODE_IPS
|
||||
assert "https://$node:8006/api2/json/version" in block
|
||||
|
||||
|
||||
def test_infra_monitoring_does_not_probe_ct116_for_pve_api():
|
||||
assert "https://192.168.68.116:8006" not in INFRA.read_text()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"node",
|
||||
["192.168.68.9", "192.168.68.5", "192.168.68.15", "192.168.68.6", "192.168.68.12"],
|
||||
)
|
||||
def test_infra_monitoring_probes_every_real_pve_node(node):
|
||||
assert node in INFRA.read_text()
|
||||
|
||||
|
||||
def test_infra_monitoring_uses_any_http_liveness_rule():
|
||||
text = INFRA.read_text()
|
||||
assert "api2/json/version" in text
|
||||
assert "ANY HTTP status" in text or "any-HTTP" in text
|
||||
assert "192.168.68.116:8006" not in _check_health_block(INFRA)
|
||||
|
||||
Reference in New Issue
Block a user