Skip to content

Commit 53ffd14

Browse files
GlucksbergGlucksberg
committed
fix(codex): require explicit final source delivery
Distinguish progress from terminal source delivery across Codex telemetry, payload suppression, terminal failure routing, and stranded-reply recovery. Preserve legacy behavior when explicit markers are absent.
1 parent 1beb0ea commit 53ffd14

24 files changed

Lines changed: 563 additions & 61 deletions

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,42 @@ describe("Codex app-server dynamic tool build", () => {
16711671
expect(shouldForceMessageTool(params)).toBe(false);
16721672
});
16731673

1674+
it("exposes the final delivery control only on Codex message-tool-only schemas", async () => {
1675+
const workspaceDir = path.join(tempDir, "workspace");
1676+
const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir);
1677+
params.disableTools = false;
1678+
params.runtimePlan = createCodexRuntimePlanFixture();
1679+
const messageTool = {
1680+
...createRuntimeDynamicTool("message"),
1681+
parameters: {
1682+
type: "object",
1683+
properties: { message: { type: "string" } },
1684+
additionalProperties: false,
1685+
},
1686+
};
1687+
setOpenClawCodingToolsFactoryForTests(() => [messageTool]);
1688+
1689+
params.sourceReplyDeliveryMode = "message_tool_only";
1690+
const sourceReplyTools = await buildDynamicToolsForTest(params, workspaceDir);
1691+
const sourceReplySchema = sourceReplyTools[0]?.parameters as {
1692+
properties?: Record<string, unknown>;
1693+
additionalProperties?: unknown;
1694+
};
1695+
1696+
expect(sourceReplySchema.properties).toMatchObject({
1697+
final: { type: "boolean" },
1698+
});
1699+
expect(sourceReplySchema.additionalProperties).toBe(false);
1700+
1701+
params.sourceReplyDeliveryMode = "automatic";
1702+
const automaticTools = await buildDynamicToolsForTest(params, workspaceDir);
1703+
const automaticSchema = automaticTools[0]?.parameters as {
1704+
properties?: Record<string, unknown>;
1705+
};
1706+
1707+
expect(automaticSchema.properties).not.toHaveProperty("final");
1708+
});
1709+
16741710
it("retains forced message policy for the registered schema override", () => {
16751711
const workspaceDir = path.join(tempDir, "workspace");
16761712
const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir);

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import {
77
buildAgentHookContextChannelFields,
88
buildEmbeddedAttemptToolRunContext,
9+
cloneAgentRuntimeToolWithParameters,
910
embeddedAgentLog,
1011
filterProviderNormalizableTools,
1112
isHostScopedAgentToolActive,
@@ -347,9 +348,13 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
347348
onToolOutcome: params.onToolOutcome,
348349
allocateToolOutcomeOrdinal: params.allocateToolOutcomeOrdinal,
349350
});
351+
const codexScopedTools = addCodexMessageToolOnlyFinalControl(
352+
allTools,
353+
params.sourceReplyDeliveryMode,
354+
);
350355
toolBuildStages.mark("create-openclaw-coding-tools");
351356
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [];
352-
const readableAllToolProjection = filterProviderNormalizableTools(allTools);
357+
const readableAllToolProjection = filterProviderNormalizableTools(codexScopedTools);
353358
preNormalizationDiagnostics.push(...readableAllToolProjection.diagnostics);
354359
const webSearchPlan = resolveCodexWebSearchPlan({
355360
config: params.config,
@@ -930,6 +935,51 @@ function hideNodeExecDynamicToolParameters(
930935
};
931936
}
932937

938+
/**
939+
* `final` is a Codex-only control for message-tool-only source delivery. Keep
940+
* it on the projected Codex schema so other agent runtimes never receive an
941+
* API contract they do not implement.
942+
*/
943+
function addCodexMessageToolOnlyFinalControl(
944+
tools: OpenClawDynamicTool[],
945+
sourceReplyDeliveryMode: EmbeddedRunAttemptParams["sourceReplyDeliveryMode"],
946+
): OpenClawDynamicTool[] {
947+
if (sourceReplyDeliveryMode !== "message_tool_only") {
948+
return tools;
949+
}
950+
return tools.map((tool) => {
951+
if (normalizeCodexDynamicToolName(tool.name) !== "message") {
952+
return tool;
953+
}
954+
return cloneAgentRuntimeToolWithParameters(
955+
tool,
956+
addCodexMessageToolOnlyFinalParameter(tool.parameters),
957+
);
958+
});
959+
}
960+
961+
function addCodexMessageToolOnlyFinalParameter(parameters: OpenClawDynamicTool["parameters"]) {
962+
if (!parameters || typeof parameters !== "object" || Array.isArray(parameters)) {
963+
return parameters;
964+
}
965+
const schema = parameters as Record<string, unknown>;
966+
const rawProperties = schema.properties;
967+
if (!rawProperties || typeof rawProperties !== "object" || Array.isArray(rawProperties)) {
968+
return parameters;
969+
}
970+
return {
971+
...schema,
972+
properties: {
973+
...rawProperties,
974+
final: {
975+
type: "boolean",
976+
description:
977+
"Set true only when this message is intended to complete the reply to the current source conversation. OpenClaw stops after confirming delivery.",
978+
},
979+
},
980+
};
981+
}
982+
933983
function resolveCodexNativeExecutionPolicyForDynamicTools(
934984
input: DynamicToolBuildParams,
935985
): CodexNativeExecutionPolicy {

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1318,21 +1318,38 @@ describe("createCodexDynamicToolBridge", () => {
13181318
]);
13191319
});
13201320

