mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(cron): verify delivery before clearing message warnings
This commit is contained in:
@@ -90,7 +90,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Google Meet: preserve Gemini Live function names when replying to realtime tool calls so Google SDK validation accepts the `FunctionResponse` payload. Fixes #72425. (#72426) Thanks @BsnizND.
|
||||
- Matrix/E2EE: stabilize recovery and broken-device QA flows while avoiding Matrix device-cleanup sync races that could leave shutdown-time crypto work running. Thanks @gumadeiras.
|
||||
- Cron: apply `cron.maxConcurrentRuns` to a dedicated `cron-nested` isolated agent-turn lane as well as cron dispatch, so parallel cron jobs no longer serialize on inner LLM execution while non-cron nested flows keep their existing lane behavior. Fixes #72707. Thanks @kagura-agent.
|
||||
- Cron: report isolated runs as successful when fallback delivery already delivered the reply, and ignore no-delivery Message/Canvas presentation warnings without hiding real execution failures. Fixes #72732 and #50170; follow-up to #54188. Thanks @zNatix, @pixeldyn, and @ChickenEggRoll.
|
||||
- Cron: report isolated runs as successful when verified cron delivery already delivered the reply, while keeping unresolved Message/Canvas tool failures fatal. Fixes #72732 and #50170; follow-up to #54188. Thanks @zNatix, @pixeldyn, and @ChickenEggRoll.
|
||||
- Cron: treat isolated run-level agent failures as job errors even when no reply payload is produced, synthesizing a safe error payload so model/provider failures increment error counters and trigger failure notifications instead of clearing as successful. Fixes #43604; carries forward #43631. Thanks @SPFAdvisors.
|
||||
- Cron: preserve exact `NO_REPLY` tool results from isolated jobs with empty final assistant turns as quiet successes instead of surfacing incomplete-turn errors. Fixes #68452; carries forward #68453. Thanks @anyech.
|
||||
- Cron: resolve failure alerts and failure-destination announcements against `session:<id>` targets before falling back to the creator session, so jobs created from group chats can notify the targeted direct session without cross-account routing errors. Refs #62777; carries forward #68535. Thanks @slideshow-dingo and @likewen-tech.
|
||||
|
||||
@@ -74,20 +74,21 @@ describe("resolveCronPayloadOutcome", () => {
|
||||
|
||||
expect(result.hasFatalErrorPayload).toBe(false);
|
||||
expect(result.embeddedRunError).toBeUndefined();
|
||||
expect(result.pendingPresentationWarningError).toBe("⚠️ ✉️ Message failed");
|
||||
expect(result.summary).toBe("Final cron report");
|
||||
expect(result.outputText).toBe("Final cron report");
|
||||
expect(result.deliveryPayloads).toEqual([{ text: "Final cron report" }]);
|
||||
});
|
||||
|
||||
it("treats trailing canvas warnings as non-fatal when earlier assistant output exists", () => {
|
||||
it("keeps trailing canvas warnings fatal even when earlier assistant output exists", () => {
|
||||
const result = resolveCronPayloadOutcome({
|
||||
payloads: [{ text: "Saved report to disk." }, { text: "⚠️ 🖼️ Canvas failed", isError: true }],
|
||||
});
|
||||
|
||||
expect(result.hasFatalErrorPayload).toBe(false);
|
||||
expect(result.summary).toBe("Saved report to disk.");
|
||||
expect(result.outputText).toBe("Saved report to disk.");
|
||||
expect(result.deliveryPayloads).toEqual([{ text: "Saved report to disk." }]);
|
||||
expect(result.hasFatalErrorPayload).toBe(true);
|
||||
expect(result.pendingPresentationWarningError).toBeUndefined();
|
||||
expect(result.embeddedRunError).toBe("⚠️ 🖼️ Canvas failed");
|
||||
expect(result.deliveryPayloads).toEqual([{ text: "⚠️ 🖼️ Canvas failed", isError: true }]);
|
||||
});
|
||||
|
||||
it("keeps standalone presentation warnings fatal when there is no cron output", () => {
|
||||
|
||||
@@ -19,6 +19,7 @@ export type CronPayloadOutcome = {
|
||||
deliveryPayloadHasStructuredContent: boolean;
|
||||
hasFatalErrorPayload: boolean;
|
||||
embeddedRunError?: string;
|
||||
pendingPresentationWarningError?: string;
|
||||
};
|
||||
|
||||
type CronDenialSignal = {
|
||||
@@ -241,13 +242,11 @@ export function resolveHeartbeatAckMaxChars(agentCfg?: { heartbeat?: { ackMaxCha
|
||||
return Math.max(0, raw);
|
||||
}
|
||||
|
||||
function isNonFatalCronPresentationWarning(text: string | undefined): boolean {
|
||||
function isCronMessagePresentationWarning(text: string | undefined): boolean {
|
||||
const normalized = normalizeOptionalString(text)?.toLowerCase();
|
||||
return (
|
||||
normalized === "⚠️ ✉️ message failed" ||
|
||||
normalized?.startsWith("⚠️ ✉️ message failed:") === true ||
|
||||
normalized === "⚠️ 🖼️ canvas failed" ||
|
||||
normalized?.startsWith("⚠️ 🖼️ canvas failed:") === true
|
||||
normalized?.startsWith("⚠️ ✉️ message failed:") === true
|
||||
);
|
||||
}
|
||||
|
||||
@@ -288,14 +287,14 @@ export function resolveCronPayloadOutcome(params: {
|
||||
params.payloads
|
||||
.slice(0, lastErrorPayloadIndex)
|
||||
.some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
|
||||
const hasNonFatalTrailingPresentationWarning =
|
||||
const hasPendingPresentationWarning =
|
||||
!params.runLevelError &&
|
||||
params.failureSignal?.fatalForCron !== true &&
|
||||
lastErrorPayloadIndex >= 0 &&
|
||||
isNonFatalCronPresentationWarning(lastErrorPayloadText) &&
|
||||
isCronMessagePresentationWarning(lastErrorPayloadText) &&
|
||||
(normalizedFinalAssistantVisibleText !== undefined || hasSuccessfulPayloadBeforeLastError);
|
||||
const hasFatalStructuredErrorPayload =
|
||||
hasErrorPayload &&
|
||||
!hasSuccessfulPayloadAfterLastError &&
|
||||
!hasNonFatalTrailingPresentationWarning;
|
||||
hasErrorPayload && !hasSuccessfulPayloadAfterLastError && !hasPendingPresentationWarning;
|
||||
const hasStructuredDeliveryPayloads = selectedDeliveryPayloads.some((payload) =>
|
||||
payloadHasStructuredDeliveryContent(payload),
|
||||
);
|
||||
@@ -370,5 +369,8 @@ export function resolveCronPayloadOutcome(params: {
|
||||
: denialSignal
|
||||
? formatCronDenialSignal(denialSignal)
|
||||
: runLevelError,
|
||||
pendingPresentationWarningError: hasPendingPresentationWarning
|
||||
? lastErrorPayloadText
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
loadRunCronIsolatedAgentTurn,
|
||||
makeCronSession,
|
||||
mockRunCronFallbackPassthrough,
|
||||
resolveCronPayloadOutcomeMock,
|
||||
resetRunCronIsolatedAgentTurnHarness,
|
||||
resolveCronDeliveryPlanMock,
|
||||
resolveDeliveryTargetMock,
|
||||
@@ -92,6 +93,20 @@ function makeMessageToolRunResult(messagingToolSentTargets: Array<Record<string,
|
||||
};
|
||||
}
|
||||
|
||||
function mockPendingMessagePresentationWarningOutcome() {
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: "Final cron report",
|
||||
outputText: "Final cron report",
|
||||
synthesizedText: "Final cron report",
|
||||
deliveryPayload: { text: "Final cron report" },
|
||||
deliveryPayloads: [{ text: "Final cron report" }],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
pendingPresentationWarningError: "⚠️ ✉️ Message failed",
|
||||
});
|
||||
}
|
||||
|
||||
describe("runCronIsolatedAgentTurn message tool policy", () => {
|
||||
let previousFastTestEnv: string | undefined;
|
||||
|
||||
@@ -753,6 +768,57 @@ describe("runCronIsolatedAgentTurn message tool policy", () => {
|
||||
);
|
||||
expect(result.delivery).not.toHaveProperty("resolved");
|
||||
});
|
||||
|
||||
it("clears pending message presentation warnings only after cron delivery succeeds", async () => {
|
||||
mockRunCronFallbackPassthrough();
|
||||
mockPendingMessagePresentationWarningOutcome();
|
||||
resolveCronDeliveryPlanMock.mockReturnValue(makeAnnounceDeliveryPlan());
|
||||
runEmbeddedPiAgentMock.mockResolvedValue({
|
||||
payloads: [{ text: "Final cron report" }, { text: "⚠️ ✉️ Message failed", isError: true }],
|
||||
meta: { agentMeta: { usage: { input: 10, output: 20 } } },
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn({
|
||||
...makeParams(),
|
||||
job: makeAnnounceMessageToolJob({
|
||||
id: "pending-message-warning-delivered",
|
||||
name: "Pending Message Warning Delivered",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
deliveryPayloads: [{ text: "Final cron report" }],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps pending message presentation warnings fatal when cron delivery does not succeed", async () => {
|
||||
mockRunCronFallbackPassthrough();
|
||||
mockPendingMessagePresentationWarningOutcome();
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({ requested: false, mode: "none" });
|
||||
runEmbeddedPiAgentMock.mockResolvedValue({
|
||||
payloads: [{ text: "Final cron report" }, { text: "⚠️ ✉️ Message failed", isError: true }],
|
||||
meta: { agentMeta: { usage: { input: 10, output: 20 } } },
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn({
|
||||
...makeParams(),
|
||||
job: makeMessageToolPolicyJob({ mode: "none" }),
|
||||
});
|
||||
|
||||
expect(result.status).toBe("error");
|
||||
expect(result.error).toBe("⚠️ ✉️ Message failed");
|
||||
expect(result.summary).toBe("Final cron report");
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
deliveryRequested: false,
|
||||
deliveryPayloads: [{ text: "Final cron report" }],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runCronIsolatedAgentTurn delivery instruction", () => {
|
||||
|
||||
@@ -848,6 +848,7 @@ async function finalizeCronRun(params: {
|
||||
deliveryPayloadHasStructuredContent,
|
||||
hasFatalErrorPayload,
|
||||
embeddedRunError,
|
||||
pendingPresentationWarningError,
|
||||
} = resolveCronPayloadOutcome({
|
||||
payloads,
|
||||
runLevelError: finalRunResult.meta?.error,
|
||||
@@ -874,6 +875,12 @@ async function finalizeCronRun(params: {
|
||||
delivery: result?.delivery,
|
||||
...telemetry,
|
||||
});
|
||||
const failPendingPresentationWarningUnlessDelivered = (delivered?: boolean) => {
|
||||
if (pendingPresentationWarningError && delivered !== true) {
|
||||
hasFatalErrorPayload = true;
|
||||
embeddedRunError = pendingPresentationWarningError;
|
||||
}
|
||||
};
|
||||
|
||||
const skipHeartbeatDelivery =
|
||||
prepared.deliveryRequested &&
|
||||
@@ -944,6 +951,9 @@ async function finalizeCronRun(params: {
|
||||
deliveryResult.result.deliveryAttempted ?? deliveryResult.deliveryAttempted,
|
||||
delivery: deliveryTrace,
|
||||
};
|
||||
failPendingPresentationWarningUnlessDelivered(
|
||||
resultWithDeliveryMeta.delivered ?? deliveryResult.delivered,
|
||||
);
|
||||
if (!hasFatalErrorPayload || deliveryResult.result.status !== "ok") {
|
||||
return resultWithDeliveryMeta;
|
||||
}
|
||||
@@ -955,6 +965,7 @@ async function finalizeCronRun(params: {
|
||||
}
|
||||
summary = deliveryResult.summary;
|
||||
outputText = deliveryResult.outputText;
|
||||
failPendingPresentationWarningUnlessDelivered(deliveryResult.delivered);
|
||||
return resolveRunOutcome({
|
||||
delivered: deliveryResult.delivered,
|
||||
deliveryAttempted: deliveryResult.deliveryAttempted,
|
||||
|
||||
Reference in New Issue
Block a user