From f3ee7772f54402506caff6ba09cc44e85a62812e Mon Sep 17 00:00:00 2001 From: zzbcode Date: Thu, 16 Jul 2026 20:21:21 +0800 Subject: [PATCH 1/2] deduplicate multi-scope hook invocations --- __tests__/hooks/handler.test.ts | 26 +++ __tests__/hooks/hook-invocation-dedup.test.ts | 91 +++++++++++ src/hooks/handler.ts | 18 +++ src/hooks/hook-invocation-dedup.ts | 149 ++++++++++++++++++ 4 files changed, 284 insertions(+) create mode 100644 __tests__/hooks/hook-invocation-dedup.test.ts create mode 100644 src/hooks/hook-invocation-dedup.ts diff --git a/__tests__/hooks/handler.test.ts b/__tests__/hooks/handler.test.ts index 32b9c11d..79006bf3 100644 --- a/__tests__/hooks/handler.test.ts +++ b/__tests__/hooks/handler.test.ts @@ -49,6 +49,10 @@ vi.mock("../../src/hooks/hook-logger", () => ({ hookLogError: vi.fn(), })); +vi.mock("../../src/hooks/hook-invocation-dedup", () => ({ + claimHookInvocation: vi.fn(() => Promise.resolve({ role: "independent" })), +})); + describe("hooks/handler", () => { let stderrSpy: ReturnType; let stdoutSpy: ReturnType; @@ -1251,6 +1255,28 @@ describe("hooks/handler", () => { ); }); + it("replays a duplicate decision without evaluating or persisting again", async () => { + const { claimHookInvocation } = await import("../../src/hooks/hook-invocation-dedup"); + vi.mocked(claimHookInvocation).mockResolvedValueOnce({ + role: "duplicate", + response: { exitCode: 2, stdout: "", stderr: "blocked once" }, + }); + mockStdin(JSON.stringify({ + session_id: "session-1", + tool_use_id: "tool-1", + tool_name: "Bash", + })); + const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator"); + const { persistHookActivity } = await import("../../src/hooks/hook-activity-store"); + + const exitCode = await handleHookEvent("PreToolUse"); + + expect(exitCode).toBe(2); + expect(stderrSpy).toHaveBeenCalledWith("blocked once"); + expect(evaluatePolicies).not.toHaveBeenCalled(); + expect(persistHookActivity).not.toHaveBeenCalled(); + }); + it("persists instruct decision as 'instruct' in activity store", async () => { const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator"); vi.mocked(evaluatePolicies).mockResolvedValueOnce({ diff --git a/__tests__/hooks/hook-invocation-dedup.test.ts b/__tests__/hooks/hook-invocation-dedup.test.ts new file mode 100644 index 00000000..aea176e9 --- /dev/null +++ b/__tests__/hooks/hook-invocation-dedup.test.ts @@ -0,0 +1,91 @@ +// @vitest-environment node +import { afterEach, describe, expect, it } from "vitest"; +import { mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { + claimHookInvocation, + setHookInvocationDedupDirForTests, +} from "../../src/hooks/hook-invocation-dedup"; + +describe("hook invocation deduplication", () => { + const tempDirs: string[] = []; + + async function useTempDir(): Promise { + const dir = await mkdtemp(join(tmpdir(), "failproofai-hook-dedup-")); + tempDirs.push(dir); + setHookInvocationDedupDirForTests(dir); + } + + afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); + }); + + it("elects one owner and replays its response to a duplicate process", async () => { + await useTempDir(); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const owner = await claimHookInvocation("PreToolUse", "claude", payload); + expect(owner.role).toBe("owner"); + if (owner.role !== "owner") throw new Error("expected owner claim"); + + const duplicatePromise = claimHookInvocation("PreToolUse", "claude", payload); + await owner.complete({ exitCode: 2, stdout: "", stderr: "blocked" }); + const duplicate = await duplicatePromise; + + expect(duplicate).toEqual({ + role: "duplicate", + response: { exitCode: 2, stdout: "", stderr: "blocked" }, + }); + await expect(claimHookInvocation("PreToolUse", "claude", payload)).resolves.toEqual({ + role: "duplicate", + response: { exitCode: 2, stdout: "", stderr: "blocked" }, + }); + }); + + it("does not combine different tool uses or hook events", async () => { + await useTempDir(); + const first = await claimHookInvocation("PreToolUse", "claude", { + session_id: "session-1", + tool_use_id: "tool-1", + }); + const differentTool = await claimHookInvocation("PreToolUse", "claude", { + session_id: "session-1", + tool_use_id: "tool-2", + }); + const differentEvent = await claimHookInvocation("PostToolUse", "claude", { + session_id: "session-1", + tool_use_id: "tool-1", + }); + + expect(first.role).toBe("owner"); + expect(differentTool.role).toBe("owner"); + expect(differentEvent.role).toBe("owner"); + if (first.role === "owner") await first.release(); + if (differentTool.role === "owner") await differentTool.release(); + if (differentEvent.role === "owner") await differentEvent.release(); + }); + + it("lets another process take ownership when the first exits before publishing", async () => { + await useTempDir(); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const first = await claimHookInvocation("PreToolUse", "claude", payload); + expect(first.role).toBe("owner"); + if (first.role !== "owner") throw new Error("expected owner claim"); + await first.release(); + + const retry = await claimHookInvocation("PreToolUse", "claude", payload); + expect(retry.role).toBe("owner"); + if (retry.role === "owner") await retry.release(); + }); + + it("only deduplicates Claude payloads with stable invocation identifiers", async () => { + await useTempDir(); + await expect(claimHookInvocation("PreToolUse", "codex", { + session_id: "session-1", + tool_use_id: "tool-1", + })).resolves.toEqual({ role: "independent" }); + await expect(claimHookInvocation("PreToolUse", "claude", { + session_id: "session-1", + })).resolves.toEqual({ role: "independent" }); + }); +}); diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index a7856bab..2126808f 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -39,6 +39,7 @@ import { resolvePermissionMode } from "./resolve-permission-mode"; import { resolveTranscriptPath } from "./resolve-transcript-path"; import { getInstanceId } from "../../lib/telemetry-id"; import { hookLogInfo, hookLogWarn } from "./hook-logger"; +import { claimHookInvocation } from "./hook-invocation-dedup"; /** * Canonicalize an event name to PascalCase. Codex sends snake_case event names @@ -92,6 +93,7 @@ export async function handleHookEvent( cli: IntegrationType = "claude", ): Promise { const startTime = performance.now(); + let invocationClaim: Awaited> | undefined; try { // Read stdin payload (Claude passes JSON) @@ -197,6 +199,14 @@ export async function handleHookEvent( } } + invocationClaim = await claimHookInvocation(eventType, cli, parsed); + if (invocationClaim.role === "duplicate") { + if (invocationClaim.response.stdout) process.stdout.write(invocationClaim.response.stdout); + if (invocationClaim.response.stderr) process.stderr.write(invocationClaim.response.stderr); + hookLogInfo(`duplicate invocation replayed event=${eventType} cli=${cli}`); + return invocationClaim.response.exitCode; + } + // Canonicalize event name (Codex sends snake_case; internals expect PascalCase) const canonicalEventType = canonicalizeEventType(eventType, cli); @@ -311,6 +321,13 @@ export async function handleHookEvent( // Evaluate policies (use canonical PascalCase event type) const result = await evaluatePolicies(canonicalEventType, parsed, session, config); + if (invocationClaim.role === "owner") { + await invocationClaim.complete({ + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + }); + } const durationMs = Math.round(performance.now() - startTime); hookLogInfo(`result=${result.decision} policy=${result.policyName ?? "none"} duration=${durationMs}ms`); @@ -376,6 +393,7 @@ export async function handleHookEvent( } return result.exitCode; } finally { + if (invocationClaim?.role === "owner") await invocationClaim.release(); // Await any un-awaited (`void trackHookEvent(...)`) events fired during // this invocation. bin/failproofai.mjs calls process.exit() the moment we // return OR throw, which would otherwise drop in-flight POSTs — notably on diff --git a/src/hooks/hook-invocation-dedup.ts b/src/hooks/hook-invocation-dedup.ts new file mode 100644 index 00000000..27f3ffd8 --- /dev/null +++ b/src/hooks/hook-invocation-dedup.ts @@ -0,0 +1,149 @@ +/** + * Cross-process deduplication for Claude hooks installed in multiple scopes. + * + * Claude starts one hook process per matching user/project/local registration. + * Those processes receive the same session + tool-use identifiers. An exclusive + * lock elects one process to evaluate policies; the others wait briefly and + * replay its response without duplicating activity or telemetry. + */ +import { createHash } from "node:crypto"; +import { mkdir, open, readFile, readdir, rename, stat, unlink, writeFile } from "node:fs/promises"; +import { homedir } from "node:os"; +import { join } from "node:path"; +import type { IntegrationType } from "./types"; + +const RESULT_TTL_MS = 5 * 60_000; +const WAIT_TIMEOUT_MS = 12_000; +const POLL_INTERVAL_MS = 10; + +let dedupDir = join(homedir(), ".failproofai", "cache", "hook-invocations"); + +export interface HookInvocationResponse { + exitCode: number; + stdout: string; + stderr: string; +} + +export type HookInvocationClaim = + | { role: "owner"; complete: (response: HookInvocationResponse) => Promise; release: () => Promise } + | { role: "duplicate"; response: HookInvocationResponse } + | { role: "independent" }; + +function invocationKey( + eventType: string, + cli: IntegrationType, + payload: Record, +): string | null { + if (cli !== "claude") return null; + const sessionId = payload.session_id; + const toolUseId = payload.tool_use_id; + if (typeof sessionId !== "string" || typeof toolUseId !== "string") return null; + return createHash("sha256").update(`${cli}\0${eventType}\0${sessionId}\0${toolUseId}`).digest("hex"); +} + +async function removeOldEntries(): Promise { + try { + const names = await readdir(dedupDir); + const now = Date.now(); + await Promise.all(names.map(async (name) => { + const path = join(dedupDir, name); + try { + if (now - (await stat(path)).mtimeMs > RESULT_TTL_MS) await unlink(path); + } catch { + // Another hook process may have removed it first. + } + })); + } catch { + // Cleanup is best-effort and must never delay or block policy evaluation. + } +} + +async function readResponse(path: string): Promise { + try { + const parsed = JSON.parse(await readFile(path, "utf8")) as Partial; + if ( + typeof parsed.exitCode === "number" + && typeof parsed.stdout === "string" + && typeof parsed.stderr === "string" + ) { + return parsed as HookInvocationResponse; + } + } catch { + // The owner has not published its response yet. + } + return null; +} + +async function readFreshResponse(path: string): Promise { + try { + if (Date.now() - (await stat(path)).mtimeMs > RESULT_TTL_MS) return null; + } catch { + return null; + } + return readResponse(path); +} + +export async function claimHookInvocation( + eventType: string, + cli: IntegrationType, + payload: Record, +): Promise { + const key = invocationKey(eventType, cli, payload); + if (!key) return { role: "independent" }; + + try { + await mkdir(dedupDir, { recursive: true }); + void removeOldEntries(); + const lockPath = join(dedupDir, `${key}.lock`); + const resultPath = join(dedupDir, `${key}.json`); + const existingResponse = await readFreshResponse(resultPath); + if (existingResponse) return { role: "duplicate", response: existingResponse }; + + try { + const handle = await open(lockPath, "wx"); + await handle.close(); + const racedResponse = await readFreshResponse(resultPath); + if (racedResponse) { + try { await unlink(lockPath); } catch { /* best-effort */ } + return { role: "duplicate", response: racedResponse }; + } + let completed = false; + return { + role: "owner", + complete: async (response) => { + try { + const tmpPath = `${resultPath}.${process.pid}.tmp`; + await writeFile(tmpPath, JSON.stringify(response), "utf8"); + await rename(tmpPath, resultPath); + completed = true; + } catch { + // Publishing is best-effort; the owner must still return its policy result. + } finally { + try { await unlink(lockPath); } catch { /* best-effort */ } + } + }, + release: async () => { + if (!completed) { + try { await unlink(lockPath); } catch { /* best-effort */ } + } + }, + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") return { role: "independent" }; + } + + const deadline = Date.now() + WAIT_TIMEOUT_MS; + while (Date.now() < deadline) { + const response = await readResponse(resultPath); + if (response) return { role: "duplicate", response }; + await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS)); + } + } catch { + // Deduplication is an optimization. Fail open into normal policy evaluation. + } + return { role: "independent" }; +} + +export function setHookInvocationDedupDirForTests(path: string): void { + dedupDir = path; +} From 046d68046ba06506ad35842bf496d991f7656471 Mon Sep 17 00:00:00 2001 From: zzbcode Date: Sat, 18 Jul 2026 18:44:43 +0800 Subject: [PATCH 2/2] harden hook deduplication identity and recovery --- __tests__/hooks/handler.test.ts | 32 ++++ __tests__/hooks/hook-invocation-dedup.test.ts | 116 +++++++++++-- src/hooks/handler.ts | 34 ++-- src/hooks/hook-invocation-dedup.ts | 160 +++++++++++++----- 4 files changed, 275 insertions(+), 67 deletions(-) diff --git a/__tests__/hooks/handler.test.ts b/__tests__/hooks/handler.test.ts index 79006bf3..17fc1e13 100644 --- a/__tests__/hooks/handler.test.ts +++ b/__tests__/hooks/handler.test.ts @@ -51,6 +51,7 @@ vi.mock("../../src/hooks/hook-logger", () => ({ vi.mock("../../src/hooks/hook-invocation-dedup", () => ({ claimHookInvocation: vi.fn(() => Promise.resolve({ role: "independent" })), + createHookRuntimeIdentity: vi.fn(() => "test-runtime"), })); describe("hooks/handler", () => { @@ -1277,6 +1278,37 @@ describe("hooks/handler", () => { expect(persistHookActivity).not.toHaveBeenCalled(); }); + it("publishes and releases the owner decision", async () => { + const { claimHookInvocation } = await import("../../src/hooks/hook-invocation-dedup"); + const complete = vi.fn(() => Promise.resolve()); + const release = vi.fn(() => Promise.resolve()); + vi.mocked(claimHookInvocation).mockResolvedValueOnce({ role: "owner", complete, release }); + const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator"); + vi.mocked(evaluatePolicies).mockResolvedValueOnce({ + exitCode: 2, + stdout: "", + stderr: "blocked by owner", + policyName: "block-sudo", + reason: "blocked", + decision: "deny", + }); + mockStdin(JSON.stringify({ + session_id: "session-1", + tool_use_id: "tool-1", + tool_name: "Bash", + })); + + const exitCode = await handleHookEvent("PreToolUse"); + + expect(exitCode).toBe(2); + expect(complete).toHaveBeenCalledWith({ + exitCode: 2, + stdout: "", + stderr: "blocked by owner", + }); + expect(release).toHaveBeenCalledOnce(); + }); + it("persists instruct decision as 'instruct' in activity store", async () => { const { evaluatePolicies } = await import("../../src/hooks/policy-evaluator"); vi.mocked(evaluatePolicies).mockResolvedValueOnce({ diff --git a/__tests__/hooks/hook-invocation-dedup.test.ts b/__tests__/hooks/hook-invocation-dedup.test.ts index aea176e9..821c7d43 100644 --- a/__tests__/hooks/hook-invocation-dedup.test.ts +++ b/__tests__/hooks/hook-invocation-dedup.test.ts @@ -1,34 +1,40 @@ // @vitest-environment node import { afterEach, describe, expect, it } from "vitest"; -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdtemp, readdir, rm, utimes, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { claimHookInvocation, + createHookRuntimeIdentity, setHookInvocationDedupDirForTests, + setHookInvocationDedupTimingForTests, } from "../../src/hooks/hook-invocation-dedup"; describe("hook invocation deduplication", () => { const tempDirs: string[] = []; + const runtimeIdentity = "runtime-a"; + let activeDir = ""; async function useTempDir(): Promise { const dir = await mkdtemp(join(tmpdir(), "failproofai-hook-dedup-")); tempDirs.push(dir); + activeDir = dir; setHookInvocationDedupDirForTests(dir); } afterEach(async () => { + setHookInvocationDedupTimingForTests(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); }); it("elects one owner and replays its response to a duplicate process", async () => { await useTempDir(); const payload = { session_id: "session-1", tool_use_id: "tool-1" }; - const owner = await claimHookInvocation("PreToolUse", "claude", payload); + const owner = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); expect(owner.role).toBe("owner"); if (owner.role !== "owner") throw new Error("expected owner claim"); - const duplicatePromise = claimHookInvocation("PreToolUse", "claude", payload); + const duplicatePromise = claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); await owner.complete({ exitCode: 2, stdout: "", stderr: "blocked" }); const duplicate = await duplicatePromise; @@ -36,7 +42,7 @@ describe("hook invocation deduplication", () => { role: "duplicate", response: { exitCode: 2, stdout: "", stderr: "blocked" }, }); - await expect(claimHookInvocation("PreToolUse", "claude", payload)).resolves.toEqual({ + await expect(claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity)).resolves.toEqual({ role: "duplicate", response: { exitCode: 2, stdout: "", stderr: "blocked" }, }); @@ -47,15 +53,15 @@ describe("hook invocation deduplication", () => { const first = await claimHookInvocation("PreToolUse", "claude", { session_id: "session-1", tool_use_id: "tool-1", - }); + }, runtimeIdentity); const differentTool = await claimHookInvocation("PreToolUse", "claude", { session_id: "session-1", tool_use_id: "tool-2", - }); + }, runtimeIdentity); const differentEvent = await claimHookInvocation("PostToolUse", "claude", { session_id: "session-1", tool_use_id: "tool-1", - }); + }, runtimeIdentity); expect(first.role).toBe("owner"); expect(differentTool.role).toBe("owner"); @@ -68,12 +74,12 @@ describe("hook invocation deduplication", () => { it("lets another process take ownership when the first exits before publishing", async () => { await useTempDir(); const payload = { session_id: "session-1", tool_use_id: "tool-1" }; - const first = await claimHookInvocation("PreToolUse", "claude", payload); + const first = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); expect(first.role).toBe("owner"); if (first.role !== "owner") throw new Error("expected owner claim"); await first.release(); - const retry = await claimHookInvocation("PreToolUse", "claude", payload); + const retry = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); expect(retry.role).toBe("owner"); if (retry.role === "owner") await retry.release(); }); @@ -83,9 +89,97 @@ describe("hook invocation deduplication", () => { await expect(claimHookInvocation("PreToolUse", "codex", { session_id: "session-1", tool_use_id: "tool-1", - })).resolves.toEqual({ role: "independent" }); + }, runtimeIdentity)).resolves.toEqual({ role: "independent" }); await expect(claimHookInvocation("PreToolUse", "claude", { session_id: "session-1", - })).resolves.toEqual({ role: "independent" }); + }, runtimeIdentity)).resolves.toEqual({ role: "independent" }); + }); + + it("keeps different binaries or merged configurations independent", async () => { + await useTempDir(); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const first = await claimHookInvocation("PreToolUse", "claude", payload, "runtime-a"); + const differentRuntime = await claimHookInvocation("PreToolUse", "claude", payload, "runtime-b"); + + expect(first.role).toBe("owner"); + expect(differentRuntime.role).toBe("owner"); + if (first.role === "owner") await first.release(); + if (differentRuntime.role === "owner") await differentRuntime.release(); + + const originalRoot = process.env.FAILPROOFAI_PACKAGE_ROOT; + process.env.FAILPROOFAI_PACKAGE_ROOT = "/tmp/failproofai-a"; + const identityA = createHookRuntimeIdentity({ enabledPolicies: ["block-sudo"] }, []); + process.env.FAILPROOFAI_PACKAGE_ROOT = "/tmp/failproofai-b"; + const identityB = createHookRuntimeIdentity({ enabledPolicies: ["block-sudo"] }, []); + const identityC = createHookRuntimeIdentity({ enabledPolicies: ["block-rm-rf"] }, []); + const identityD = createHookRuntimeIdentity( + { enabledPolicies: ["block-rm-rf"] }, + [{ name: "custom-a", fn: async () => ({ decision: "deny" }) }], + ); + const identityE = createHookRuntimeIdentity( + { enabledPolicies: ["block-rm-rf"] }, + [{ name: "custom-a", fn: async () => ({ decision: "deny" }) }], + "/tmp/another-project", + ); + if (originalRoot === undefined) delete process.env.FAILPROOFAI_PACKAGE_ROOT; + else process.env.FAILPROOFAI_PACKAGE_ROOT = originalRoot; + + expect(identityA).not.toBe(identityB); + expect(identityB).not.toBe(identityC); + expect(identityC).not.toBe(identityD); + expect(identityD).not.toBe(identityE); + }); + + it("takes over immediately when the lock owner process is dead", async () => { + await useTempDir(); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const crashedOwner = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); + expect(crashedOwner.role).toBe("owner"); + const lockName = (await readdir(activeDir)).find((name) => name.endsWith(".lock")); + if (!lockName) throw new Error("expected lock file"); + await writeFile(join(activeDir, lockName), JSON.stringify({ pid: 2_147_483_647 }), "utf8"); + + const replacement = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); + + expect(replacement.role).toBe("owner"); + if (replacement.role === "owner") await replacement.release(); + }); + + it("falls back independently after the bounded wait for a live owner", async () => { + await useTempDir(); + setHookInvocationDedupTimingForTests({ waitTimeoutMs: 30, pollIntervalMs: 5 }); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const owner = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); + expect(owner.role).toBe("owner"); + + await expect( + claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity), + ).resolves.toEqual({ role: "independent" }); + if (owner.role === "owner") await owner.release(); + }); + + it("replaces an expired result and publishes the new response", async () => { + await useTempDir(); + const payload = { session_id: "session-1", tool_use_id: "tool-1" }; + const first = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); + if (first.role !== "owner") throw new Error("expected owner claim"); + await first.complete({ exitCode: 0, stdout: "old", stderr: "" }); + const resultName = (await readdir(activeDir)).find((name) => name.endsWith(".json")); + if (!resultName) throw new Error("expected result file"); + const old = new Date(Date.now() - 10_000); + await utimes(join(activeDir, resultName), old, old); + setHookInvocationDedupTimingForTests({ resultTtlMs: 1_000 }); + + const replacement = await claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity); + expect(replacement.role).toBe("owner"); + if (replacement.role !== "owner") throw new Error("expected replacement owner"); + await replacement.complete({ exitCode: 2, stdout: "", stderr: "new" }); + + await expect( + claimHookInvocation("PreToolUse", "claude", payload, runtimeIdentity), + ).resolves.toEqual({ + role: "duplicate", + response: { exitCode: 2, stdout: "", stderr: "new" }, + }); }); }); diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index 2126808f..3bf25f9e 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -39,7 +39,12 @@ import { resolvePermissionMode } from "./resolve-permission-mode"; import { resolveTranscriptPath } from "./resolve-transcript-path"; import { getInstanceId } from "../../lib/telemetry-id"; import { hookLogInfo, hookLogWarn } from "./hook-logger"; -import { claimHookInvocation } from "./hook-invocation-dedup"; +import { claimHookInvocation, createHookRuntimeIdentity } from "./hook-invocation-dedup"; + +function writeHookResponse(response: { stdout: string; stderr: string }): void { + if (response.stdout) process.stdout.write(response.stdout); + if (response.stderr) process.stderr.write(response.stderr); +} /** * Canonicalize an event name to PascalCase. Codex sends snake_case event names @@ -199,14 +204,6 @@ export async function handleHookEvent( } } - invocationClaim = await claimHookInvocation(eventType, cli, parsed); - if (invocationClaim.role === "duplicate") { - if (invocationClaim.response.stdout) process.stdout.write(invocationClaim.response.stdout); - if (invocationClaim.response.stderr) process.stderr.write(invocationClaim.response.stderr); - hookLogInfo(`duplicate invocation replayed event=${eventType} cli=${cli}`); - return invocationClaim.response.exitCode; - } - // Canonicalize event name (Codex sends snake_case; internals expect PascalCase) const canonicalEventType = canonicalizeEventType(eventType, cli); @@ -257,6 +254,18 @@ export async function handleHookEvent( const customHooksList = loadResult.hooks; const conventionHookNames = new Set(loadResult.conventionSources.flatMap((s) => s.hookNames)); + invocationClaim = await claimHookInvocation( + eventType, + cli, + parsed, + createHookRuntimeIdentity(config, customHooksList, session.cwd), + ); + if (invocationClaim.role === "duplicate") { + writeHookResponse(invocationClaim.response); + hookLogInfo(`duplicate invocation replayed event=${eventType} cli=${cli}`); + return invocationClaim.response.exitCode; + } + for (const hook of customHooksList) { const hookName = hook.name; const conventionScope = (hook as CustomHook & { __conventionScope?: string }).__conventionScope; @@ -331,12 +340,7 @@ export async function handleHookEvent( const durationMs = Math.round(performance.now() - startTime); hookLogInfo(`result=${result.decision} policy=${result.policyName ?? "none"} duration=${durationMs}ms`); - if (result.stdout) { - process.stdout.write(result.stdout); - } - if (result.stderr) { - process.stderr.write(result.stderr); - } + writeHookResponse(result); // Persist activity to disk (visible in /policies activity tab) const activityEntry = { diff --git a/src/hooks/hook-invocation-dedup.ts b/src/hooks/hook-invocation-dedup.ts index 27f3ffd8..7d544ad8 100644 --- a/src/hooks/hook-invocation-dedup.ts +++ b/src/hooks/hook-invocation-dedup.ts @@ -10,13 +10,18 @@ import { createHash } from "node:crypto"; import { mkdir, open, readFile, readdir, rename, stat, unlink, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; +import { version } from "../../package.json"; +import type { CustomHook, HooksConfig } from "./policy-types"; import type { IntegrationType } from "./types"; -const RESULT_TTL_MS = 5 * 60_000; -const WAIT_TIMEOUT_MS = 12_000; -const POLL_INTERVAL_MS = 10; +const DEFAULT_RESULT_TTL_MS = 10 * 60_000; +const DEFAULT_WAIT_TIMEOUT_MS = 60_000; +const DEFAULT_POLL_INTERVAL_MS = 10; let dedupDir = join(homedir(), ".failproofai", "cache", "hook-invocations"); +let resultTtlMs = DEFAULT_RESULT_TTL_MS; +let waitTimeoutMs = DEFAULT_WAIT_TIMEOUT_MS; +let pollIntervalMs = DEFAULT_POLL_INTERVAL_MS; export interface HookInvocationResponse { exitCode: number; @@ -33,12 +38,33 @@ function invocationKey( eventType: string, cli: IntegrationType, payload: Record, + runtimeIdentity: string, ): string | null { if (cli !== "claude") return null; const sessionId = payload.session_id; const toolUseId = payload.tool_use_id; if (typeof sessionId !== "string" || typeof toolUseId !== "string") return null; - return createHash("sha256").update(`${cli}\0${eventType}\0${sessionId}\0${toolUseId}`).digest("hex"); + return createHash("sha256") + .update(`${cli}\0${eventType}\0${sessionId}\0${toolUseId}\0${runtimeIdentity}`) + .digest("hex"); +} + +export function createHookRuntimeIdentity( + config: HooksConfig, + customHooks: CustomHook[], + cwd?: string, +): string { + const packageRoot = process.env.FAILPROOFAI_PACKAGE_ROOT ?? process.argv[1] ?? "unknown"; + const customHookIdentity = customHooks.map((hook) => ({ + name: hook.name, + description: hook.description, + match: hook.match, + implementation: String(hook.fn), + conventionScope: (hook as CustomHook & { __conventionScope?: string }).__conventionScope, + })); + return createHash("sha256") + .update(JSON.stringify({ version, packageRoot, cwd, config, customHookIdentity })) + .digest("hex"); } async function removeOldEntries(): Promise { @@ -48,7 +74,7 @@ async function removeOldEntries(): Promise { await Promise.all(names.map(async (name) => { const path = join(dedupDir, name); try { - if (now - (await stat(path)).mtimeMs > RESULT_TTL_MS) await unlink(path); + if (now - (await stat(path)).mtimeMs > resultTtlMs) await unlink(path); } catch { // Another hook process may have removed it first. } @@ -76,7 +102,7 @@ async function readResponse(path: string): Promise { try { - if (Date.now() - (await stat(path)).mtimeMs > RESULT_TTL_MS) return null; + if (Date.now() - (await stat(path)).mtimeMs > resultTtlMs) return null; } catch { return null; } @@ -87,8 +113,9 @@ export async function claimHookInvocation( eventType: string, cli: IntegrationType, payload: Record, + runtimeIdentity: string, ): Promise { - const key = invocationKey(eventType, cli, payload); + const key = invocationKey(eventType, cli, payload, runtimeIdentity); if (!key) return { role: "independent" }; try { @@ -99,44 +126,67 @@ export async function claimHookInvocation( const existingResponse = await readFreshResponse(resultPath); if (existingResponse) return { role: "duplicate", response: existingResponse }; - try { - const handle = await open(lockPath, "wx"); - await handle.close(); - const racedResponse = await readFreshResponse(resultPath); - if (racedResponse) { - try { await unlink(lockPath); } catch { /* best-effort */ } - return { role: "duplicate", response: racedResponse }; + while (true) { + try { + const handle = await open(lockPath, "wx"); + await handle.writeFile(JSON.stringify({ pid: process.pid }), "utf8"); + await handle.close(); + const racedResponse = await readFreshResponse(resultPath); + if (racedResponse) { + try { await unlink(lockPath); } catch { /* best-effort */ } + return { role: "duplicate", response: racedResponse }; + } + let completed = false; + return { + role: "owner", + complete: async (response) => { + const tmpPath = `${resultPath}.${process.pid}.tmp`; + try { + const serialized = JSON.stringify(response); + await writeFile(tmpPath, serialized, "utf8"); + try { + await rename(tmpPath, resultPath); + } catch { + // Windows does not reliably replace an existing destination. + await writeFile(resultPath, serialized, "utf8"); + try { await unlink(tmpPath); } catch { /* best-effort */ } + } + completed = true; + } catch { + // Publishing is best-effort; the owner must still return its policy result. + } finally { + try { await unlink(lockPath); } catch { /* best-effort */ } + } + }, + release: async () => { + if (!completed) { + try { await unlink(lockPath); } catch { /* best-effort */ } + } + }, + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") return { role: "independent" }; } - let completed = false; - return { - role: "owner", - complete: async (response) => { + + const deadline = Date.now() + waitTimeoutMs; + let retryClaim = false; + while (Date.now() < deadline) { + const response = await readResponse(resultPath); + if (response) return { role: "duplicate", response }; + + const ownerPid = await readOwnerPid(lockPath); + if (ownerPid !== null && !isProcessAlive(ownerPid)) { try { - const tmpPath = `${resultPath}.${process.pid}.tmp`; - await writeFile(tmpPath, JSON.stringify(response), "utf8"); - await rename(tmpPath, resultPath); - completed = true; + await unlink(lockPath); + retryClaim = true; + break; } catch { - // Publishing is best-effort; the owner must still return its policy result. - } finally { - try { await unlink(lockPath); } catch { /* best-effort */ } - } - }, - release: async () => { - if (!completed) { - try { await unlink(lockPath); } catch { /* best-effort */ } + // Another waiting process may have taken over first. } - }, - }; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "EEXIST") return { role: "independent" }; - } - - const deadline = Date.now() + WAIT_TIMEOUT_MS; - while (Date.now() < deadline) { - const response = await readResponse(resultPath); - if (response) return { role: "duplicate", response }; - await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS)); + } + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + } + if (!retryClaim) return { role: "independent" }; } } catch { // Deduplication is an optimization. Fail open into normal policy evaluation. @@ -144,6 +194,34 @@ export async function claimHookInvocation( return { role: "independent" }; } +async function readOwnerPid(lockPath: string): Promise { + try { + const parsed = JSON.parse(await readFile(lockPath, "utf8")) as { pid?: unknown }; + return typeof parsed.pid === "number" && Number.isInteger(parsed.pid) ? parsed.pid : null; + } catch { + return null; + } +} + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + export function setHookInvocationDedupDirForTests(path: string): void { dedupDir = path; } + +export function setHookInvocationDedupTimingForTests(options?: { + resultTtlMs?: number; + waitTimeoutMs?: number; + pollIntervalMs?: number; +}): void { + resultTtlMs = options?.resultTtlMs ?? DEFAULT_RESULT_TTL_MS; + waitTimeoutMs = options?.waitTimeoutMs ?? DEFAULT_WAIT_TIMEOUT_MS; + pollIntervalMs = options?.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS; +}