mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 09:38:39 +00:00
fix(cron): suppress fallback summary after attempted announce delivery
This commit is contained in:
@@ -39,6 +39,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Security/Browser temp paths: harden trace/download output-path handling against symlink-root and symlink-parent escapes with realpath-based write-path checks plus secure fallback tmp-dir validation that fails closed on unsafe fallback links. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
|
- Security/Browser temp paths: harden trace/download output-path handling against symlink-root and symlink-parent escapes with realpath-based write-path checks plus secure fallback tmp-dir validation that fails closed on unsafe fallback links. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
|
||||||
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
|
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
|
||||||
- Cron/Message multi-account routing: honor explicit `delivery.accountId` for isolated cron delivery resolution, and when `message.send` omits `accountId`, fall back to the sending agent's bound channel account instead of defaulting to the global account. (#27015, #26975) Thanks @lbo728 and @stakeswky.
|
- Cron/Message multi-account routing: honor explicit `delivery.accountId` for isolated cron delivery resolution, and when `message.send` omits `accountId`, fall back to the sending agent's bound channel account instead of defaulting to the global account. (#27015, #26975) Thanks @lbo728 and @stakeswky.
|
||||||
|
- Cron/Announce duplicate guard: track attempted announce/direct delivery separately from confirmed `delivered`, and suppress fallback main-session cron summaries when delivery was already attempted to avoid duplicate end-user sends in uncertain-ack paths. (#27018)
|
||||||
- Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
|
- Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
|
||||||
- Security/Gateway auth: require pairing for operator device-identity sessions authenticated with shared token auth so unpaired devices cannot self-assign operator scopes. Thanks @tdjackey for reporting.
|
- Security/Gateway auth: require pairing for operator device-identity sessions authenticated with shared token auth so unpaired devices cannot self-assign operator scopes. Thanks @tdjackey for reporting.
|
||||||
- Security/Gateway WebSocket auth: enforce origin checks for direct browser WebSocket clients beyond Control UI/Webchat, apply password-auth failure throttling to browser-origin loopback attempts (including localhost), and block silent auto-pairing for non-Control-UI browser clients to prevent cross-origin brute-force and session takeover chains. This ships in the next npm release (`2026.2.25`). Thanks @luz-oasis for reporting.
|
- Security/Gateway WebSocket auth: enforce origin checks for direct browser WebSocket clients beyond Control UI/Webchat, apply password-auth failure throttling to browser-origin loopback attempts (including localhost), and block silent auto-pairing for non-Control-UI browser clients to prevent cross-origin brute-force and session takeover chains. This ships in the next npm release (`2026.2.25`). Thanks @luz-oasis for reporting.
|
||||||
|
|||||||
@@ -56,6 +56,7 @@ async function expectBestEffortTelegramNotDelivered(
|
|||||||
|
|
||||||
expect(res.status).toBe("ok");
|
expect(res.status).toBe("ok");
|
||||||
expect(res.delivered).toBe(false);
|
expect(res.delivered).toBe(false);
|
||||||
|
expect(res.deliveryAttempted).toBe(true);
|
||||||
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
|
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
|
||||||
expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1);
|
expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
@@ -287,6 +288,33 @@ describe("runCronIsolatedAgentTurn", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("marks attempted when announce delivery reports false and best-effort is enabled", async () => {
|
||||||
|
await withTempCronHome(async (home) => {
|
||||||
|
const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" });
|
||||||
|
const deps = createCliDeps();
|
||||||
|
mockAgentPayloads([{ text: "hello from cron" }]);
|
||||||
|
vi.mocked(runSubagentAnnounceFlow).mockResolvedValueOnce(false);
|
||||||
|
|
||||||
|
const res = await runTelegramAnnounceTurn({
|
||||||
|
home,
|
||||||
|
storePath,
|
||||||
|
deps,
|
||||||
|
delivery: {
|
||||||
|
mode: "announce",
|
||||||
|
channel: "telegram",
|
||||||
|
to: "123",
|
||||||
|
bestEffort: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe("ok");
|
||||||
|
expect(res.delivered).toBe(false);
|
||||||
|
expect(res.deliveryAttempted).toBe(true);
|
||||||
|
expect(runSubagentAnnounceFlow).toHaveBeenCalledTimes(1);
|
||||||
|
expect(deps.sendMessageTelegram).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("ignores structured direct delivery failures when best-effort is enabled", async () => {
|
it("ignores structured direct delivery failures when best-effort is enabled", async () => {
|
||||||
await expectBestEffortTelegramNotDelivered({
|
await expectBestEffortTelegramNotDelivered({
|
||||||
text: "hello from cron",
|
text: "hello from cron",
|
||||||
|
|||||||
@@ -117,6 +117,7 @@ type DispatchCronDeliveryParams = {
|
|||||||
export type DispatchCronDeliveryState = {
|
export type DispatchCronDeliveryState = {
|
||||||
result?: RunCronAgentTurnResult;
|
result?: RunCronAgentTurnResult;
|
||||||
delivered: boolean;
|
delivered: boolean;
|
||||||
|
deliveryAttempted: boolean;
|
||||||
summary?: string;
|
summary?: string;
|
||||||
outputText?: string;
|
outputText?: string;
|
||||||
synthesizedText?: string;
|
synthesizedText?: string;
|
||||||
@@ -134,6 +135,7 @@ export async function dispatchCronDelivery(
|
|||||||
// `true` means we confirmed at least one outbound send reached the target.
|
// `true` means we confirmed at least one outbound send reached the target.
|
||||||
// Keep this strict so timer fallback can safely decide whether to wake main.
|
// Keep this strict so timer fallback can safely decide whether to wake main.
|
||||||
let delivered = params.skipMessagingToolDelivery;
|
let delivered = params.skipMessagingToolDelivery;
|
||||||
|
let deliveryAttempted = params.skipMessagingToolDelivery;
|
||||||
const failDeliveryTarget = (error: string) =>
|
const failDeliveryTarget = (error: string) =>
|
||||||
params.withRunSession({
|
params.withRunSession({
|
||||||
status: "error",
|
status: "error",
|
||||||
@@ -141,6 +143,7 @@ export async function dispatchCronDelivery(
|
|||||||
errorKind: "delivery-target",
|
errorKind: "delivery-target",
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -162,9 +165,11 @@ export async function dispatchCronDelivery(
|
|||||||
return params.withRunSession({
|
return params.withRunSession({
|
||||||
status: "error",
|
status: "error",
|
||||||
error: params.abortReason(),
|
error: params.abortReason(),
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
deliveryAttempted = true;
|
||||||
const deliveryResults = await deliverOutboundPayloads({
|
const deliveryResults = await deliverOutboundPayloads({
|
||||||
cfg: params.cfgWithAgentDefaults,
|
cfg: params.cfgWithAgentDefaults,
|
||||||
channel: delivery.channel,
|
channel: delivery.channel,
|
||||||
@@ -187,6 +192,7 @@ export async function dispatchCronDelivery(
|
|||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
error: String(err),
|
error: String(err),
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -277,9 +283,11 @@ export async function dispatchCronDelivery(
|
|||||||
return params.withRunSession({
|
return params.withRunSession({
|
||||||
status: "error",
|
status: "error",
|
||||||
error: params.abortReason(),
|
error: params.abortReason(),
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
deliveryAttempted = true;
|
||||||
const didAnnounce = await runSubagentAnnounceFlow({
|
const didAnnounce = await runSubagentAnnounceFlow({
|
||||||
childSessionKey: params.agentSessionKey,
|
childSessionKey: params.agentSessionKey,
|
||||||
childRunId: `${params.job.id}:${params.runSessionId}:${params.runStartedAt}`,
|
childRunId: `${params.job.id}:${params.runSessionId}:${params.runStartedAt}`,
|
||||||
@@ -315,6 +323,7 @@ export async function dispatchCronDelivery(
|
|||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
error: message,
|
error: message,
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -327,6 +336,7 @@ export async function dispatchCronDelivery(
|
|||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
error: String(err),
|
error: String(err),
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -345,6 +355,7 @@ export async function dispatchCronDelivery(
|
|||||||
return {
|
return {
|
||||||
result: failDeliveryTarget(params.resolvedDelivery.error.message),
|
result: failDeliveryTarget(params.resolvedDelivery.error.message),
|
||||||
delivered,
|
delivered,
|
||||||
|
deliveryAttempted,
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
synthesizedText,
|
synthesizedText,
|
||||||
@@ -357,9 +368,11 @@ export async function dispatchCronDelivery(
|
|||||||
status: "ok",
|
status: "ok",
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
|
deliveryAttempted,
|
||||||
...params.telemetry,
|
...params.telemetry,
|
||||||
}),
|
}),
|
||||||
delivered,
|
delivered,
|
||||||
|
deliveryAttempted,
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
synthesizedText,
|
synthesizedText,
|
||||||
@@ -383,6 +396,7 @@ export async function dispatchCronDelivery(
|
|||||||
return {
|
return {
|
||||||
result: directResult,
|
result: directResult,
|
||||||
delivered,
|
delivered,
|
||||||
|
deliveryAttempted,
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
synthesizedText,
|
synthesizedText,
|
||||||
@@ -395,6 +409,7 @@ export async function dispatchCronDelivery(
|
|||||||
return {
|
return {
|
||||||
result: announceResult,
|
result: announceResult,
|
||||||
delivered,
|
delivered,
|
||||||
|
deliveryAttempted,
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
synthesizedText,
|
synthesizedText,
|
||||||
@@ -406,6 +421,7 @@ export async function dispatchCronDelivery(
|
|||||||
|
|
||||||
return {
|
return {
|
||||||
delivered,
|
delivered,
|
||||||
|
deliveryAttempted,
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
synthesizedText,
|
synthesizedText,
|
||||||
|
|||||||
@@ -77,6 +77,12 @@ export type RunCronAgentTurnResult = {
|
|||||||
* messages. See: https://github.com/openclaw/openclaw/issues/15692
|
* messages. See: https://github.com/openclaw/openclaw/issues/15692
|
||||||
*/
|
*/
|
||||||
delivered?: boolean;
|
delivered?: boolean;
|
||||||
|
/**
|
||||||
|
* `true` when cron attempted announce/direct delivery for this run.
|
||||||
|
* This is tracked separately from `delivered` because some announce paths
|
||||||
|
* cannot guarantee a final delivery ack synchronously.
|
||||||
|
*/
|
||||||
|
deliveryAttempted?: boolean;
|
||||||
} & CronRunOutcome &
|
} & CronRunOutcome &
|
||||||
CronRunTelemetry;
|
CronRunTelemetry;
|
||||||
|
|
||||||
@@ -565,7 +571,7 @@ export async function runCronIsolatedAgentTurn(params: {
|
|||||||
const embeddedRunError = hasErrorPayload
|
const embeddedRunError = hasErrorPayload
|
||||||
? (lastErrorPayloadText ?? "cron isolated run returned an error payload")
|
? (lastErrorPayloadText ?? "cron isolated run returned an error payload")
|
||||||
: undefined;
|
: undefined;
|
||||||
const resolveRunOutcome = (params?: { delivered?: boolean }) =>
|
const resolveRunOutcome = (params?: { delivered?: boolean; deliveryAttempted?: boolean }) =>
|
||||||
withRunSession({
|
withRunSession({
|
||||||
status: hasErrorPayload ? "error" : "ok",
|
status: hasErrorPayload ? "error" : "ok",
|
||||||
...(hasErrorPayload
|
...(hasErrorPayload
|
||||||
@@ -574,6 +580,7 @@ export async function runCronIsolatedAgentTurn(params: {
|
|||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
delivered: params?.delivered,
|
delivered: params?.delivered,
|
||||||
|
deliveryAttempted: params?.deliveryAttempted,
|
||||||
...telemetry,
|
...telemetry,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -619,14 +626,23 @@ export async function runCronIsolatedAgentTurn(params: {
|
|||||||
withRunSession,
|
withRunSession,
|
||||||
});
|
});
|
||||||
if (deliveryResult.result) {
|
if (deliveryResult.result) {
|
||||||
|
const resultWithDeliveryMeta: RunCronAgentTurnResult = {
|
||||||
|
...deliveryResult.result,
|
||||||
|
deliveryAttempted:
|
||||||
|
deliveryResult.result.deliveryAttempted ?? deliveryResult.deliveryAttempted,
|
||||||
|
};
|
||||||
if (!hasErrorPayload || deliveryResult.result.status !== "ok") {
|
if (!hasErrorPayload || deliveryResult.result.status !== "ok") {
|
||||||
return deliveryResult.result;
|
return resultWithDeliveryMeta;
|
||||||
}
|
}
|
||||||
return resolveRunOutcome({ delivered: deliveryResult.result.delivered });
|
return resolveRunOutcome({
|
||||||
|
delivered: deliveryResult.result.delivered,
|
||||||
|
deliveryAttempted: resultWithDeliveryMeta.deliveryAttempted,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
const delivered = deliveryResult.delivered;
|
const delivered = deliveryResult.delivered;
|
||||||
|
const deliveryAttempted = deliveryResult.deliveryAttempted;
|
||||||
summary = deliveryResult.summary;
|
summary = deliveryResult.summary;
|
||||||
outputText = deliveryResult.outputText;
|
outputText = deliveryResult.outputText;
|
||||||
|
|
||||||
return resolveRunOutcome({ delivered });
|
return resolveRunOutcome({ delivered, deliveryAttempted });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -625,6 +625,28 @@ describe("CronService", () => {
|
|||||||
await store.cleanup();
|
await store.cleanup();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not post isolated summary to main when announce delivery was attempted", async () => {
|
||||||
|
const runIsolatedAgentJob = vi.fn(async () => ({
|
||||||
|
status: "ok" as const,
|
||||||
|
summary: "done",
|
||||||
|
delivered: false,
|
||||||
|
deliveryAttempted: true,
|
||||||
|
}));
|
||||||
|
const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } =
|
||||||
|
await createIsolatedAnnounceHarness(runIsolatedAgentJob);
|
||||||
|
await runIsolatedAnnounceJobAndWait({
|
||||||
|
cron,
|
||||||
|
events,
|
||||||
|
name: "weekly attempted",
|
||||||
|
status: "ok",
|
||||||
|
});
|
||||||
|
expect(runIsolatedAgentJob).toHaveBeenCalledTimes(1);
|
||||||
|
expect(enqueueSystemEvent).not.toHaveBeenCalled();
|
||||||
|
expect(requestHeartbeatNow).not.toHaveBeenCalled();
|
||||||
|
cron.stop();
|
||||||
|
await store.cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
it("migrates legacy payload.provider to payload.channel on load", async () => {
|
it("migrates legacy payload.provider to payload.channel on load", async () => {
|
||||||
const rawJob = createLegacyDeliveryMigrationJob({
|
const rawJob = createLegacyDeliveryMigrationJob({
|
||||||
id: "legacy-1",
|
id: "legacy-1",
|
||||||
|
|||||||
@@ -80,6 +80,11 @@ export type CronServiceDeps = {
|
|||||||
* https://github.com/openclaw/openclaw/issues/15692
|
* https://github.com/openclaw/openclaw/issues/15692
|
||||||
*/
|
*/
|
||||||
delivered?: boolean;
|
delivered?: boolean;
|
||||||
|
/**
|
||||||
|
* `true` when announce/direct delivery was attempted for this run, even
|
||||||
|
* if the final per-message ack status is uncertain.
|
||||||
|
*/
|
||||||
|
deliveryAttempted?: boolean;
|
||||||
} & CronRunOutcome &
|
} & CronRunOutcome &
|
||||||
CronRunTelemetry
|
CronRunTelemetry
|
||||||
>;
|
>;
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ type TimedCronRunOutcome = CronRunOutcome &
|
|||||||
CronRunTelemetry & {
|
CronRunTelemetry & {
|
||||||
jobId: string;
|
jobId: string;
|
||||||
delivered?: boolean;
|
delivered?: boolean;
|
||||||
|
deliveryAttempted?: boolean;
|
||||||
startedAt: number;
|
startedAt: number;
|
||||||
endedAt: number;
|
endedAt: number;
|
||||||
};
|
};
|
||||||
@@ -606,7 +607,9 @@ export async function executeJobCore(
|
|||||||
state: CronServiceState,
|
state: CronServiceState,
|
||||||
job: CronJob,
|
job: CronJob,
|
||||||
abortSignal?: AbortSignal,
|
abortSignal?: AbortSignal,
|
||||||
): Promise<CronRunOutcome & CronRunTelemetry & { delivered?: boolean }> {
|
): Promise<
|
||||||
|
CronRunOutcome & CronRunTelemetry & { delivered?: boolean; deliveryAttempted?: boolean }
|
||||||
|
> {
|
||||||
const resolveAbortError = () => ({
|
const resolveAbortError = () => ({
|
||||||
status: "error" as const,
|
status: "error" as const,
|
||||||
error: timeoutErrorMessage(),
|
error: timeoutErrorMessage(),
|
||||||
@@ -729,17 +732,22 @@ export async function executeJobCore(
|
|||||||
return { status: "error", error: timeoutErrorMessage() };
|
return { status: "error", error: timeoutErrorMessage() };
|
||||||
}
|
}
|
||||||
|
|
||||||
// Post a short summary back to the main session — but only when the
|
// Post a short summary back to the main session only when announce
|
||||||
// isolated run did NOT already deliver its output to the target channel.
|
// delivery was requested and we are confident no outbound delivery path
|
||||||
// When `res.delivered` is true the announce flow (or direct outbound
|
// ran. If delivery was attempted but final ack is uncertain, suppress the
|
||||||
// delivery) already sent the result, so posting the summary to main
|
// main summary to avoid duplicate user-facing sends.
|
||||||
// would wake the main agent and cause a duplicate message.
|
|
||||||
// See: https://github.com/openclaw/openclaw/issues/15692
|
// See: https://github.com/openclaw/openclaw/issues/15692
|
||||||
const summaryText = res.summary?.trim();
|
const summaryText = res.summary?.trim();
|
||||||
const deliveryPlan = resolveCronDeliveryPlan(job);
|
const deliveryPlan = resolveCronDeliveryPlan(job);
|
||||||
const suppressMainSummary =
|
const suppressMainSummary =
|
||||||
res.status === "error" && res.errorKind === "delivery-target" && deliveryPlan.requested;
|
res.status === "error" && res.errorKind === "delivery-target" && deliveryPlan.requested;
|
||||||
if (summaryText && deliveryPlan.requested && !res.delivered && !suppressMainSummary) {
|
if (
|
||||||
|
summaryText &&
|
||||||
|
deliveryPlan.requested &&
|
||||||
|
!res.delivered &&
|
||||||
|
res.deliveryAttempted !== true &&
|
||||||
|
!suppressMainSummary
|
||||||
|
) {
|
||||||
const prefix = "Cron";
|
const prefix = "Cron";
|
||||||
const label =
|
const label =
|
||||||
res.status === "error" ? `${prefix} (error): ${summaryText}` : `${prefix}: ${summaryText}`;
|
res.status === "error" ? `${prefix} (error): ${summaryText}` : `${prefix}: ${summaryText}`;
|
||||||
@@ -762,6 +770,7 @@ export async function executeJobCore(
|
|||||||
error: res.error,
|
error: res.error,
|
||||||
summary: res.summary,
|
summary: res.summary,
|
||||||
delivered: res.delivered,
|
delivered: res.delivered,
|
||||||
|
deliveryAttempted: res.deliveryAttempted,
|
||||||
sessionId: res.sessionId,
|
sessionId: res.sessionId,
|
||||||
sessionKey: res.sessionKey,
|
sessionKey: res.sessionKey,
|
||||||
model: res.model,
|
model: res.model,
|
||||||
|
|||||||
Reference in New Issue
Block a user