mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 07:04:32 +00:00
fix(cron): skip isError payloads when picking summary/delivery content (#21454)
* fix(cron): skip isError payloads when picking summary/delivery content buildEmbeddedRunPayloads appends isError warnings as the last payload. Three functions in helpers.ts iterate last-to-first and pick the error over real agent output. Use two-pass selection: prefer non-error payloads, fall back to error-only when no real content exists. Fixes: pickSummaryFromPayloads, pickLastNonEmptyTextFromPayloads, pickLastDeliverablePayload — all now accept and filter isError. * Changelog: note cron payload isError filtering (#21454) --------- Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
- 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.
|
- 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/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.
|
- 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.
|
- 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.
|
- 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.
|
||||||
|
|||||||
86
src/cron/isolated-agent/helpers.test.ts
Normal file
86
src/cron/isolated-agent/helpers.test.ts
Normal file
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -9,6 +9,7 @@ type DeliveryPayload = {
|
|||||||
mediaUrl?: string;
|
mediaUrl?: string;
|
||||||
mediaUrls?: string[];
|
mediaUrls?: string[];
|
||||||
channelData?: Record<string, unknown>;
|
channelData?: Record<string, unknown>;
|
||||||
|
isError?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
export function pickSummaryFromOutput(text: string | undefined) {
|
export function pickSummaryFromOutput(text: string | undefined) {
|
||||||
@@ -20,7 +21,18 @@ export function pickSummaryFromOutput(text: string | undefined) {
|
|||||||
return clean.length > limit ? `${truncateUtf16Safe(clean, limit)}…` : clean;
|
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--) {
|
for (let i = payloads.length - 1; i >= 0; i--) {
|
||||||
const summary = pickSummaryFromOutput(payloads[i]?.text);
|
const summary = pickSummaryFromOutput(payloads[i]?.text);
|
||||||
if (summary) {
|
if (summary) {
|
||||||
@@ -30,7 +42,18 @@ export function pickSummaryFromPayloads(payloads: Array<{ text?: string | undefi
|
|||||||
return undefined;
|
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--) {
|
for (let i = payloads.length - 1; i >= 0; i--) {
|
||||||
const clean = (payloads[i]?.text ?? "").trim();
|
const clean = (payloads[i]?.text ?? "").trim();
|
||||||
if (clean) {
|
if (clean) {
|
||||||
@@ -41,13 +64,23 @@ export function pickLastNonEmptyTextFromPayloads(payloads: Array<{ text?: string
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function pickLastDeliverablePayload(payloads: DeliveryPayload[]) {
|
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--) {
|
for (let i = payloads.length - 1; i >= 0; i--) {
|
||||||
const payload = payloads[i];
|
if (payloads[i]?.isError) {
|
||||||
const text = (payload?.text ?? "").trim();
|
continue;
|
||||||
const hasMedia = Boolean(payload?.mediaUrl) || (payload?.mediaUrls?.length ?? 0) > 0;
|
}
|
||||||
const hasChannelData = Object.keys(payload?.channelData ?? {}).length > 0;
|
if (isDeliverable(payloads[i])) {
|
||||||
if (text || hasMedia || hasChannelData) {
|
return payloads[i];
|
||||||
return payload;
|
}
|
||||||
|
}
|
||||||
|
for (let i = payloads.length - 1; i >= 0; i--) {
|
||||||
|
if (isDeliverable(payloads[i])) {
|
||||||
|
return payloads[i];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return undefined;
|
return undefined;
|
||||||
|
|||||||
Reference in New Issue
Block a user