diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..04348bc --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,19 @@ +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-node@v4 + with: + node-version: "20" + - run: node --test test/ diff --git a/bin/govern.mjs b/bin/govern.mjs index 3fd775c..d758c52 100755 --- a/bin/govern.mjs +++ b/bin/govern.mjs @@ -64,7 +64,7 @@ const ACP_GOVERN = process.env.ACP_API_BASE || "https://govern.agenticcontrolplane.com"; -const PLUGIN_VERSION = "0.6.7"; +const PLUGIN_VERSION = "0.6.8"; // Identifies the calling client to the server (per-client policy routing). // Each client's hooks.json sets this env var at invocation time: @@ -152,7 +152,12 @@ if (!token) process.exit(0); let input; try { - input = JSON.parse(readFileSync("/dev/stdin", "utf8")); + // Read stdin as a stream. readFileSync("/dev/stdin") throws EAGAIN on + // Linux when the parent hands over a non-blocking pipe (Node's own + // child_process does), which made the hook exit silently there. + const chunks = []; + for await (const chunk of process.stdin) chunks.push(chunk); + input = JSON.parse(Buffer.concat(chunks).toString("utf8")); } catch { process.exit(0); } @@ -434,6 +439,8 @@ async function handlePreToolUse() { /* PostToolUse */ /* ------------------------------------------------------------------ */ +const NOTICES_OFF = /^(off|0|false)$/i.test(process.env.ACP_SHADOW ?? ""); + async function handlePostToolUse() { let outputStr = ""; try { @@ -463,11 +470,18 @@ async function handlePostToolUse() { clearTimeout(timeout); if (!res.ok) { process.exit(0); } const data = await res.json(); + // Gateway notices (cost advisories, shadow counterfactuals) ride the + // `notice` field. Same contract as the Claude Code plugin: show it to the + // person as systemMessage, never to the model; ACP_SHADOW=off silences it. + // gatewaystack-connect#1334 — this plugin used to drop every notice. + const lines = []; if (data.action === "redact" || data.action === "block") { - process.stdout.write(JSON.stringify({ - systemMessage: `[ACP] ${data.action === "block" ? "Blocked" : "Flagged"}: ${data.reason || "governance policy"}`, - })); + lines.push(`[ACP] ${data.action === "block" ? "Blocked" : "Flagged"}: ${data.reason || "governance policy"}`); + } + if (!NOTICES_OFF && typeof data.notice === "string" && data.notice.trim()) { + lines.push(data.notice.trim().slice(0, 2000)); } + if (lines.length) process.stdout.write(JSON.stringify({ systemMessage: lines.join("\n") })); } catch { // silent pass-through } finally { clearTimeout(timeout); } diff --git a/test/conformance.test.mjs b/test/conformance.test.mjs new file mode 100644 index 0000000..354b845 --- /dev/null +++ b/test/conformance.test.mjs @@ -0,0 +1,275 @@ +// ACP plugin conformance adapter (davidcrowe/gatewaystack-connect#1344). +// +// Drives the REAL bin/govern.mjs entry point (the same script hooks.json +// invokes for PostToolUse) against a fake gateway on 127.0.0.1:0, using the +// shared corpus vendored at test/fixtures/plugin-corpus.json. See that repo's +// conformance/plugin-corpus.json for the capability contracts. +// +// Run with: node --test test/ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { join, dirname } from "node:path"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { createHash } from "node:crypto"; +import http from "node:http"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const GOVERN = join(HERE, "..", "bin", "govern.mjs"); +const CORPUS_PATH = join(HERE, "fixtures", "plugin-corpus.json"); +const PINNED_FINGERPRINT = "aa186d3fb3e7d18c"; +const PLUGIN_NAME = "codex-acp-plugin"; + +/* ------------------------------------------------------------------ */ +/* Fingerprint gate */ +/* ------------------------------------------------------------------ */ + +const corpusBytes = readFileSync(CORPUS_PATH); +const fingerprint = createHash("sha256").update(corpusBytes).digest("hex").slice(0, 16); + +test("vendored corpus matches the pinned fingerprint", () => { + assert.equal( + fingerprint, + PINNED_FINGERPRINT, + `test/fixtures/plugin-corpus.json hashes to ${fingerprint}, pinned is ${PINNED_FINGERPRINT}. ` + + "Re-vendor a byte-identical copy from davidcrowe/gatewaystack-connect:conformance/plugin-corpus.json." + ); +}); + +const corpus = JSON.parse(corpusBytes.toString("utf8")); +const MARKER = corpus.marker; +const casesById = Object.fromEntries(corpus.cases.map((c) => [c.id, c])); +const rows = corpus.harnesses.filter((h) => h.plugin === PLUGIN_NAME); +const rowFor = (capability) => rows.find((r) => r.capability === capability); + +// codex-acp-plugin's bin/govern.mjs adopted Claude Code's hook wire format +// (see the HARNESS comment at the top of govern.mjs): a shell command's +// native tool_name is "Bash", the same as Claude Code, not the corpus's +// generic "shell". Declared explicitly per the corpus's adapterMust note. +const CANONICAL_TOOL_NAME_MAP = { shell: "Bash" }; + +/* ------------------------------------------------------------------ */ +/* EXPECTED_DIVERGENCES */ +/* */ +/* A case listed here is expected to FAIL on this branch. The adapter */ +/* asserts that it actually fails, so a real fix flips this red until */ +/* the entry is removed — a divergence can't be quietly forgotten, and */ +/* a regression can't slip in silently either. */ +/* ------------------------------------------------------------------ */ + +// #1334 (notice-shown) was fixed on this branch; no divergences remain. +const EXPECTED_DIVERGENCES = []; + +test("EXPECTED_DIVERGENCES is exactly what's recorded", () => { + assert.deepEqual( + EXPECTED_DIVERGENCES.map((d) => d.case).sort(), + [] + ); +}); + +const isExpectedDivergence = (caseId) => EXPECTED_DIVERGENCES.some((d) => d.case === caseId); + +/* ------------------------------------------------------------------ */ +/* Fake gateway */ +/* ------------------------------------------------------------------ */ + +function startFakeGateway(postToolOutputReply) { + const requests = []; + const server = http.createServer((req, res) => { + let raw = ""; + req.on("data", (chunk) => (raw += chunk)); + req.on("end", () => { + let body = null; + try { + body = raw ? JSON.parse(raw) : null; + } catch { + body = raw; + } + requests.push({ method: req.method, path: req.url, body }); + res.setHeader("Content-Type", "application/json"); + if (req.method === "POST" && req.url === "/govern/tool-output") { + res.end(JSON.stringify(postToolOutputReply)); + } else { + res.end(JSON.stringify({ decision: "allow" })); + } + }); + }); + return { server, requests }; +} + +function listen(server) { + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => resolve(server.address().port)); + }); +} + +function close(server) { + return new Promise((resolve) => server.close(() => resolve())); +} + +/* ------------------------------------------------------------------ */ +/* Driving the real entry point */ +/* ------------------------------------------------------------------ */ + +// Isolates HOME per call so no first-per-session marker or credential file +// on the developer's machine can suppress a notice or short-circuit the +// hook. Only ACP_SHADOW is ever set explicitly (via extraEnv); it is never +// inherited because we build the child's env from scratch, not by spreading +// process.env. +// +// Uses async spawn rather than execFileSync deliberately: the fake gateway +// below runs in-process (same event loop as this test file). A *synchronous* +// child-process call would block that event loop while waiting for the +// child to exit, and the child's fetch to our own server would then never +// get serviced — a self-deadlock that silently resolves only once the +// child's own fetch abort timer fires. Async spawn keeps the event loop free +// so the in-process server can answer while we await the child's exit. +function runHook(event, extraEnv, gatewayBase) { + return new Promise((resolve) => { + const homeDir = mkdtempSync(join(tmpdir(), "acp-conf-home-")); + const env = { + PATH: process.env.PATH, + HOME: homeDir, + // Dummy credential string — never a real one. + ACP_BEARER_TOKEN: "gsk_conformance_dummy_token_0000", + ACP_GOVERN_BASE: gatewayBase, + ACP_API_BASE: gatewayBase, + ACP_HARNESS: "codex", + ACP_CLIENT: "codex-plugin", + ...extraEnv, + }; + const child = spawn("node", [GOVERN], { env }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (d) => (stdout += d)); + child.stderr.on("data", (d) => (stderr += d)); + child.on("close", () => resolve({ stdout, stderr })); + child.stdin.end(JSON.stringify(event)); + }); +} + +function markerVisible(result) { + return result.stdout.includes(MARKER) || result.stderr.includes(MARKER); +} + +/* ------------------------------------------------------------------ */ +/* notice capability */ +/* ------------------------------------------------------------------ */ + +const noticeRow = rowFor("notice"); + +test(`notice capability row is "${noticeRow?.status}" for ${PLUGIN_NAME}`, () => { + assert.ok(noticeRow, "corpus has no notice row for this plugin"); +}); + +if (noticeRow?.status === "not-possible") { + test("notice: not-possible row states a reason", () => { + assert.ok(noticeRow.reason && noticeRow.reason.length > 0); + }); +} else { + test('notice-shown: gateway "notice" text reaches stdout systemMessage [EXPECTED DIVERGENCE #1334]', async () => { + const c = casesById["notice-shown"]; + const { server, requests } = startFakeGateway(c.gatewayReply); + const port = await listen(server); + const event = { + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { command: `echo ${MARKER}` }, + tool_response: `${MARKER}\n`, + session_id: "acpconf-session-notice-shown", + permission_mode: "default", + }; + const result = await runHook(event, c.env, `http://127.0.0.1:${port}`); + await close(server); + + // Guard against a false pass: confirm the plugin actually reached our + // fake gateway before drawing any conclusion from what did or didn't + // show up on stdout/stderr. (An in-process fake gateway driven by a + // *synchronous* child-process call would never see this request at all — + // the sync call blocks the event loop that the server needs to answer — + // and a "marker absent" read would then be indistinguishable from the + // real #1334 bug. runHook uses async spawn precisely to avoid that.) + const posted = requests.find((r) => r.method === "POST" && r.path === "/govern/tool-output"); + assert.ok(posted, "plugin never POSTed to /govern/tool-output — cannot conclude anything about notice display"); + + const seen = markerVisible(result); + assert.equal(isExpectedDivergence(c.id), false); + assert.equal(seen, c.expect.personSees, "#1334: the notice marker must appear on stdout/stderr"); + }); + + test("notice-shadow-off: ACP_SHADOW=off keeps the marker off stdout/stderr", async () => { + const c = casesById["notice-shadow-off"]; + const { server, requests } = startFakeGateway(c.gatewayReply); + const port = await listen(server); + const event = { + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { command: `echo ${MARKER}` }, + tool_response: `${MARKER}\n`, + session_id: "acpconf-session-notice-shadow-off", + permission_mode: "default", + }; + const result = await runHook(event, c.env, `http://127.0.0.1:${port}`); + await close(server); + // Same false-pass guard as notice-shown: confirm the request actually + // arrived before trusting the absence of the marker. + const posted = requests.find((r) => r.method === "POST" && r.path === "/govern/tool-output"); + assert.ok(posted, "plugin never POSTed to /govern/tool-output — cannot conclude anything about notice display"); + // This plugin currently drops every notice regardless of ACP_SHADOW, so + // this case trivially passes today (nothing is shown either way). It + // still runs for real so a future fix that reads data.notice but forgets + // to gate on ACP_SHADOW would be caught here. + assert.equal(markerVisible(result), false); + }); +} + +/* ------------------------------------------------------------------ */ +/* post-tool capability */ +/* ------------------------------------------------------------------ */ + +const postToolRow = rowFor("post-tool"); + +test(`post-tool capability row is "${postToolRow?.status}" for ${PLUGIN_NAME}`, () => { + assert.ok(postToolRow, "corpus has no post-tool row for this plugin"); +}); + +if (postToolRow?.status === "not-possible") { + test("post-tool: not-possible row states a reason", () => { + assert.ok(postToolRow.reason && postToolRow.reason.length > 0); + }); +} else { + test("post-tool-fields: native PostToolUse payload reaches /govern/tool-output with required fields", async () => { + const c = casesById["post-tool-fields"]; + const { server, requests } = startFakeGateway(c.gatewayReply); + const port = await listen(server); + const nativeToolName = CANONICAL_TOOL_NAME_MAP[c.call.tool] ?? c.call.tool; + const event = { + hook_event_name: "PostToolUse", + tool_name: nativeToolName, + tool_input: { command: c.call.command }, + tool_response: c.call.output, + session_id: c.call.sessionId, + permission_mode: "default", + }; + await runHook(event, c.env, `http://127.0.0.1:${port}`); + await close(server); + + const posted = requests.find((r) => r.method === "POST" && r.path === "/govern/tool-output"); + assert.ok(posted, "plugin never POSTed to /govern/tool-output"); + const body = posted.body; + assert.equal(body.hook_event_name, "PostToolUse"); + assert.equal( + body.tool_name, + nativeToolName, + `tool_name must equal the native tool name fed in ("${nativeToolName}") or the declared canonical mapping ` + + `(CANONICAL_TOOL_NAME_MAP: corpus "shell" -> native "Bash")` + ); + assert.ok(body.tool_input && typeof body.tool_input === "object", "tool_input must be an object"); + assert.ok(JSON.stringify(body.tool_input).includes(MARKER), "tool_input JSON must contain the marker"); + assert.ok(JSON.stringify(body.tool_output).includes(MARKER), "tool_output JSON must contain the marker"); + assert.equal(typeof body.session_id, "string"); + assert.ok(body.session_id.length > 0, "session_id must be a non-empty string"); + }); +} diff --git a/test/fixtures/plugin-corpus.json b/test/fixtures/plugin-corpus.json new file mode 100644 index 0000000..8f0d2de --- /dev/null +++ b/test/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/test/post-notice.test.mjs b/test/post-notice.test.mjs new file mode 100644 index 0000000..ccb8ce0 --- /dev/null +++ b/test/post-notice.test.mjs @@ -0,0 +1,53 @@ +// PostToolUse shows the gateway's `notice` to the person (gatewaystack-connect#1334). +// Run with: node --test test/ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { createServer } from "node:http"; +import { fileURLToPath } from "node:url"; +import { join, dirname } from "node:path"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; + +const GOVERN = join(dirname(fileURLToPath(import.meta.url)), "..", "bin", "govern.mjs"); + +async function withGateway(reply, fn) { + const server = createServer((req, res) => { + let body = ""; + req.on("data", (c) => { body += c; }); + req.on("end", () => { res.setHeader("content-type", "application/json"); res.end(JSON.stringify(reply)); }); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + try { return await fn(`http://127.0.0.1:${server.address().port}`); } finally { server.close(); } +} + +function runPost(base, extraEnv = {}) { + return new Promise((resolve, reject) => { + const child = execFile("node", [GOVERN], { + env: { ...process.env, HOME: mkdtempSync(join(tmpdir(), "acp-notice-")), ACP_BEARER_TOKEN: "test-token", ACP_GOVERN_BASE: base, ACP_API_BASE: base, ...extraEnv }, + encoding: "utf8", + }, (err, stdout) => (err ? reject(err) : resolve(stdout))); + child.stdin.end(JSON.stringify({ hook_event_name: "PostToolUse", tool_name: "shell", tool_input: { command: "ls" }, tool_response: "a b", session_id: "s1", tool_use_id: "c1" })); + }); +} + +test("a pass with a notice shows it as systemMessage", async () => { + const out = await withGateway({ action: "pass", notice: "[ACP cost] Context is 400k tokens." }, runPost); + assert.equal(JSON.parse(out).systemMessage, "[ACP cost] Context is 400k tokens."); +}); + +test("a block and a notice are both shown, block first", async () => { + const out = await withGateway({ action: "block", reason: "secret", notice: "[ACP cost] x" }, runPost); + assert.equal(JSON.parse(out).systemMessage, "[ACP] Blocked: secret\n[ACP cost] x"); +}); + +test("no notice and a pass writes nothing", async () => { + const out = await withGateway({ action: "pass" }, runPost); + assert.equal(out, ""); +}); + +test("ACP_SHADOW=off silences notices but not blocks", async () => { + assert.equal(await withGateway({ action: "pass", notice: "n" }, (b) => runPost(b, { ACP_SHADOW: "off" })), ""); + const out = await withGateway({ action: "block", reason: "r", notice: "n" }, (b) => runPost(b, { ACP_SHADOW: "off" })); + assert.equal(JSON.parse(out).systemMessage, "[ACP] Blocked: r"); +});