Skip to content

Commit b961220

Browse files
LanNguyenSinguyen-si-ppclaude
authored
test: cover addLedgerFact errors, tool-name-aliases, hook E2E, CLI smoke (L7) (#311)
Closes the L7 test-audit gaps with four new test-only files (49 tests), home isolated via tmp dirs (never the real ~/.harness/): - ledger-add: addLedgerFact error paths (empty command, spawn ENOENT, timeout with verified child kill, server-exit stderr surfacing, JSON-RPC error, and an OK round-trip baseline) via fake MCP scripts under a tmp dir. - tool-name-aliases: expandToolNameAliases / expandCodexHookMatchPattern / extractShellCommand, incl. MCP name-variant edge cases, pinned exactly. - pack hook pre-tool-use: subprocess E2E spawning the real dist/cli/main.js with JSON on stdin, asserting the allow + fail-open paths. - CLI smoke: --help, --version (read dynamically from package.json), and unknown-command exit 64, via the real binary. Review fixes (reviewer subagent): pin HARNESS_HOME under the tmp dir in the hook E2E so machine/project override layers can't resolve against the real home; soften the server-exit stderr assertion to the stable "grounding-mcp exited:" invariant (the stderr tail rides a documented exit-vs-data race); assert the malformed-JSON loud-degradation stderr line. Deferred to follow-up 5839b59e: deny/ask-path E2E envelope coverage and switching addLedgerFact stderr capture to the 'close' event. Refs: task 56a834d2 Co-authored-by: Lan Nguyen Si <nguyen-si@publicplan.de> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 023e264 commit b961220

4 files changed

Lines changed: 647 additions & 0 deletions

File tree

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Subprocess smoke tests for the built harness CLI binary (dist/cli/main.js).
2+
//
3+
// These tests spawn the REAL binary rather than calling run() in-process.
4+
// This exercises the full entry path including the
5+
// `HARNESS_ALLOW_REAL_GENERATED_DIR=1` assignment in src/cli/main.ts that
6+
// only fires in an actual binary invocation, and ensures the dist/ artefact
7+
// matches the expected surface (version string, exit codes).
8+
//
9+
// They complement the in-process tests in tests/cli/program.test.ts, which
10+
// test CLI logic but cannot exercise the real binary entry point.
11+
//
12+
// Build prerequisite: dist/cli/main.js must be up-to-date. Run
13+
// `npm run build` before this suite if you are iterating on CLI code.
14+
//
15+
// Home-dir isolation: --help, --version, and an unknown command do not
16+
// load the harness manifest, so no HARNESS_HOME override is needed.
17+
18+
import * as fs from "node:fs";
19+
import * as path from "node:path";
20+
import { spawnSync } from "node:child_process";
21+
import { fileURLToPath } from "node:url";
22+
import { describe, expect, it } from "vitest";
23+
24+
const __filename = fileURLToPath(import.meta.url);
25+
const REPO_ROOT = path.resolve(path.dirname(__filename), "..", "..");
26+
const MAIN_JS = path.join(REPO_ROOT, "dist", "cli", "main.js");
27+
28+
// Read the expected version from package.json so the test stays correct
29+
// across bumps without a manual update.
30+
const PKG_VERSION = (
31+
JSON.parse(fs.readFileSync(path.join(REPO_ROOT, "package.json"), "utf8")) as {
32+
version: string;
33+
}
34+
).version;
35+
36+
function spawn(args: string[]): { status: number | null; stdout: string; stderr: string } {
37+
const result = spawnSync("node", [MAIN_JS, ...args], {
38+
encoding: "utf8",
39+
timeout: 15_000,
40+
env: { ...process.env },
41+
});
42+
return {
43+
status: result.status,
44+
stdout: result.stdout as string,
45+
stderr: result.stderr as string,
46+
};
47+
}
48+
49+
describe("CLI subprocess smoke — --help", () => {
50+
it("exits 0", () => {
51+
expect(spawn(["--help"]).status).toBe(0);
52+
});
53+
54+
it("prints a Usage: banner to stdout", () => {
55+
expect(spawn(["--help"]).stdout).toMatch(/Usage:/);
56+
});
57+
58+
it("produces no stderr output", () => {
59+
expect(spawn(["--help"]).stderr).toBe("");
60+
});
61+
});
62+
63+
describe("CLI subprocess smoke — --version", () => {
64+
it("exits 0", () => {
65+
expect(spawn(["--version"]).status).toBe(0);
66+
});
67+
68+
it("prints exactly the package.json version on stdout", () => {
69+
// The trim() strips the trailing newline Commander appends.
70+
expect(spawn(["--version"]).stdout.trim()).toBe(PKG_VERSION);
71+
});
72+
73+
it("produces no stderr output", () => {
74+
expect(spawn(["--version"]).stderr).toBe("");
75+
});
76+
});
77+
78+
describe("CLI subprocess smoke — unknown command", () => {
79+
it("exits 64 (EX_USAGE) for an unrecognised command", () => {
80+
expect(spawn(["totally-unknown-cmd-xyz"]).status).toBe(64);
81+
});
82+
83+
it("emits an 'unknown command' message to stderr", () => {
84+
expect(spawn(["totally-unknown-cmd-xyz"]).stderr).toMatch(/unknown command/i);
85+
});
86+
87+
it("produces no stdout output", () => {
88+
expect(spawn(["totally-unknown-cmd-xyz"]).stdout).toBe("");
89+
});
90+
});
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// E2E subprocess tests for `harness pack hook pre-tool-use`.
2+
//
3+
// Spawns the REAL built CLI (dist/cli/main.js) as a child process to verify
4+
// the complete hook entry path — manifest load, pack lookup, decision, stdout
5+
// decision envelope — without mocking internals.
6+
//
7+
// Home-dir isolation: we pass `--config <tmpdir>/harness.yaml` AND set
8+
// HARNESS_HOME to a tmp path. `--config` only overrides the base manifest path;
9+
// the loader still resolves the machine/project override layers under the
10+
// harness home (resolveHomeDir honors $HARNESS_HOME before any disk lookup), so
11+
// without HARNESS_HOME a real ~/.harness/machines override could merge into the
12+
// planted manifest and change the decision. With both set, the child reads and
13+
// writes only under the tmp dir, never the operator's real ~/.harness/.
14+
//
15+
// Deterministic allow path: the planted harness.yaml declares NO
16+
// policy_packs[], so the hook allows with "pack not declared in manifest,
17+
// allowing." before it ever reaches the ledger or approval-marker checks.
18+
// This gives a zero-dependency, fast, reproducible assertion.
19+
//
20+
// Why subprocess (not in-process): main.ts sets
21+
// HARNESS_ALLOW_REAL_GENERATED_DIR=1 before importing. Running the module
22+
// in-process inside vitest would skip that assignment and trip the
23+
// resolvePaths() isolation guard. A subprocess gets a clean module state.
24+
25+
import * as fs from "node:fs";
26+
import * as os from "node:os";
27+
import * as path from "node:path";
28+
import { spawnSync } from "node:child_process";
29+
import { fileURLToPath } from "node:url";
30+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
31+
32+
const __filename = fileURLToPath(import.meta.url);
33+
const REPO_ROOT = path.resolve(path.dirname(__filename), "..", "..");
34+
const MAIN_JS = path.join(REPO_ROOT, "dist", "cli", "main.js");
35+
36+
// Minimal valid harness.yaml with NO policy_packs declared.
37+
// Hook will allow immediately with "pack not declared in manifest, allowing."
38+
const MANIFEST_NO_PACKS = `version: 1
39+
hooks: []
40+
policies: []
41+
tools:
42+
builtin:
43+
known: [Bash, Edit, Write]
44+
`;
45+
46+
let tmpDir: string;
47+
48+
beforeEach(() => {
49+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "harness-hook-e2e-"));
50+
});
51+
52+
afterEach(() => {
53+
fs.rmSync(tmpDir, { recursive: true, force: true });
54+
});
55+
56+
function runHook(
57+
configPath: string,
58+
stdinPayload: string,
59+
): { status: number | null; stdout: string; stderr: string } {
60+
// Strip session-id env vars so the test controls which code path the hook
61+
// takes (otherwise the dev host's $CLAUDE_CODE_SESSION_ID could influence
62+
// the decision for a pack-declared manifest).
63+
const childEnv = { ...process.env };
64+
delete childEnv["CLAUDE_CODE_SESSION_ID"];
65+
delete childEnv["CLAUDE_SESSION_ID"];
66+
// Pin the harness home under the tmp dir so the machine/project override
67+
// layers cannot resolve against the operator's real ~/.harness/.
68+
childEnv["HARNESS_HOME"] = path.join(tmpDir, "home");
69+
70+
const result = spawnSync(
71+
"node",
72+
[MAIN_JS, "pack", "hook", "pre-tool-use", "--config", configPath],
73+
{
74+
input: stdinPayload,
75+
encoding: "utf8",
76+
timeout: 15_000,
77+
env: childEnv,
78+
},
79+
);
80+
return {
81+
status: result.status,
82+
stdout: result.stdout as string,
83+
stderr: result.stderr as string,
84+
};
85+
}
86+
87+
describe("pack hook pre-tool-use — subprocess E2E (allow path)", () => {
88+
it("exits 0 with empty stdout when the pack is not declared in the manifest", () => {
89+
const configPath = path.join(tmpDir, "harness.yaml");
90+
fs.writeFileSync(configPath, MANIFEST_NO_PACKS, "utf8");
91+
92+
const event = JSON.stringify({
93+
session_id: "sess-hook-e2e-1",
94+
tool_name: "Edit",
95+
tool_input: { file_path: "/some/file.ts", old_string: "x", new_string: "y" },
96+
});
97+
98+
const { status, stdout, stderr } = runHook(configPath, event);
99+
100+
expect(status).toBe(0);
101+
// Allow path: hook writes nothing to stdout (only block/ask emit JSON)
102+
expect(stdout.trim()).toBe("");
103+
// The hook always writes a diagnostic line to stderr
104+
expect(stderr).toContain("not declared in manifest");
105+
});
106+
107+
it("exits 0 with empty stdout on malformed stdin JSON (fail-open contract)", () => {
108+
// When stdin is not valid JSON, the hook falls through to allow rather
109+
// than erroring, so a broken event injector never hard-blocks the session.
110+
const configPath = path.join(tmpDir, "harness.yaml");
111+
fs.writeFileSync(configPath, MANIFEST_NO_PACKS, "utf8");
112+
113+
const { status, stdout, stderr } = runHook(configPath, "{not valid json}");
114+
115+
expect(status).toBe(0);
116+
expect(stdout.trim()).toBe("");
117+
// Loud degradation: the fail-open path must announce why on stderr, so a
118+
// silent-swallow regression is caught.
119+
expect(stderr).toContain("malformed event JSON on stdin");
120+
});
121+
122+
it("exits 0 with empty stdout on empty stdin", () => {
123+
const configPath = path.join(tmpDir, "harness.yaml");
124+
fs.writeFileSync(configPath, MANIFEST_NO_PACKS, "utf8");
125+
126+
const { status, stdout } = runHook(configPath, "");
127+
128+
expect(status).toBe(0);
129+
expect(stdout.trim()).toBe("");
130+
});
131+
});

0 commit comments

Comments
 (0)