diff --git a/src/cron/isolated-agent/delivery-dispatch.error-guard.test.ts b/src/cron/isolated-agent/delivery-dispatch.error-guard.test.ts index 34ff372a75c..fec7f8e17e8 100644 --- a/src/cron/isolated-agent/delivery-dispatch.error-guard.test.ts +++ b/src/cron/isolated-agent/delivery-dispatch.error-guard.test.ts @@ -149,9 +149,10 @@ describe("isLikelyRawErrorOutput", () => { expect(isLikelyRawErrorOutput(text)).toBe(true); }); - it("detects HTTP status code error patterns", () => { - expect(isLikelyRawErrorOutput("error: status code 500 from upstream")).toBe(true); - expect(isLikelyRawErrorOutput("Error response with code 429 - rate limited")).toBe(true); + it("does not flag HTTP status code error patterns in prose", () => { + expect(isLikelyRawErrorOutput("error: status code 500 from upstream")).toBe(false); + expect(isLikelyRawErrorOutput("Error response with code 429 - rate limited")).toBe(false); + expect(isLikelyRawErrorOutput("Server returned error code 503")).toBe(false); }); it("does not flag normal agent output", () => { @@ -211,11 +212,10 @@ describe("dispatchCronDelivery — error output guard", () => { const state = await dispatchCronDelivery(params); expect(state.delivered).toBe(false); - expect(state.deliveryAttempted).toBe(false); expect(state.result).toBeUndefined(); expect(deliverOutboundPayloads).not.toHaveBeenCalled(); expect(logWarn).toHaveBeenCalledWith( - expect.stringContaining("suppressed announce delivery of raw error output"), + expect.stringContaining("suppressed delivery of raw error output"), ); }); @@ -226,7 +226,6 @@ describe("dispatchCronDelivery — error output guard", () => { const state = await dispatchCronDelivery(params); expect(state.delivered).toBe(false); - expect(state.deliveryAttempted).toBe(false); expect(deliverOutboundPayloads).not.toHaveBeenCalled(); }); @@ -248,17 +247,6 @@ describe("dispatchCronDelivery — error output guard", () => { expect(state.result).toBeUndefined(); }); - it("skips delivery for HTTP 500 error pattern in text", async () => { - const params = makeBaseParams({ - synthesizedText: "error: status code 500 from model provider", - }); - const state = await dispatchCronDelivery(params); - - expect(state.delivered).toBe(false); - expect(state.deliveryAttempted).toBe(false); - expect(deliverOutboundPayloads).not.toHaveBeenCalled(); - }); - it("preserves summary and outputText in the returned state even when skipping", async () => { const errorText = "Error: ECONNREFUSED 127.0.0.1:18789"; const params = makeBaseParams({ synthesizedText: errorText }); @@ -276,17 +264,17 @@ describe("dispatchCronDelivery — error output guard", () => { error: { type: "server_error", message: "fail" }, }); const params = makeBaseParams({ synthesizedText: undefined }); - // Override deliveryPayloads directly with error text + // Override deliveryPayloads directly with error text and route through + // deliverViaDirect (structured content path) so the guard is reached. params.deliveryPayloads = [{ text: errorJson }]; + (params as Record).deliveryPayloadHasStructuredContent = true; const state = await dispatchCronDelivery(params); expect(state.delivered).toBe(false); - expect(state.deliveryAttempted).toBe(false); expect(deliverOutboundPayloads).not.toHaveBeenCalled(); expect(logWarn).toHaveBeenCalledWith( - expect.stringContaining("suppressed announce delivery of raw error output"), + expect.stringContaining("suppressed delivery of raw error output"), ); - expect(logWarn).toHaveBeenCalledWith(expect.stringContaining("deliveryPayloads")); }); it("allows delivery when only some payloads match error patterns", async () => { @@ -304,22 +292,6 @@ describe("dispatchCronDelivery — error output guard", () => { expect(deliverOutboundPayloads).toHaveBeenCalledTimes(1); }); - it("guard fires even when delivery is not explicitly requested", async () => { - const errorJson = JSON.stringify({ - type: "error", - error: { type: "server_error", message: "fail" }, - }); - const params = makeBaseParams({ - synthesizedText: errorJson, - deliveryRequested: false, - }); - const state = await dispatchCronDelivery(params); - - expect(state.delivered).toBe(false); - expect(state.deliveryAttempted).toBe(false); - expect(deliverOutboundPayloads).not.toHaveBeenCalled(); - }); - it("allows delivery when synthesizedText has error but deliveryPayloads has valid content", async () => { const errorJson = JSON.stringify({ type: "error", @@ -335,21 +307,4 @@ describe("dispatchCronDelivery — error output guard", () => { expect(state.delivered).toBe(true); expect(deliverOutboundPayloads).toHaveBeenCalledTimes(1); }); - - it("preserves shared-delivery flags when skipMessagingToolDelivery is true", async () => { - const errorJson = JSON.stringify({ - type: "error", - error: { type: "server_error", message: "fail" }, - }); - const params = makeBaseParams({ synthesizedText: errorJson }); - // Simulate shared-delivery path: agent already sent via message tool - (params as Record).skipMessagingToolDelivery = true; - const state = await dispatchCronDelivery(params); - - // Error guard should suppress announce delivery but preserve the - // existing message-tool delivery state. - expect(state.delivered).toBe(true); - expect(state.deliveryAttempted).toBe(true); - expect(deliverOutboundPayloads).not.toHaveBeenCalled(); - }); }); diff --git a/src/cron/isolated-agent/delivery-dispatch.ts b/src/cron/isolated-agent/delivery-dispatch.ts index 93f652ea479..822754cf8d2 100644 --- a/src/cron/isolated-agent/delivery-dispatch.ts +++ b/src/cron/isolated-agent/delivery-dispatch.ts @@ -290,7 +290,6 @@ async function retryTransientDirectCronDelivery(params: { const RAW_ERROR_OUTPUT_PATTERNS: readonly RegExp[] = [ /^\s*\{[\s\S]*"(?:type|error|code)":\s*"(?:error|server_error|invalid_request)/, /^\s*(?:Error|TypeError|RangeError|SyntaxError|ReferenceError):/, - /\b(?:error|Error)\b.*\b(?:status|code)\b.*\b(?:5\d{2}|4\d{2})\b/, /^\s*\{[\s\S]*"(?:message|error)":\s*"An error occurred/, /\berror"?:\s*\{[\s\S]*"(?:type|code)":\s*"(?:server_error|invalid_request|rate_limit)/, ]; @@ -320,43 +319,6 @@ export async function dispatchCronDelivery( let synthesizedText = params.synthesizedText; let deliveryPayloads = params.deliveryPayloads; - // Guard: never deliver raw error output (provider JSON errors, runtime - // exceptions) to user-facing channels. The error is still logged internally - // and visible via `cron runs`, but should not be posted to channels. - // - // The delivery path prefers deliveryPayloads when non-empty, so we only - // suppress based on synthesizedText when there are no valid payloads that - // would take priority (e.g., media or structured content). When payloads - // exist, we check whether ALL of them contain error text instead. - // See: https://github.com/openclaw/openclaw/issues/42243 - const hasActivePayloads = deliveryPayloads.length > 0; - const errorInSynthesized = - !hasActivePayloads && synthesizedText && isLikelyRawErrorOutput(synthesizedText); - const errorInPayloads = - hasActivePayloads && - deliveryPayloads.every((p) => typeof p.text === "string" && isLikelyRawErrorOutput(p.text)); - if (errorInSynthesized || errorInPayloads) { - const source = errorInSynthesized ? "synthesizedText" : "deliveryPayloads"; - const chars = errorInSynthesized - ? synthesizedText!.length - : deliveryPayloads.reduce((n, p) => n + (p.text?.length ?? 0), 0); - logWarn( - `[cron:${params.job.id}] suppressed announce delivery of raw error output (${source}, ${chars} chars)`, - ); - // Preserve shared-delivery state: when skipMessagingToolDelivery is true, - // the agent already sent via the message tool — suppressing the error - // guard's announce delivery should not overwrite that existing delivery - // state in cron logs/metadata. - return { - delivered: skipMessagingToolDelivery, - deliveryAttempted: skipMessagingToolDelivery, - summary, - outputText, - synthesizedText, - deliveryPayloads, - }; - } - // Shared callers can treat a matching message-tool send as the completed // delivery path. Cron-owned callers keep this false so direct cron delivery // remains the only source of delivered state. @@ -392,6 +354,19 @@ export async function dispatchCronDelivery( if (payloadsForDelivery.length === 0) { return null; } + // Guard: never deliver raw error output (provider JSON errors, runtime + // exceptions) to user-facing channels. Runs AFTER subagent orchestration + // so interim error-shaped parent messages don't suppress valid final output. + // See: https://github.com/openclaw/openclaw/issues/42243 + const allPayloadsAreErrors = payloadsForDelivery.every( + (p) => typeof p.text === "string" && isLikelyRawErrorOutput(p.text), + ); + if (allPayloadsAreErrors) { + logWarn( + `[cron:${params.job.id}] suppressed delivery of raw error output (${payloadsForDelivery.length} payloads)`, + ); + return null; + } if (params.isAborted()) { return params.withRunSession({ status: "error",