fix(codex): preserve legacy termination when final is omitted

Omitted final on a confirmed message-tool-only source reply keeps main's terminate-on-delivery semantics and records a completed marker; only explicit final=false defers termination. Project the Codex-only final schema property in place on attempt-fresh tools instead of adding a public plugin-SDK clone export, resolving the SDK surface budget failure.
This commit is contained in:
Ayaan Zaidi
2026-07-13 09:29:15 +05:30
parent d8a84a29cb
commit 4dc6e182c3
6 changed files with 49 additions and 42 deletions
@@ -1676,15 +1676,18 @@ describe("Codex app-server dynamic tool build", () => {
const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir);
params.disableTools = false;
params.runtimePlan = createCodexRuntimePlanFixture();
const messageTool = {
...createRuntimeDynamicTool("message"),
parameters: {
type: "object",
properties: { message: { type: "string" } },
additionalProperties: false,
// Mirror production createOpenClawCodingTools: attempt-fresh tool instances
// per build, never a shared object reused across delivery modes.
setOpenClawCodingToolsFactoryForTests(() => [
{
...createRuntimeDynamicTool("message"),
parameters: {
type: "object",
properties: { message: { type: "string" } },
additionalProperties: false,
},
},
};
setOpenClawCodingToolsFactoryForTests(() => [messageTool]);
]);
params.sourceReplyDeliveryMode = "message_tool_only";
const sourceReplyTools = await buildDynamicToolsForTest(params, workspaceDir);
@@ -6,7 +6,6 @@
import {
buildAgentHookContextChannelFields,
buildEmbeddedAttemptToolRunContext,
cloneAgentRuntimeToolWithParameters,
embeddedAgentLog,
filterProviderNormalizableTools,
isHostScopedAgentToolActive,
@@ -947,15 +946,17 @@ function addCodexMessageToolOnlyFinalControl(
if (sourceReplyDeliveryMode !== "message_tool_only") {
return tools;
}
return tools.map((tool) => {
// allTools is attempt-fresh from createOpenClawCodingTools inside
// buildDynamicTools — never a shared/cached instance across attempts or
// delivery modes. Project the Codex-only `final` property in place so
// WeakMap ownership metadata stays attached without a public SDK clone helper.
for (const tool of tools) {
if (normalizeCodexDynamicToolName(tool.name) !== "message") {
return tool;
continue;
}
return cloneAgentRuntimeToolWithParameters(
tool,
addCodexMessageToolOnlyFinalParameter(tool.parameters),
);
});
tool.parameters = addCodexMessageToolOnlyFinalParameter(tool.parameters);
}
return tools;
}
function addCodexMessageToolOnlyFinalParameter(parameters: OpenClawDynamicTool["parameters"]) {
@@ -1318,7 +1318,28 @@ describe("createCodexDynamicToolBridge", () => {
]);
});
it("requires final=true before a delivered message-tool-only source reply terminates", async () => {
it("marks delivered message-tool-only source replies as terminal when final is omitted", async () => {
const bridge = createBridgeWithToolResult(
"message",
textToolResult("Sent.", { messageId: "imessage-6264" }),
{ sourceReplyDeliveryMode: "message_tool_only" },
);
const result = await handleMessageToolCall(bridge, {
action: "send",
message: "visible reply",
});
expect(result).toEqual(expectInputText("Sent."));
expect(result.terminate).toBe(true);
expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true);
expect(bridge.telemetry.messagingToolSentTargets.at(-1)).toMatchObject({
sourceReplyFinal: true,
});
expect(Object.keys(result)).not.toContain("terminate");
});
it("requires explicit final=false to keep a delivered message-tool-only source reply non-terminal", async () => {
const bridge = createBridgeWithToolResult(
"message",
textToolResult("Sent.", { messageId: "imessage-6264" }),
@@ -1383,7 +1404,6 @@ describe("createCodexDynamicToolBridge", () => {
const result = await handleMessageToolCall(bridge, {
action: "send",
message: "visible reply",
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1435,7 +1455,6 @@ describe("createCodexDynamicToolBridge", () => {
messageId: "853",
message: "visible reply",
buttons: [],
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1482,7 +1501,6 @@ describe("createCodexDynamicToolBridge", () => {
target: "+1 (206) 910-6512",
messageId: "853",
message: "visible reply",
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1522,7 +1540,6 @@ describe("createCodexDynamicToolBridge", () => {
messageId: "857",
message: "visible reply",
buttons: [],
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1559,7 +1576,6 @@ describe("createCodexDynamicToolBridge", () => {
messageId: "861",
message: "visible reply",
buttons: [],
final: true,
});
expect(result).toEqual(expectInputText(receiptText));
@@ -1640,7 +1656,6 @@ describe("createCodexDynamicToolBridge", () => {
messageId: "863",
message: "visible reply",
buttons: [],
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1662,7 +1677,6 @@ describe("createCodexDynamicToolBridge", () => {
messageId: "865",
message: "visible reply",
buttons: [],
final: true,
});
expect(result).toEqual(expectInputText("Sent."));
@@ -1756,7 +1770,6 @@ describe("createCodexDynamicToolBridge", () => {
const firstResult = await handleMessageToolCall(bridge, {
action: "send",
message: "visible reply",
final: true,
});
const secondResult = await bridge.handleToolCall({
threadId: "thread-1",
@@ -689,16 +689,17 @@ export function createCodexDynamicToolBridge(params: {
toolName === "message" &&
!resultIsError &&
(rawResult.terminate === true || result.terminate === true);
const explicitFinalSourceReply =
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
toolName === "message" &&
executedArgs.final === true;
const hasExplicitFinalControl = typeof executedArgs.final === "boolean";
// Omitted final on a confirmed source reply must degrade to legacy
// terminate-on-delivery (completed marker), never progress; otherwise
// stranded-reply recovery re-delivers a duplicate of that reply.
const sourceReplyFinal =
params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" &&
toolName === "message" &&
(toolConfirmedSourceReply || deliveredSourceReply || receiptConfirmedSourceReply)
? explicitFinalSourceReply || (toolConfirmedSourceReply && !hasExplicitFinalControl)
? hasExplicitFinalControl
? executedArgs.final === true
: true
: undefined;
collectToolTelemetry({
toolName,
@@ -723,7 +724,7 @@ export function createCodexDynamicToolBridge(params: {
)) ||
isToolResultYield(rawResult) ||
isToolResultYield(result) ||
(explicitFinalSourceReply && (deliveredSourceReply || receiptConfirmedSourceReply)),
((deliveredSourceReply || receiptConfirmedSourceReply) && executedArgs.final !== false),
);
const asyncStarted =
isAsyncStartedToolResult(rawResult) || isAsyncStartedToolResult(result);
-10
View File
@@ -74,16 +74,6 @@ function copyRuntimeToolMetadata(source: AgentTool, target: AgentTool): void {
copyToolTerminalPresentation(source as never, target as never);
}
/** Clone a runtime tool with a projected schema while preserving WeakMap-backed ownership data. */
export function cloneAgentRuntimeToolWithParameters<TTool extends AgentTool>(
source: TTool,
parameters: TTool["parameters"],
): TTool {
const target = { ...source, parameters } as TTool;
copyRuntimeToolMetadata(source, target);
return target;
}
// Duplicate names cannot be matched by map lookup alone, so same-index matches
// take precedence and unique-name fallback covers cloned arrays.
function preserveRuntimeToolMetadata<TSchemaType extends TSchema = TSchema, TResult = unknown>(
-1
View File
@@ -220,7 +220,6 @@ export function queueAgentHarnessMessage(
}
export { disposeRegisteredAgentHarnesses } from "../agents/harness/registry.js";
export {
cloneAgentRuntimeToolWithParameters,
logAgentRuntimeToolDiagnostics,
normalizeAgentRuntimeTools,
} from "../agents/runtime-plan/tools.js";