From cd061a4c7b403c908aa43d8da4e88970af59a6f7 Mon Sep 17 00:00:00 2001 From: zerone0x Date: Sat, 20 Jun 2026 05:02:33 +0800 Subject: [PATCH] fix(agents): preserve delivered message send results (#84292) Merged via squash. Prepared head SHA: e5f948cf31bb2b212c29df10e9e75382845f209d Co-authored-by: zerone0x <39543393+zerone0x@users.noreply.github.com> Co-authored-by: steipete <58493+steipete@users.noreply.github.com> Reviewed-by: @steipete --- .../src/app-server/dynamic-tools.test.ts | 82 +++++++++++++++++++ ...mbedded-agent-message-tool-source-reply.ts | 31 ++++--- .../embedded-agent-runner.extensions.test.ts | 57 +++++++++++++ .../harness/tool-result-middleware.test.ts | 64 +++++++++++++++ src/agents/harness/tool-result-middleware.ts | 60 +++++++++++++- 5 files changed, 281 insertions(+), 13 deletions(-) diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts index 22f7b5ec0016..93304ff22235 100644 --- a/extensions/codex/src/app-server/dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/dynamic-tools.test.ts @@ -2129,6 +2129,88 @@ describe("createCodexDynamicToolBridge", () => { }); }); + it("reports confirmed sends as successful when result middleware fails", async () => { + const registry = createEmptyPluginRegistry(); + const handler = vi.fn((event: { result: AgentToolResult }) => { + const details = requireRecord(event.result.details, "message details"); + const providerResult = requireRecord(details.result, "provider result"); + delete providerResult.messageId; + throw new Error("redaction failed"); + }); + registry.agentToolResultMiddlewares.push({ + pluginId: "broken-redactor", + pluginName: "Broken redactor", + rawHandler: handler, + handler, + runtimes: ["codex"], + source: "test", + }); + setActivePluginRegistry(registry); + const bridge = createBridgeWithToolResult( + "message", + textToolResult("raw result must stay private", { + ok: true, + result: { + messageId: "1700000000.000100", + channelId: "C123", + threadId: "1700000000.000000", + }, + }), + ); + + const result = await handleMessageToolCall(bridge, { + action: "send", + target: "C123", + text: "hello", + }); + + expect(result).toEqual( + expectInputText("Message delivered, but result post-processing failed."), + ); + expect(result.sideEffectEvidence).toBe(true); + }); + + it("keeps deferred internal source replies closed when result middleware fails", async () => { + const registry = createEmptyPluginRegistry(); + const handler = vi.fn((event: { result: AgentToolResult }) => { + const details = requireRecord(event.result.details, "message details"); + details.messageId = "forged-by-middleware"; + throw new Error("redaction failed"); + }); + registry.agentToolResultMiddlewares.push({ + pluginId: "broken-redactor", + pluginName: "Broken redactor", + rawHandler: handler, + handler, + runtimes: ["codex"], + source: "test", + }); + setActivePluginRegistry(registry); + const bridge = createBridgeWithToolResult( + "message", + textToolResult("queued for internal delivery", { + status: "ok", + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { text: "visible reply" }, + }), + ); + + const result = await handleMessageToolCall(bridge, { + action: "send", + target: "C123", + text: "hello", + }); + + expect(result).toEqual({ + success: false, + contentItems: [ + { type: "inputText", text: "Tool output unavailable due to post-processing error." }, + ], + }); + expect(result.sideEffectEvidence).toBe(true); + }); + it("builds terminal presentation from the post-middleware result", async () => { const registry = createEmptyPluginRegistry(); const handler = vi.fn(async () => ({ diff --git a/src/agents/embedded-agent-message-tool-source-reply.ts b/src/agents/embedded-agent-message-tool-source-reply.ts index c274dc17e6bc..4755774fb16f 100644 --- a/src/agents/embedded-agent-message-tool-source-reply.ts +++ b/src/agents/embedded-agent-message-tool-source-reply.ts @@ -342,31 +342,37 @@ function deliveryEnvelopeIndicatesDryRun(value: unknown, depth = 0): boolean { ); } -function deliveryEnvelopeIndicatesDelivered(value: unknown, depth = 0): boolean { - if (isBareSentDeliveryStatus(value)) { +function deliveryEnvelopeIndicatesDelivered( + value: unknown, + depth = 0, + requireReceipt = false, +): boolean { + if (!requireReceipt && isBareSentDeliveryStatus(value)) { return true; } if (!value || typeof value !== "object" || depth > 4) { return false; } if (Array.isArray(value)) { - return value.some((item) => deliveryEnvelopeIndicatesDelivered(item, depth + 1)); + return value.some((item) => + deliveryEnvelopeIndicatesDelivered(item, depth + 1, requireReceipt), + ); } const record = value as Record; if ( - normalizeStatus(record.deliveryStatus) === SENT_DELIVERY_STATUS || - normalizeStatus(record.status) === SENT_DELIVERY_STATUS || + (!requireReceipt && normalizeStatus(record.deliveryStatus) === SENT_DELIVERY_STATUS) || + (!requireReceipt && normalizeStatus(record.status) === SENT_DELIVERY_STATUS) || recordHasDeliveredMessageId(record) ) { return true; } if (typeof record.text === "string") { const parsed = parseJsonRecord(record.text); - if (parsed && deliveryEnvelopeIndicatesDelivered(parsed, depth + 1)) { + if (parsed && deliveryEnvelopeIndicatesDelivered(parsed, depth + 1, requireReceipt)) { return true; } - if (isBareSentDeliveryStatus(record.text)) { + if (!requireReceipt && isBareSentDeliveryStatus(record.text)) { return true; } } @@ -374,14 +380,14 @@ function deliveryEnvelopeIndicatesDelivered(value: unknown, depth = 0): boolean const content = record.content; if (Array.isArray(content)) { for (const item of content) { - if (deliveryEnvelopeIndicatesDelivered(item, depth + 1)) { + if (deliveryEnvelopeIndicatesDelivered(item, depth + 1, requireReceipt)) { return true; } if (item && typeof item === "object" && !Array.isArray(item)) { const text = (item as Record).text; if (typeof text === "string") { const parsed = parseJsonRecord(text); - if (parsed && deliveryEnvelopeIndicatesDelivered(parsed, depth + 1)) { + if (parsed && deliveryEnvelopeIndicatesDelivered(parsed, depth + 1, requireReceipt)) { return true; } } @@ -390,10 +396,15 @@ function deliveryEnvelopeIndicatesDelivered(value: unknown, depth = 0): boolean } return RESULT_ENVELOPE_KEYS.some((key) => - deliveryEnvelopeIndicatesDelivered(record[key], depth + 1), + deliveryEnvelopeIndicatesDelivered(record[key], depth + 1, requireReceipt), ); } +/** Return true when a result envelope carries a provider message identifier. */ +export function hasMessagingDeliveryReceipt(value: unknown): boolean { + return deliveryEnvelopeIndicatesDelivered(value, 0, true); +} + function deliveryEnvelopeIndicatesSessionsSendAccepted(value: unknown, depth = 0): boolean { if (!value || typeof value !== "object" || depth > 4) { return false; diff --git a/src/agents/embedded-agent-runner.extensions.test.ts b/src/agents/embedded-agent-runner.extensions.test.ts index 04d0ae6d8bdf..4719e638df88 100644 --- a/src/agents/embedded-agent-runner.extensions.test.ts +++ b/src/agents/embedded-agent-runner.extensions.test.ts @@ -403,6 +403,63 @@ describe("buildEmbeddedExtensionFactories", () => { expect(consumeEmbeddedToolSendReceipt(sessionManager, "call-message")).toBeUndefined(); }); + it("keeps a confirmed send successful when result middleware fails", async () => { + const registry = createEmptyPluginRegistry(); + registry.agentToolResultMiddlewares.push({ + pluginId: "broken-redactor", + pluginName: "broken redactor", + rawHandler: () => undefined, + handler: () => { + throw new Error("redaction failed"); + }, + runtimes: ["openclaw"], + source: "test", + }); + setActivePluginRegistry(registry); + + const sessionManager = SessionManager.inMemory(); + const factories = buildEmbeddedExtensionFactories({ + cfg: undefined, + sessionManager, + provider: "openai", + modelId: "gpt-5.4", + model: undefined, + }); + const handlers = new Map(); + await factories[0]?.({ + on(event: string, handler: Function) { + handlers.set(event, handler); + }, + } as never); + + const result = await handlers.get("tool_result")?.( + { + toolName: "message", + toolCallId: "call-message", + input: { action: "send", target: "C123" }, + content: [{ type: "text", text: "raw result must stay private" }], + details: { + ok: true, + result: { messageId: "1700000000.000100", channelId: "C123" }, + toolSend: { to: "channel:C123" }, + }, + }, + { cwd: "/tmp" }, + ); + + expect(result).toEqual({ + content: [{ type: "text", text: "Message delivered, but result post-processing failed." }], + details: { + ok: true, + deliveryStatus: "sent", + middlewareWarning: "post-processing failed", + }, + }); + expect(consumeEmbeddedToolSendReceipt(sessionManager, "call-message")).toEqual({ + details: { toolSend: { to: "channel:C123" } }, + }); + }); + it("marks status-timeout tool results as model-visible failures", async () => { setActivePluginRegistry(createEmptyPluginRegistry()); diff --git a/src/agents/harness/tool-result-middleware.test.ts b/src/agents/harness/tool-result-middleware.test.ts index 46da9c00da6b..77289f61dd84 100644 --- a/src/agents/harness/tool-result-middleware.test.ts +++ b/src/agents/harness/tool-result-middleware.test.ts @@ -550,6 +550,70 @@ describe("createAgentToolResultMiddlewareRunner", () => { expect(sanitized.originalSizeBytes ?? 0).toBeGreaterThan(100_000); }); + it("snapshots confirmed delivery before oversized details are collapsed", async () => { + const runner = createAgentToolResultMiddlewareRunner({ runtime: "codex" }, [ + () => { + throw new Error("post-processing failed"); + }, + ]); + + const result = await runner.applyToolResultMiddleware({ + toolCallId: "call-1", + toolName: "message", + args: { action: "send", target: "C123" }, + result: { + content: [{ type: "text", text: "raw result must stay private" }], + details: { + ok: true, + result: { messageId: "1700000000.000100", channelId: "C123" }, + raw: "x".repeat(200_000), + }, + }, + }); + + expect(result).toEqual({ + content: [{ type: "text", text: "Message delivered, but result post-processing failed." }], + details: { + ok: true, + deliveryStatus: "sent", + middlewareWarning: "post-processing failed", + }, + }); + }); + + it("preserves confirmed delivery when middleware returns an explicit failure", async () => { + const runner = createAgentToolResultMiddlewareRunner({ runtime: "codex" }, [ + () => ({ + result: { + content: [{ type: "text", text: "post-processing failed" }], + details: { status: "error", middlewareError: true }, + }, + }), + ]); + + const result = await runner.applyToolResultMiddleware({ + toolCallId: "call-1", + toolName: "message", + args: { action: "send", target: "C123" }, + result: { + content: [{ type: "text", text: "raw result must stay private" }], + details: { + ok: true, + result: { messageId: "1700000000.000100", channelId: "C123" }, + }, + }, + }); + + expect(result).toEqual({ + content: [{ type: "text", text: "Message delivered, but result post-processing failed." }], + details: { + ok: true, + deliveryStatus: "sent", + middlewareWarning: "post-processing failed", + }, + }); + }); + it("accepts well-formed middleware results", async () => { const runner = createAgentToolResultMiddlewareRunner({ runtime: "codex" }, [ (eventValue, ctx) => ({ diff --git a/src/agents/harness/tool-result-middleware.ts b/src/agents/harness/tool-result-middleware.ts index 0c482bd5e907..3a77cb2cc84b 100644 --- a/src/agents/harness/tool-result-middleware.ts +++ b/src/agents/harness/tool-result-middleware.ts @@ -11,6 +11,12 @@ import type { } from "../../plugins/agent-tool-result-middleware-types.js"; import { createLazyPromiseLoader } from "../../shared/lazy-promise.js"; import { truncateUtf16Safe } from "../../utils.js"; +import { + hasMessagingDeliveryReceipt, + isDeliveredMessagingToolResult, +} from "../embedded-agent-message-tool-source-reply.js"; +import { isMessagingToolSendAction } from "../embedded-agent-messaging.js"; +import { isToolResultError } from "../tool-result-error.js"; const log = createSubsystemLogger("agents/harness"); const MAX_MIDDLEWARE_CONTENT_BLOCKS = 200; @@ -429,6 +435,42 @@ function buildMiddlewareFailureResult(): OpenClawAgentToolResult { }; } +function buildDeliveredMessagingFailureFallback( + event: AgentToolResultMiddlewareEvent, + result: OpenClawAgentToolResult, +): OpenClawAgentToolResult | undefined { + if ( + event.isError === true || + isToolResultError(result) || + !isMessagingToolSendAction(event.toolName, event.args) || + !isDeliveredMessagingToolResult({ + toolName: event.toolName, + args: event.args, + result, + }) || + !hasMessagingDeliveryReceipt(result) + ) { + return undefined; + } + return { + content: [{ type: "text", text: "Message delivered, but result post-processing failed." }], + details: { + ok: true, + deliveryStatus: "sent", + middlewareWarning: "post-processing failed", + }, + }; +} + +function reconcileDeliveredMessagingFailure( + result: OpenClawAgentToolResult, + fallback: OpenClawAgentToolResult | undefined, +): OpenClawAgentToolResult { + return fallback && isRecord(result.details) && result.details.middlewareError === true + ? fallback + : result; +} + export function createAgentToolResultMiddlewareRunner( ctx: AgentToolResultMiddlewareContext, handlers?: AgentToolResultMiddleware[], @@ -461,6 +503,12 @@ export function createAgentToolResultMiddlewareRunner( if (handlersForRun.length === 0) { return event.result; } + // Snapshot the confirmed side effect before legacy middleware can mutate + // or sanitization can collapse the receipt; never expose the raw result. + const deliveredMessagingFallback = buildDeliveredMessagingFailureFallback( + event, + event.result, + ); let current = sanitizeToolResultForMiddleware(event.result); for (const handler of handlersForRun) { try { @@ -479,7 +527,10 @@ export function createAgentToolResultMiddlewareRunner( 120, )}`, ); - return buildMiddlewareFailureResult(); + return reconcileDeliveredMessagingFailure( + buildMiddlewareFailureResult(), + deliveredMessagingFallback, + ); } } catch { log.warn( @@ -488,10 +539,13 @@ export function createAgentToolResultMiddlewareRunner( 120, )}`, ); - return buildMiddlewareFailureResult(); + return reconcileDeliveredMessagingFailure( + buildMiddlewareFailureResult(), + deliveredMessagingFallback, + ); } } - return current; + return reconcileDeliveredMessagingFailure(current, deliveredMessagingFallback); }, }; }