diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..d1d3353 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,21 @@ +name: test + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: python -m pip install --upgrade pip + - run: pip install -e ".[dev]" + - run: pytest -q diff --git a/pyproject.toml b/pyproject.toml index e585141..76b1f70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "acp-hermes" -version = "0.3.0" +version = "0.3.1" description = "ACP governance plugin for Hermes Agent — audit and veto every tool call via the Agentic Control Plane." readme = "README.md" license = { text = "MIT" } diff --git a/src/acp_hermes/__init__.py b/src/acp_hermes/__init__.py index 123a94c..9b70ef4 100644 --- a/src/acp_hermes/__init__.py +++ b/src/acp_hermes/__init__.py @@ -52,7 +52,7 @@ # Only used when the package is imported from a source tree without dist # metadata (tests, `python -m` from a checkout). tests/test_cli.py pins it # to pyproject's version so a release bump can't leave it behind again. -_FALLBACK_VERSION = "0.3.0" +_FALLBACK_VERSION = "0.3.1" def _dist_version() -> str: @@ -202,6 +202,10 @@ def _post_json(path: str, body: dict[str, Any], token: str) -> dict[str, Any] | return _post_json_detail(path, body, token)[0] +def _notices_off() -> bool: + return os.environ.get("ACP_SHADOW", "").strip().lower() in ("off", "0", "false") + + def _block(message: str) -> dict[str, str]: return {"action": "block", "message": message} @@ -318,6 +322,13 @@ def _post_tool_call( posted, _failure = _post_json_detail("/govern/tool-output", body, token) if posted is None: _requeue_lapses(lapses) + # Gateway notices (cost advisories, shadow) ride `notice`. Hermes has no + # message channel on the post hook, so they go to stderr, the surface this + # plugin already uses for the person. Same contract as the Claude Code + # plugin: ACP_SHADOW=off silences them (gatewaystack-connect#1334). + notice = posted.get("notice") if isinstance(posted, dict) else None + if isinstance(notice, str) and notice.strip() and not _notices_off(): + sys.stderr.write(notice.strip()[:2000] + "\n") def _post_api_request( diff --git a/tests/fixtures/plugin-corpus.json b/tests/fixtures/plugin-corpus.json new file mode 100644 index 0000000..8f0d2de --- /dev/null +++ b/tests/fixtures/plugin-corpus.json @@ -0,0 +1,90 @@ +{ + "corpus": "acp-plugin-conformance", + "version": 1, + "canonicalHome": "davidcrowe/gatewaystack-connect:conformance/plugin-corpus.json", + "tracking": "davidcrowe/gatewaystack-connect#1344 (L1 shared conformance corpus, build step 1)", + "purpose": "One table of plugin capabilities, run against EVERY ACP harness plugin through a thin per-plugin adapter. The adapter drives the plugin's real entry point against a fake gateway on 127.0.0.1 (or a stubbed fetch/urlopen where the repo already does that) and asserts on what the person would see or on what the plugin sent. Unit tests stayed green while four plugins dropped every gateway notice (#1334); this corpus is the seam test that would have caught it.", + "vendoring": "Each plugin vendors a byte-identical copy at test/fixtures/plugin-corpus.json (tests/fixtures/ for pytest repos). Its adapter pins the fingerprint below and fails when the copy differs. To change the corpus: edit this file, recompute the fingerprint, and update every plugin's copy and pin in the same change.", + "fingerprint": "sha256 of the raw file bytes, first 16 hex characters. Byte hashing (not a hash of a re-serialised object) so Node and Python compute the same value without agreeing on JSON serialisation.", + "divergences": "A plugin that fails a supported capability records it in its adapter's EXPECTED_DIVERGENCES list with the issue number. The adapter asserts the list exactly: a NEW failure fails CI, and a FIX also fails CI until the entry is removed. Neither can be quietly forgotten. A divergence is not a not-possible row: not-possible means the harness gives the plugin no way to do it.", + "marker": "ACPCONF7F3A", + "capabilities": { + "notice": { + "contract": "When the gateway's reply to POST /govern/tool-output contains notice: \"\", that text reaches the person-visible channel of the harness (stdout systemMessage, stderr, a toast, a UI notify call, a logger the harness shows, or an editor protocol message). ACP_SHADOW=off silences it on the client side.", + "adapterMust": "Answer every other gateway path with an allow verdict. Isolate HOME (or the plugin's state dir) to a temp directory so first-per-session markers from the developer's machine cannot suppress the notice." + }, + "post-tool": { + "contract": "Given the harness's NATIVE post-tool payload, the plugin's outgoing POST /govern/tool-output request body carries tool_name, tool_input, tool_output, session_id and hook_event_name \"PostToolUse\".", + "adapterMust": "Build the payload in the harness's own native shape (its own field names and tool name) from the canonical call below. Assert tool_name equals the native tool name the adapter fed in, or the plugin's documented canonical mapping of it (the adapter names that mapping explicitly)." + } + }, + "cases": [ + { + "id": "notice-shown", + "capability": "notice", + "env": {}, + "gatewayReply": { "decision": "allow", "notice": "ACPCONF7F3A shadow mode: this call would have been held for review" }, + "expect": { "personSees": true, "contains": "ACPCONF7F3A" }, + "issue": "#1334", + "why": "Codex, OpenCode, Hermes and fx dropped every notice while their unit tests stayed green." + }, + { + "id": "notice-shadow-off", + "capability": "notice", + "env": { "ACP_SHADOW": "off" }, + "gatewayReply": { "decision": "allow", "notice": "ACPCONF7F3A shadow mode: this call would have been held for review" }, + "expect": { "personSees": false, "contains": "ACPCONF7F3A" }, + "issue": "#1334", + "why": "ACP_SHADOW=off is the client-side belt to the server's own shadow switch." + }, + { + "id": "post-tool-fields", + "capability": "post-tool", + "env": {}, + "call": { + "tool": "shell", + "command": "echo ACPCONF7F3A", + "output": "ACPCONF7F3A\n", + "sessionId": "acpconf-session-0001" + }, + "gatewayReply": { "decision": "allow" }, + "expect": { + "method": "POST", + "path": "/govern/tool-output", + "hook_event_name": "PostToolUse", + "tool_name": "equals the native tool name fed in, or the adapter's declared canonical mapping", + "tool_input": "a JSON object whose serialisation contains the marker", + "tool_output": "a value whose serialisation contains the marker", + "session_id": "a non-empty string" + }, + "issue": "#1344", + "why": "fx's post-tool call sends no tool_name or tool_input, so the gateway cannot scan or attribute the output." + } + ], + "harnesses": [ + { "plugin": "claude-code-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "claude-code-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "codex-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "codex-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "opencode-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "opencode-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "hermes-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "hermes-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "pi-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "pi-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "grok-build-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "grok-build-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "antigravity-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "antigravity-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "openclaw-acp-plugin", "capability": "notice", "status": "not-possible", "reason": "OpenClaw exposes no after-tool hook to plugins, so the plugin never calls /govern/tool-output and has no reply to surface. Revisit when OpenClaw ships one." }, + { "plugin": "openclaw-acp-plugin", "capability": "post-tool", "status": "not-possible", "reason": "OpenClaw exposes no after-tool hook to plugins, so there is no native post-tool payload to forward. Revisit when OpenClaw ships one." }, + { "plugin": "dsh-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "dsh-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "fx-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "fx-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "muse-code-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "muse-code-acp-plugin", "capability": "post-tool", "status": "supported" }, + { "plugin": "prime-agent-acp-plugin", "capability": "notice", "status": "supported" }, + { "plugin": "prime-agent-acp-plugin", "capability": "post-tool", "status": "supported" } + ] +} diff --git a/tests/test_conformance.py b/tests/test_conformance.py new file mode 100644 index 0000000..1534ad7 --- /dev/null +++ b/tests/test_conformance.py @@ -0,0 +1,258 @@ +"""ACP plugin conformance adapter for hermes-acp-plugin. + +Drives the REAL entry point (acp_hermes._post_tool_call, the function that +acp_hermes.register() wires onto Hermes's native "post_tool_call" hook name) +against a fake gateway, using the shared corpus vendored at +tests/fixtures/plugin-corpus.json (see davidcrowe/gatewaystack-connect#1344). + +Canonical field mapping (declared per the corpus's post-tool contract): + - tool_name: hermes has NO tool-name canonicalisation table. The native + tool name Hermes hands the hook is forwarded byte for byte as + body["tool_name"]. So case.call["tool"] ("shell") is fed straight in as + the hook's tool_name positional arg and expected back unchanged. + - session_id: acp_hermes._post_tool_call's outgoing wire body sets + body["session_id"] = task_id — NOT the hook's separate `session_id=` + kwarg (see src/acp_hermes/__init__.py). So case.call["sessionId"] is fed + in as this hook's `task_id=` kwarg, which is what a real Hermes session + id would occupy on the wire. + +Run with PYTHONPATH pointing at THIS WORKTREE's src/ so the code under test +is the worktree's, not any installed copy: + + PYTHONPATH=/src -m pytest tests/ -q + +test_worktree_module_is_under_test asserts acp_hermes.__file__ resolves +under this worktree's path, so a wrong PYTHONPATH fails loudly instead of +silently testing the wrong code. +""" + +from __future__ import annotations + +import contextlib +import hashlib +import http.server +import json +import threading +from pathlib import Path + +import pytest + +import acp_hermes + +FIXTURE = Path(__file__).parent / "fixtures" / "plugin-corpus.json" +PINNED_FINGERPRINT = "aa186d3fb3e7d18c" + + +def _load_corpus() -> dict: + raw = FIXTURE.read_bytes() + fp = hashlib.sha256(raw).hexdigest()[:16] + if fp != PINNED_FINGERPRINT: + raise AssertionError( + f"vendored plugin-corpus.json fingerprint {fp} != pinned " + f"{PINNED_FINGERPRINT}; re-vendor from " + "davidcrowe/gatewaystack-connect:conformance/plugin-corpus.json" + ) + return json.loads(raw) + + +CORPUS = _load_corpus() +MARKER = CORPUS["marker"] +CASES = {c["id"]: c for c in CORPUS["cases"]} +PLUGIN_ROWS = { + row["capability"]: row + for row in CORPUS["harnesses"] + if row["plugin"] == "hermes-acp-plugin" +} + +# A case listed here is EXPECTED to currently fail the corpus contract. +# Each entry is asserted to actually fail (so a fix turns this file red +# until the entry is removed), and the full list is checked against every +# case's actual outcome (so an undeclared NEW failure also turns this file +# red instead of silently passing). +# #1334 (notice-shown) was fixed on this branch; no divergences remain. +EXPECTED_DIVERGENCES: list[dict] = [] +_DIVERGENT_CASE_IDS = {d["case"] for d in EXPECTED_DIVERGENCES} + + +def test_fingerprint_pinned() -> None: + assert hashlib.sha256(FIXTURE.read_bytes()).hexdigest()[:16] == PINNED_FINGERPRINT + + +def test_plugin_rows_present_and_supported() -> None: + assert PLUGIN_ROWS["notice"]["status"] == "supported" + assert PLUGIN_ROWS["post-tool"]["status"] == "supported" + + +def test_worktree_module_is_under_test() -> None: + # Guards against a bad PYTHONPATH silently exercising an installed copy + # (e.g. the primary checkout's .venv site-packages) instead of this + # worktree's src/. + # Must hold in any checkout (a CI clone, a worktree), so compare against + # this repo's own src/ rather than a folder name. + from pathlib import Path + + repo_src = (Path(__file__).resolve().parents[1] / "src").resolve() + module = Path(acp_hermes.__file__).resolve() + assert repo_src in module.parents, ( + f"acp_hermes imported from {str(module)!r}, not {str(repo_src)!r} — " + "check PYTHONPATH" + ) + + +# --- fake gateway ------------------------------------------------------------ + + +@contextlib.contextmanager +def fake_gateway(reply_for_tool_output: dict): + """Real HTTP server on 127.0.0.1:0. /govern/tool-output -> reply_for_tool_output. + Every other path -> {"decision": "allow"}. Records every request.""" + requests: list[dict] = [] + + class Handler(http.server.BaseHTTPRequestHandler): + def do_POST(self) -> None: # noqa: N802 - stdlib method name + length = int(self.headers.get("Content-Length", "0")) + raw = self.rfile.read(length) if length else b"" + try: + body = json.loads(raw) if raw else None + except json.JSONDecodeError: + body = raw.decode("utf-8", "replace") + requests.append({"method": "POST", "path": self.path, "body": body}) + reply = reply_for_tool_output if self.path == "/govern/tool-output" else {"decision": "allow"} + payload = json.dumps(reply).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, *_args: object) -> None: # silence stderr noise + pass + + server = http.server.HTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server, requests + finally: + server.shutdown() + thread.join(timeout=2) + + +@pytest.fixture(autouse=True) +def _isolate_env(tmp_path, monkeypatch): + # Isolate HOME so no first-per-session marker or real credential on the + # developer's machine can influence the plugin. + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("ACP_SHADOW", raising=False) + monkeypatch.delenv("ACP_BEARER_TOKEN", raising=False) + acp_dir = tmp_path / ".acp" + acp_dir.mkdir() + # Dummy credential in the plugin's own expected shape/prefix. Never real. + (acp_dir / "credentials").write_text("gsk_conformance_dummy_not_real\n") + yield + + +def _call_post_tool(monkeypatch, server, env: dict, *, tool: str, command: str, output: str, session_id: str) -> None: + for k, v in env.items(): + monkeypatch.setenv(k, v) + port = server.server_address[1] + monkeypatch.setenv("ACP_API_BASE", f"http://127.0.0.1:{port}") + acp_hermes._post_tool_call( + tool, + {"command": command}, + result=output, + task_id=session_id, # see module docstring: lands in body["session_id"] + duration_ms=5, + ) + + +# --- notice capability -------------------------------------------------------- + + +@pytest.mark.parametrize("case_id", ["notice-shown", "notice-shadow-off"]) +def test_notice_channel_stdout_stderr(case_id, capsys, monkeypatch) -> None: + """Channels asserted: process stdout and stderr — acp_hermes has no + toast/logger/UI-notify surface, so these are the only person-visible + channels its process can use.""" + case = CASES[case_id] + with fake_gateway(case["gatewayReply"]) as (server, _requests): + _call_post_tool( + monkeypatch, + server, + case["env"], + tool="shell", + command=f"echo {MARKER}", + output=f"{MARKER}\n", + session_id="acpconf-session-0001", + ) + out, err = capsys.readouterr() + seen = (MARKER in out) or (MARKER in err) + expected = case["expect"]["personSees"] + + if case_id in _DIVERGENT_CASE_IDS: + assert seen != expected, ( + f"{case_id}: now matches the corpus contract (seen={seen}); " + "fixed, remove the EXPECTED_DIVERGENCES entry" + ) + else: + assert seen == expected, f"stdout={out!r} stderr={err!r}" + + +def test_expected_divergences_matches_actual_failures(monkeypatch, capsys) -> None: + """Recompute pass/fail for every notice case independently of the test + above and assert EXPECTED_DIVERGENCES is exactly the set that actually + fails — so an undeclared NEW failure (or a silently-fixed one) also + turns this file red.""" + actual_failures = [] + for case_id in ("notice-shown", "notice-shadow-off"): + case = CASES[case_id] + with fake_gateway(case["gatewayReply"]) as (server, _requests): + _call_post_tool( + monkeypatch, + server, + case["env"], + tool="shell", + command=f"echo {MARKER}", + output=f"{MARKER}\n", + session_id="acpconf-session-0001", + ) + out, err = capsys.readouterr() + seen = (MARKER in out) or (MARKER in err) + if seen != case["expect"]["personSees"]: + actual_failures.append(case_id) + monkeypatch.delenv("ACP_SHADOW", raising=False) + + assert sorted(actual_failures) == sorted(_DIVERGENT_CASE_IDS) + + +# --- post-tool capability ----------------------------------------------------- + +# Declared canonical mapping: hermes performs no tool-name canonicalisation, +# so the native tool name is expected back on the wire unchanged. +NATIVE_TOOL_NAME = "shell" + + +def test_post_tool_fields_reach_gateway(monkeypatch) -> None: + case = CASES["post-tool-fields"] + call = case["call"] + with fake_gateway(case["gatewayReply"]) as (server, requests): + _call_post_tool( + monkeypatch, + server, + case["env"], + tool=call["tool"], + command=call["command"], + output=call["output"], + session_id=call["sessionId"], + ) + + posts = [r for r in requests if r["path"] == "/govern/tool-output" and r["method"] == "POST"] + assert len(posts) == 1, f"expected exactly one POST /govern/tool-output, got {requests!r}" + body = posts[0]["body"] + + assert body["hook_event_name"] == "PostToolUse" + assert body["tool_name"] == NATIVE_TOOL_NAME + assert isinstance(body["tool_input"], dict) + assert MARKER in json.dumps(body["tool_input"]) + assert MARKER in json.dumps(body["tool_output"]) + assert isinstance(body["session_id"], str) and body["session_id"] diff --git a/tests/test_hooks.py b/tests/test_hooks.py index d5c95fd..6ee2554 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -143,6 +143,22 @@ def test_post_observational_no_return(tmp_path): assert result is None +def test_post_shows_gateway_notice_on_stderr(tmp_path, capsys): + _write_token(tmp_path) + with patch("urllib.request.urlopen", _fake_urlopen({"action": "pass", "notice": "[ACP cost] Context is 400k tokens."})): + result = _post_tool_call("terminal", {"command": "ls"}, result="ok", task_id="t", duration_ms=1) + assert result is None + assert "[ACP cost] Context is 400k tokens." in capsys.readouterr().err + + +def test_post_notice_silenced_by_acp_shadow_off(tmp_path, capsys, monkeypatch): + _write_token(tmp_path) + monkeypatch.setenv("ACP_SHADOW", "off") + with patch("urllib.request.urlopen", _fake_urlopen({"action": "pass", "notice": "[ACP cost] x"})): + _post_tool_call("terminal", {}, result="ok", task_id="t", duration_ms=1) + assert "[ACP cost] x" not in capsys.readouterr().err + + def test_post_truncates_large_payload(tmp_path): _write_token(tmp_path) big = "x" * (300 * 1024) # 300 KB, over the 200 KB ceiling