Skip to content

Commit 81666e5

Browse files
pashpashpashpashpashpash
authored
fix codex dynamic tool hooks (#70965)
1 parent 4e4aeac commit 81666e5

3 files changed

Lines changed: 309 additions & 2 deletions

File tree

‎extensions/codex/src/app-server/dynamic-tools.test.ts‎

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
22
import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-harness";
33
import { afterEach, describe, expect, it, vi } from "vitest";
4+
import { wrapToolWithBeforeToolCallHook } from "../../../../src/agents/pi-tools.before-tool-call.js";
45
import {
56
initializeGlobalHookRunner,
67
resetGlobalHookRunner,
@@ -33,6 +34,13 @@ function mediaResult(mediaUrl: string, audioAsVoice?: boolean): AgentToolResult<
3334
};
3435
}
3536

37+
function textToolResult(text: string, details: unknown = {}): AgentToolResult<unknown> {
38+
return {
39+
content: [{ type: "text", text }],
40+
details,
41+
};
42+
}
43+
3644
function createBridgeWithToolResult(toolName: string, toolResult: AgentToolResult<unknown>) {
3745
return createCodexDynamicToolBridge({
3846
tools: [
@@ -281,4 +289,292 @@ describe("createCodexDynamicToolBridge", () => {
281289
);
282290
});
283291
});
292+
293+
it("runs before_tool_call for unwrapped dynamic tools before execution", async () => {
294+
const beforeToolCall = vi.fn(async () => ({ params: { mode: "safe" } }));
295+
const afterToolCall = vi.fn();
296+
initializeGlobalHookRunner(
297+
createMockPluginRegistry([
298+
{ hookName: "before_tool_call", handler: beforeToolCall },
299+
{ hookName: "after_tool_call", handler: afterToolCall },
300+
]),
301+
);
302+
303+
const execute = vi.fn(async () => textToolResult("done", { ok: true }));
304+
const bridge = createCodexDynamicToolBridge({
305+
tools: [createTool({ name: "exec", execute })],
306+
signal: new AbortController().signal,
307+
hookContext: {
308+
agentId: "agent-1",
309+
sessionId: "session-1",
310+
sessionKey: "agent:agent-1:session-1",
311+
runId: "run-1",
312+
},
313+
});
314+
315+
const result = await bridge.handleToolCall({
316+
threadId: "thread-1",
317+
turnId: "turn-1",
318+
callId: "call-1",
319+
namespace: null,
320+
tool: "exec",
321+
arguments: { command: "pwd" },
322+
});
323+
324+
expect(result).toEqual(expectInputText("done"));
325+
expect(beforeToolCall).toHaveBeenCalledWith(
326+
expect.objectContaining({
327+
toolName: "exec",
328+
toolCallId: "call-1",
329+
runId: "run-1",
330+
params: { command: "pwd" },
331+
}),
332+
expect.objectContaining({
333+
agentId: "agent-1",
334+
sessionId: "session-1",
335+
sessionKey: "agent:agent-1:session-1",
336+
runId: "run-1",
337+
toolCallId: "call-1",
338+
}),
339+
);
340+
expect(execute).toHaveBeenCalledWith(
341+
"call-1",
342+
{ command: "pwd", mode: "safe" },
343+
expect.any(AbortSignal),
344+
undefined,
345+
);
346+
await vi.waitFor(() => {
347+
expect(afterToolCall).toHaveBeenCalledWith(
348+
expect.objectContaining({
349+
toolName: "exec",
350+
toolCallId: "call-1",
351+
params: { command: "pwd", mode: "safe" },
352+
result: expect.objectContaining({
353+
content: [{ type: "text", text: "done" }],
354+
details: { ok: true },
355+
}),
356+
}),
357+
expect.objectContaining({
358+
agentId: "agent-1",
359+
sessionId: "session-1",
360+
sessionKey: "agent:agent-1:session-1",
361+
runId: "run-1",
362+
toolCallId: "call-1",
363+
}),
364+
);
365+
});
366+
});
367+
368+
it("does not execute dynamic tools blocked by before_tool_call", async () => {
369+
const beforeToolCall = vi.fn(async () => ({
370+
block: true,
371+
blockReason: "blocked by policy",
372+
}));
373+
const afterToolCall = vi.fn();
374+
initializeGlobalHookRunner(
375+
createMockPluginRegistry([
376+
{ hookName: "before_tool_call", handler: beforeToolCall },
377+
{ hookName: "after_tool_call", handler: afterToolCall },
378+
]),
379+
);
380+
const execute = vi.fn(async () => textToolResult("should not run"));
381+
const bridge = createCodexDynamicToolBridge({
382+
tools: [createTool({ name: "message", execute })],
383+
signal: new AbortController().signal,
384+
hookContext: { runId: "run-blocked" },
385+
});
386+
387+
const result = await handleMessageToolCall(bridge, {
388+
action: "send",
389+
text: "blocked",
390+
provider: "telegram",
391+
to: "chat-1",
392+
});
393+
394+
expect(result).toEqual({
395+
success: false,
396+
contentItems: [{ type: "inputText", text: "blocked by policy" }],
397+
});
398+
expect(execute).not.toHaveBeenCalled();
399+
expect(bridge.telemetry.didSendViaMessagingTool).toBe(false);
400+
await vi.waitFor(() => {
401+
expect(afterToolCall).toHaveBeenCalledWith(
402+
expect.objectContaining({
403+
toolName: "message",
404+
toolCallId: "call-1",
405+
params: {
406+
action: "send",
407+
text: "blocked",
408+
provider: "telegram",
409+
to: "chat-1",
410+
},
411+
error: "blocked by policy",
412+
}),
413+
expect.objectContaining({
414+
runId: "run-blocked",
415+
toolCallId: "call-1",
416+
}),
417+
);
418+
});
419+
});
420+
421+
it("applies dynamic tool result middleware before after_tool_call observes the result", async () => {
422+
const events: string[] = [];
423+
const beforeToolCall = vi.fn(async () => {
424+
events.push("before_tool_call");
425+
return { params: { mode: "safe" } };
426+
});
427+
const afterToolCall = vi.fn(async (event) => {
428+
events.push("after_tool_call");
429+
expect(event).toMatchObject({
430+
params: { command: "status", mode: "safe" },
431+
result: {
432+
content: [{ type: "text", text: "compacted output" }],
433+
details: { stage: "middleware" },
434+
},
435+
});
436+
});
437+
initializeGlobalHookRunner(
438+
createMockPluginRegistry([
439+
{ hookName: "before_tool_call", handler: beforeToolCall },
440+
{ hookName: "after_tool_call", handler: afterToolCall },
441+
]),
442+
);
443+
const registry = createEmptyPluginRegistry();
444+
const factory = async (codex: {
445+
on: (
446+
event: "tool_result",
447+
handler: (event: any) => Promise<{ result: AgentToolResult<unknown> }>,
448+
) => void;
449+
}) => {
450+
codex.on("tool_result", async (event) => {
451+
events.push("middleware");
452+
expect(event.args).toEqual({ command: "status" });
453+
return {
454+
result: {
455+
...event.result,
456+
content: [{ type: "text", text: "compacted output" }],
457+
details: { stage: "middleware" },
458+
},
459+
};
460+
});
461+
};
462+
registry.codexAppServerExtensionFactories.push({
463+
pluginId: "tokenjuice",
464+
pluginName: "Tokenjuice",
465+
rawFactory: factory,
466+
factory,
467+
source: "test",
468+
});
469+
setActivePluginRegistry(registry);
470+
const execute = vi.fn(async () => {
471+
events.push("execute");
472+
return textToolResult("raw output", { stage: "execute" });
473+
});
474+
const bridge = createCodexDynamicToolBridge({
475+
tools: [createTool({ name: "exec", execute })],
476+
signal: new AbortController().signal,
477+
hookContext: { runId: "run-middleware" },
478+
});
479+
480+
const result = await bridge.handleToolCall({
481+
threadId: "thread-1",
482+
turnId: "turn-1",
483+
callId: "call-1",
484+
namespace: null,
485+
tool: "exec",
486+
arguments: { command: "status" },
487+
});
488+
489+
expect(result).toEqual(expectInputText("compacted output"));
490+
await vi.waitFor(() => {
491+
expect(events).toEqual(["before_tool_call", "execute", "middleware", "after_tool_call"]);
492+
});
493+
});
494+
495+
it("reports dynamic tool execution errors through after_tool_call without stranding the turn", async () => {
496+
const beforeToolCall = vi.fn(async () => ({ params: { timeoutSec: 1 } }));
497+
const afterToolCall = vi.fn();
498+
initializeGlobalHookRunner(
499+
createMockPluginRegistry([
500+
{ hookName: "before_tool_call", handler: beforeToolCall },
501+
{ hookName: "after_tool_call", handler: afterToolCall },
502+
]),
503+
);
504+
const execute = vi.fn(async () => {
505+
throw new Error("tool failed");
506+
});
507+
const bridge = createCodexDynamicToolBridge({
508+
tools: [createTool({ name: "exec", execute })],
509+
signal: new AbortController().signal,
510+
hookContext: { runId: "run-error" },
511+
});
512+
513+
const result = await bridge.handleToolCall({
514+
threadId: "thread-1",
515+
turnId: "turn-1",
516+
callId: "call-err",
517+
namespace: null,
518+
tool: "exec",
519+
arguments: { command: "false" },
520+
});
521+
522+
expect(result).toEqual({
523+
success: false,
524+
contentItems: [{ type: "inputText", text: "tool failed" }],
525+
});
526+
expect(execute).toHaveBeenCalledWith(
527+
"call-err",
528+
{ command: "false", timeoutSec: 1 },
529+
expect.any(AbortSignal),
530+
undefined,
531+
);
532+
await vi.waitFor(() => {
533+
expect(afterToolCall).toHaveBeenCalledWith(
534+
expect.objectContaining({
535+
toolName: "exec",
536+
toolCallId: "call-err",
537+
params: { command: "false", timeoutSec: 1 },
538+
error: "tool failed",
539+
}),
540+
expect.objectContaining({
541+
runId: "run-error",
542+
toolCallId: "call-err",
543+
}),
544+
);
545+
});
546+
});
547+
548+
it("does not double-wrap dynamic tools that already have before_tool_call", async () => {
549+
const beforeToolCall = vi.fn(async () => ({ params: { mode: "safe" } }));
550+
initializeGlobalHookRunner(
551+
createMockPluginRegistry([{ hookName: "before_tool_call", handler: beforeToolCall }]),
552+
);
553+
const execute = vi.fn(async () => textToolResult("done"));
554+
const tool = wrapToolWithBeforeToolCallHook(createTool({ name: "exec", execute }), {
555+
runId: "run-wrapped",
556+
});
557+
const bridge = createCodexDynamicToolBridge({
558+
tools: [tool],
559+
signal: new AbortController().signal,
560+
hookContext: { runId: "run-wrapped" },
561+
});
562+
563+
await bridge.handleToolCall({
564+
threadId: "thread-1",
565+
turnId: "turn-1",
566+
callId: "call-wrapped",
567+
namespace: null,
568+
tool: "exec",
569+
arguments: { command: "pwd" },
570+
});
571+
572+
expect(beforeToolCall).toHaveBeenCalledTimes(1);
573+
expect(execute).toHaveBeenCalledWith(
574+
"call-wrapped",
575+
{ command: "pwd", mode: "safe" },
576+
expect.any(AbortSignal),
577+
undefined,
578+
);
579+
});
284580
});

