From 53d5ef5756a69846651d17b20c69ddb62ddcb3c4 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Mon, 20 Jul 2026 17:37:48 +0800 Subject: [PATCH] Emit tool approval metadata for Codex tool spans --- .../src/agents/codex/event-processor.test.ts | 14 ++++- .../src/agents/codex/event-processor.ts | 63 ++++++++++++++++++- plugins/trace-codex/src/test-helpers.ts | 9 +++ 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/plugins/trace-codex/src/agents/codex/event-processor.test.ts b/plugins/trace-codex/src/agents/codex/event-processor.test.ts index dac10fc..720830a 100644 --- a/plugins/trace-codex/src/agents/codex/event-processor.test.ts +++ b/plugins/trace-codex/src/agents/codex/event-processor.test.ts @@ -724,7 +724,12 @@ describe("CodexEventProcessor: tool spans", () => { span_attributes: { name: "exec_command", type: "tool" }, input: '{"cmd":"ls"}', output: "file.txt", - metadata: { tool_name: "exec_command", call_id: "call_1", turn_id: "t1" }, + metadata: { + tool_name: "exec_command", + call_id: "call_1", + turn_id: "t1", + tool_approval: "approved", + }, ended: true, children: [], }, @@ -764,6 +769,7 @@ describe("CodexEventProcessor: tool spans", () => { span_attributes: { name: "apply_patch", type: "tool" }, input: "*** Begin Patch", output: "Success", + metadata: { tool_approval: "approved" }, ended: true, }, ], @@ -829,6 +835,8 @@ describe("CodexEventProcessor: tool spans", () => { { span_attributes: { name: "exec_command", type: "tool" }, input: "{}", + metadata: { tool_approval: "approved" }, + error: "Tool output missing before turn ended", ended: true, children: [], }, @@ -1141,8 +1149,9 @@ describe("CodexEventProcessor: permissions", () => { { // The failed sandboxed attempt: no permission annotation. span_attributes: { name: "exec_command", type: "tool" }, - metadata: { call_id: "attempt", permission: undefined }, + metadata: { call_id: "attempt", permission: undefined, tool_approval: "approved" }, output: "Error: network blocked", + error: "Error: network blocked", ended: true, }, { @@ -1154,6 +1163,7 @@ describe("CodexEventProcessor: permissions", () => { sandbox_permissions: "require_escalated", justification: "Need network access", }, + tool_approval: "approved", }, tags: ["permission-request"], output: "ok", diff --git a/plugins/trace-codex/src/agents/codex/event-processor.ts b/plugins/trace-codex/src/agents/codex/event-processor.ts index 0f22529..8dc4116 100644 --- a/plugins/trace-codex/src/agents/codex/event-processor.ts +++ b/plugins/trace-codex/src/agents/codex/event-processor.ts @@ -81,6 +81,7 @@ const POST_TOOL_USE = "PostToolUse"; const PRE_COMPACT = "PreCompact"; const POST_COMPACT = "PostCompact"; const SPAWN_AGENT_TOOL = "spawn_agent"; +const MISSING_TOOL_OUTPUT_ERROR = "Tool output missing before turn ended"; // Tag applied to a tool span whose call requested escalated permissions, so // permission-gated actions are easy to find/filter in Braintrust. const PERMISSION_TAG = "permission-request"; @@ -607,6 +608,53 @@ function skillLoadMetadata(info: SkillLoadInfo | undefined): Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function conciseErrorText(value: unknown, fallback: string): string { + if (typeof value === "string") return value.split("\n")[0] || fallback; + if (value instanceof Error) return value.message || fallback; + if (isObject(value)) { + const candidate = value.error ?? value.message ?? value.stderr ?? value.output ?? value.result; + if (typeof candidate === "string") return candidate.split("\n")[0] || fallback; + } + return fallback; +} + +function classifyToolOutput(output: unknown): { error?: string } { + if (isObject(output)) { + if (output.is_error === true || output.isError === true) { + return { error: conciseErrorText(output, "Tool execution failed") }; + } + if (output.status === "error" || output.status === "failed") { + return { error: conciseErrorText(output, "Tool execution failed") }; + } + if (output.error !== undefined) { + return { + error: conciseErrorText(output.error, "Tool execution failed"), + }; + } + const exitCode = output.exit_code ?? output.exitCode; + if (typeof exitCode === "number" && exitCode !== 0) { + return { + error: conciseErrorText(output, `Exit code ${exitCode}`), + }; + } + } + + if (typeof output === "string") { + const firstLine = output.split("\n")[0] ?? output; + if (/^Error:/i.test(firstLine)) return { error: firstLine }; + const exitMatch = /^Exit code\s+(-?\d+)/i.exec(firstLine); + if (exitMatch !== null && Number(exitMatch[1]) !== 0) { + return { error: firstLine }; + } + } + + return {}; +} + // ConversationItems (chat messages / reasoning) are already plain JSON, so they // round-trip through a snapshot unchanged. These two helpers just bridge the // nominal type boundary between the live union and the snapshot's record array @@ -1935,8 +1983,13 @@ export class CodexEventProcessor implements EventProcessor { return; } const endTime = isoToUnixSeconds(record.timestamp); + const { error } = classifyToolOutput(output); try { - entry.span.log({ output }); + entry.span.log({ + output, + metadata: { tool_approval: "approved" }, + ...(error !== undefined ? { error } : {}), + }); entry.span.end(endTime !== undefined ? { endTime } : undefined); // A tool is a child of its turn; advance the turn's boundary so a following // LLM call (the model resuming after the tool result) starts here. @@ -1963,6 +2016,10 @@ export class CodexEventProcessor implements EventProcessor { for (const [callId, entry] of scope.openTools) { if (entry.turnId !== turnId) continue; try { + entry.span.log({ + metadata: { tool_approval: "approved" }, + error: MISSING_TOOL_OUTPUT_ERROR, + }); entry.span.end(endTime !== undefined ? { endTime } : undefined); } catch (err) { this.logger.error("codex processor: failed to end dangling tool span", { @@ -1985,6 +2042,10 @@ export class CodexEventProcessor implements EventProcessor { ): void { for (const [callId, entry] of scope.openTools) { try { + entry.span.log({ + metadata: { tool_approval: "approved" }, + error: MISSING_TOOL_OUTPUT_ERROR, + }); entry.span.end(endArgs); } catch (err) { this.logger.error("codex processor: failed to end dangling tool span", { diff --git a/plugins/trace-codex/src/test-helpers.ts b/plugins/trace-codex/src/test-helpers.ts index 2f99eeb..07dc660 100644 --- a/plugins/trace-codex/src/test-helpers.ts +++ b/plugins/trace-codex/src/test-helpers.ts @@ -103,6 +103,7 @@ export interface CapturedSpan { span_attributes?: { name?: string; type?: string }; input?: unknown; output?: unknown; + error?: unknown; metadata?: Record; tags?: string[]; metrics?: { start?: number; end?: number } & Record; @@ -158,6 +159,7 @@ export interface SpanTree { type?: string; input?: unknown; output?: unknown; + error?: unknown; metadata?: Record; tags?: string[]; metrics?: { start?: number; end?: number } & Record; @@ -237,6 +239,7 @@ export function spansToTree(rawSpans: CapturedSpan[]): SpanTree | null { type: span.span_attributes?.type, input: span.input, output: span.output, + error: span.error, metadata: span.metadata, tags: span.tags, metrics: span.metrics, @@ -255,6 +258,7 @@ export interface ExpectedSpan { span_attributes?: { name?: string | RegExp; type?: string }; input?: unknown; output?: unknown; + error?: unknown; metadata?: Record; /** If set, assert each listed tag is present on the span. */ tags?: string[]; @@ -320,6 +324,11 @@ export function diffSpan(actual: SpanTree | null, expected: ExpectedSpan, path: const e = JSON.stringify(expected.output); if (a !== e) diffs.push(`${path}.output: expected ${e}, got ${a}`); } + if (expected.error !== undefined) { + const a = JSON.stringify(actual.error); + const e = JSON.stringify(expected.error); + if (a !== e) diffs.push(`${path}.error: expected ${e}, got ${a}`); + } if (expected.metadata !== undefined) { for (const [key, value] of Object.entries(expected.metadata)) { // Compare with sorted keys so nested-object key order doesn't matter (the