no-mistakes(review): Ignore commented infisical paths; normalize probe-model tests

docs/probe-drift-round2-evidence.md

@@ -240,7 +240,7 @@ The health script prints `📍 executed from: script=… cwd=…` and includes
 $ pwd -P
 /root/.treehouse/prose-contracts-9ce5f3/3/prose-contracts
 $ python3 -m pytest -q
-23 passed
+22 passed
 $ shellcheck scripts/prose-lint.sh
 (clean)
 ```

scripts/agent-health-check.py

@@ -490,6 +490,26 @@ def check_config_integrity():
 # CHECK 6: Wrapper/CLI Integrity (NEW)
 # ═══════════════════════════════════════════════════════════════════
 
+def _infisical_invocation_paths(wrapper_body):
+    """Absolute infisical paths the wrapper actually invokes.
+
+    Only executed (non-comment) lines count, and only a path followed by a real
+    infisical subcommand (e.g. `/usr/bin/infisical run`) is treated as an
+    invocation. A note such as `# migrated from /usr/local/bin/infisical` is
+    prose, not a call, so it must not manufacture a dangling-path false alarm.
+    """
+    paths = []
+    for line in wrapper_body.splitlines():
+        code = line.split("#", 1)[0]
+        for _m in re.finditer(
+            r"(/[A-Za-z0-9._/-]*infisical)\s+(?:run|export|secrets|login|logout)\b",
+            code,
+        ):
+            if _m.group(1) not in paths:
+                paths.append(_m.group(1))
+    return paths
+
+
 def check_wrapper_integrity():
     """Verify the hermes CLI wrapper exists and can reach hermes-real."""
     for name, agent in AGENTS.items():
@@ -525,29 +545,32 @@ def check_wrapper_integrity():
         # the first 20 lines, so koonimo's wrapper — which DOES reference
         # /usr/bin/infisical, just past line 20 — false-failed as "path may be
         # wrong". Read the full body, accept a no-infisical wrapper, and verify
-        # that any absolute infisical path the wrapper hardcodes actually exists
-        # (PATH resolution alone is not enough — a dangling /usr/bin/infisical is
-        # a broken wrapper even when a different infisical is on PATH).
+        # the absolute infisical path(s) the wrapper actually invokes. Only
+        # executed invocation lines count: a comment or dead prose mentioning a
+        # removed path (litellm-api-keys.prose.md documents
+        # `rm -f /usr/local/bin/infisical`) must not false-fail a wrapper whose
+        # real invocation works.
         wrapper_body = ssh(host, "cat /root/.local/bin/hermes 2>/dev/null", user=user) or ""
+        invoked_paths = _infisical_invocation_paths(wrapper_body)
         if "infisical" in wrapper_body:
-            inf_paths = []
-            for _m in re.finditer(r"(/[A-Za-z0-9._/-]*infisical)", wrapper_body):
-                if _m.group(1) not in inf_paths:
-                    inf_paths.append(_m.group(1))
-            dangling = []
-            for _p in inf_paths:
-                _exists = ssh(host, f"test -x {_p} && echo OK || echo MISS", user=user)
-                if not _exists or _exists.strip().splitlines()[-1] != "OK":
-                    dangling.append(_p)
-            if inf_paths:
-                if dangling:
+            if invoked_paths:
+                missing = []
+                for _p in invoked_paths:
+                    _exists = ssh(host, f"test -x {_p} && echo OK || echo MISS", user=user)
+                    if not _exists or _exists.strip().splitlines()[-1] != "OK":
+                        missing.append(_p)
+                if len(missing) == len(invoked_paths):
                     inf_actual = ssh(host, "command -v infisical 2>/dev/null", user=user)
                     suffix = f" (infisical at {inf_actual})" if inf_actual else ""
-                    print(f"  ❌ {name}: wrapper hardcodes missing infisical path(s) "
-                          f"{', '.join(dangling)}{suffix}")
+                    print(f"  ❌ {name}: wrapper invokes infisical via missing path(s) "
+                          f"{', '.join(missing)}{suffix}")
                     _fail(f"wrapper-infisical-path:{name}", name)
-                elif "/usr/bin/infisical" not in wrapper_body:
-                    print(f"  ⚠️  {name}: wrapper infisical path differs — informational")
+                elif missing:
+                    print(f"  ⚠️  {name}: wrapper has an unused/missing infisical path "
+                          f"({', '.join(missing)}) but a working invocation — informational")
+                elif "/usr/bin/infisical" not in invoked_paths:
+                    print(f"  ⚠️  {name}: wrapper infisical path differs "
+                          f"({', '.join(invoked_paths)}) — informational")
                 else:
                     print(f"  ✅ {name}: wrapper infisical path OK")
             else:

tests/test_probe_drift.py

@@ -172,6 +172,9 @@ def _stub_wrapper_ssh(ahc, monkeypatch, wrapper_body, test_x_result="OK", comman
         if cmd.startswith("grep -c 'LITELLM_API_KEY'"):
             return "1"
         if cmd.startswith("test -x "):
+            path = cmd[len("test -x "):].split()[0]
+            if isinstance(test_x_result, dict):
+                return test_x_result.get(path, "MISS")
             return test_x_result
         if cmd.startswith("command -v infisical"):
             return command_v
@@ -213,6 +216,21 @@ def test_existing_absolute_infisical_path_passes(ahc, monkeypatch, capsys):
     assert "wrapper infisical path OK" in out
 
 
+def test_comment_mentioning_removed_infisical_path_is_not_failed(ahc, monkeypatch, capsys):
+    # litellm-api-keys.prose.md documents `rm -f /usr/local/bin/infisical`; a
+    # wrapper comment about that migration must not manufacture a dangling path
+    # when the real invocation (/usr/bin/infisical) is present and executable.
+    _stub_wrapper_ssh(ahc, monkeypatch,
+                      "#!/bin/bash\n# migrated from /usr/local/bin/infisical\n"
+                      "exec /usr/bin/infisical run -- hermes-real \"$@\"\n",
+                      test_x_result={"/usr/bin/infisical": "OK",
+                                     "/usr/local/bin/infisical": "MISS"})
+    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("""\
@@ -324,41 +342,100 @@ def _check_health_block(contract):
     return match.group(1)
 
 
