Skip to content

Commit 31ee040

Browse files
committed
deduplicate multi-scope hook invocations
1 parent 446484b commit 31ee040

4 files changed

Lines changed: 284 additions & 0 deletions

File tree

‎__tests__/hooks/handler.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ vi.mock("../../src/hooks/hook-logger", () => ({
4949
hookLogError: vi.fn(),
5050
}));
5151

52+
vi.mock("../../src/hooks/hook-invocation-dedup", () => ({
53+
claimHookInvocation: vi.fn(() => Promise.resolve({ role: "independent" })),
54+
}));
55+
5256
describe("hooks/handler", () => {
5357
let stderrSpy: ReturnType<typeof vi.spyOn>;
5458
let stdoutSpy: ReturnType<typeof vi.spyOn>;
@@ -1251,6 +1255,28 @@ describe("hooks/handler", () => {
12511255
);
12521256
});
12531257

1258+
it("replays a duplicate decision without evaluating or persisting again", async () => {
1259+
const { claimHookInvocation } = await import("../../src/hooks/hook-invocation-dedup");
1260+
vi.mocked(claimHookInvocation).mockResolvedValueOnce({
1261+
role: "duplicate",
1262+
response: { exitCode: 2, stdout: "", stderr: "blocked once" },
1263+
});
1264+
mockStdin(JSON.stringify({
1265+
session_id: "session-1",
1266+
tool_use_id: "tool-1",
1267+
tool_name: "Bash",
1268+
}));
1269+
const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator");
1270+
const { persistHookActivity } = await import("../../src/hooks/hook-activity-store");
1271+
1272+
const exitCode = await handleHookEvent("PreToolUse");
1273+
1274+
expect(exitCode).toBe(2);
1275+
expect(stderrSpy).toHaveBeenCalledWith("blocked once");
1276+
expect(evaluatePolicies).not.toHaveBeenCalled();
1277+
expect(persistHookActivity).not.toHaveBeenCalled();
1278+
});
1279+
12541280
it("persists instruct decision as 'instruct' in activity store", async () => {
12551281
const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator");
12561282
vi.mocked(evaluatePolicies).mockResolvedValueOnce({
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// @vitest-environment node
2+
import { afterEach, describe, expect, it } from "vitest";
3+
import { mkdtemp, rm } from "node:fs/promises";
4+
import { join } from "node:path";
5+
import { tmpdir } from "node:os";
6+
import {
7+
claimHookInvocation,
8+
setHookInvocationDedupDirForTests,
9+
} from "../../src/hooks/hook-invocation-dedup";
10+
11+
describe("hook invocation deduplication", () => {
12+
const tempDirs: string[] = [];
13+
14+
async function useTempDir(): Promise<void> {
15+
const dir = await mkdtemp(join(tmpdir(), "failproofai-hook-dedup-"));
16+
tempDirs.push(dir);
17+
setHookInvocationDedupDirForTests(dir);
18+
}
19+
20+
afterEach(async () => {
21+
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
22+
});
23+
24+
it("elects one owner and replays its response to a duplicate process", async () => {
25+
await useTempDir();
26+
const payload = { session_id: "session-1", tool_use_id: "tool-1" };
27+
const owner = await claimHookInvocation("PreToolUse", "claude", payload);
28+
expect(owner.role).toBe("owner");
29+
if (owner.role !== "owner") throw new Error("expected owner claim");
30+
31+
const duplicatePromise = claimHookInvocation("PreToolUse", "claude", payload);
32+
await owner.complete({ exitCode: 2, stdout: "", stderr: "blocked" });
33+
const duplicate = await duplicatePromise;
34+
35+
expect(duplicate).toEqual({
36+
role: "duplicate",
37+
response: { exitCode: 2, stdout: "", stderr: "blocked" },
38+
});
39+
await expect(claimHookInvocation("PreToolUse", "claude", payload)).resolves.toEqual({
40+
role: "duplicate",
41+
response: { exitCode: 2, stdout: "", stderr: "blocked" },
42+
});
43+
});
44+
45+
it("does not combine different tool uses or hook events", async () => {
46+
await useTempDir();
47+
const first = await claimHookInvocation("PreToolUse", "claude", {
48+
session_id: "session-1",
49+
tool_use_id: "tool-1",
50+
});
51+
const differentTool = await claimHookInvocation("PreToolUse", "claude", {
52+
session_id: "session-1",
53+
tool_use_id: "tool-2",
54+
});
55+
const differentEvent = await claimHookInvocation("PostToolUse", "claude", {
56+
session_id: "session-1",
57+
tool_use_id: "tool-1",
58+
});
59+
60+
expect(first.role).toBe("owner");
61+
expect(differentTool.role).toBe("owner");
62+
expect(differentEvent.role).toBe("owner");
63+
if (first.role === "owner") await first.release();
64+
if (differentTool.role === "owner") await differentTool.release();
65+
if (differentEvent.role === "owner") await differentEvent.release();
66+
});
67+
68+
it("lets another process take ownership when the first exits before publishing", async () => {
69+
await useTempDir();
70+
const payload = { session_id: "session-1", tool_use_id: "tool-1" };
71+
const first = await claimHookInvocation("PreToolUse", "claude", payload);
72+
expect(first.role).toBe("owner");
73+
if (first.role !== "owner") throw new Error("expected owner claim");
74+
await first.release();
75+
76+
const retry = await claimHookInvocation("PreToolUse", "claude", payload);
77+
expect(retry.role).toBe("owner");
78+
if (retry.role === "owner") await retry.release();
79+
});
80+
81+
it("only deduplicates Claude payloads with stable invocation identifiers", async () => {
82+
await useTempDir();
83+
await expect(claimHookInvocation("PreToolUse", "codex", {
84+
session_id: "session-1",
85+
tool_use_id: "tool-1",
86+
})).resolves.toEqual({ role: "independent" });
87+
await expect(claimHookInvocation("PreToolUse", "claude", {
88+
session_id: "session-1",
89+
})).resolves.toEqual({ role: "independent" });
90+
});
91+
});

‎src/hooks/handler.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { resolvePermissionMode } from "./resolve-permission-mode";
3939
import { resolveTranscriptPath } from "./resolve-transcript-path";
4040
import { getInstanceId } from "../../lib/telemetry-id";
4141
import { hookLogInfo, hookLogWarn } from "./hook-logger";
42+
import { claimHookInvocation } from "./hook-invocation-dedup";
4243

4344
/**
4445
* Canonicalize an event name to PascalCase. Codex sends snake_case event names
@@ -92,6 +93,7 @@ export async function handleHookEvent(
9293
cli: IntegrationType = "claude",
9394
): Promise<number> {
9495
const startTime = performance.now();
96+
let invocationClaim: Awaited<ReturnType<typeof claimHookInvocation>> | undefined;
9597
try {
9698

9799
// Read stdin payload (Claude passes JSON)
@@ -197,6 +199,14 @@ export async function handleHookEvent(
197199
}
198200
}
199201

202+
invocationClaim = await claimHookInvocation(eventType, cli, parsed);
203+
if (invocationClaim.role === "duplicate") {
204+
if (invocationClaim.response.stdout) process.stdout.write(invocationClaim.response.stdout);
205+
if (invocationClaim.response.stderr) process.stderr.write(invocationClaim.response.stderr);
206+
hookLogInfo(`duplicate invocation replayed event=${eventType} cli=${cli}`);
207+
return invocationClaim.response.exitCode;
208+
}
209+
200210
// Canonicalize event name (Codex sends snake_case; internals expect PascalCase)
201211
const canonicalEventType = canonicalizeEventType(eventType, cli);
202212

@@ -311,6 +321,13 @@ export async function handleHookEvent(
311321

312322
// Evaluate policies (use canonical PascalCase event type)
313323
const result = await evaluatePolicies(canonicalEventType, parsed, session, config);
324+
if (invocationClaim.role === "owner") {
325+
await invocationClaim.complete({
326+
exitCode: result.exitCode,
327+
stdout: result.stdout,
328+
stderr: result.stderr,
329+
});
330+
}
314331
const durationMs = Math.round(performance.now() - startTime);
315332
hookLogInfo(`result=${result.decision} policy=${result.policyName ?? "none"} duration=${durationMs}ms`);
316333

@@ -376,6 +393,7 @@ export async function handleHookEvent(
376393
}
377394
return result.exitCode;
378395
} finally {
396+
if (invocationClaim?.role === "owner") await invocationClaim.release();
379397
// Await any un-awaited (`void trackHookEvent(...)`) events fired during
380398
// this invocation. bin/failproofai.mjs calls process.exit() the moment we
381399
// return OR throw, which would otherwise drop in-flight POSTs — notably on

‎src/hooks/hook-invocation-dedup.ts‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/**
2+
* Cross-process deduplication for Claude hooks installed in multiple scopes.
3+
*
4+
* Claude starts one hook process per matching user/project/local registration.
5+
* Those processes receive the same session + tool-use identifiers. An exclusive
6+
* lock elects one process to evaluate policies; the others wait briefly and
7+
* replay its response without duplicating activity or telemetry.
8+
*/
9+
import { createHash } from "node:crypto";
10+
import { mkdir, open, readFile, readdir, rename, stat, unlink, writeFile } from "node:fs/promises";
11+
import { homedir } from "node:os";
12+
import { join } from "node:path";
13+
import type { IntegrationType } from "./types";
14+
15+
const RESULT_TTL_MS = 5 * 60_000;
16+
const WAIT_TIMEOUT_MS = 12_000;
17+
const POLL_INTERVAL_MS = 10;
18+
19+
let dedupDir = join(homedir(), ".failproofai", "cache", "hook-invocations");
20+
21+
export interface HookInvocationResponse {
22+
exitCode: number;
23+
stdout: string;
24+
stderr: string;
25+
}
26+
27+
export type HookInvocationClaim =
28+
| { role: "owner"; complete: (response: HookInvocationResponse) => Promise<void>; release: () => Promise<void> }
29+
| { role: "duplicate"; response: HookInvocationResponse }
30+
| { role: "independent" };
31+
32+
function invocationKey(
33+
eventType: string,
34+
cli: IntegrationType,
35+
payload: Record<string, unknown>,
36+
): string | null {
37+
if (cli !== "claude") return null;
38+
const sessionId = payload.session_id;
39+
const toolUseId = payload.tool_use_id;
40+
if (typeof sessionId !== "string" || typeof toolUseId !== "string") return null;
41+
return createHash("sha256").update(`${cli}\0${eventType}\0${sessionId}\0${toolUseId}`).digest("hex");
42+
}
43+
44+
async function removeOldEntries(): Promise<void> {
45+
try {
46+
const names = await readdir(dedupDir);
47+
const now = Date.now();
48+
await Promise.all(names.map(async (name) => {
49+
const path = join(dedupDir, name);
50+
try {
51+
if (now - (await stat(path)).mtimeMs > RESULT_TTL_MS) await unlink(path);
52+
} catch {
53+
// Another hook process may have removed it first.
54+
}
55+
}));
56+
} catch {
57+
// Cleanup is best-effort and must never delay or block policy evaluation.
58+
}
59+
}
60+
61+
async function readResponse(path: string): Promise<HookInvocationResponse | null> {
62+
try {
63+
const parsed = JSON.parse(await readFile(path, "utf8")) as Partial<HookInvocationResponse>;
64+
if (
65+
typeof parsed.exitCode === "number"
66+
&& typeof parsed.stdout === "string"
67+
&& typeof parsed.stderr === "string"
68+
) {
69+
return parsed as HookInvocationResponse;
70+
}
71+
} catch {
72+
// The owner has not published its response yet.
73+
}
74+
return null;
75+
}
76+
77+
async function readFreshResponse(path: string): Promise<HookInvocationResponse | null> {
78+
try {
79+
if (Date.now() - (await stat(path)).mtimeMs > RESULT_TTL_MS) return null;
80+
} catch {
81+
return null;
82+
}
83+
return readResponse(path);
84+
}
85+
86+
export async function claimHookInvocation(
87+
eventType: string,
88+
cli: IntegrationType,
89+
payload: Record<string, unknown>,
90+
): Promise<HookInvocationClaim> {
91+
const key = invocationKey(eventType, cli, payload);
92+
if (!key) return { role: "independent" };
93+
94+
try {
95+
await mkdir(dedupDir, { recursive: true });
96+
void removeOldEntries();
97+
const lockPath = join(dedupDir, `${key}.lock`);
98+
const resultPath = join(dedupDir, `${key}.json`);
99+
const existingResponse = await readFreshResponse(resultPath);
100+
if (existingResponse) return { role: "duplicate", response: existingResponse };
101+
102+
try {
103+
const handle = await open(lockPath, "wx");
104+
await handle.close();
105+
const racedResponse = await readFreshResponse(resultPath);
106+
if (racedResponse) {
107+
try { await unlink(lockPath); } catch { /* best-effort */ }
108+
return { role: "duplicate", response: racedResponse };
109+
}
110+
let completed = false;
111+
return {
112+
role: "owner",
113+
complete: async (response) => {
114+
try {
115+
const tmpPath = `${resultPath}.${process.pid}.tmp`;
116+
await writeFile(tmpPath, JSON.stringify(response), "utf8");
117+
await rename(tmpPath, resultPath);
118+
completed = true;
119+
} catch {
120+
// Publishing is best-effort; the owner must still return its policy result.
121+
} finally {
122+
try { await unlink(lockPath); } catch { /* best-effort */ }
123+
}
124+
},
125+
release: async () => {
126+
if (!completed) {
127+
try { await unlink(lockPath); } catch { /* best-effort */ }
128+
}
129+
},
130+
};
131+
} catch (error) {
132+
if ((error as NodeJS.ErrnoException).code !== "EEXIST") return { role: "independent" };
133+
}
134+
135+
const deadline = Date.now() + WAIT_TIMEOUT_MS;
136+
while (Date.now() < deadline) {
137+
const response = await readResponse(resultPath);
138+
if (response) return { role: "duplicate", response };
139+
await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS));
140+
}
141+
} catch {
142+
// Deduplication is an optimization. Fail open into normal policy evaluation.
143+
}
144+
return { role: "independent" };
145+
}
146+
147+
export function setHookInvocationDedupDirForTests(path: string): void {
148+
dedupDir = path;
149+
}

0 commit comments

Comments
 (0)