Skip to content

Commit fa6761d

Browse files
timvisher-ddclaude
andcommitted
fix: defer PostToolUse hook execution when callback not yet registered
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1c641cd commit fa6761d

File tree

2 files changed

+222
-8
lines changed

2 files changed

+222
-8
lines changed

src/tests/tools.test.ts

Lines changed: 171 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
BetaBashCodeExecutionToolResultBlockParam,
1212
} from "@anthropic-ai/sdk/resources/beta.mjs";
1313
import { toAcpNotifications, ToolUseCache, Logger } from "../acp-agent.js";
14-
import { toolUpdateFromToolResult, createPostToolUseHook } from "../tools.js";
14+
import { toolUpdateFromToolResult, createPostToolUseHook, registerHookCallback } from "../tools.js";
1515

1616
describe("rawOutput in tool call updates", () => {
1717
const mockClient = {} as AgentSideConnection;
@@ -873,7 +873,7 @@ describe("Bash terminal output", () => {
873873
const hook = createPostToolUseHook(mockLogger);
874874
await hook(
875875
{
876-
hook_event_name: "PostToolUse",
876+
hook_event_name: "PostToolUse" as const,
877877
tool_name: "Edit",
878878
tool_input: {
879879
file_path: "/Users/test/project/file.ts",
@@ -951,7 +951,7 @@ describe("Bash terminal output", () => {
951951
const hook = createPostToolUseHook(mockLogger);
952952
await hook(
953953
{
954-
hook_event_name: "PostToolUse",
954+
hook_event_name: "PostToolUse" as const,
955955
tool_name: "Edit",
956956
tool_input: {
957957
file_path: "/Users/test/project/file.ts",
@@ -1031,7 +1031,7 @@ describe("Bash terminal output", () => {
10311031
const hook = createPostToolUseHook(mockLogger);
10321032
await hook(
10331033
{
1034-
hook_event_name: "PostToolUse",
1034+
hook_event_name: "PostToolUse" as const,
10351035
tool_name: "Bash",
10361036
tool_input: { command: "echo hi" },
10371037
tool_response: "hi",
@@ -1131,7 +1131,7 @@ describe("Bash terminal output", () => {
11311131
const hook = createPostToolUseHook(mockLogger);
11321132
await hook(
11331133
{
1134-
hook_event_name: "PostToolUse",
1134+
hook_event_name: "PostToolUse" as const,
11351135
tool_name: "Bash",
11361136
tool_input: { command: "ls -la" },
11371137
tool_response: "file1.txt",
@@ -1212,7 +1212,7 @@ describe("Bash terminal output", () => {
12121212
const hook = createPostToolUseHook(mockLogger);
12131213
await hook(
12141214
{
1215-
hook_event_name: "PostToolUse",
1215+
hook_event_name: "PostToolUse" as const,
12161216
tool_name: "Bash",
12171217
tool_input: { command: "echo hi" },
12181218
tool_response: "hi",
@@ -1234,4 +1234,169 @@ describe("Bash terminal output", () => {
12341234
expect(hookMeta.terminal_exit).toBeUndefined();
12351235
});
12361236
});
1237+
1238+
describe("PostToolUse callback execution contract", () => {
1239+
// These tests verify the observable contract between PostToolUse
1240+
// hooks and registerHookCallback, regardless of implementation:
1241+
//
1242+
// 1. Callback registered THEN hook fires → callback executes
1243+
// 2. Hook fires THEN callback registered → callback still executes
1244+
// 3. No errors logged in either ordering
1245+
// 4. Callback receives correct toolInput and toolResponse
1246+
// 5. Multiple hooks with mixed ordering don't interfere
1247+
//
1248+
// A helper that builds the hook input object for a given tool call.
1249+
function postToolUseInput(
1250+
toolUseId: string,
1251+
toolName: string,
1252+
toolInput: unknown = {},
1253+
toolResponse: unknown = "",
1254+
) {
1255+
return {
1256+
hook_event_name: "PostToolUse" as const,
1257+
tool_name: toolName,
1258+
tool_input: toolInput,
1259+
tool_response: toolResponse,
1260+
tool_use_id: toolUseId,
1261+
session_id: "test-session",
1262+
transcript_path: "/tmp/test",
1263+
cwd: "/tmp",
1264+
};
1265+
}
1266+
1267+
it("executes callback when registered before hook fires", async () => {
1268+
const received: { id: string; input: unknown; response: unknown }[] = [];
1269+
1270+
registerHookCallback("toolu_before_1", {
1271+
onPostToolUseHook: async (id, input, response) => {
1272+
received.push({ id, input, response });
1273+
},
1274+
});
1275+
1276+
const hook = createPostToolUseHook(mockLogger);
1277+
const result = await hook(
1278+
postToolUseInput("toolu_before_1", "Bash", { command: "ls" }, "file.txt"),
1279+
"toolu_before_1",
1280+
{ signal: AbortSignal.abort() },
1281+
);
1282+
1283+
expect(result).toEqual({ continue: true });
1284+
expect(received).toHaveLength(1);
1285+
expect(received[0]).toEqual({
1286+
id: "toolu_before_1",
1287+
input: { command: "ls" },
1288+
response: "file.txt",
1289+
});
1290+
});
1291+
1292+
it("executes callback when registered after hook fires", async () => {
1293+
const received: { id: string; input: unknown; response: unknown }[] = [];
1294+
const hook = createPostToolUseHook(mockLogger);
1295+
1296+
// Hook fires first — no callback registered yet.
1297+
const hookPromise = hook(
1298+
postToolUseInput("toolu_after_1", "Read", { file_path: "/tmp/f" }, "contents"),
1299+
"toolu_after_1",
1300+
{ signal: AbortSignal.abort() },
1301+
);
1302+
1303+
// Registration arrives on the next tick (simulates streaming lag).
1304+
await new Promise((r) => setTimeout(r, 5));
1305+
registerHookCallback("toolu_after_1", {
1306+
onPostToolUseHook: async (id, input, response) => {
1307+
received.push({ id, input, response });
1308+
},
1309+
});
1310+
1311+
const result = await hookPromise;
1312+
1313+
expect(result).toEqual({ continue: true });
1314+
expect(received).toHaveLength(1);
1315+
expect(received[0]).toEqual({
1316+
id: "toolu_after_1",
1317+
input: { file_path: "/tmp/f" },
1318+
response: "contents",
1319+
});
1320+
});
1321+
1322+
it("does not log errors regardless of registration ordering", async () => {
1323+
const errors: string[] = [];
1324+
const spyLogger: Logger = {
1325+
log: () => {},
1326+
error: (...args: any[]) => {
1327+
errors.push(args.map(String).join(" "));
1328+
},
1329+
};
1330+
1331+
const hook = createPostToolUseHook(spyLogger);
1332+
1333+
// Case A: register-then-fire
1334+
registerHookCallback("toolu_order_a", {
1335+
onPostToolUseHook: async () => {},
1336+
});
1337+
await hook(postToolUseInput("toolu_order_a", "Bash"), "toolu_order_a", {
1338+
signal: AbortSignal.abort(),
1339+
});
1340+
1341+
// Case B: fire-then-register
1342+
const hookPromise = hook(postToolUseInput("toolu_order_b", "Grep"), "toolu_order_b", {
1343+
signal: AbortSignal.abort(),
1344+
});
1345+
await new Promise((r) => setTimeout(r, 5));
1346+
registerHookCallback("toolu_order_b", {
1347+
onPostToolUseHook: async () => {},
1348+
});
1349+
await hookPromise;
1350+
1351+
expect(errors).toHaveLength(0);
1352+
});
1353+
1354+
it("keeps hooks independent when some are pre-registered and some are late", async () => {
1355+
const callOrder: string[] = [];
1356+
const hook = createPostToolUseHook(mockLogger);
1357+
1358+
// Register callback A upfront.
1359+
registerHookCallback("toolu_mix_a", {
1360+
onPostToolUseHook: async (id) => {
1361+
callOrder.push(id);
1362+
},
1363+
});
1364+
1365+
// Fire hook B first (no registration yet), then hook A.
1366+
const hookBPromise = hook(postToolUseInput("toolu_mix_b", "Read"), "toolu_mix_b", {
1367+
signal: AbortSignal.abort(),
1368+
});
1369+
1370+
await hook(postToolUseInput("toolu_mix_a", "Bash"), "toolu_mix_a", {
1371+
signal: AbortSignal.abort(),
1372+
});
1373+
1374+
// A should have executed already.
1375+
expect(callOrder).toEqual(["toolu_mix_a"]);
1376+
1377+
// Now register B — its hook should complete.
1378+
registerHookCallback("toolu_mix_b", {
1379+
onPostToolUseHook: async (id) => {
1380+
callOrder.push(id);
1381+
},
1382+
});
1383+
1384+
await hookBPromise;
1385+
expect(callOrder).toEqual(["toolu_mix_a", "toolu_mix_b"]);
1386+
});
1387+
1388+
it("always returns { continue: true } even in the race case", async () => {
1389+
const hook = createPostToolUseHook(mockLogger);
1390+
1391+
const hookPromise = hook(postToolUseInput("toolu_continue_1", "Agent"), "toolu_continue_1", {
1392+
signal: AbortSignal.abort(),
1393+
});
1394+
1395+
registerHookCallback("toolu_continue_1", {
1396+
onPostToolUseHook: async () => {},
1397+
});
1398+
1399+
expect(await hookPromise).toEqual({ continue: true });
1400+
});
1401+
});
12371402
});

src/tools.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,25 @@ const toolUseCallbacks: {
728728
};
729729
} = {};
730730

731+
/*
732+
* When the SDK fires PostToolUse before the streaming handler has called
733+
* registerHookCallback(), we store a deferred here. registerHookCallback()
734+
* checks this map and resolves the deferred so the waiting hook can proceed.
735+
*/
736+
const pendingHooks: {
737+
[toolUseId: string]: {
738+
resolve: (
739+
cb: (toolUseID: string, toolInput: unknown, toolResponse: unknown) => Promise<void>,
740+
) => void;
741+
promise: Promise<
742+
(toolUseID: string, toolInput: unknown, toolResponse: unknown) => Promise<void>
743+
>;
744+
};
745+
} = {};
746+
747+
/** Maximum time (ms) to wait for a callback registration before giving up. */
748+
const HOOK_WAIT_TIMEOUT_MS = 5_000;
749+
731750
/* Setup callbacks that will be called when receiving hooks from Claude Code */
732751
export const registerHookCallback = (
733752
toolUseID: string,
@@ -744,6 +763,12 @@ export const registerHookCallback = (
744763
toolUseCallbacks[toolUseID] = {
745764
onPostToolUseHook,
746765
};
766+
767+
// If a PostToolUse hook is already waiting for this ID, hand it the callback.
768+
if (onPostToolUseHook && pendingHooks[toolUseID]) {
769+
pendingHooks[toolUseID].resolve(onPostToolUseHook);
770+
delete pendingHooks[toolUseID];
771+
}
747772
};
748773

749774
/* A callback for Claude Code that is called when receiving a PostToolUse hook */
@@ -767,8 +792,32 @@ export const createPostToolUseHook =
767792
await onPostToolUseHook(toolUseID, input.tool_input, input.tool_response);
768793
delete toolUseCallbacks[toolUseID]; // Cleanup after execution
769794
} else {
770-
logger.error(`No onPostToolUseHook found for tool use ID: ${toolUseID}`);
771-
delete toolUseCallbacks[toolUseID];
795+
// The SDK fired PostToolUse before the streaming handler called
796+
// registerHookCallback(). Wait for registration (with timeout).
797+
let resolve: (typeof pendingHooks)[string]["resolve"];
798+
const promise = new Promise<
799+
(toolUseID: string, toolInput: unknown, toolResponse: unknown) => Promise<void>
800+
>((r) => {
801+
resolve = r;
802+
});
803+
pendingHooks[toolUseID] = { resolve: resolve!, promise };
804+
805+
const cb = await Promise.race([
806+
promise,
807+
new Promise<null>((r) => setTimeout(() => r(null), HOOK_WAIT_TIMEOUT_MS)),
808+
]);
809+
810+
delete pendingHooks[toolUseID];
811+
812+
if (cb) {
813+
await cb(toolUseID, input.tool_input, input.tool_response);
814+
delete toolUseCallbacks[toolUseID];
815+
} else {
816+
logger.error(
817+
`PostToolUse hook timed out waiting for callback registration: ${toolUseID}`,
818+
);
819+
delete toolUseCallbacks[toolUseID];
820+
}
772821
}
773822
}
774823
}

0 commit comments

Comments
 (0)