mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-13 17:07:40 +00:00
fix(agents): preserve delivered message send results (#84292)
Merged via squash.
Prepared head SHA: e5f948cf31
Co-authored-by: zerone0x <39543393+zerone0x@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Reviewed-by: @steipete
This commit is contained in:
@@ -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<unknown> }) => {
|
||||
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<unknown> }) => {
|
||||
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 () => ({
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<string, unknown>).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;
|
||||
|
||||
@@ -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<string, Function>();
|
||||
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());
|
||||
|
||||
|
||||
@@ -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) => ({
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user