fix codex dynamic tool hooks (#70965)

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

@@ -1,6 +1,7 @@
 import type { AgentToolResult } from "@mariozechner/pi-agent-core";
 import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-harness";
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { wrapToolWithBeforeToolCallHook } from "../../../../src/agents/pi-tools.before-tool-call.js";
 import {
   initializeGlobalHookRunner,
   resetGlobalHookRunner,
@@ -33,6 +34,13 @@ function mediaResult(mediaUrl: string, audioAsVoice?: boolean): AgentToolResult<
   };
 }
 
+function textToolResult(text: string, details: unknown = {}): AgentToolResult<unknown> {
+  return {
+    content: [{ type: "text", text }],
+    details,
+  };
+}
+
 function createBridgeWithToolResult(toolName: string, toolResult: AgentToolResult<unknown>) {
   return createCodexDynamicToolBridge({
     tools: [
@@ -281,4 +289,292 @@ describe("createCodexDynamicToolBridge", () => {
       );
     });
   });
+
+  it("runs before_tool_call for unwrapped dynamic tools before execution", async () => {
+    const beforeToolCall = vi.fn(async () => ({ params: { mode: "safe" } }));
+    const afterToolCall = vi.fn();
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        { hookName: "before_tool_call", handler: beforeToolCall },
+        { hookName: "after_tool_call", handler: afterToolCall },
+      ]),
+    );
+
+    const execute = vi.fn(async () => textToolResult("done", { ok: true }));
+    const bridge = createCodexDynamicToolBridge({
+      tools: [createTool({ name: "exec", execute })],
+      signal: new AbortController().signal,
+      hookContext: {
+        agentId: "agent-1",
+        sessionId: "session-1",
+        sessionKey: "agent:agent-1:session-1",
+        runId: "run-1",
+      },
+    });
+
+    const result = await bridge.handleToolCall({
+      threadId: "thread-1",
+      turnId: "turn-1",
+      callId: "call-1",
+      namespace: null,
+      tool: "exec",
+      arguments: { command: "pwd" },
+    });
+
+    expect(result).toEqual(expectInputText("done"));
+    expect(beforeToolCall).toHaveBeenCalledWith(
+      expect.objectContaining({
+        toolName: "exec",
+        toolCallId: "call-1",
+        runId: "run-1",
+        params: { command: "pwd" },
+      }),
+      expect.objectContaining({
+        agentId: "agent-1",
+        sessionId: "session-1",
+        sessionKey: "agent:agent-1:session-1",
+        runId: "run-1",
+        toolCallId: "call-1",
+      }),
+    );
+    expect(execute).toHaveBeenCalledWith(
+      "call-1",
+      { command: "pwd", mode: "safe" },
+      expect.any(AbortSignal),
+      undefined,
+    );
+    await vi.waitFor(() => {
+      expect(afterToolCall).toHaveBeenCalledWith(
+        expect.objectContaining({
+          toolName: "exec",
+          toolCallId: "call-1",
+          params: { command: "pwd", mode: "safe" },
+          result: expect.objectContaining({
+            content: [{ type: "text", text: "done" }],
+            details: { ok: true },
+          }),
+        }),
+        expect.objectContaining({
+          agentId: "agent-1",
+          sessionId: "session-1",
+          sessionKey: "agent:agent-1:session-1",
+          runId: "run-1",
+          toolCallId: "call-1",
+        }),
+      );
+    });
+  });
+
+  it("does not execute dynamic tools blocked by before_tool_call", async () => {
+    const beforeToolCall = vi.fn(async () => ({
+      block: true,
+      blockReason: "blocked by policy",
+    }));
+    const afterToolCall = vi.fn();
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        { hookName: "before_tool_call", handler: beforeToolCall },
+        { hookName: "after_tool_call", handler: afterToolCall },
+      ]),
+    );
+    const execute = vi.fn(async () => textToolResult("should not run"));
+    const bridge = createCodexDynamicToolBridge({
+      tools: [createTool({ name: "message", execute })],
+      signal: new AbortController().signal,
+      hookContext: { runId: "run-blocked" },
+    });
+
+    const result = await handleMessageToolCall(bridge, {
+      action: "send",
+      text: "blocked",
+      provider: "telegram",
+      to: "chat-1",
+    });
+
+    expect(result).toEqual({
+      success: false,
+      contentItems: [{ type: "inputText", text: "blocked by policy" }],
+    });
+    expect(execute).not.toHaveBeenCalled();
+    expect(bridge.telemetry.didSendViaMessagingTool).toBe(false);
+    await vi.waitFor(() => {
+      expect(afterToolCall).toHaveBeenCalledWith(
+        expect.objectContaining({
+          toolName: "message",
+          toolCallId: "call-1",
+          params: {
+            action: "send",
+            text: "blocked",
+            provider: "telegram",
+            to: "chat-1",
+          },
+          error: "blocked by policy",
+        }),
+        expect.objectContaining({
+          runId: "run-blocked",
+          toolCallId: "call-1",
+        }),
+      );
+    });
+  });
+
+  it("applies dynamic tool result middleware before after_tool_call observes the result", async () => {
+    const events: string[] = [];
+    const beforeToolCall = vi.fn(async () => {
+      events.push("before_tool_call");
+      return { params: { mode: "safe" } };
+    });
+    const afterToolCall = vi.fn(async (event) => {
+      events.push("after_tool_call");
+      expect(event).toMatchObject({
+        params: { command: "status", mode: "safe" },
+        result: {
+          content: [{ type: "text", text: "compacted output" }],
+          details: { stage: "middleware" },
+        },
+      });
+    });
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        { hookName: "before_tool_call", handler: beforeToolCall },
+        { hookName: "after_tool_call", handler: afterToolCall },
+      ]),
+    );
+    const registry = createEmptyPluginRegistry();
+    const factory = async (codex: {
+      on: (
+        event: "tool_result",
+        handler: (event: any) => Promise<{ result: AgentToolResult<unknown> }>,
+      ) => void;
+    }) => {
+      codex.on("tool_result", async (event) => {
+        events.push("middleware");
+        expect(event.args).toEqual({ command: "status" });
+        return {
+          result: {
+            ...event.result,
+            content: [{ type: "text", text: "compacted output" }],
+            details: { stage: "middleware" },
+          },
+        };
+      });
+    };
+    registry.codexAppServerExtensionFactories.push({
+      pluginId: "tokenjuice",
+      pluginName: "Tokenjuice",
+      rawFactory: factory,
+      factory,
+      source: "test",
+    });
+    setActivePluginRegistry(registry);
+    const execute = vi.fn(async () => {
+      events.push("execute");
+      return textToolResult("raw output", { stage: "execute" });
+    });
+    const bridge = createCodexDynamicToolBridge({
+      tools: [createTool({ name: "exec", execute })],
+      signal: new AbortController().signal,
+      hookContext: { runId: "run-middleware" },
+    });
+
+    const result = await bridge.handleToolCall({
+      threadId: "thread-1",
+      turnId: "turn-1",
+      callId: "call-1",
+      namespace: null,
+      tool: "exec",
+      arguments: { command: "status" },
+    });
+
+    expect(result).toEqual(expectInputText("compacted output"));
+    await vi.waitFor(() => {
+      expect(events).toEqual(["before_tool_call", "execute", "middleware", "after_tool_call"]);
+    });
+  });
+
+  it("reports dynamic tool execution errors through after_tool_call without stranding the turn", async () => {
+    const beforeToolCall = vi.fn(async () => ({ params: { timeoutSec: 1 } }));
+    const afterToolCall = vi.fn();
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        { hookName: "before_tool_call", handler: beforeToolCall },
+        { hookName: "after_tool_call", handler: afterToolCall },
+      ]),
+    );
+    const execute = vi.fn(async () => {
+      throw new Error("tool failed");
+    });
+    const bridge = createCodexDynamicToolBridge({
+      tools: [createTool({ name: "exec", execute })],
+      signal: new AbortController().signal,
+      hookContext: { runId: "run-error" },
+    });
+
+    const result = await bridge.handleToolCall({
+      threadId: "thread-1",
+      turnId: "turn-1",
+      callId: "call-err",
+      namespace: null,
+      tool: "exec",
+      arguments: { command: "false" },
+    });
+
+    expect(result).toEqual({
+      success: false,
+      contentItems: [{ type: "inputText", text: "tool failed" }],
+    });
+    expect(execute).toHaveBeenCalledWith(
+      "call-err",
+      { command: "false", timeoutSec: 1 },
+      expect.any(AbortSignal),
+      undefined,
+    );
+    await vi.waitFor(() => {
+      expect(afterToolCall).toHaveBeenCalledWith(
+        expect.objectContaining({
+          toolName: "exec",
+          toolCallId: "call-err",
+          params: { command: "false", timeoutSec: 1 },
+          error: "tool failed",
+        }),
+        expect.objectContaining({
+          runId: "run-error",
+          toolCallId: "call-err",
+        }),
+      );
+    });
+  });
+
+  it("does not double-wrap dynamic tools that already have before_tool_call", async () => {
+    const beforeToolCall = vi.fn(async () => ({ params: { mode: "safe" } }));
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([{ hookName: "before_tool_call", handler: beforeToolCall }]),
+    );
+    const execute = vi.fn(async () => textToolResult("done"));
+    const tool = wrapToolWithBeforeToolCallHook(createTool({ name: "exec", execute }), {
+      runId: "run-wrapped",
+    });
+    const bridge = createCodexDynamicToolBridge({
+      tools: [tool],
+      signal: new AbortController().signal,
+      hookContext: { runId: "run-wrapped" },
+    });
+
+    await bridge.handleToolCall({
+      threadId: "thread-1",
+      turnId: "turn-1",
+      callId: "call-wrapped",
+      namespace: null,
+      tool: "exec",
+      arguments: { command: "pwd" },
+    });
+
+    expect(beforeToolCall).toHaveBeenCalledTimes(1);
+    expect(execute).toHaveBeenCalledWith(
+      "call-wrapped",
+      { command: "pwd", mode: "safe" },
+      expect.any(AbortSignal),
+      undefined,
+    );
+  });
 });

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

