Skip to content

Commit 7cda26a

Browse files
joshavantjoshavant
authored
Handle Codex turns missing completion (#85107)
* fix(codex): handle missing turn completion * docs: add changelog for Codex completion fix
1 parent 6115087 commit 7cda26a

15 files changed

Lines changed: 683 additions & 147 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ Docs: https://docs.openclaw.ai
4343
- Update/doctor: prune stale local bundled plugin install records that point at old compiled bundled output so current bundled plugin schemas win after upgrade. (#84863) Thanks @fuller-stack-dev.
4444
- Providers/Ollama: preserve native Ollama tool-call IDs across assistant replay so Gemini over Ollama Cloud can keep its hidden function-call thought-signature handle.
4545
- Discord: keep session recovery and `/stop` abort ownership on the source dispatch lane while bound ACP turns continue routing to their target session, so stalled pre-run work and late replies are cleared instead of leaking after stop. Fixes #84477. (#85100) Thanks @joshavant.
46+
- Codex app-server: mark missing turn completion after observed execution as replay-unsafe and release the session so follow-up turns can run. Fixes #84076. (#85107) Thanks @joshavant.
4647
- PDF tool: time out idle remote PDF body reads after 120 seconds so stalled remote documents return an error instead of wedging the session. Fixes #68649. (#84768) Thanks @luoyanglang.
4748
- Diagnostics/OpenTelemetry plugin: suppress handled OTLP exporter promise rejections so collector shutdowns no longer crash the Gateway. (#81085) Thanks @luoyanglang.
4849
- Media/audio: skip empty structured sherpa-onnx transcripts instead of treating the raw JSON payload as spoken text. (#84667) Thanks @TurboTheTurtle.

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,11 +758,62 @@ describe("createCodexDynamicToolBridge", () => {
758758
success: false,
759759
contentItems: [{ type: "inputText", text: "failed output" }],
760760
});
761+
expect(result.sideEffectEvidence).toBe(true);
761762
const event = requireRecord(callArg(handler, 0, 0, "middleware event"), "middleware event");
762763
expect(event.isError).toBe(true);
763764
expectContextFields(callArg(handler, 0, 1, "middleware context"), { runtime: "codex" });
764765
});
765766

767+
it("marks executed dynamic tool results as side-effect evidence", async () => {
768+
const bridge = createBridgeWithToolResult("exec", textToolResult("done"));
769+
770+
const result = await bridge.handleToolCall({
771+
threadId: "thread-1",
772+
turnId: "turn-1",
773+
callId: "call-1",
774+
namespace: null,
775+
tool: "exec",
776+
arguments: { command: "pwd" },
777+
});
778+
779+
expect(result).toEqual(expectInputText("done"));
780+
expect(result.sideEffectEvidence).toBe(true);
781+
});
782+
783+
it("does not mark pre-execution argument failures as side-effect evidence", async () => {
784+
const execute = vi.fn(async () => textToolResult("should not run"));
785+
const bridge = createCodexDynamicToolBridge({
786+
tools: [
787+
createTool({
788+
name: "exec",
789+
execute,
790+
...({
791+
prepareArguments: () => {
792+
throw new Error("invalid arguments");
793+
},
794+
} as { prepareArguments: () => never }),
795+
}),
796+
],
797+
signal: new AbortController().signal,
798+
});
799+
800+
const result = await bridge.handleToolCall({
801+
threadId: "thread-1",
802+
turnId: "turn-1",
803+
callId: "call-1",
804+
namespace: null,
805+
tool: "exec",
806+
arguments: {},
807+
});
808+
809+
expect(result).toEqual({
810+
success: false,
811+
contentItems: [{ type: "inputText", text: "invalid arguments" }],
812+
});
813+
expect(result.sideEffectEvidence).toBeUndefined();
814+
expect(execute).not.toHaveBeenCalled();
815+
});
816+
766817
it("uses raw tool provenance for media trust after middleware rewrites details", async () => {
767818
const registry = createEmptyPluginRegistry();
768819
const handler = vi.fn(async (event: { result: AgentToolResult<unknown> }) => ({
@@ -1082,6 +1133,7 @@ describe("createCodexDynamicToolBridge", () => {
10821133
success: false,
10831134
contentItems: [{ type: "inputText", text: "blocked by policy" }],
10841135
});
1136+
expect(result.sideEffectEvidence).toBeUndefined();
10851137
expect(execute).not.toHaveBeenCalled();
10861138
expect(bridge.telemetry.didSendViaMessagingTool).toBe(false);
10871139
await vi.waitFor(() => {

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

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,10 @@ export function createCodexDynamicToolBridge(params: {
125125
const args = jsonObjectToRecord(call.arguments);
126126
const startedAt = Date.now();
127127
const signal = composeAbortSignals(params.signal, options?.signal);
128+
let didStartExecution = false;
128129
try {
129130
const preparedArgs = tool.prepareArguments ? tool.prepareArguments(args) : args;
131+
didStartExecution = true;
130132
const rawResult = await tool.execute(call.callId, preparedArgs, signal);
131133
const rawIsError = isToolResultError(rawResult);
132134
const middlewareResult = await middlewareRunner.applyToolResultMiddleware({
@@ -167,12 +169,16 @@ export function createCodexDynamicToolBridge(params: {
167169
result,
168170
startedAt,
169171
});
170-
return withDiagnosticTerminalType(
171-
{
172-
contentItems: convertToolContents(result.content, toolResultMaxChars),
173-
success: !resultIsError,
174-
},
175-
inferToolResultDiagnosticTerminalType(result, resultIsError),
172+
const terminalType = inferToolResultDiagnosticTerminalType(result, resultIsError);
173+
return withSideEffectEvidence(
174+
withDiagnosticTerminalType(
175+
{
176+
contentItems: convertToolContents(result.content, toolResultMaxChars),
177+
success: !resultIsError,
178+
},
179+
terminalType,
180+
),
181+
terminalType !== "blocked",
176182
);
177183
} catch (error) {
178184
collectToolTelemetry({
@@ -194,17 +200,20 @@ export function createCodexDynamicToolBridge(params: {
194200
error: error instanceof Error ? error.message : String(error),
195201
startedAt,
196202
});
197-
return withDiagnosticTerminalType(
198-
{
199-
contentItems: [
200-
{
201-
type: "inputText",
202-
text: error instanceof Error ? error.message : String(error),
203-
},
204-
],
205-
success: false,
206-
},
207-
"error",
203+
return withSideEffectEvidence(
204+
withDiagnosticTerminalType(
205+
{
206+
contentItems: [
207+
{
208+
type: "inputText",
209+
text: error instanceof Error ? error.message : String(error),
210+
},
211+
],
212+
success: false,
213+
},
214+
"error",
215+
),
216+
didStartExecution,
208217
);
209218
}
210219
},
@@ -230,7 +239,6 @@ function createCodexDynamicToolSpec(params: {
230239
deferLoading: true,
231240
};
232241
}
233-
234242
function toToolResultHookContext(
235243
ctx: CodexDynamicToolHookContext | undefined,
236244
): CodexToolResultHookContext {
@@ -463,6 +471,21 @@ function withDiagnosticTerminalType<T extends CodexDynamicToolCallResponse>(
463471
return response;
464472
}
465473

474+
function withSideEffectEvidence<T extends CodexDynamicToolCallResponse>(
475+
response: T,
476+
sideEffectEvidence: boolean,
477+
): T {
478+
if (!sideEffectEvidence) {
479+
return response;
480+
}
481+
Object.defineProperty(response, "sideEffectEvidence", {
482+
configurable: true,
483+
enumerable: false,
484+
value: true,
485+
});
486+
return response;
487+
}
488+
466489
function normalizeToolResultMaxChars(maxChars: number): number {
467490
return typeof maxChars === "number" && Number.isFinite(maxChars) && maxChars > 0
468491
? Math.floor(maxChars)

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

Lines changed: 95 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1991,6 +1991,95 @@ describe("CodexAppServerEventProjector", () => {
19911991
expect(payload.text).toContain("```txt\nopened\n```");
19921992
});
19931993

1994+
it("keeps side-effect evidence for dynamic tools that error after execution", async () => {
1995+
const projector = await createProjector();
1996+
1997+
projector.recordDynamicToolCall({
1998+
callId: "call-process-kill",
1999+
tool: "process",
2000+
arguments: { action: "kill", sessionId: "session-1" },
2001+
});
2002+
projector.recordDynamicToolResult({
2003+
callId: "call-process-kill",
2004+
tool: "process",
2005+
success: false,
2006+
terminalType: "error",
2007+
sideEffectEvidence: true,
2008+
contentItems: [{ type: "inputText", text: "process exited" }],
2009+
});
2010+
2011+
const result = projector.buildResult(buildEmptyToolTelemetry());
2012+
2013+
expect(result.replayMetadata).toEqual({ hadPotentialSideEffects: true, replaySafe: false });
2014+
});
2015+
2016+
it("does not keep side-effect evidence for pre-execution dynamic tool errors", async () => {
2017+
const projector = await createProjector();
2018+
2019+
projector.recordDynamicToolCall({
2020+
callId: "call-unknown-message",
2021+
tool: "message",
2022+
arguments: { action: "send", text: "hello" },
2023+
});
2024+
projector.recordDynamicToolResult({
2025+
callId: "call-unknown-message",
2026+
tool: "message",
2027+
success: false,
2028+
terminalType: "error",
2029+
contentItems: [{ type: "inputText", text: "Unknown OpenClaw tool: message" }],
2030+
});
2031+
2032+
const result = projector.buildResult(buildEmptyToolTelemetry());
2033+
2034+
expect(result.replayMetadata).toEqual({ hadPotentialSideEffects: false, replaySafe: true });
2035+
});
2036+
2037+
it("does not mark blocked dynamic tools as side-effecting", async () => {
2038+
const projector = await createProjector();
2039+
2040+
projector.recordDynamicToolCall({
2041+
callId: "call-bash-blocked",
2042+
tool: "bash",
2043+
arguments: { command: "touch blocked.txt" },
2044+
});
2045+
projector.recordDynamicToolResult({
2046+
callId: "call-bash-blocked",
2047+
tool: "bash",
2048+
success: false,
2049+
terminalType: "blocked",
2050+
sideEffectEvidence: true,
2051+
contentItems: [{ type: "inputText", text: "blocked" }],
2052+
});
2053+
2054+
const result = projector.buildResult(buildEmptyToolTelemetry());
2055+
2056+
expect(result.replayMetadata).toEqual({ hadPotentialSideEffects: false, replaySafe: true });
2057+
});
2058+
2059+
it("treats completed native MCP tool calls as side-effect evidence", async () => {
2060+
const projector = await createProjector();
2061+
2062+
await projector.handleNotification({
2063+
method: "item/completed",
2064+
params: {
2065+
threadId: "thread-1",
2066+
turnId: "turn-1",
2067+
item: {
2068+
id: "mcp-1",
2069+
type: "mcpToolCall",
2070+
server: "github",
2071+
tool: "create_issue",
2072+
status: "completed",
2073+
arguments: { title: "check replay safety" },
2074+
},
2075+
},
2076+
});
2077+
2078+
const result = projector.buildResult(buildEmptyToolTelemetry());
2079+
2080+
expect(result.replayMetadata).toEqual({ hadPotentialSideEffects: true, replaySafe: false });
2081+
});
2082+
19942083
it("suppresses transcript progress for message-like tools", async () => {
19952084
const onAgentEvent = vi.fn();
19962085
const onToolResult = vi.fn();
@@ -2021,7 +2110,7 @@ describe("CodexAppServerEventProjector", () => {
20212110
expect(onToolResult).not.toHaveBeenCalled();
20222111
});
20232112

2024-
it("suppresses transcript progress for activity-log bash commands", async () => {
2113+
it("does not parse shell command text to suppress transcript progress", async () => {
20252114
const onAgentEvent = vi.fn();
20262115
const onToolResult = vi.fn();
20272116
const projector = await createProjector({
@@ -2047,12 +2136,11 @@ describe("CodexAppServerEventProjector", () => {
20472136
contentItems: [{ type: "inputText", text: "Logged: [web_search] Grilled salmon research" }],
20482137
});
20492138

2050-
const toolEvents = onAgentEvent.mock.calls.filter(([event]) => {
2051-
const record = requireRecord(event, "agent event");
2052-
return record.stream === "tool";
2053-
});
2054-
expect(toolEvents).toHaveLength(0);
2055-
expect(onToolResult).not.toHaveBeenCalled();
2139+
expect(onAgentEvent).not.toHaveBeenCalled();
2140+
const toolProgressText = onToolResult.mock.calls
2141+
.map(([payload]) => (payload as { text?: string }).text ?? "")
2142+
.join("\n");
2143+
expect(toolProgressText).toContain("log_activity.sh");
20562144

20572145
const result = projector.buildResult(buildEmptyToolTelemetry());
20582146
expect(result.messagesSnapshot.some((message) => message.role === "toolResult")).toBe(true);

0 commit comments

Comments
 (0)