From 670d1236a4a86c65be1a4841b742268e45357cca Mon Sep 17 00:00:00 2001 From: anantvardhanpandey Date: Wed, 29 Jul 2026 13:16:03 +0530 Subject: [PATCH] fix(hooks): deduplicate hook handler invocations when installed in multiple scopes (#58) --- __tests__/hooks/dedup-invocation.test.ts | 71 +++++++++++++++++++ src/hooks/dedup-invocation.ts | 87 ++++++++++++++++++++++++ src/hooks/handler.ts | 9 +++ 3 files changed, 167 insertions(+) create mode 100644 __tests__/hooks/dedup-invocation.test.ts create mode 100644 src/hooks/dedup-invocation.ts diff --git a/__tests__/hooks/dedup-invocation.test.ts b/__tests__/hooks/dedup-invocation.test.ts new file mode 100644 index 00000000..71f48b61 --- /dev/null +++ b/__tests__/hooks/dedup-invocation.test.ts @@ -0,0 +1,71 @@ +// @vitest-environment node +import { describe, it, expect, beforeEach, vi } from "vitest"; + +vi.mock("node:fs", async () => { + const actual = await vi.importActual("node:fs"); + let mockStore: Record = {}; + return { + ...actual, + existsSync: vi.fn((path: string) => path in mockStore), + readFileSync: vi.fn((path: string) => mockStore[path] ?? "{}"), + writeFileSync: vi.fn((path: string, content: string) => { + mockStore[path] = content; + }), + mkdirSync: vi.fn(), + __resetMockStore: () => { + mockStore = {}; + }, + }; +}); + +describe("hooks/dedup-invocation", () => { + beforeEach(async () => { + vi.resetAllMocks(); + const fs = await import("node:fs"); + (fs as unknown as { __resetMockStore: () => void }).__resetMockStore(); + }); + + it("returns isDuplicate: false on first invocation", async () => { + const { isDuplicateInvocation } = await import("../../src/hooks/dedup-invocation"); + const result = isDuplicateInvocation("claude", "PreToolUse", { + session_id: "s1", + tool_name: "Bash", + tool_input: { command: "ls" }, + }); + expect(result.isDuplicate).toBe(false); + }); + + it("returns isDuplicate: true when identical invocation occurs within deduplication window", async () => { + const { isDuplicateInvocation, recordInvocation } = await import("../../src/hooks/dedup-invocation"); + const payload = { + session_id: "s1", + tool_name: "Bash", + tool_input: { command: "ls" }, + }; + + recordInvocation("claude", "PreToolUse", payload, 0); + + const check = isDuplicateInvocation("claude", "PreToolUse", payload); + expect(check.isDuplicate).toBe(true); + expect(check.exitCode).toBe(0); + }); + + it("returns isDuplicate: false when tool_input or session differs", async () => { + const { isDuplicateInvocation, recordInvocation } = await import("../../src/hooks/dedup-invocation"); + const payload1 = { + session_id: "s1", + tool_name: "Bash", + tool_input: { command: "ls" }, + }; + const payload2 = { + session_id: "s1", + tool_name: "Bash", + tool_input: { command: "pwd" }, + }; + + recordInvocation("claude", "PreToolUse", payload1, 0); + + const check = isDuplicateInvocation("claude", "PreToolUse", payload2); + expect(check.isDuplicate).toBe(false); + }); +}); diff --git a/src/hooks/dedup-invocation.ts b/src/hooks/dedup-invocation.ts new file mode 100644 index 00000000..cb53255a --- /dev/null +++ b/src/hooks/dedup-invocation.ts @@ -0,0 +1,87 @@ +import { readFileSync, writeFileSync, existsSync, mkdirSync } from "node:fs"; +import { join } from "node:path"; +import { homedir } from "node:os"; + +const DEDUP_CACHE_DIR = join(homedir(), ".failproofai", "cache"); +const DEDUP_CACHE_FILE = join(DEDUP_CACHE_DIR, "dedup-invocations.json"); +const DEDUP_WINDOW_MS = 2000; + +interface DedupRecord { + key: string; + timestamp: number; + exitCode: number; +} + +export function isDuplicateInvocation( + cli: string, + eventType: string, + parsed: Record, +): { isDuplicate: boolean; exitCode: number } { + try { + const session = (parsed.session_id as string) ?? ""; + const tool = (parsed.tool_name as string) ?? ""; + const toolInput = parsed.tool_input ? JSON.stringify(parsed.tool_input) : ""; + const key = `${cli}:${eventType}:${session}:${tool}:${toolInput}`; + + const now = Date.now(); + let records: Record = {}; + + if (existsSync(DEDUP_CACHE_FILE)) { + try { + const raw = readFileSync(DEDUP_CACHE_FILE, "utf8"); + records = JSON.parse(raw) as Record; + } catch { + records = {}; + } + } + + const existing = records[key]; + if (existing && now - existing.timestamp < DEDUP_WINDOW_MS) { + return { isDuplicate: true, exitCode: existing.exitCode }; + } + } catch { + // Fail-open: if dedup check fails, treat as not duplicate + } + return { isDuplicate: false, exitCode: 0 }; +} + +export function recordInvocation( + cli: string, + eventType: string, + parsed: Record, + exitCode: number, +): void { + try { + const session = (parsed.session_id as string) ?? ""; + const tool = (parsed.tool_name as string) ?? ""; + const toolInput = parsed.tool_input ? JSON.stringify(parsed.tool_input) : ""; + const key = `${cli}:${eventType}:${session}:${tool}:${toolInput}`; + + const now = Date.now(); + let records: Record = {}; + + mkdirSync(DEDUP_CACHE_DIR, { recursive: true }); + + if (existsSync(DEDUP_CACHE_FILE)) { + try { + const raw = readFileSync(DEDUP_CACHE_FILE, "utf8"); + records = JSON.parse(raw) as Record; + } catch { + records = {}; + } + } + + // Clean up stale records older than 10 seconds to keep cache small + const cleaned: Record = {}; + for (const [k, v] of Object.entries(records)) { + if (now - v.timestamp < 10_000) { + cleaned[k] = v; + } + } + + cleaned[key] = { key, timestamp: now, exitCode }; + writeFileSync(DEDUP_CACHE_FILE, JSON.stringify(cleaned), "utf8"); + } catch { + // Non-blocking write + } +} diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index 69a33fb1..43fb8d38 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 { isDuplicateInvocation, recordInvocation } from "./dedup-invocation"; /** * Canonicalize an event name to PascalCase. Codex sends snake_case event names @@ -221,6 +222,13 @@ export async function handleHookEvent( parsed.tool_input = canonicalInput; } + // Deduplicate duplicate hook handler invocations from multiple scopes + const { isDuplicate, exitCode: prevExitCode } = isDuplicateInvocation(cli, canonicalEventType, parsed); + if (isDuplicate) { + hookLogInfo(`deduplicated duplicate hook invocation from multiple scopes: event=${canonicalEventType} cli=${cli}`); + return prevExitCode; + } + // Extract session metadata from payload const sessionId = parsed.session_id as string | undefined; const session: SessionMetadata = { @@ -377,6 +385,7 @@ export async function handleHookEvent( // Telemetry is best-effort — never block the hook } } + recordInvocation(cli, canonicalEventType, parsed, result.exitCode); return result.exitCode; } finally { // Await any un-awaited (`void trackHookEvent(...)`) events fired during