1321-
it("marks delivered message-tool-only source replies as terminal", async () => {
1321+
it("requires final=true before a delivered message-tool-only source reply terminates", async () => {
13221322
const bridge = createBridgeWithToolResult(
13231323
"message",
13241324
textToolResult("Sent.", { messageId: "imessage-6264" }),
13251325
{ sourceReplyDeliveryMode: "message_tool_only" },
13261326
);
13271327

1328+
const progressResult = await handleMessageToolCall(bridge, {
1329+
action: "send",
1330+
message: "visible reply",
1331+
final: false,
1332+
});
1333+
1334+
expect(progressResult).toEqual(expectInputText("Sent."));
1335+
expect(progressResult.terminate).toBeUndefined();
1336+
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1337+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1338+
sourceReplyFinal: false,
1339+
});
1340+
13281341
const result = await handleMessageToolCall(bridge, {
13291342
action: "send",
13301343
message: "visible reply",
1344+
final: true,
13311345
});
13321346

13331347
expect(result).toEqual(expectInputText("Sent."));
13341348
expect(result.terminate).toBe(true);
13351349
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1350+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1351+
sourceReplyFinal: true,
1352+
});
13361353
expect(Object.keys(result)).not.toContain("terminate");
13371354
});
13381355

@@ -1366,6 +1383,7 @@ describe("createCodexDynamicToolBridge", () => {
13661383
const result = await handleMessageToolCall(bridge, {
13671384
action: "send",
13681385
message: "visible reply",
1386+
final: true,
13691387
});
13701388

13711389
expect(result).toEqual(expectInputText("Sent."));
@@ -1417,11 +1435,15 @@ describe("createCodexDynamicToolBridge", () => {
14171435
messageId: "853",
14181436
message: "visible reply",
14191437
buttons: [],
1438+
final: true,
14201439
});
14211440

14221441
expect(result).toEqual(expectInputText("Sent."));
14231442
expect(result.terminate).toBe(true);
14241443
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1444+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1445+
sourceReplyFinal: true,
1446+
});
14251447
expect(Object.keys(result)).not.toContain("terminate");
14261448
});
14271449

@@ -1460,6 +1482,7 @@ describe("createCodexDynamicToolBridge", () => {
14601482
target: "+1 (206) 910-6512",
14611483
messageId: "853",
14621484
message: "visible reply",
1485+
final: true,
14631486
});
14641487

14651488
expect(result).toEqual(expectInputText("Sent."));
@@ -1499,6 +1522,7 @@ describe("createCodexDynamicToolBridge", () => {
14991522
messageId: "857",
15001523
message: "visible reply",
15011524
buttons: [],
1525+
final: true,
15021526
});
15031527

