mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
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.
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -309,6 +309,43 @@ function emitCodexAppServerEvent(
|
||||
}
|
||||
}
|
||||
|
||||
function toTranscriptToolResult(response: CodexDynamicToolCallResponse): Record<string, unknown> {
|
||||
const sanitized = sanitizeCodexToolResponse(response);
|
||||
const contentItems = Array.isArray(sanitized.contentItems) ? sanitized.contentItems : [];
|
||||
const result: Record<string, unknown> = {
|
||||
...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<string, unknown> {
|
||||
if (!item || typeof item !== "object") {
|
||||
return { type: "text", text: "" };
|
||||
}
|
||||
const record = item as Record<string, unknown>;
|
||||
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<typeof runAgentHarnessAgentEndHook>[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,
|
||||
|
||||
Reference in New Issue
Block a user