From 60d716e652cd582a56088aed51e9a1f7810c1310 Mon Sep 17 00:00:00 2001 From: Ben Badejo <188106718+bdjben@users.noreply.github.com> Date: Mon, 8 Jun 2026 18:05:16 +0300 Subject: [PATCH] fix: normalize Codex dynamic tool progress results Normalize Codex dynamic tool progress result payloads to TUI-compatible content arrays after sanitization, while stripping protocol-only fields from the emitted event. Includes regression coverage for sanitized dynamic tool text/image progress output. Thanks @bdjben. --- .../run-attempt.dynamic-tools.test.ts | 11 ++- .../codex/src/app-server/run-attempt.test.ts | 95 +++++++++++++++++++ .../codex/src/app-server/run-attempt.ts | 40 +++++++- 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/extensions/codex/src/app-server/run-attempt.dynamic-tools.test.ts b/extensions/codex/src/app-server/run-attempt.dynamic-tools.test.ts index 23ff89fc19d7..48d2076d6a18 100644 --- a/extensions/codex/src/app-server/run-attempt.dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/run-attempt.dynamic-tools.test.ts @@ -128,7 +128,11 @@ describe("runCodexAppServerAttempt dynamic tools", () => { isError?: boolean; name?: string; phase?: string; - result?: { success?: boolean }; + result?: { + content?: Array<{ text?: string; type?: string; url?: string }>; + contentItems?: unknown; + success?: unknown; + }; toolCallId?: string; }; stream?: string; @@ -150,7 +154,10 @@ describe("runCodexAppServerAttempt dynamic tools", () => { expect(resultEvent?.data?.name).toBe("lookup"); expect(resultEvent?.data?.toolCallId).toBe("call-1"); expect(resultEvent?.data?.isError).toBe(true); - expect(resultEvent?.data?.result?.success).toBe(false); + expect(resultEvent?.data?.result).not.toHaveProperty("success"); + expect(resultEvent?.data?.result).not.toHaveProperty("contentItems"); + expect(resultEvent?.data?.result?.content?.[0]?.type).toBe("text"); + expect(resultEvent?.data?.result?.content?.[0]?.text).toBe("Unknown OpenClaw tool: lookup"); expect(JSON.stringify(agentEvents)).not.toContain("plain-secret-value-12345"); const globalStartEvent = globalAgentEvents.find( (event) => event.stream === "tool" && event.data.phase === "start", diff --git a/extensions/codex/src/app-server/run-attempt.test.ts b/extensions/codex/src/app-server/run-attempt.test.ts index db4b2de0ea8d..445f0f8b5367 100644 --- a/extensions/codex/src/app-server/run-attempt.test.ts +++ b/extensions/codex/src/app-server/run-attempt.test.ts @@ -1162,6 +1162,101 @@ describe("runCodexAppServerAttempt", () => { ]); }); + it("emits TUI-compatible tool events for Codex dynamic tool calls", async () => { + const sessionFile = path.join(tempDir, "session-tool-events.jsonl"); + const workspaceDir = path.join(tempDir, "workspace-tool-events"); + const harness = createStartedThreadHarness(); + const params = createParams(sessionFile, workspaceDir); + const onRunAgentEvent = vi.fn(); + params.timeoutMs = 60_000; + params.onAgentEvent = onRunAgentEvent; + + const run = runCodexAppServerAttempt(params); + await harness.waitForMethod("turn/start"); + + await expect( + harness.handleServerRequest({ + id: "request-tool-1", + method: "item/tool/call", + params: { + threadId: "thread-1", + turnId: "turn-1", + callId: "call-1", + namespace: null, + tool: "python", + arguments: { code: "print('hi')" }, + }, + }), + ).resolves.toMatchObject({ + success: false, + contentItems: [{ type: "inputText", text: "Unknown OpenClaw tool: python" }], + }); + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await run; + + expect(onRunAgentEvent).toHaveBeenCalledWith({ + stream: "tool", + data: { + phase: "start", + name: "python", + toolCallId: "call-1", + args: { code: "print('hi')" }, + }, + }); + expect(onRunAgentEvent).toHaveBeenCalledWith({ + stream: "tool", + data: { + phase: "result", + name: "python", + toolCallId: "call-1", + isError: true, + result: { + content: [{ type: "text", text: "Unknown OpenClaw tool: python" }], + }, + }, + }); + const resultEvent = onRunAgentEvent.mock.calls + .map(([event]) => event) + .find( + ( + event, + ): event is { + data: { + phase: "result"; + result: { content?: unknown; contentItems?: unknown; success?: unknown }; + }; + stream: "tool"; + } => event.stream === "tool" && event.data?.phase === "result", + ); + expect(resultEvent?.data.result).not.toHaveProperty("success"); + expect(resultEvent?.data.result).not.toHaveProperty("contentItems"); + }); + + it("maps sanitized dynamic tool output into transcript progress content", () => { + const rawToolSecret = "sk-abcdefghijklmnopqrstuvwxyz1234567890"; // pragma: allowlist secret + const result = testing.toTranscriptToolResultForTests({ + success: true, + contentItems: [ + { type: "inputText", text: `lookup result: ${rawToolSecret}` }, + { type: "inputImage", imageUrl: "data:image/png;base64,abc" }, + { type: "unsupportedCodexOutput", imageUrl: "data:image/png;base64,ignored" }, + ], + }); + const content = result.content as Array<{ text?: string; type?: string; url?: string }>; + + expect(result).not.toHaveProperty("success"); + expect(result).not.toHaveProperty("contentItems"); + expect(content[0]).toEqual({ type: "text", text: expect.any(String) }); + expect(content[0]?.text).toContain("lookup result:"); + expect(content[0]?.text).not.toContain(rawToolSecret); + expect(content[1]).toEqual({ type: "image", url: "data:image/png;base64,abc" }); + expect(content[2]).toEqual({ + type: "text", + text: "[Unsupported Codex dynamic tool output: unsupportedCodexOutput]", + }); + expect(JSON.stringify(result)).not.toContain(rawToolSecret); + }); + it("keeps leading delivery hints out of the Codex current user request", async () => { const sessionFile = path.join(tempDir, "session-delivery-hint.jsonl"); const workspaceDir = path.join(tempDir, "workspace-delivery-hint"); diff --git a/extensions/codex/src/app-server/run-attempt.ts b/extensions/codex/src/app-server/run-attempt.ts index f4f010033ad6..35d68434b45a 100644 --- a/extensions/codex/src/app-server/run-attempt.ts +++ b/extensions/codex/src/app-server/run-attempt.ts @@ -309,6 +309,43 @@ function emitCodexAppServerEvent( } } +function toTranscriptToolResult(response: CodexDynamicToolCallResponse): Record { + const sanitized = sanitizeCodexToolResponse(response); + const contentItems = Array.isArray(sanitized.contentItems) ? sanitized.contentItems : []; + const result: Record = { + ...sanitized, + // Progress events are UI/transcript-facing; map only sanitized content so + // event redaction cannot be bypassed by raw dynamic tool output. + content: contentItems.map(toTranscriptToolResultContentItem), + }; + delete result.contentItems; + delete result.success; + return result; +} + +function toTranscriptToolResultContentItem(item: unknown): Record { + if (!item || typeof item !== "object") { + return { type: "text", text: "" }; + } + const record = item as Record; + if (record.type === "inputText") { + return { type: "text", text: typeof record.text === "string" ? record.text : "" }; + } + if (record.type === "inputImage") { + return typeof record.imageUrl === "string" + ? { type: "image", url: record.imageUrl } + : { type: "text", text: formatUnsupportedCodexDynamicToolOutput(record.type) }; + } + return { type: "text", text: formatUnsupportedCodexDynamicToolOutput(record.type) }; +} + +function formatUnsupportedCodexDynamicToolOutput(type: unknown): string { + const rawType = typeof type === "string" ? type.replace(/\s+/g, " ").trim() : ""; + const label = rawType ? rawType.slice(0, 80) : "unknown"; + const suffix = rawType.length > 80 ? "..." : ""; + return `[Unsupported Codex dynamic tool output: ${label}${suffix}]`; +} + type CodexAgentEndHookParams = Parameters[0]; function shouldAwaitCodexAgentEndHook(params: EmbeddedRunAttemptParams): boolean { @@ -1717,7 +1754,7 @@ export async function runCodexAppServerAttempt( toolCallId: call.callId, ...(toolMeta ? { meta: toolMeta } : {}), isError: !protocolResponse.success, - result: sanitizeCodexToolResponse(progressResponse), + result: toTranscriptToolResult(progressResponse), }, }); } @@ -2770,6 +2807,7 @@ export const testing = { shouldEnableCodexAppServerNativeToolSurface, shouldForceMessageTool, hasPendingDynamicToolTerminalDiagnostic, + toTranscriptToolResultForTests: toTranscriptToolResult, withCodexStartupTimeout, setOpenClawCodingToolsFactoryForTests, resetOpenClawCodingToolsFactoryForTests,