‎extensions/codex/src/app-server/dynamic-tools.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import {
44
createCodexAppServerToolResultExtensionRunner,
55
extractToolResultMediaArtifact,
66
filterToolResultMediaUrls,
7+
isToolWrappedWithBeforeToolCallHook,
78
isMessagingTool,
89
isMessagingToolSendAction,
910
runAgentHarnessAfterToolCallHook,
1011
type AnyAgentTool,
1112
type MessagingToolSend,
13+
wrapToolWithBeforeToolCallHook,
1214
} from "openclaw/plugin-sdk/agent-harness-runtime";
1315
import {
1416
type CodexDynamicToolCallOutputContentItem,
@@ -42,7 +44,12 @@ export function createCodexDynamicToolBridge(params: {
4244
runId?: string;
4345
};
4446
}): CodexDynamicToolBridge {
45-
const toolMap = new Map(params.tools.map((tool) => [tool.name, tool]));
47+
const tools = params.tools.map((tool) =>
48+
isToolWrappedWithBeforeToolCallHook(tool)
49+
? tool
50+
: wrapToolWithBeforeToolCallHook(tool, params.hookContext),
51+
);
52+
const toolMap = new Map(tools.map((tool) => [tool.name, tool]));
4653
const telemetry: CodexDynamicToolBridge["telemetry"] = {
4754
didSendViaMessagingTool: false,
4855
messagingToolSentTexts: [],
@@ -54,7 +61,7 @@ export function createCodexDynamicToolBridge(params: {
5461
const extensionRunner = createCodexAppServerToolResultExtensionRunner(params.hookContext ?? {});
5562

5663
return {
57-
specs: params.tools.map((tool) => ({
64+
specs: tools.map((tool) => ({
5865
name: tool.name,
5966
description: tool.description,
6067
inputSchema: toJsonValue(tool.parameters),

‎src/plugin-sdk/agent-harness-runtime.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ export { resolveSandboxContext } from "../agents/sandbox.js";
6868
export { isSubagentSessionKey } from "../routing/session-key.js";
6969
export { acquireSessionWriteLock } from "../agents/session-write-lock.js";
7070
export { emitSessionTranscriptUpdate } from "../sessions/transcript-events.js";
71+
export {
72+
isToolWrappedWithBeforeToolCallHook,
73+
wrapToolWithBeforeToolCallHook,
74+
} from "../agents/pi-tools.before-tool-call.js";
7175
export {
7276
resolveAgentHarnessBeforePromptBuildResult,
7377
runAgentHarnessAfterCompactionHook,

0 commit comments

Comments
 (0)