-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 _loop_nodes(block):
+    nodes = []
+    for line in block.splitlines():
+        match = re.match(r"\s*for\s+\w+\s+in\s+(.+?);?\s*do\b", line)
+        if match:
+            nodes = match.group(1).split()
+    return nodes
+
+
+def _record(url):
+    """Normalize a URL into a probe record: host, port, path, expected status."""
+    match = re.match(r"https?://([^/\s\"')]+)(/[^\s\"')]*)?", url)
+    assert match, f"unparseable probe URL: {url}"
+    hostport = match.group(1)
+    if "@" in hostport:
+        hostport = hostport.split("@", 1)[1]
+    if hostport.startswith("["):
+        host, port = hostport[1:hostport.index("]")], None
+    elif ":" in hostport:
+        host, raw_port = hostport.rsplit(":", 1)
+        port = int(raw_port) if raw_port.isdigit() else None
+    else:
+        host, port = hostport, None
+    return {"host": host, "port": port, "path": match.group(2) or "/",
+            "expected": None}
+
+
+def _probes(block):
+    """Parse the executable check-health bash block into a normalized probe model.
+
+    Comments are not probes; an `# Expected: <status>` comment annotates the
+    preceding probe. URLs using the block's shell-loop variable `$node` are
+    expanded over the loop's node list.
+    """
+    loop_nodes = _loop_nodes(block)
+    probes = []
+    last = None
+    for raw in block.splitlines():
+        stripped = raw.strip()
+        if stripped.startswith("#"):
+            expected = re.search(r"Expected:\s*(\d{3})", stripped, re.I)
+            if expected and last is not None:
+                last["expected"] = int(expected.group(1))
+            continue
+        for url in re.findall(r"https?://[^\s\"')]+", raw):
+            hosts = loop_nodes if "$node" in url else [None]
+            for node in hosts:
+                record = _record(url.replace("$node", node) if node else url)
+                probes.append(record)
+                last = record
+    return probes
 
 
 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
+    probes = _probes(_check_health_block(GPU))
+    targets = {(p["host"], p["port"], p["path"]) for p in probes}
+    assert ("192.168.68.8", 8080, "/health") in targets
+    assert ("192.168.68.110", 8080, "/health") in targets
 
 
 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)
+    probes = _probes(_check_health_block(GPU))
+    gpu_hosts = {"192.168.68.8", "192.168.68.110", "192.168.68.15"}
+    offenders = [p for p in probes
+                 if p["host"] in gpu_hosts and p["port"] in (None, 80)]
+    assert offenders == []
 
 
-def test_gpu_monitor_documents_router_301_as_alive():
-    block = _check_health_block(GPU)
-    assert "/health/unified" in block
-    assert "301" in block
+def test_probe_model_flags_explicit_port_80_on_gpu_host():
+    # Regression: a bare-port probe may be spelled with an explicit :80.
+    block = ("curl -s -o /dev/null -w '%{http_code}' "
+             "http://192.168.68.8:80/health\n")
+    gpu_hosts = {"192.168.68.8", "192.168.68.110", "192.168.68.15"}
+    offenders = [p for p in _probes(block)
+                 if p["host"] in gpu_hosts and p["port"] in (None, 80)]
+    assert offenders and offenders[0]["port"] == 80
+
+
+def test_gpu_monitor_treats_router_301_as_alive():
+    probes = _probes(_check_health_block(GPU))
+    unified = [p for p in probes
+               if p["host"] == "192.168.68.116" and p["path"] == "/health/unified"]
+    assert unified, "router /health/unified probe missing"
+    assert unified[0]["expected"] == 301
 
 
 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
+    probes = _probes(_check_health_block(INFRA))
+    pve = {(p["host"], p["port"], p["path"]) for p in probes if p["port"] == 8006}
+    assert {host for host, _, _ in pve} == PVE_NODE_IPS
+    assert {path for _, _, path in pve} == {"/api2/json/version"}
 
 
 def test_infra_monitoring_does_not_probe_ct116_for_pve_api():
-    assert "192.168.68.116:8006" not in _check_health_block(INFRA)
+    probes = _probes(_check_health_block(INFRA))
+    assert not any(p["host"] == "192.168.68.116" and p["port"] == 8006
+                   for p in probes)