Skip to content

Commit 8e6f966

Browse files
joshavantjoshavant
authored
fix(codex): continue turns after progress replies (#108487)
* fix(codex): defer omitted source reply finality * test(codex): refresh source reply prompt snapshots
1 parent 4d4b176 commit 8e6f966

9 files changed

Lines changed: 214 additions & 57 deletions

File tree

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

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
type CodexDynamicToolSpec,
3232
type JsonValue,
3333
} from "./protocol.js";
34+
import { settleCodexSourceReplyFinality } from "./source-reply-finality.js";
3435

3536
const CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE = "openclaw";
3637

@@ -1388,7 +1389,7 @@ describe("createCodexDynamicToolBridge", () => {
13881389
]);
13891390
});
13901391

1391-
it("marks delivered message-tool-only source replies as terminal when final is omitted", async () => {
1392+
it("keeps omitted source-reply finality non-terminal until a successful attempt settles", async () => {
13921393
const bridge = createBridgeWithToolResult(
13931394
"message",
13941395
textToolResult("Sent.", { messageId: "imessage-6264" }),
@@ -1401,15 +1402,97 @@ describe("createCodexDynamicToolBridge", () => {
14011402
});
14021403

14031404
expect(result).toEqual(expectInputText("Sent."));
1404-
expect(result.terminate).toBe(true);
1405+
expect(result.terminate).toBeUndefined();
14051406
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1407+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).not.toHaveProperty("sourceReplyFinal");
1408+
1409+
expect(settleCodexSourceReplyFinality(bridge.telemetry, true)).toBe(true);
1410+
14061411
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
14071412
sourceReplyFinal: true,
14081413
});
14091414
expect(Object.keys(result)).not.toContain("terminate");
14101415
});
14111416