@@ -4,11 +4,13 @@ import {
   createCodexAppServerToolResultExtensionRunner,
   extractToolResultMediaArtifact,
   filterToolResultMediaUrls,
+  isToolWrappedWithBeforeToolCallHook,
   isMessagingTool,
   isMessagingToolSendAction,
   runAgentHarnessAfterToolCallHook,
   type AnyAgentTool,
   type MessagingToolSend,
+  wrapToolWithBeforeToolCallHook,
 } from "openclaw/plugin-sdk/agent-harness-runtime";
 import {
   type CodexDynamicToolCallOutputContentItem,
@@ -42,7 +44,12 @@ export function createCodexDynamicToolBridge(params: {
     runId?: string;
   };
 }): CodexDynamicToolBridge {
-  const toolMap = new Map(params.tools.map((tool) => [tool.name, tool]));
+  const tools = params.tools.map((tool) =>
+    isToolWrappedWithBeforeToolCallHook(tool)
+      ? tool
+      : wrapToolWithBeforeToolCallHook(tool, params.hookContext),
+  );
+  const toolMap = new Map(tools.map((tool) => [tool.name, tool]));
   const telemetry: CodexDynamicToolBridge["telemetry"] = {
     didSendViaMessagingTool: false,
     messagingToolSentTexts: [],
@@ -54,7 +61,7 @@ export function createCodexDynamicToolBridge(params: {
   const extensionRunner = createCodexAppServerToolResultExtensionRunner(params.hookContext ?? {});
 
   return {
-    specs: params.tools.map((tool) => ({
+    specs: tools.map((tool) => ({
       name: tool.name,
       description: tool.description,
       inputSchema: toJsonValue(tool.parameters),