mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 19:35:28 -06:00
refactor(message): own WebChat termination at sender
Co-authored-by: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com>
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<ReturnType<NonNullable<Agent["afterToolCall"]>>>;
|
||||
@@ -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<ReturnType<NonNullable<Agent["afterToolCall"]>>>;
|
||||
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,
|
||||
|
||||
@@ -20,17 +20,21 @@ function argsRecordForToolCall(context: AfterToolCallContext): Record<string, un
|
||||
: {};
|
||||
}
|
||||
|
||||
/** A completed source reply ends the turn only for message-tool-only delivery
|
||||
* or the internal UI sink; external current-source receipts remain nonterminal. */
|
||||
function isTerminalMessageToolSourceReply(params: {
|
||||
/**
|
||||
* Determines whether a `message.send` tool call delivered a visible source reply
|
||||
* in message-tool-only delivery mode. Only implicit-route, non-dry-run,
|
||||
* delivered sends qualify; explicit routes and errors are not source replies.
|
||||
*/
|
||||
function isDeliveredMessageToolOnlySourceReply(params: {
|
||||
sourceReplyDeliveryMode?: SourceReplyDeliveryMode;
|
||||
context: AfterToolCallContext;
|
||||
hookResult?: AfterToolCallResult;
|
||||
}): boolean {
|
||||
const resultDetails = readToolResultDetails(params.context.result);
|
||||
const deliveryFact = readEmbeddedMessageDeliveryFact(resultDetails?.messageDelivery);
|
||||
const deliveryFact = readEmbeddedMessageDeliveryFact(
|
||||
readToolResultDetails(params.context.result)?.messageDelivery,
|
||||
);
|
||||
const isError = params.hookResult?.isError ?? params.context.isError;
|
||||
const delivered = isDeliveredMessageToolOnlySourceReplyResult({
|
||||
return isDeliveredMessageToolOnlySourceReplyResult({
|
||||
sourceReplyDeliveryMode: params.sourceReplyDeliveryMode,
|
||||
toolName: params.context.toolCall.name,
|
||||
args: argsRecordForToolCall(params.context),
|
||||
@@ -44,25 +48,22 @@ function isTerminalMessageToolSourceReply(params: {
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
return (
|
||||
delivered &&
|
||||
(params.sourceReplyDeliveryMode === "message_tool_only" ||
|
||||
resultDetails?.sourceReplySink === "internal-ui" ||
|
||||
readToolResultDetails(params.hookResult)?.sourceReplySink === "internal-ui")
|
||||
);
|
||||
}
|
||||
|
||||
/** Installs an after-tool hook that records and settles completed source replies. */
|
||||
export function installMessageToolTerminalHook(params: {
|
||||
/** Installs an after-tool hook that records source reply delivery evidence. */
|
||||
export function installMessageToolOnlyTerminalHook(params: {
|
||||
agent: Agent;
|
||||
sourceReplyDeliveryMode?: SourceReplyDeliveryMode;
|
||||
onDeliveredSourceReply?: () => 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,
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<InternalSourceReplyToolDetails> {
|
||||
const action = payload.dryRun ? "Prepared" : "Sent";
|
||||
const sink = payload.sourceReplySink ? ` via ${payload.sourceReplySink}` : "";
|
||||
@@ -316,6 +318,7 @@ function buildInternalSourceReplyToolResult(
|
||||
},
|
||||
],
|
||||
details: payload,
|
||||
...(terminatesTurn ? { terminate: true } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user