1412-
it("requires explicit final=false to keep a delivered message-tool-only source reply non-terminal", async () => {
1417+
it("settles omitted source-reply finality as progress when the attempt fails", async () => {
1418+
const bridge = createBridgeWithToolResult(
1419+
"message",
1420+
{
1421+
...textToolResult("Sent.", { messageId: "imessage-6264" }),
1422+
terminate: true,
1423+
},
1424+
{ sourceReplyDeliveryMode: "message_tool_only" },
1425+
);
1426+
1427+
const result = await handleMessageToolCall(bridge, {
1428+
action: "send",
1429+
message: "visible reply",
1430+
});
1431+
expect(result.terminate).toBeUndefined();
1432+
expect(settleCodexSourceReplyFinality(bridge.telemetry, false)).toBe(false);
1433+
1434+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1435+
sourceReplyFinal: false,
1436+
});
1437+
});
1438+
1439+
it("settles only the latest omitted source reply as final after success", async () => {
1440+
const bridge = createBridgeWithToolResult(
1441+
"message",
1442+
textToolResult("Sent.", { messageId: "imessage-6264" }),
1443+
{ sourceReplyDeliveryMode: "message_tool_only" },
1444+
);
1445+
1446+
await handleMessageToolCall(bridge, { action: "send", message: "first update" });
1447+
await handleMessageToolCall(bridge, { action: "send", message: "second update" });
1448+
settleCodexSourceReplyFinality(bridge.telemetry, true);
1449+
1450+
expect(
1451+
bridge.telemetry.messagingToolSentTargets.map((target) => target.sourceReplyFinal),
1452+
).toEqual([false, true]);
1453+
});
1454+
1455+
it("does not promote an omitted reply past a later explicit progress reply", async () => {
1456+
const bridge = createBridgeWithToolResult(
1457+
"message",
1458+
textToolResult("Sent.", { messageId: "imessage-6264" }),
1459+
{ sourceReplyDeliveryMode: "message_tool_only" },
1460+
);
1461+
1462+
await handleMessageToolCall(bridge, { action: "send", message: "first update" });
1463+
await handleMessageToolCall(bridge, {
1464+
action: "send",
1465+
message: "still working",
1466+
final: false,
1467+
});
1468+
settleCodexSourceReplyFinality(bridge.telemetry, true);
1469+
1470+
expect(
1471+
bridge.telemetry.messagingToolSentTargets.map((target) => target.sourceReplyFinal),
1472+
).toEqual([false, false]);
1473+
});
1474+
1475+
it("keeps a later explicit final reply authoritative over an omitted reply", async () => {
1476+
const bridge = createBridgeWithToolResult(
1477+
"message",
1478+
textToolResult("Sent.", { messageId: "imessage-6264" }),
1479+
{ sourceReplyDeliveryMode: "message_tool_only" },
1480+
);
1481+
1482+
await handleMessageToolCall(bridge, { action: "send", message: "first update" });
1483+
await handleMessageToolCall(bridge, {
1484+
action: "send",
1485+
message: "finished",
1486+
final: true,
1487+
});
1488+
settleCodexSourceReplyFinality(bridge.telemetry, true);
1489+
1490+
expect(
1491+
bridge.telemetry.messagingToolSentTargets.map((target) => target.sourceReplyFinal),
1492+
).toEqual([false, true]);
1493+
});
1494+
1495+
it("honors explicit finality for delivered message-tool-only source replies", async () => {
14131496
const bridge = createBridgeWithToolResult(
14141497
"message",
14151498
textToolResult("Sent.", { messageId: "imessage-6264" }),
@@ -1474,6 +1557,7 @@ describe("createCodexDynamicToolBridge", () => {
14741557
const result = await handleMessageToolCall(bridge, {
14751558
action: "send",
14761559
message: "visible reply",
1560+
final: true,
14771561
});
14781562

14791563
expect(result).toEqual(expectInputText("Sent."));
@@ -1525,6 +1609,7 @@ describe("createCodexDynamicToolBridge", () => {
15251609
messageId: "853",
15261610
message: "visible reply",
15271611
buttons: [],
1612+
final: true,
15281613
});
15291614

15301615
expect(result).toEqual(expectInputText("Sent."));
@@ -1571,6 +1656,7 @@ describe("createCodexDynamicToolBridge", () => {
15711656
target: "+1 (206) 910-6512",
15721657
messageId: "853",
15731658
message: "visible reply",
1659+
final: true,
15741660
});
15751661

15761662
expect(result).toEqual(expectInputText("Sent."));
@@ -1610,6 +1696,7 @@ describe("createCodexDynamicToolBridge", () => {
16101696
messageId: "857",
16111697
message: "visible reply",
16121698
buttons: [],
1699+
final: true,
16131700
});
16141701

16151702
expect(result).toEqual(expectInputText("Sent."));
@@ -1646,6 +1733,7 @@ describe("createCodexDynamicToolBridge", () => {
16461733
messageId: "861",
16471734
message: "visible reply",
16481735
buttons: [],
1736+
final: true,
16491737
});
16501738

16511739
expect(result).toEqual(expectInputText(receiptText));
@@ -1726,6 +1814,7 @@ describe("createCodexDynamicToolBridge", () => {
17261814
messageId: "863",
17271815
message: "visible reply",
17281816
buttons: [],
1817+
final: true,
17291818
});
17301819

17311820
expect(result).toEqual(expectInputText("Sent."));
@@ -1747,6 +1836,7 @@ describe("createCodexDynamicToolBridge", () => {
17471836
messageId: "865",
17481837
message: "visible reply",
17491838
buttons: [],
1839+
final: true,
17501840
});
17511841

17521842
expect(result).toEqual(expectInputText("Sent."));
@@ -1755,7 +1845,7 @@ describe("createCodexDynamicToolBridge", () => {
17551845
expect(Object.keys(result)).not.toContain("terminate");
17561846
});
17571847

1758-
it("records message-tool-owned terminal replies as delivered source replies", async () => {
1848+
it("defers omitted finality even when the message tool returns legacy termination", async () => {
17591849
const bridge = createBridgeWithToolResult(
17601850
"message",
17611851
{
@@ -1775,8 +1865,12 @@ describe("createCodexDynamicToolBridge", () => {
17751865
});
17761866

17771867
expect(result).toEqual(expectInputText("Sent."));
1778-
expect(result.terminate).toBe(true);
1868+
expect(result.terminate).toBeUndefined();
17791869
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1870+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).not.toHaveProperty("sourceReplyFinal");
1871+
1872+
settleCodexSourceReplyFinality(bridge.telemetry, true);
1873+
17801874
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
17811875
sourceReplyFinal: true,
17821876
});
@@ -1850,7 +1944,7 @@ describe("createCodexDynamicToolBridge", () => {
18501944
arguments: { action: "inspect" },
18511945
});
18521946

1853-
expect(firstResult.terminate).toBe(true);
1947+
expect(firstResult.terminate).toBeUndefined();
18541948
expect(bridge.telemetry.didSendViaMessagingTool).toBe(true);
18551949
expect(secondResult).toEqual(expectInputText("No message sent."));
18561950
expect(secondResult.terminate).toBeUndefined();

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

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
type CodexDynamicToolSpec,
6262
type JsonValue,
6363
} from "./protocol.js";
64+
import { recordCodexSourceReplyDeliveryIntent } from "./source-reply-finality.js";
6465
import { resolveCodexToolAbortTerminalReason } from "./tool-abort-terminal-reason.js";
6566

6667
type CodexDynamicToolHookContext = {
@@ -688,18 +689,16 @@ export function createCodexDynamicToolBridge(params: {
688689
!resultIsError &&
689690
(rawResult.terminate === true || result.terminate === true);
690691
const hasExplicitFinalControl = typeof executedArgs.final === "boolean";
691-
// Omitted final on a confirmed source reply must degrade to legacy
692-
// terminate-on-delivery (completed marker), never progress; otherwise
693-
// stranded-reply recovery re-delivers a duplicate of that reply.
694-
const sourceReplyFinal =
692+
const confirmedSourceReply =
695693
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
696694
toolName === "message" &&
697-
(toolConfirmedSourceReply || deliveredSourceReply || receiptConfirmedSourceReply)
698-
? hasExplicitFinalControl
699-
? executedArgs.final === true
700-
: true
701-
: undefined;
702-
collectToolTelemetry({
695+
(toolConfirmedSourceReply || deliveredSourceReply || receiptConfirmedSourceReply);
696+
const sourceReplyFinal = confirmedSourceReply
697+
? hasExplicitFinalControl
698+
? executedArgs.final === true
699+
: undefined
700+
: undefined;
701+
const sourceReplyRecord = collectToolTelemetry({
703702
toolName,
704703
args: executedArgs,
705704
result,
@@ -709,20 +708,26 @@ export function createCodexDynamicToolBridge(params: {
709708
messagingTarget: confirmedMessagingTarget,
710709
sourceReplyFinal,
711710
});
711+
if (confirmedSourceReply && sourceReplyRecord) {
712+
recordCodexSourceReplyDeliveryIntent(telemetry, {
713+
record: sourceReplyRecord,
714+
final: sourceReplyFinal,
715+
});
716+
}
712717
if (deliveredSourceReply || receiptConfirmedSourceReply || toolConfirmedSourceReply) {
713718
telemetry.didDeliverSourceReplyViaMessageTool = true;
714719
}
720+
const defersInferredSourceReplyTermination =
721+
confirmedSourceReply && executedArgs.final !== true;
715722
withDynamicToolTermination(
716723
response,
717724
((rawResult.terminate === true || result.terminate === true) &&
718-
!(
719-
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
720-
toolName === "message" &&
721-
executedArgs.final === false
722-
)) ||
725+
!defersInferredSourceReplyTermination) ||
726+
// Yield is an explicit owner-level turn handoff, not termination
727+
// inferred from source-reply delivery, so finality does not mask it.
723728
isToolResultYield(rawResult) ||
724729
isToolResultYield(result) ||
725-
((deliveredSourceReply || receiptConfirmedSourceReply) && executedArgs.final !== false),
730+
(confirmedSourceReply && executedArgs.final === true),
726731
);
727732
const asyncStarted =
728733
isAsyncStartedToolResult(rawResult) || isAsyncStartedToolResult(result);
@@ -1146,9 +1151,9 @@ function collectToolTelemetry(params: {
11461151
isError: boolean;
11471152
messagingTarget?: MessagingToolSend;
11481153
sourceReplyFinal?: boolean;
1149-
}): void {
1154+
}): MessagingToolSend | MessagingToolSourceReplyPayload | undefined {
11501155
if (params.isError) {
1151-
return;
1156+
return undefined;
11521157
}
11531158
if (!params.isError && params.toolName === "cron" && isCronAddAction(params.args)) {
11541159
params.telemetry.successfulCronAdds = (params.telemetry.successfulCronAdds ?? 0) + 1;
@@ -1180,11 +1185,11 @@ function collectToolTelemetry(params: {
11801185
}
11811186
}
11821187
if (!isMessagingTool(params.toolName)) {
1183-
return;
1188+
return undefined;
11841189
}
11851190
const isMessagingSendAction = isMessagingToolSendAction(params.toolName, params.args);
11861191
if (!isMessagingSendAction && !params.messagingTarget) {
1187-
return;
1192+
return undefined;
11881193
}
11891194
if (
11901195
!isMessagingSendAction &&
@@ -1196,26 +1201,27 @@ function collectToolTelemetry(params: {
11961201
isError: params.isError,
11971202
})
11981203
) {
1199-
return;
1204+
return undefined;
12001205
}
12011206
params.telemetry.didSendViaMessagingTool = true;
12021207
const sourceReplyPayload = extractInternalSourceReplyPayload(params.result?.details);
12031208
if (sourceReplyPayload) {
1204-
params.telemetry.messagingToolSourceReplyPayloads.push({
1209+
const record = {
12051210
...sourceReplyPayload,
12061211
...(params.sourceReplyFinal !== undefined
12071212
? { sourceReplyFinal: params.sourceReplyFinal }
12081213
: {}),
1209-
});
1210-
return;
1214+
};
1215+
params.telemetry.messagingToolSourceReplyPayloads.push(record);
1216+
return record;
12111217
}
12121218
const text = readFirstString(params.args, ["text", "message", "body", "content"]);
12131219
if (text) {
12141220
params.telemetry.messagingToolSentTexts.push(text);
12151221
}
12161222
const mediaUrls = collectMediaUrls(params.args);
12171223
params.telemetry.messagingToolSentMediaUrls.push(...mediaUrls);
1218-
params.telemetry.messagingToolSentTargets.push({
1224+
const record = {
12191225
...(params.messagingTarget ?? {
12201226
tool: params.toolName,
12211227
provider: readFirstString(params.args, ["provider", "channel"]) ?? params.toolName,
@@ -1226,7 +1232,9 @@ function collectToolTelemetry(params: {
12261232
...(text ? { text } : {}),
12271233
...(mediaUrls.length > 0 ? { mediaUrls } : {}),
12281234
...(params.sourceReplyFinal !== undefined ? { sourceReplyFinal: params.sourceReplyFinal } : {}),
1229-
});
1235+
};
1236+
params.telemetry.messagingToolSentTargets.push(record);
1237+
return record;
12301238
}
12311239
function extractInternalSourceReplyPayload(
12321240
details: unknown,

‎extensions/codex/src/app-server/message-tool-final-control.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function addCodexMessageToolOnlyFinalParameter(parameters: unknown): unknown {
4545
final: {
4646
type: "boolean",
4747
description:
48-
"Set true only when this message is intended to complete the reply to the current source conversation. OpenClaw stops after confirming delivery.",
48+
"Set false for progress or true to complete the current source reply. If omitted, OpenClaw continues and resolves the latest omitted source reply when the turn ends.",
4949
},
5050
},
5151
};

‎extensions/codex/src/app-server/run-attempt-finalize.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
} from "./run-attempt-state.js";
3636
import type { prepareCodexAttemptTurnRequest } from "./run-attempt-turn-request.js";
3737
import type { CodexAttemptTurnState } from "./run-attempt-turn-state.js";
38+
import { settleCodexSourceReplyFinality } from "./source-reply-finality.js";
3839
import { normalizeCodexTrajectoryError, recordCodexTrajectoryCompletion } from "./trajectory.js";
3940
import { codexTranscriptMirrorRuntime } from "./transcript-mirror.js";
4041
import {
@@ -270,14 +271,24 @@ export async function finalizeCodexAttempt(
270271
!state.terminalTurnNotificationQueued &&
271272
!state.timedOut &&
272273
clientClosedPromptErrorForFinal === undefined;
273-
const attemptSucceeded =
274+
const turnSucceeded =
274275
!finalAborted &&
275276
!effectiveTimedOut &&
276277
(finalPromptError === null || finalPromptError === undefined) &&
277-
result.agentHarnessResultClassification === undefined &&
278278
(completedTurnStatus === "completed" ||
279279
recoveredTurnWatchTimeout ||
280280
completedWithoutTerminalNotification);
281+
// buildResult retains the bridge's delivery records. Resolve omitted final
282+
// intent only after the authoritative turn outcome is known, before any
283+
// terminal observer consumes the result.
284+
const completedSourceReply = settleCodexSourceReplyFinality(toolBridge.telemetry, turnSucceeded);
285+
if (completedSourceReply) {
286+
// Harness classification only sees assistant/reasoning/plan projections.
287+
// A reply delivered entirely through the source message tool is visible
288+
// output, so an empty/reasoning-only classification is stale at this point.
289+
result.agentHarnessResultClassification = undefined;
290+
}
291+
const attemptSucceeded = turnSucceeded && result.agentHarnessResultClassification === undefined;
281292
terminalState.sharedAbortAllowedAfterTerminalOutcome = shouldKeepCodexSharedAbortOpen({
282293
trigger: params.trigger,
283294
result,

0 commit comments

Comments
 (0)