15041528
expect(result).toEqual(expectInputText("Sent."));
@@ -1535,6 +1559,7 @@ describe("createCodexDynamicToolBridge", () => {
15351559
messageId: "861",
15361560
message: "visible reply",
15371561
buttons: [],
1562+
final: true,
15381563
});
15391564

15401565
expect(result).toEqual(expectInputText(receiptText));
@@ -1615,6 +1640,7 @@ describe("createCodexDynamicToolBridge", () => {
16151640
messageId: "863",
16161641
message: "visible reply",
16171642
buttons: [],
1643+
final: true,
16181644
});
16191645

16201646
expect(result).toEqual(expectInputText("Sent."));
@@ -1636,6 +1662,7 @@ describe("createCodexDynamicToolBridge", () => {
16361662
messageId: "865",
16371663
message: "visible reply",
16381664
buttons: [],
1665+
final: true,
16391666
});
16401667

16411668
expect(result).toEqual(expectInputText("Sent."));
@@ -1666,9 +1693,39 @@ describe("createCodexDynamicToolBridge", () => {
16661693
expect(result).toEqual(expectInputText("Sent."));
16671694
expect(result.terminate).toBe(true);
16681695
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
1696+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1697+
sourceReplyFinal: true,
1698+
});
16691699
expect(Object.keys(result)).not.toContain("terminate");
16701700
});
16711701

