Skip to content

Commit a09f6b1

Browse files
steipetesteipete
authored
fix(codex): preserve terminal outcome ordering (#93287)
1 parent d1b33a6 commit a09f6b1

13 files changed

Lines changed: 531 additions & 32 deletions

‎extensions/codex/src/app-server/attempt-notifications.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,31 @@ export function readCodexNotificationItem(
436436
: undefined;
437437
}
438438

439+
/** Reads the stable call id from a model-emitted raw tool item. */
440+
export function readRawResponseToolCallId(
441+
notification: CodexServerNotification,
442+
): string | undefined {
443+
if (notification.method !== "rawResponseItem/completed" || !isJsonObject(notification.params)) {
444+
return undefined;
445+
}
446+
const item = isJsonObject(notification.params.item) ? notification.params.item : undefined;
447+
if (!item) {
448+
return undefined;
449+
}
450+
switch (readString(item, "type")) {
451+
case "custom_tool_call":
452+
case "function_call":
453+
case "local_shell_call":
454+
case "tool_search_call":
455+
return readString(item, "call_id");
456+
case "image_generation_call":
457+
case "web_search_call":
458+
return readString(item, "id");
459+
default:
460+
return undefined;
461+
}
462+
}
463+
439464
/** Maps Codex item types to the tool name shown in execution progress. */
440465
export function codexExecutionToolName(item: CodexThreadItem): string | undefined {
441466
if (item.type === "dynamicToolCall" && typeof item.tool === "string") {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,13 +583,15 @@ describe("Codex app-server dynamic tool build", () => {
583583
});
584584
});
585585

586-
it("forwards the tool outcome observer into Codex dynamic tools", async () => {
586+
it("forwards tool outcome ordering into Codex dynamic tools", async () => {
587587
const sessionFile = path.join(tempDir, "session.jsonl");
588588
const workspaceDir = path.join(tempDir, "workspace");
589589
const params = createParams(sessionFile, workspaceDir);
590590
const onToolOutcome = vi.fn();
591+
const allocateToolOutcomeOrdinal = vi.fn(() => 0);
591592
params.disableTools = false;
592593
params.onToolOutcome = onToolOutcome;
594+
params.allocateToolOutcomeOrdinal = allocateToolOutcomeOrdinal;
593595
params.runtimePlan = createCodexRuntimePlanFixture();
594596
const factoryOptions: unknown[] = [];
595597
setOpenClawCodingToolsFactoryForTests((options) => {
@@ -599,7 +601,10 @@ describe("Codex app-server dynamic tool build", () => {
599601

600602
await buildDynamicToolsForTest(params, workspaceDir, { sandbox: null as never });
601603

602-
expect(factoryOptions[0]).toMatchObject({ onToolOutcome });
604+
expect(factoryOptions[0]).toMatchObject({
605+
onToolOutcome,
606+
allocateToolOutcomeOrdinal,
607+
});
603608
});
604609

605610
it("preserves before-tool wrapping through Codex runtime normalization", async () => {

‎extensions/codex/src/app-server/dynamic-tool-build.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
291291
toolBuildStages.mark(name);
292292
},
293293
onToolOutcome: params.onToolOutcome,
294+
allocateToolOutcomeOrdinal: params.allocateToolOutcomeOrdinal,
294295
});
295296
toolBuildStages.mark("create-openclaw-coding-tools");
296297
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [];

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ describe("dynamic tool execution helpers", () => {
183183
vi.useFakeTimers();
184184
let capturedSignal: AbortSignal | undefined;
185185
const onTimeout = vi.fn();
186+
const onFallbackSelected = vi.fn();
186187
const onAgentToolResult = vi.fn();
187188
const response = handleDynamicToolCallWithTimeout({
188189
call: {
@@ -202,6 +203,7 @@ describe("dynamic tool execution helpers", () => {
202203
signal: new AbortController().signal,
203204
timeoutMs: 1,
204205
onAgentToolResult,
206+
onFallbackSelected,
205207
onTimeout,
206208
});
207209

@@ -217,6 +219,7 @@ describe("dynamic tool execution helpers", () => {
217219
],
218220
});
219221
expect(capturedSignal?.aborted).toBe(true);
222+
expect(onFallbackSelected).toHaveBeenCalledOnce();
220223
expect(onTimeout).toHaveBeenCalledTimes(1);
221224
expect(onAgentToolResult).toHaveBeenCalledWith({
222225
toolName: "message",

‎extensions/codex/src/app-server/dynamic-tool-execution.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,9 @@ export async function handleDynamicToolCallWithTimeout(params: {
126126
toolBridge: Pick<CodexDynamicToolBridge, "handleToolCall">;
127127
signal: AbortSignal;
128128
timeoutMs: number;
129+
toolCallOrdinal?: number;
129130
onAgentToolResult?: EmbeddedRunAttemptParams["onAgentToolResult"];
131+
onFallbackSelected?: () => void;
130132
onTimeout?: () => void;
131133
}): Promise<CodexDynamicToolCallResponse> {
132134
// Timeout or run abort can win while a tool ignores cancellation. Keep the
@@ -159,6 +161,7 @@ export async function handleDynamicToolCallWithTimeout(params: {
159161
};
160162
if (params.signal.aborted) {
161163
const message = "OpenClaw dynamic tool call aborted before execution.";
164+
params.onFallbackSelected?.();
162165
notifyFailedToolResult(message);
163166
return failedDynamicToolResponse(message);
164167
}
@@ -169,6 +172,7 @@ export async function handleDynamicToolCallWithTimeout(params: {
169172
let resolveAbort: ((response: CodexDynamicToolCallResponse) => void) | undefined;
170173
const abortFromRun = () => {
171174
const message = "OpenClaw dynamic tool call aborted.";
175+
params.onFallbackSelected?.();
172176
controller.abort(params.signal.reason ?? new Error(message));
173177
notifyFailedToolResult(message);
174178
resolveAbort?.(failedDynamicToolResponse(message, { sideEffectEvidence: true }));
@@ -181,6 +185,7 @@ export async function handleDynamicToolCallWithTimeout(params: {
181185
timeout = setTimeout(() => {
182186
timedOut = true;
183187
const timeoutDetails = formatDynamicToolTimeoutDetails({ call: params.call, timeoutMs });
188+
params.onFallbackSelected?.();
184189
controller.abort(new Error(timeoutDetails.responseMessage));
185190
params.onTimeout?.();
186191
embeddedAgentLog.warn("codex dynamic tool call timed out", {
@@ -204,6 +209,7 @@ export async function handleDynamicToolCallWithTimeout(params: {
204209
params.toolBridge.handleToolCall(params.call, {
205210
signal: controller.signal,
206211
onAgentToolResult: notifyAgentToolResult,
212+
toolCallOrdinal: params.toolCallOrdinal,
207213
}),
208214
abortPromise,
209215
timeoutPromise,

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ type CodexDynamicToolHookContext = {
6666
replyToMode?: "off" | "first" | "all" | "batched";
6767
hasRepliedRef?: { value: boolean };
6868
onToolOutcome?: EmbeddedRunAttemptParams["onToolOutcome"];
69+
allocateToolOutcomeOrdinal?: EmbeddedRunAttemptParams["allocateToolOutcomeOrdinal"];
6970
};
7071

7172
type CodexToolResultHookContext = Omit<CodexDynamicToolHookContext, "config">;
@@ -107,6 +108,7 @@ export type CodexDynamicToolBridge = {
107108
options?: {
108109
signal?: AbortSignal;
109110
onAgentToolResult?: EmbeddedRunAttemptParams["onAgentToolResult"];
111+
toolCallOrdinal?: number;
110112
},
111113
) => Promise<CodexDynamicToolCallResponse>;
112114
telemetry: {
@@ -227,6 +229,7 @@ export function createCodexDynamicToolBridge(params: {
227229
isError: true,
228230
observer: params.hookContext?.onToolOutcome,
229231
toolName: call.tool,
232+
toolCallOrdinal: options?.toolCallOrdinal,
230233
});
231234
notifyAgentToolResult(
232235
options?.onAgentToolResult,
@@ -326,6 +329,7 @@ export function createCodexDynamicToolBridge(params: {
326329
isError: resultIsError,
327330
observer: params.hookContext?.onToolOutcome,
328331
toolName,
332+
toolCallOrdinal: options?.toolCallOrdinal,
329333
});
330334
const messagingTelemetryArgs = applyCurrentMessageProvider(
331335
toolName,
@@ -393,6 +397,7 @@ export function createCodexDynamicToolBridge(params: {
393397
isError: true,
394398
observer: params.hookContext?.onToolOutcome,
395399
toolName,
400+
toolCallOrdinal: options?.toolCallOrdinal,
396401
});
397402
notifyAgentToolResult(options?.onAgentToolResult, toolName, failedResult, true);
398403
collectToolTelemetry({

‎extensions/codex/src/app-server/event-projector.test.ts‎

Lines changed: 96 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,22 +2788,24 @@ describe("CodexAppServerEventProjector", () => {
27882788
terminalPresentation = observation.terminalPresentation;
27892789
},
27902790
});
2791+
const item = {
2792+
type: "commandExecution",
2793+
id: "command-clear-presentation",
2794+
command: "git status --short",
2795+
cwd: "/workspace",
2796+
processId: null,
2797+
source: "agent",
2798+
status: "completed",
2799+
commandActions: [{ type: "unknown", command: "git status --short" }],
2800+
aggregatedOutput: "",
2801+
exitCode: 0,
2802+
durationMs: 1,
2803+
};
27912804

2805+
await projector.handleNotification(forCurrentTurn("item/started", { item }));
27922806
await projector.handleNotification(
27932807
forCurrentTurn("item/completed", {
2794-
item: {
2795-
type: "commandExecution",
2796-
id: "command-clear-presentation",
2797-
command: "git status --short",
2798-
cwd: "/workspace",
2799-
processId: null,
2800-
source: "agent",
2801-
status: "completed",
2802-
commandActions: [{ type: "unknown", command: "git status --short" }],
2803-
aggregatedOutput: "",
2804-
exitCode: 0,
2805-
durationMs: 1,
2806-
},
2808+
item,
28072809
}),
28082810
);
28092811

@@ -2826,9 +2828,90 @@ describe("CodexAppServerEventProjector", () => {
28262828
id: "image-view-clear-presentation",
28272829
path: "/workspace/reference.png",
28282830
},
2831+
{
2832+
type: "dynamicToolCall",
2833+
id: "stale-dynamic-tool",
2834+
turnId: "turn-old",
2835+
tool: "web_fetch",
2836+
status: "completed",
2837+
},
2838+
]),
2839+
);
2840+
2841+
expect(terminalPresentation).toBeUndefined();
2842+
});
2843+
2844+
it("keeps a later dynamic presentation over an earlier snapshot-only native tool", async () => {
2845+
let terminalPresentation: string | undefined = "later dynamic result";
2846+
let latestOrdinal = 1;
2847+
let nextOrdinal = 0;
2848+
const projector = await createProjector({
2849+
...(await createParams()),
2850+
allocateToolOutcomeOrdinal: () => nextOrdinal++,
2851+
onToolOutcome: (observation) => {
2852+
const ordinal = observation.toolCallOrdinal ?? latestOrdinal + 1;
2853+
if (ordinal >= latestOrdinal) {
2854+
latestOrdinal = ordinal;
2855+
terminalPresentation = observation.terminalPresentation;
2856+
}
2857+
},
2858+
});
2859+
const nativeItem = {
2860+
type: "imageView",
2861+
id: "image-view-before-dynamic",
2862+
path: "/workspace/reference.png",
2863+
};
2864+
2865+
await projector.handleNotification(
2866+
forCurrentTurn("item/completed", {
2867+
item: nativeItem,
2868+
}),
2869+
);
2870+
2871+
await projector.handleNotification(
2872+
turnCompleted([
2873+
nativeItem,
2874+
{
2875+
type: "dynamicToolCall",
2876+
id: "dynamic-after-image-view",
2877+
turnId: TURN_ID,
2878+
tool: "web_fetch",
2879+
status: "completed",
2880+
},
2881+
{
2882+
type: "imageView",
2883+
id: "stale-image-view",
2884+
turnId: "turn-old",
2885+
path: "/workspace/stale.png",
2886+
},
28292887
]),
28302888
);
28312889

2890+
expect(terminalPresentation).toBe("later dynamic result");
2891+
});
2892+
2893+
it("clears a prior presentation for a completion-only native item without a turn snapshot", async () => {
2894+
let terminalPresentation: string | undefined = "stale dynamic result";
2895+
let nextOrdinal = 1;
2896+
const projector = await createProjector({
2897+
...(await createParams()),
2898+
allocateToolOutcomeOrdinal: () => nextOrdinal++,
2899+
onToolOutcome: (observation) => {
2900+
terminalPresentation = observation.terminalPresentation;
2901+
},
2902+
});
2903+
2904+
await projector.handleNotification(
2905+
forCurrentTurn("item/completed", {
2906+
item: {
2907+
type: "imageView",
2908+
id: "completion-only-image-view",
2909+
path: "/workspace/reference.png",
2910+
},
2911+
}),
2912+
);
2913+
await projector.handleNotification(turnCompleted([]));
2914+
28322915
expect(terminalPresentation).toBeUndefined();
28332916
});
28342917

‎extensions/codex/src/app-server/event-projector.ts‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ export class CodexAppServerEventProjector {
171171
{ toolName: string; meta?: string; asyncStarted?: boolean }
172172
>();
173173
private readonly terminalPresentationClearedItemIds = new Set<string>();
174+
private readonly nativeToolOutcomeOrdinals = new Map<string, number>();
174175
private readonly sideEffectingToolItemIds = new Set<string>();
175176
private readonly sideEffectingDynamicToolCallIds = new Set<string>();
176177
private readonly toolTranscriptMessages: AgentMessage[] = [];
@@ -217,6 +218,21 @@ export class CodexAppServerEventProjector {
217218
return finalItem !== undefined && this.completedItemIds.has(finalItem.itemId);
218219
}
219220

221+
/** Resolves the shared model-order position for a native tool item. */
222+
recordNativeToolOutcome(item: CodexThreadItem | undefined): void {
223+
if (
224+
!item ||
225+
this.nativeToolOutcomeOrdinals.has(item.id) ||
226+
!shouldClearTerminalPresentationForNativeItem(item)
227+
) {
228+
return;
229+
}
230+
const ordinal = this.params.allocateToolOutcomeOrdinal?.(item.id);
231+
if (ordinal !== undefined) {
232+
this.nativeToolOutcomeOrdinals.set(item.id, ordinal);
233+
}
234+
}
235+
220236
async handleNotification(notification: CodexServerNotification): Promise<void> {
221237
const params = isJsonObject(notification.params) ? notification.params : undefined;
222238
if (!params) {
@@ -583,6 +599,7 @@ export class CodexAppServerEventProjector {
583599
if (itemId) {
584600
this.activeItemIds.add(itemId);
585601
}
602+
this.recordNativeToolOutcome(item);
586603
if (item?.type === "contextCompaction" && itemId) {
587604
this.activeCompactionItemIds.add(itemId);
588605
await runAgentHarnessBeforeCompactionHook({
@@ -623,6 +640,7 @@ export class CodexAppServerEventProjector {
623640

624641
private async handleItemCompleted(params: JsonObject): Promise<void> {
625642
const item = readItem(params.item);
643+
this.recordNativeToolOutcome(item);
626644
this.clearTerminalPresentationForNativeItem(item);
627645
const itemId = item?.id ?? readString(params, "itemId") ?? readString(params, "id");
628646
if (itemId) {
@@ -766,8 +784,23 @@ export class CodexAppServerEventProjector {
766784
"codex app-server turn failed";
767785
this.promptErrorSource = "prompt";
768786
}
769-
for (const item of turn.items ?? []) {
770-
this.clearTerminalPresentationForNativeItem(item);
787+
const turnItems = turn.items ?? [];
788+
// The final snapshot is authoritative when item notifications were omitted.
789+
// Only its last relevant tool may change the terminal presentation.
790+
for (let index = turnItems.length - 1; index >= 0; index -= 1) {
791+
const item = turnItems[index];
792+
if (!item || !this.isCurrentTurnSnapshotItem(item)) {
793+
continue;
794+
}
795+
if (item?.type === "dynamicToolCall") {
796+
break;
797+
}
798+
if (shouldClearTerminalPresentationForNativeItem(item)) {
799+
this.clearTerminalPresentationForNativeItem(item);
800+
break;
801+
}
802+
}
803+
for (const item of turnItems) {
771804
this.rememberAssistantPhase(item);
772805
if (item.type === "agentMessage" && typeof item.text === "string") {
773806
this.rememberAssistantItem(item.id);
@@ -1132,11 +1165,13 @@ export class CodexAppServerEventProjector {
11321165
) {
11331166
return;
11341167
}
1168+
const toolCallOrdinal = this.nativeToolOutcomeOrdinals.get(item.id);
11351169
this.terminalPresentationClearedItemIds.add(item.id);
11361170
this.params.onToolOutcome?.({
11371171
toolName: itemName(item) ?? item.type,
11381172
argsHash: "",
11391173
resultHash: "",
1174+
...(toolCallOrdinal !== undefined ? { toolCallOrdinal } : {}),
11401175
terminalPresentation: undefined,
11411176
presentationOnly: true,
11421177
});

0 commit comments

Comments
 (0)