diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts index c59f530b0caa..1e319e380794 100644 --- a/extensions/codex/src/app-server/dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/dynamic-tools.test.ts @@ -2312,12 +2312,6 @@ describe("createCodexDynamicToolBridge", () => { status: "ok", deliveryStatus: "sent", sourceReplySink: "internal-ui", - sourceReplyRoute: "current-source", - messageDelivery: { - status: "settled", - partialDelivery: false, - createdThreadIds: [], - }, sourceReply: { text: "visible reply", mediaUrls: ["/tmp/reply.png"], @@ -2331,9 +2325,7 @@ describe("createCodexDynamicToolBridge", () => { }); expect(result).toEqual(expectInputText("Sent to current chat.")); - expect(result.terminate).toBe(true); expect(bridge.telemetry.didSendViaMessagingTool).toBe(true); - expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true); expect(bridge.telemetry.messagingToolSentTexts).toEqual([]); expect(bridge.telemetry.messagingToolSentMediaUrls).toEqual([]); expect(bridge.telemetry.messagingToolSentTargets).toEqual([]); @@ -2342,64 +2334,10 @@ describe("createCodexDynamicToolBridge", () => { text: "visible reply", mediaUrl: "/tmp/reply.png", mediaUrls: ["/tmp/reply.png"], - sourceReplyFinal: true, }, ]); }); - it("keeps explicit internal UI progress nonterminal", async () => { - const toolResult = textToolResult("Sent to current chat.", { - status: "ok", - deliveryStatus: "sent", - sourceReplySink: "internal-ui", - sourceReplyRoute: "current-source", - messageDelivery: { - status: "settled", - partialDelivery: false, - createdThreadIds: [], - }, - sourceReply: { text: "still working" }, - }); - const bridge = createBridgeWithToolResult("message", toolResult); - - const result = await handleMessageToolCall(bridge, { - action: "send", - message: "still working", - final: false, - }); - - expect(result.terminate).toBeUndefined(); - expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true); - expect(bridge.telemetry.messagingToolSourceReplyPayloads).toEqual([ - { text: "still working", sourceReplyFinal: false }, - ]); - }); - - it("keeps automatic external current-source replies nonterminal", async () => { - const bridge = createBridgeWithToolResult( - "message", - textToolResult("Sent.", { - ok: true, - sourceReplyRoute: "current-source", - messageDelivery: { - status: "settled", - partialDelivery: false, - createdThreadIds: [], - }, - }), - { sourceReplyDeliveryMode: "automatic" }, - ); - - const result = await handleMessageToolCall(bridge, { - action: "thread-reply", - threadId: "thread-1", - message: "first update", - }); - - expect(result.terminate).toBeUndefined(); - expect(bridge.telemetry.didDeliverSourceReplyViaMessageTool).toBe(true); - }); - it("treats omitted source-reply finality as terminal", async () => { const bridge = createBridgeWithToolResult( "message", diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts index 75d3cdcfbff4..2f257aebf142 100644 --- a/extensions/codex/src/app-server/dynamic-tools.ts +++ b/extensions/codex/src/app-server/dynamic-tools.ts @@ -867,15 +867,9 @@ export function createCodexDynamicToolBridge(params: { !resultIsError && (rawResult.terminate === true || result.terminate === true); const confirmedSourceReply = + params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" && toolName === "message" && - // Internal UI receipts are current-source facts even when the outer run - // otherwise uses automatic final replies; completed sends end this turn. - ((deliveredSourceReply && - (params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" || - asOptionalRecord(rawResult.details)?.sourceReplySink === "internal-ui" || - asOptionalRecord(result.details)?.sourceReplySink === "internal-ui")) || - (params.hookContext?.sourceReplyDeliveryMode === "message_tool_only" && - (toolConfirmedSourceReply || receiptConfirmedSourceReply))); + (toolConfirmedSourceReply || deliveredSourceReply || receiptConfirmedSourceReply); const sourceReplyFinal = confirmedSourceReply ? executedArgs.final !== false : undefined; collectToolTelemetry({ toolName, diff --git a/src/agents/embedded-agent-runner/run/attempt-session-prepare.ts b/src/agents/embedded-agent-runner/run/attempt-session-prepare.ts index b6aeecfc1b56..c05204d6d8bd 100644 --- a/src/agents/embedded-agent-runner/run/attempt-session-prepare.ts +++ b/src/agents/embedded-agent-runner/run/attempt-session-prepare.ts @@ -50,7 +50,7 @@ import { resolveExistingAttemptTranscriptState } from "./attempt-transcript-help import type { EmbeddedAttemptTranscriptLifecycle } from "./attempt-transcript-lifecycle.js"; import { createUserTranscriptContextRegistry } from "./attempt-user-transcript-context-registry.js"; import { installCodeModeRepairHook } from "./code-mode-repair.js"; -import { installMessageToolTerminalHook } from "./message-tool-terminal.js"; +import { installMessageToolOnlyTerminalHook } from "./message-tool-terminal.js"; import { reconcilePrePersistedCurrentUserTurn } from "./pre-persisted-user-turn.js"; import { resolveSessionBoundaryPromptCacheKey } from "./session-boundary-prompt-cache-key.js"; import { notifyToolActivity } from "./tool-activity-heartbeat.js"; @@ -225,7 +225,7 @@ export async function prepareEmbeddedAttemptAgentSession(input: { const markSourceReplyDelivered = () => { didDeliverSourceReplyViaMessageTool = true; }; - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent: activeSession.agent, sourceReplyDeliveryMode: attempt.sourceReplyDeliveryMode, onDeliveredSourceReply: markSourceReplyDelivered, diff --git a/src/agents/embedded-agent-runner/run/attempt-session.test.ts b/src/agents/embedded-agent-runner/run/attempt-session.test.ts index f6db33f0f214..3c04b5a66e0f 100644 --- a/src/agents/embedded-agent-runner/run/attempt-session.test.ts +++ b/src/agents/embedded-agent-runner/run/attempt-session.test.ts @@ -12,7 +12,7 @@ const hoisted = vi.hoisted(() => ({ createPreparedEmbeddedAgentSettingsManager: vi.fn(), getGlobalHookRunner: vi.fn(), installCodeModeRepairHook: vi.fn(), - installMessageToolTerminalHook: vi.fn(), + installMessageToolOnlyTerminalHook: vi.fn(), prepareEmbeddedAttemptClientTools: vi.fn(), resolveEffectiveCompactionMode: vi.fn(), isSilentOverflowProneModel: vi.fn(), @@ -63,7 +63,7 @@ vi.mock("./code-mode-repair.js", () => ({ installCodeModeRepairHook: hoisted.installCodeModeRepairHook, })); vi.mock("./message-tool-terminal.js", () => ({ - installMessageToolTerminalHook: hoisted.installMessageToolTerminalHook, + installMessageToolOnlyTerminalHook: hoisted.installMessageToolOnlyTerminalHook, })); vi.mock("./tool-activity-heartbeat.js", () => ({ notifyToolActivity: hoisted.notifyToolActivity, @@ -143,7 +143,7 @@ function createInput(options?: { hoisted.applySystemPromptToSession.mockImplementation(() => { events.push("apply-system-prompt"); }); - hoisted.installMessageToolTerminalHook.mockImplementation( + hoisted.installMessageToolOnlyTerminalHook.mockImplementation( (input: { onDeliveredSourceReply?: () => void }) => { events.push("install-terminal-hook"); onDeliveredSourceReply = input.onDeliveredSourceReply; diff --git a/src/agents/embedded-agent-runner/run/message-tool-terminal.test.ts b/src/agents/embedded-agent-runner/run/message-tool-terminal.test.ts index 98ad0ac4fab3..9cb4f9eae7ab 100644 --- a/src/agents/embedded-agent-runner/run/message-tool-terminal.test.ts +++ b/src/agents/embedded-agent-runner/run/message-tool-terminal.test.ts @@ -1,12 +1,13 @@ -// Message-tool terminal tests cover completed current-source replies across -// explicit message-tool-only and internal UI delivery modes. +// Message-tool delivery tests cover message_tool_only delivery, where a +// successful source message send records source reply evidence without ending +// the run before the model can observe the tool result. import type { Agent, AfterToolCallContext } from "openclaw/plugin-sdk/agent-core"; import { describe, expect, it, vi } from "vitest"; -import { installMessageToolTerminalHook } from "./message-tool-terminal.js"; +import { installMessageToolOnlyTerminalHook } from "./message-tool-terminal.js"; async function recordsDeliveredSourceReply(params: { sourceReplyDeliveryMode?: Parameters< - typeof installMessageToolTerminalHook + typeof installMessageToolOnlyTerminalHook >[0]["sourceReplyDeliveryMode"]; context: AfterToolCallContext; hookResult?: Awaited>>; @@ -15,7 +16,7 @@ async function recordsDeliveredSourceReply(params: { ? { afterToolCall: vi.fn(async () => params.hookResult) } : {}) as unknown as Agent; const onDeliveredSourceReply = vi.fn(); - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, onDeliveredSourceReply, @@ -27,14 +28,14 @@ async function recordsDeliveredSourceReply(params: { type TerminalHookCase = { label: string; sourceReplyDeliveryMode?: Parameters< - typeof installMessageToolTerminalHook + typeof installMessageToolOnlyTerminalHook >[0]["sourceReplyDeliveryMode"]; context: AfterToolCallContext; hookResult?: Awaited>>; expected: boolean; }; -describe("message-tool source replies", () => { +describe("message-tool-only source replies", () => { it.each([ { label: "implicit successful send", @@ -76,22 +77,12 @@ describe("message-tool source replies", () => { expected: true, }, { - label: "automatic internal source reply", + label: "automatic delivery mode", sourceReplyDeliveryMode: "automatic", context: createAfterToolCallContext({ toolName: "message", args: { action: "send", message: "visible reply" }, }), - expected: true, - }, - { - label: "automatic external send", - sourceReplyDeliveryMode: "automatic", - context: createAfterToolCallContext({ - toolName: "message", - args: { action: "send", message: "visible reply" }, - result: createDirectSendResult({ messageId: "discord-message-automatic" }), - }), expected: false, }, { @@ -188,7 +179,7 @@ describe("message-tool source replies", () => { })); const agent = { afterToolCall: previousAfterToolCall } as unknown as Agent; const onDeliveredSourceReply = vi.fn(); - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent, sourceReplyDeliveryMode: "message_tool_only", onDeliveredSourceReply, @@ -213,7 +204,7 @@ describe("message-tool source replies", () => { it("terminates after a delivered completed source reply", async () => { const agent = {} as unknown as Agent; const onDeliveredSourceReply = vi.fn(); - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent, sourceReplyDeliveryMode: "message_tool_only", onDeliveredSourceReply, @@ -232,7 +223,7 @@ describe("message-tool source replies", () => { it("continues after delivered progress", async () => { const agent = {} as unknown as Agent; - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent, sourceReplyDeliveryMode: "message_tool_only", }); @@ -247,67 +238,6 @@ describe("message-tool source replies", () => { ).resolves.toBeUndefined(); }); - it("terminates automatic internal source replies but preserves explicit progress", async () => { - const agent = {} as unknown as Agent; - installMessageToolTerminalHook({ - agent, - sourceReplyDeliveryMode: "automatic", - }); - - await expect( - agent.afterToolCall?.( - createAfterToolCallContext({ - toolName: "message", - args: { action: "send", message: "visible reply" }, - }), - ), - ).resolves.toEqual({ terminate: true }); - await expect( - agent.afterToolCall?.( - createAfterToolCallContext({ - toolName: "message", - args: { action: "send", message: "still working", final: false }, - }), - ), - ).resolves.toBeUndefined(); - }); - - it("keeps automatic external current-source replies nonterminal", async () => { - const agent = {} as unknown as Agent; - const onDeliveredSourceReply = vi.fn(); - installMessageToolTerminalHook({ - agent, - sourceReplyDeliveryMode: "automatic", - onDeliveredSourceReply, - }); - - await expect( - agent.afterToolCall?.( - createAfterToolCallContext({ - toolName: "message", - args: { - action: "thread-reply", - threadId: "thread-1", - message: "first update", - }, - result: { - content: [{ type: "text", text: '{"ok":true}' }], - details: { - ok: true, - sourceReplyRoute: "current-source", - messageDelivery: { - status: "settled", - partialDelivery: false, - createdThreadIds: [], - }, - }, - }, - }), - ), - ).resolves.toBeUndefined(); - expect(onDeliveredSourceReply).not.toHaveBeenCalled(); - }); - it("leaves existing after-tool-call output alone when the send failed", async () => { const previousAfterToolCall = vi.fn(async () => ({ content: [{ type: "text" as const, text: "failed" }], @@ -316,7 +246,7 @@ describe("message-tool source replies", () => { })); const agent = { afterToolCall: previousAfterToolCall } as unknown as Agent; const onDeliveredSourceReply = vi.fn(); - installMessageToolTerminalHook({ + installMessageToolOnlyTerminalHook({ agent, sourceReplyDeliveryMode: "message_tool_only", onDeliveredSourceReply, @@ -337,6 +267,19 @@ describe("message-tool source replies", () => { expect(previousAfterToolCall).toHaveBeenCalledTimes(1); expect(onDeliveredSourceReply).not.toHaveBeenCalled(); }); + + it("does not install a wrapper for non-message-tool-only delivery", async () => { + const previousAfterToolCall = vi.fn(async () => ({ + details: { untouched: true }, + })); + const agent = { afterToolCall: previousAfterToolCall } as unknown as Agent; + installMessageToolOnlyTerminalHook({ + agent, + sourceReplyDeliveryMode: "automatic", + }); + + expect(agent.afterToolCall).toBe(previousAfterToolCall); + }); }); function createAfterToolCallContext(params: { @@ -345,9 +288,6 @@ function createAfterToolCallContext(params: { isError?: boolean; result?: AfterToolCallContext["result"]; }): AfterToolCallContext { - const hasExplicitRoute = ["channel", "target", "to", "channelId", "provider"].some( - (key) => typeof params.args[key] === "string" && params.args[key].trim().length > 0, - ); return { assistantMessage: createToolCallAssistant(params.toolName, params.args), toolCall: { @@ -367,7 +307,6 @@ function createAfterToolCallContext(params: { details: { status: "ok", deliveryStatus: "sent", - ...(hasExplicitRoute ? {} : { sourceReplyRoute: "current-source" }), messageDelivery: { status: params.args.dryRun ? "dryRun" : params.isError ? "failed" : "settled", partialDelivery: false, diff --git a/src/agents/embedded-agent-runner/run/message-tool-terminal.ts b/src/agents/embedded-agent-runner/run/message-tool-terminal.ts index 8ade955d8b0c..daf59946066e 100644 --- a/src/agents/embedded-agent-runner/run/message-tool-terminal.ts +++ b/src/agents/embedded-agent-runner/run/message-tool-terminal.ts @@ -20,17 +20,21 @@ function argsRecordForToolCall(context: AfterToolCallContext): Record void; }): void { + if (params.sourceReplyDeliveryMode !== "message_tool_only") { + return; + } const previousAfterToolCall = params.agent.afterToolCall?.bind(params.agent); params.agent.afterToolCall = async (context, signal) => { const hookResult = await previousAfterToolCall?.(context, signal); if ( - isTerminalMessageToolSourceReply({ + isDeliveredMessageToolOnlySourceReply({ sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, context, hookResult, diff --git a/src/agents/tools/message-tool.internal-source-reply.integration.test.ts b/src/agents/tools/message-tool.internal-source-reply.integration.test.ts index 4a901dd2364f..c642016bf10b 100644 --- a/src/agents/tools/message-tool.internal-source-reply.integration.test.ts +++ b/src/agents/tools/message-tool.internal-source-reply.integration.test.ts @@ -29,6 +29,7 @@ describe("WebChat message tool internal source reply", () => { message: "Visible progress from the message tool.", final: false, }); + expect(toolResult.terminate).toBeUndefined(); expect(toolResult.details).toMatchObject({ channel: "webchat", target: "current-run", diff --git a/src/infra/outbound/message-action-runner.send-validation.test.ts b/src/infra/outbound/message-action-runner.send-validation.test.ts index b27d8e57c21a..efdc5b8755d2 100644 --- a/src/infra/outbound/message-action-runner.send-validation.test.ts +++ b/src/infra/outbound/message-action-runner.send-validation.test.ts @@ -46,6 +46,7 @@ describe("runMessageAction send validation", () => { }, sessionKey: "agent:main", sourceReplyDeliveryMode: "message_tool_only", + sourceReplyFinal: true, }); expect(result).toMatchObject({ @@ -72,6 +73,7 @@ describe("runMessageAction send validation", () => { text: "Sent visible reply to the current source conversation via internal-ui.", }, ]); + expect(result.toolResult?.terminate).toBe(true); expect(result.toolResult?.details).toEqual({ status: "ok", deliveryStatus: "sent", diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index 88c250dca7a6..bb0fe5562ed4 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -268,6 +268,7 @@ async function handleInternalSourceReplySendAction( ...(sourceReply.mediaUrls?.length ? { mediaUrls: sourceReply.mediaUrls } : {}), dryRun, }; + const terminatesTurn = input.sourceReplyFinal === true && !dryRun; return withSendNormalization( { kind: "send", @@ -276,7 +277,7 @@ async function handleInternalSourceReplySendAction( to: "current-run", handledBy: "internal-source", payload, - toolResult: buildInternalSourceReplyToolResult(payload), + toolResult: buildInternalSourceReplyToolResult(payload, terminatesTurn), dryRun, }, sourceReply.normalization, @@ -305,6 +306,7 @@ type InternalSourceReplyToolDetails = { function buildInternalSourceReplyToolResult( payload: InternalSourceReplyToolDetails, + terminatesTurn: boolean, ): AgentToolResult { const action = payload.dryRun ? "Prepared" : "Sent"; const sink = payload.sourceReplySink ? ` via ${payload.sourceReplySink}` : ""; @@ -316,6 +318,7 @@ function buildInternalSourceReplyToolResult( }, ], details: payload, + ...(terminatesTurn ? { terminate: true } : {}), }; }