1702+
it("lets explicit progress override legacy message-tool-owned termination", async () => {
1703+
const bridge = createBridgeWithToolResult(
1704+
"message",
1705+
{
1706+
...textToolResult("Sent.", { ok: true }),
1707+
terminate: true,
1708+
} as AgentToolResult<unknown>,
1709+
{ sourceReplyDeliveryMode: "message_tool_only" },
1710+
);
1711+
1712+
const result = await handleMessageToolCall(bridge, {
1713+
action: "reply",
1714+
channel: "imessage",
1715+
target: "+12069106512",
1716+
messageId: "868",
1717+
message: "Still working.",
1718+
buttons: [],
1719+
final: false,
1720+
});
1721+
1722+
expect(result).toEqual(expectInputText("Sent."));
1723+
expect(result.terminate).toBeUndefined();
1724+
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
1725+
sourceReplyFinal: false,
1726+
});
1727+
});
1728+
16721729
it("does not treat bare send telemetry as delivered message-tool-only source reply evidence", async () => {
16731730
const bridge = createBridgeWithToolResult("message", textToolResult("Sent."), {
16741731
sourceReplyDeliveryMode: "message_tool_only",
@@ -1699,6 +1756,7 @@ describe("createCodexDynamicToolBridge", () => {
16991756
const firstResult = await handleMessageToolCall(bridge, {
17001757
action: "send",
17011758
message: "visible reply",
1759+
final: true,
17021760
});
17031761
const secondResult = await bridge.handleToolCall({
17041762
threadId: "thread-1",
@@ -1726,6 +1784,7 @@ describe("createCodexDynamicToolBridge", () => {
17261784
action: "send",
17271785
target: "channel:other",
17281786
message: "cross-channel reply",
1787+
final: true,
17291788
});
17301789

17311790
expect(result).toEqual(expectInputText("Sent."));

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

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -629,15 +629,6 @@ export function createCodexDynamicToolBridge(params: {
629629
!rawIsError && messagingTarget
630630
? extractMessagingToolSendResult(messagingTarget, telemetryRawResult)
631631
: messagingTarget;
632-
collectToolTelemetry({
633-
toolName,
634-
args: executedArgs,
635-
result,
636-
mediaTrustResult: telemetryRawResult,
637-
telemetry,
638-
isError: resultIsError,
639-
messagingTarget: confirmedMessagingTarget,
640-
});
641632
const terminalType =
642633
resultFailureKind === "blocked" ? "blocked" : resultIsError ? "error" : "completed";
643634
const contentItems = convertToolContents(result.content, toolResultMaxChars);
@@ -698,17 +689,41 @@ export function createCodexDynamicToolBridge(params: {
698689
toolName === "message" &&
699690
!resultIsError &&
700691
(rawResult.terminate === true || result.terminate === true);
692+
const explicitFinalSourceReply =
693+
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
694+
toolName === "message" &&
695+
executedArgs.final === true;
696+
const hasExplicitFinalControl = typeof executedArgs.final === "boolean";
697+
const sourceReplyFinal =
698+
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
699+
toolName === "message" &&
700+
(toolConfirmedSourceReply || deliveredSourceReply || receiptConfirmedSourceReply)
701+
? explicitFinalSourceReply || (toolConfirmedSourceReply && !hasExplicitFinalControl)
702+
: undefined;
703+
collectToolTelemetry({
704+
toolName,
705+
args: executedArgs,
706+
result,
707+
mediaTrustResult: telemetryRawResult,
708+
telemetry,
709+
isError: resultIsError,
710+
messagingTarget: confirmedMessagingTarget,
711+
sourceReplyFinal,
712+
});
701713
if (deliveredSourceReply || receiptConfirmedSourceReply || toolConfirmedSourceReply) {
702714
telemetry.didDeliverSourceReplyViaMessageTool = true;
703715
}
704716
withDynamicToolTermination(
705717
response,
706-
rawResult.terminate === true ||
707-
result.terminate === true ||
718+
((rawResult.terminate === true || result.terminate === true) &&
719+
!(
720+
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
721+
toolName === "message" &&
722+
executedArgs.final === false
723+
)) ||
708724
isToolResultYield(rawResult) ||
709725
isToolResultYield(result) ||
710-
deliveredSourceReply ||
711-
receiptConfirmedSourceReply,
726+
(explicitFinalSourceReply && (deliveredSourceReply || receiptConfirmedSourceReply)),
712727
);
713728
const asyncStarted =
714729
isAsyncStartedToolResult(rawResult) || isAsyncStartedToolResult(result);
@@ -1153,6 +1168,7 @@ function collectToolTelemetry(params: {
11531168
telemetry: CodexDynamicToolBridge["telemetry"];
11541169
isError: boolean;
11551170
messagingTarget?: MessagingToolSend;
1171+
sourceReplyFinal?: boolean;
11561172
}): void {
11571173
if (params.isError) {
11581174
return;
@@ -1208,7 +1224,12 @@ function collectToolTelemetry(params: {
12081224
params.telemetry.didSendViaMessagingTool = true;
12091225
const sourceReplyPayload = extractInternalSourceReplyPayload(params.result?.details);
12101226
if (sourceReplyPayload) {
1211-
params.telemetry.messagingToolSourceReplyPayloads.push(sourceReplyPayload);
1227+
params.telemetry.messagingToolSourceReplyPayloads.push({
1228+
...sourceReplyPayload,
1229+
...(params.sourceReplyFinal !== undefined
1230+
? { sourceReplyFinal: params.sourceReplyFinal }
1231+
: {}),
1232+
});
12121233
return;
12131234
}
12141235
const text = readFirstString(params.args, ["text", "message", "body", "content"]);
@@ -1227,6 +1248,7 @@ function collectToolTelemetry(params: {
12271248
}),
12281249
...(text ? { text } : {}),
12291250
...(mediaUrls.length > 0 ? { mediaUrls } : {}),
1251+
...(params.sourceReplyFinal !== undefined ? { sourceReplyFinal: params.sourceReplyFinal } : {}),
12301252
});
12311253
}
12321254

‎extensions/codex/src/app-server/thread-lifecycle.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,25 @@ describe("Codex app-server native code mode config", () => {
532532
expect(instructions).not.toContain("Deferred searchable OpenClaw dynamic tools available");
533533
});
534534

535+
it("instructs Codex to mark only completed message-tool-only source replies final", () => {
536+
const params = createAttemptParams({ provider: "openai" });
537+
params.sourceReplyDeliveryMode = "message_tool_only";
538+
539+
const instructions = buildDeveloperInstructions(params, {
540+
dynamicTools: [
541+
{
542+
type: "function",
543+
name: "message",
544+
description: "Send a message",
545+
inputSchema: { type: "object" },
546+
},
547+
],
548+
});
549+
550+
expect(instructions).toContain("For progress, set `final=false`.");
551+
expect(instructions).toContain("set `final=true`");
552+
});
553+
535554
it("keeps durable dynamic tool fingerprints scoped to loading mode", () => {
536555
const inputSchema = {
537556
type: "object",

0 commit comments

Comments
 (0)