mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(cron): channel failure alerts drop when global webhook failure destination is set (#102445)
* fix(cron): keep channel-shaped failure destinations from inheriting webhook mode * test(cron): prove channel failure destination announces under global webhook * test(gateway): prove cron channel failure destination under global webhook * fix(cron): harden failure destination mode inference Tighten the resolver invariant, reuse focused test fixtures, and keep the Gateway proof at the real finished-event boundary. Co-authored-by: wuqingxuan <wu.qingxuan@xydigit.com> Co-authored-by: Hakan Baysal <hakan.baysal@trmix.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Hakan Baysal <hakan.baysal@trmix.com>
This commit is contained in:
co-authored by
Peter Steinberger
Hakan Baysal
parent
b8f365a82b
commit
e99af07369
@@ -189,14 +189,18 @@ export function resolveFailureDestination(
|
||||
if (hasJobAccountIdField) {
|
||||
accountId = jobAccountId;
|
||||
}
|
||||
if (hasJobModeField) {
|
||||
const globalMode = globalConfig?.mode ?? "announce";
|
||||
const resolvedJobMode = jobMode ?? "announce";
|
||||
// Naming a channel makes this an announce route even when mode is omitted;
|
||||
// inheriting webhook here would reinterpret the chat target as a URL.
|
||||
const jobImpliesAnnounce = !hasJobModeField && jobChannel !== undefined;
|
||||
if (hasJobModeField || jobImpliesAnnounce) {
|
||||
const effectiveJobMode = jobImpliesAnnounce ? "announce" : jobMode;
|
||||
const globalMode = mode ?? "announce";
|
||||
const resolvedJobMode = effectiveJobMode ?? "announce";
|
||||
if (!jobToExplicitValue && globalMode !== resolvedJobMode) {
|
||||
// Do not carry an inherited target across modes; an announce chat is not a webhook URL.
|
||||
to = undefined;
|
||||
}
|
||||
mode = jobMode;
|
||||
mode = effectiveJobMode;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -235,6 +235,60 @@ describe("resolveFailureDestination", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves a channel-shaped job override without mode to announce despite a global webhook default (#102235)", () => {
|
||||
const plan = resolveFailureDestination(
|
||||
makeCronJob({
|
||||
delivery: {
|
||||
mode: "none",
|
||||
failureDestination: { channel: "slack", to: "#alerts" },
|
||||
},
|
||||
}),
|
||||
{ mode: "webhook", to: "https://hook.example/cron" },
|
||||
);
|
||||
expect(plan).toEqual({
|
||||
mode: "announce",
|
||||
channel: "slack",
|
||||
to: "#alerts",
|
||||
accountId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("clears an inherited global webhook URL when a channel-only override implies announce (#102235)", () => {
|
||||
const plan = resolveFailureDestination(
|
||||
makeCronJob({
|
||||
delivery: {
|
||||
mode: "none",
|
||||
failureDestination: { channel: "slack" },
|
||||
},
|
||||
}),
|
||||
{ mode: "webhook", to: "https://hook.example/cron" },
|
||||
);
|
||||
expect(plan).toEqual({
|
||||
mode: "announce",
|
||||
channel: "slack",
|
||||
to: undefined,
|
||||
accountId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps inheriting a global webhook mode for a to-only override without channel or mode", () => {
|
||||
const plan = resolveFailureDestination(
|
||||
makeCronJob({
|
||||
delivery: {
|
||||
mode: "none",
|
||||
failureDestination: { to: "https://other.example/hook" },
|
||||
},
|
||||
}),
|
||||
{ mode: "webhook", to: "https://hook.example/cron" },
|
||||
);
|
||||
expect(plan).toEqual({
|
||||
mode: "webhook",
|
||||
channel: undefined,
|
||||
to: "https://other.example/hook",
|
||||
accountId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns null for webhook mode without destination URL", () => {
|
||||
const plan = resolveFailureDestination(
|
||||
makeCronJob({
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
// including URL redaction for invalid webhook destinations.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { CliDeps } from "../cli/deps.types.js";
|
||||
import { makeCronJob } from "../cron/delivery.test-helpers.js";
|
||||
import type { CronJob } from "../cron/types.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
@@ -141,6 +142,55 @@ describe("dispatchGatewayCronFinishedNotifications", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("announces channel-shaped failure destinations without mode under a global webhook default (#102235)", () => {
|
||||
const logger = { warn: vi.fn() };
|
||||
const job = makeCronJob({
|
||||
id: "cron-channel-fd-no-mode",
|
||||
name: "channel fd no mode",
|
||||
delivery: {
|
||||
mode: "none",
|
||||
failureDestination: { channel: "slack", to: "#alerts" },
|
||||
},
|
||||
});
|
||||
|
||||
dispatchGatewayCronFinishedNotifications({
|
||||
evt: {
|
||||
jobId: job.id,
|
||||
action: "finished",
|
||||
status: "error",
|
||||
error: "boom",
|
||||
},
|
||||
job,
|
||||
deps: {} as CliDeps,
|
||||
logger,
|
||||
resolveCronAgent: () => ({ agentId: "main", cfg: {} }),
|
||||
globalFailureDestination: {
|
||||
mode: "webhook",
|
||||
to: "https://hook.example/cron",
|
||||
},
|
||||
});
|
||||
|
||||
expect(mocks.sendFailureNotificationAnnounce).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.anything(),
|
||||
"main",
|
||||
job.id,
|
||||
{
|
||||
channel: "slack",
|
||||
to: "#alerts",
|
||||
accountId: undefined,
|
||||
sessionKey: undefined,
|
||||
inheritSessionThread: false,
|
||||
},
|
||||
'⚠️ Cron job "channel fd no mode" failed: boom',
|
||||
);
|
||||
expect(logger.warn).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ jobId: job.id }),
|
||||
"cron: failure destination webhook URL is invalid, skipping",
|
||||
);
|
||||
expect(mocks.fetchWithSsrFGuard).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("redacts command action-required summaries before webhook completion delivery", async () => {
|
||||
const logger = { warn: vi.fn() };
|
||||
const sensitiveSummary =
|
||||
|
||||
@@ -367,7 +367,8 @@ function expectFailureAnnounceCall(params: {
|
||||
jobId: string;
|
||||
channel: string;
|
||||
to?: string;
|
||||
sessionKey: string;
|
||||
sessionKey?: string;
|
||||
inheritSessionThread?: false;
|
||||
message: string;
|
||||
}) {
|
||||
expect(sendFailureNotificationAnnounceMock).toHaveBeenCalledTimes(1);
|
||||
@@ -383,6 +384,7 @@ function expectFailureAnnounceCall(params: {
|
||||
to: params.to,
|
||||
accountId: undefined,
|
||||
sessionKey: params.sessionKey,
|
||||
...(params.inheritSessionThread === false ? { inheritSessionThread: false } : {}),
|
||||
});
|
||||
expect(args[5]).toBe(params.message);
|
||||
}
|
||||
@@ -1816,6 +1818,63 @@ describe("gateway server cron", () => {
|
||||
}
|
||||
}, 45_000);
|
||||
|
||||
test("announces channel-shaped failure destinations without mode under a global webhook default (#102235)", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-fd-channel-no-mode-",
|
||||
cronEnabled: false,
|
||||
});
|
||||
|
||||
await writeCronConfig({
|
||||
cron: {
|
||||
failureDestination: {
|
||||
mode: "webhook",
|
||||
to: "https://hook.example/cron",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { server, ws } = await startServerWithClient();
|
||||
await connectOk(ws);
|
||||
|
||||
try {
|
||||
sendFailureNotificationAnnounceMock.mockClear();
|
||||
fetchWithSsrFGuardMock.mockClear();
|
||||
cronIsolatedRun.mockResolvedValueOnce({ status: "error", summary: "delivery failed" });
|
||||
|
||||
const jobId = await addWebhookCronJob({
|
||||
ws,
|
||||
name: "channel fd no mode",
|
||||
sessionTarget: "isolated",
|
||||
delivery: {
|
||||
mode: "none",
|
||||
failureDestination: {
|
||||
channel: "slack",
|
||||
to: "#alerts",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const finished = waitForCronEvent(
|
||||
ws,
|
||||
(payload) => payload?.jobId === jobId && payload?.action === "finished",
|
||||
);
|
||||
await runCronJobForce(ws, jobId);
|
||||
await finished;
|
||||
|
||||
expectFailureAnnounceCall({
|
||||
jobId,
|
||||
channel: "slack",
|
||||
to: "#alerts",
|
||||
sessionKey: undefined,
|
||||
inheritSessionThread: false,
|
||||
message: '⚠️ Cron job "channel fd no mode" failed: unknown error',
|
||||
});
|
||||
expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await cleanupCronTestRun({ ws, server, prevSkipCron });
|
||||
}
|
||||
}, 45_000);
|
||||
|
||||
test("prefers sessionTarget session context for failure announcements over creator sessionKey", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-failure-session-target-",
|
||||
|
||||
Reference in New Issue
Block a user