diff --git a/CHANGELOG.md b/CHANGELOG.md index ff3cb120efd..e031d97ac10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -207,6 +207,7 @@ Docs: https://docs.openclaw.ai - Feishu/Doc create permissions: remove caller-controlled owner fields from `feishu_doc` create and bind optional grant behavior to trusted Feishu requester context (`grant_to_requester`), preventing principal selection via tool arguments. (#31184) Thanks @Takhoffman. - Routing/Binding peer-kind parity: treat `peer.kind` `group` and `channel` as equivalent for binding scope matching (while keeping `direct` separate) so Slack/public channel bindings do not silently fall through. Landed from contributor PR #31135 by @Sid-Qin. Thanks @Sid-Qin. - Cron/Store EBUSY fallback: retry `rename` on `EBUSY` and use `copyFile` fallback on Windows when replacing cron store files so busy-file contention no longer causes false write failures. (#16932) Thanks @sudhanva-chakra. +- Cron/Isolated payload selection: ignore `isError` payloads when deriving summary/output/delivery payload fallbacks, while preserving error-only fallback behavior when no non-error payload exists. (#21454) Thanks @Diaspar4u. - Agents/FS workspace default: honor documented host file-tool default `tools.fs.workspaceOnly=false` when unset so host `write`/`edit` calls are not incorrectly workspace-restricted unless explicitly enabled. Landed from contributor PR #31128 by @SaucePackets. Thanks @SaucePackets. - Cron/Timer hot-loop guard: enforce a minimum timer re-arm delay when stale past-due jobs would otherwise trigger repeated `setTimeout(0)` loops, preventing event-loop saturation and log-flood behavior. (#29853) Thanks @FlamesCN. - Gateway/CLI session recovery: handle expired CLI session IDs gracefully by clearing stale session state and retrying without crashing gateway runs. Landed from contributor PR #31090 by @frankekn. Thanks @frankekn. diff --git a/src/cron/isolated-agent/helpers.test.ts b/src/cron/isolated-agent/helpers.test.ts new file mode 100644 index 00000000000..31e533170f8 --- /dev/null +++ b/src/cron/isolated-agent/helpers.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "vitest"; +import { + pickLastDeliverablePayload, + pickLastNonEmptyTextFromPayloads, + pickSummaryFromPayloads, +} from "./helpers.js"; + +describe("pickSummaryFromPayloads", () => { + it("picks real text over error payload", () => { + const payloads = [ + { text: "Here is your summary" }, + { text: "Tool error: rate limited", isError: true }, + ]; + expect(pickSummaryFromPayloads(payloads)).toBe("Here is your summary"); + }); + + it("falls back to error payload when no real text exists", () => { + const payloads = [{ text: "Tool error: rate limited", isError: true }]; + expect(pickSummaryFromPayloads(payloads)).toBe("Tool error: rate limited"); + }); + + it("returns undefined for empty payloads", () => { + expect(pickSummaryFromPayloads([])).toBeUndefined(); + }); + + it("treats isError: undefined as non-error", () => { + const payloads = [ + { text: "normal text", isError: undefined }, + { text: "error text", isError: true }, + ]; + expect(pickSummaryFromPayloads(payloads)).toBe("normal text"); + }); +}); + +describe("pickLastNonEmptyTextFromPayloads", () => { + it("picks real text over error payload", () => { + const payloads = [{ text: "Real output" }, { text: "Service error", isError: true }]; + expect(pickLastNonEmptyTextFromPayloads(payloads)).toBe("Real output"); + }); + + it("falls back to error payload when no real text exists", () => { + const payloads = [{ text: "Service error", isError: true }]; + expect(pickLastNonEmptyTextFromPayloads(payloads)).toBe("Service error"); + }); + + it("returns undefined for empty payloads", () => { + expect(pickLastNonEmptyTextFromPayloads([])).toBeUndefined(); + }); + + it("treats isError: undefined as non-error", () => { + const payloads = [ + { text: "good", isError: undefined }, + { text: "bad", isError: true }, + ]; + expect(pickLastNonEmptyTextFromPayloads(payloads)).toBe("good"); + }); +}); + +describe("pickLastDeliverablePayload", () => { + it("picks real payload over error payload", () => { + const real = { text: "Delivered content" }; + const error = { text: "Error warning", isError: true as const }; + expect(pickLastDeliverablePayload([real, error])).toBe(real); + }); + + it("falls back to error payload when no real payload exists", () => { + const error = { text: "Error warning", isError: true as const }; + expect(pickLastDeliverablePayload([error])).toBe(error); + }); + + it("returns undefined for empty payloads", () => { + expect(pickLastDeliverablePayload([])).toBeUndefined(); + }); + + it("picks media payload over error text payload", () => { + const media = { mediaUrl: "https://example.com/img.png" }; + const error = { text: "Error warning", isError: true as const }; + expect(pickLastDeliverablePayload([media, error])).toBe(media); + }); + + it("treats isError: undefined as non-error", () => { + const normal = { text: "ok", isError: undefined }; + const error = { text: "bad", isError: true as const }; + expect(pickLastDeliverablePayload([normal, error])).toBe(normal); + }); +}); diff --git a/src/cron/isolated-agent/helpers.ts b/src/cron/isolated-agent/helpers.ts index d4d42b20fe5..c74b65d1bb0 100644 --- a/src/cron/isolated-agent/helpers.ts +++ b/src/cron/isolated-agent/helpers.ts @@ -9,6 +9,7 @@ type DeliveryPayload = { mediaUrl?: string; mediaUrls?: string[]; channelData?: Record; + isError?: boolean; }; export function pickSummaryFromOutput(text: string | undefined) { @@ -20,7 +21,18 @@ export function pickSummaryFromOutput(text: string | undefined) { return clean.length > limit ? `${truncateUtf16Safe(clean, limit)}…` : clean; } -export function pickSummaryFromPayloads(payloads: Array<{ text?: string | undefined }>) { +export function pickSummaryFromPayloads( + payloads: Array<{ text?: string | undefined; isError?: boolean }>, +) { + for (let i = payloads.length - 1; i >= 0; i--) { + if (payloads[i]?.isError) { + continue; + } + const summary = pickSummaryFromOutput(payloads[i]?.text); + if (summary) { + return summary; + } + } for (let i = payloads.length - 1; i >= 0; i--) { const summary = pickSummaryFromOutput(payloads[i]?.text); if (summary) { @@ -30,7 +42,18 @@ export function pickSummaryFromPayloads(payloads: Array<{ text?: string | undefi return undefined; } -export function pickLastNonEmptyTextFromPayloads(payloads: Array<{ text?: string | undefined }>) { +export function pickLastNonEmptyTextFromPayloads( + payloads: Array<{ text?: string | undefined; isError?: boolean }>, +) { + for (let i = payloads.length - 1; i >= 0; i--) { + if (payloads[i]?.isError) { + continue; + } + const clean = (payloads[i]?.text ?? "").trim(); + if (clean) { + return clean; + } + } for (let i = payloads.length - 1; i >= 0; i--) { const clean = (payloads[i]?.text ?? "").trim(); if (clean) { @@ -41,13 +64,23 @@ export function pickLastNonEmptyTextFromPayloads(payloads: Array<{ text?: string } export function pickLastDeliverablePayload(payloads: DeliveryPayload[]) { + const isDeliverable = (p: DeliveryPayload) => { + const text = (p?.text ?? "").trim(); + const hasMedia = Boolean(p?.mediaUrl) || (p?.mediaUrls?.length ?? 0) > 0; + const hasChannelData = Object.keys(p?.channelData ?? {}).length > 0; + return text || hasMedia || hasChannelData; + }; for (let i = payloads.length - 1; i >= 0; i--) { - const payload = payloads[i]; - const text = (payload?.text ?? "").trim(); - const hasMedia = Boolean(payload?.mediaUrl) || (payload?.mediaUrls?.length ?? 0) > 0; - const hasChannelData = Object.keys(payload?.channelData ?? {}).length > 0; - if (text || hasMedia || hasChannelData) { - return payload; + if (payloads[i]?.isError) { + continue; + } + if (isDeliverable(payloads[i])) { + return payloads[i]; + } + } + for (let i = payloads.length - 1; i >= 0; i--) { + if (isDeliverable(payloads[i])) { + return payloads[i]; } } return undefined;