From 332cdd7acaf30c049113fd29f1ce48970b5154b8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 06:34:31 +0100 Subject: [PATCH] fix(cron): route failure alerts via target session --- CHANGELOG.md | 1 + src/cron/delivery-preview.test.ts | 54 +++++++++++++++++++++ src/cron/delivery-preview.ts | 8 ++-- src/cron/delivery.failure-notify.test.ts | 7 ++- src/cron/delivery.ts | 6 ++- src/cron/session-target.ts | 22 +++++++++ src/gateway/server-cron.ts | 24 +++++++--- src/gateway/server.cron.test.ts | 60 ++++++++++++++++++++++++ 8 files changed, 170 insertions(+), 12 deletions(-) create mode 100644 src/cron/delivery-preview.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 793f6b37c4f..553d6463caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai - 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: 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:` 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. - Cron: classify isolated runs as errors from structured embedded-run execution-denial metadata, with final-output marker fallback for `SYSTEM_RUN_DENIED`, `INVALID_REQUEST`, and approval-binding refusals, so blocked commands no longer appear green in cron history. Fixes #67172; carries forward #67186. Thanks @oc-gh-dr, @hclsys, and @1yihui. - Onboarding/GitHub Copilot: add manifest-owned `--github-copilot-token` support for non-interactive setup, including env fallback, tokenRef storage in ref mode, saved-profile reuse, and current Copilot default-model wiring. Refs #50002 and supersedes #50003. Thanks @scottgl9. - Gateway/install: add a validated `--wrapper`/`OPENCLAW_WRAPPER` service install path that persists executable LaunchAgent/systemd wrappers across forced reinstalls, updates, and doctor repairs instead of falling back to raw node/bun `ProgramArguments`. Fixes #69400. (#72445) Thanks @willtmc. diff --git a/src/cron/delivery-preview.test.ts b/src/cron/delivery-preview.test.ts new file mode 100644 index 00000000000..31e5ea0a78b --- /dev/null +++ b/src/cron/delivery-preview.test.ts @@ -0,0 +1,54 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { makeCronJob } from "./delivery.test-helpers.js"; + +const mocks = vi.hoisted(() => ({ + resolveDeliveryTarget: vi.fn(), +})); + +vi.mock("./isolated-agent/delivery-target.js", () => ({ + resolveDeliveryTarget: mocks.resolveDeliveryTarget, +})); + +const { resolveCronDeliveryPreview } = await import("./delivery-preview.js"); + +describe("resolveCronDeliveryPreview", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.resolveDeliveryTarget.mockResolvedValue({ + ok: true, + channel: "telegram", + to: "direct-123", + mode: "implicit", + }); + }); + + it("prefers sessionTarget session context over creator sessionKey", async () => { + const job = makeCronJob({ + agentId: "avery", + sessionTarget: "session:agent:avery:telegram:direct:direct-123", + sessionKey: "agent:avery:telegram:group:ops:sender:direct-123", + delivery: undefined, + }); + + const preview = await resolveCronDeliveryPreview({ + cfg: {} as never, + job, + }); + + expect(mocks.resolveDeliveryTarget).toHaveBeenCalledWith( + {}, + "avery", + { + channel: "last", + to: undefined, + threadId: undefined, + accountId: undefined, + sessionKey: "agent:avery:telegram:direct:direct-123", + }, + { dryRun: true }, + ); + expect(preview.detail).toBe( + "resolved from last, session agent:avery:telegram:direct:direct-123", + ); + }); +}); diff --git a/src/cron/delivery-preview.ts b/src/cron/delivery-preview.ts index 80cc9d37936..2cac095873e 100644 --- a/src/cron/delivery-preview.ts +++ b/src/cron/delivery-preview.ts @@ -2,6 +2,7 @@ import { resolveDefaultAgentId } from "../agents/agent-scope-config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { resolveCronDeliveryPlan } from "./delivery-plan.js"; import { resolveDeliveryTarget } from "./isolated-agent/delivery-target.js"; +import { resolveCronDeliverySessionKey } from "./session-target.js"; import type { CronDeliveryPreview, CronJob } from "./types.js"; function formatTarget(channel?: string, to?: string | null): string { @@ -50,6 +51,7 @@ export async function resolveCronDeliveryPreview(params: { const requestedChannel = plan.channel ?? "last"; const agentId = params.job.agentId?.trim() || params.defaultAgentId || resolveDefaultAgentId(params.cfg); + const deliverySessionKey = resolveCronDeliverySessionKey(params.job); const resolved = await resolveDeliveryTarget( params.cfg, agentId, @@ -58,7 +60,7 @@ export async function resolveCronDeliveryPreview(params: { to: plan.to, threadId: plan.threadId, accountId: plan.accountId, - sessionKey: params.job.sessionKey, + sessionKey: deliverySessionKey, }, { dryRun: true }, ); @@ -68,7 +70,7 @@ export async function resolveCronDeliveryPreview(params: { detail: formatDeliveryDetail({ requestedChannel, resolved: false, - sessionKey: params.job.sessionKey, + sessionKey: deliverySessionKey, error: resolved.error.message, }), }; @@ -78,7 +80,7 @@ export async function resolveCronDeliveryPreview(params: { detail: formatDeliveryDetail({ requestedChannel, resolved: true, - sessionKey: params.job.sessionKey, + sessionKey: deliverySessionKey, }), }; } diff --git a/src/cron/delivery.failure-notify.test.ts b/src/cron/delivery.failure-notify.test.ts index aee7b3be020..9755895b269 100644 --- a/src/cron/delivery.failure-notify.test.ts +++ b/src/cron/delivery.failure-notify.test.ts @@ -95,7 +95,7 @@ describe("sendFailureNotificationAnnounce", () => { ); }); - it("passes sessionKey through to delivery-target resolution", async () => { + it("uses sessionKey for delivery-target resolution and outbound context", async () => { await sendFailureNotificationAnnounce( {} as never, {} as never, @@ -114,6 +114,11 @@ describe("sendFailureNotificationAnnounce", () => { accountId: undefined, sessionKey: "agent:main:telegram:direct:123:thread:99", }); + expect(mocks.buildOutboundSessionContext).toHaveBeenCalledWith({ + cfg: {}, + agentId: "main", + sessionKey: "agent:main:telegram:direct:123:thread:99", + }); }); it("does not send when target resolution fails", async () => { diff --git a/src/cron/delivery.ts b/src/cron/delivery.ts index 423079eabe2..cec0e8a3d2b 100644 --- a/src/cron/delivery.ts +++ b/src/cron/delivery.ts @@ -51,10 +51,14 @@ export async function sendFailureNotificationAnnounce( } const identity = resolveAgentOutboundIdentity(cfg, agentId); + const deliverySessionKey = + typeof target.sessionKey === "string" && target.sessionKey.trim() + ? target.sessionKey.trim() + : `cron:${jobId}:failure`; const session = buildOutboundSessionContext({ cfg, agentId, - sessionKey: `cron:${jobId}:failure`, + sessionKey: deliverySessionKey, }); const abortController = new AbortController(); diff --git a/src/cron/session-target.ts b/src/cron/session-target.ts index f592db264af..f742818cad4 100644 --- a/src/cron/session-target.ts +++ b/src/cron/session-target.ts @@ -14,3 +14,25 @@ export function assertSafeCronSessionTargetId(sessionId: string): string { } return trimmed; } + +export function resolveCronSessionTargetSessionKey( + sessionTarget?: string | null, +): string | undefined { + if (typeof sessionTarget !== "string" || !sessionTarget.startsWith("session:")) { + return undefined; + } + return assertSafeCronSessionTargetId(sessionTarget.slice(8)); +} + +export function resolveCronDeliverySessionKey(job: { + sessionTarget?: string | null; + sessionKey?: string | null; +}): string | undefined { + const sessionTargetKey = resolveCronSessionTargetSessionKey(job.sessionTarget); + if (sessionTargetKey) { + return sessionTargetKey; + } + return typeof job.sessionKey === "string" && job.sessionKey.trim() + ? job.sessionKey.trim() + : undefined; +} diff --git a/src/gateway/server-cron.ts b/src/gateway/server-cron.ts index 98e0334956b..cb59f80e6cf 100644 --- a/src/gateway/server-cron.ts +++ b/src/gateway/server-cron.ts @@ -23,7 +23,10 @@ import { resolveCronRunLogPruneOptions, } from "../cron/run-log.js"; import { CronService } from "../cron/service.js"; -import { assertSafeCronSessionTargetId } from "../cron/session-target.js"; +import { + resolveCronDeliverySessionKey, + resolveCronSessionTargetSessionKey, +} from "../cron/session-target.js"; import { resolveCronStorePath } from "../cron/store.js"; import { normalizeHttpWebhookUrl } from "../cron/webhook-url.js"; import { formatErrorMessage } from "../infra/errors.js"; @@ -32,6 +35,7 @@ import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; import { SsrFBlockedError } from "../infra/net/ssrf.js"; import { deliverOutboundPayloads } from "../infra/outbound/deliver.js"; +import { buildOutboundSessionContext } from "../infra/outbound/session-context.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; import { getChildLogger } from "../logging.js"; import { normalizeAgentId, toAgentStoreSessionKey } from "../routing/session-key.js"; @@ -332,10 +336,7 @@ export function buildGatewayCronService(params: { }, runIsolatedAgentJob: async ({ job, message, abortSignal }) => { const { agentId, cfg: runtimeConfig } = resolveCronAgent(job.agentId); - let sessionKey = `cron:${job.id}`; - if (job.sessionTarget.startsWith("session:")) { - sessionKey = assertSafeCronSessionTargetId(job.sessionTarget.slice(8)); - } + const sessionKey = resolveCronSessionTargetSessionKey(job.sessionTarget) ?? `cron:${job.id}`; try { return await runCronIsolatedAgentTurn({ cfg: runtimeConfig, @@ -395,14 +396,21 @@ export function buildGatewayCronService(params: { return; } + const deliverySessionKey = resolveCronDeliverySessionKey(job); const target = await resolveDeliveryTarget(runtimeConfig, agentId, { channel, to, accountId, + sessionKey: deliverySessionKey, }); if (!target.ok) { throw target.error; } + const session = buildOutboundSessionContext({ + cfg: runtimeConfig, + agentId, + sessionKey: deliverySessionKey ?? `cron:${job.id}:failure`, + }); await deliverOutboundPayloads({ cfg: runtimeConfig, channel: target.channel, @@ -411,6 +419,7 @@ export function buildGatewayCronService(params: { threadId: target.threadId, payloads: [{ text }], deps: createOutboundSendDeps(params.deps), + session, }); }, log: getChildLogger({ module: "cron", storePath }), @@ -470,6 +479,7 @@ export function buildGatewayCronService(params: { if (!isBestEffort) { const failureMessage = `Cron job "${job.name}" failed: ${evt.error ?? "unknown error"}`; const failureDest = resolveFailureDestination(job, params.cfg.cron?.failureDestination); + const deliverySessionKey = resolveCronDeliverySessionKey(job); if (failureDest) { // Explicit failureDestination configured — use it @@ -518,7 +528,7 @@ export function buildGatewayCronService(params: { channel: failureDest.channel, to: failureDest.to, accountId: failureDest.accountId, - sessionKey: job.sessionKey, + sessionKey: deliverySessionKey, }, `⚠️ ${failureMessage}`, ); @@ -537,7 +547,7 @@ export function buildGatewayCronService(params: { channel: primaryPlan.channel, to: primaryPlan.to, accountId: primaryPlan.accountId, - sessionKey: job.sessionKey, + sessionKey: deliverySessionKey, }, `⚠️ ${failureMessage}`, ); diff --git a/src/gateway/server.cron.test.ts b/src/gateway/server.cron.test.ts index b8cd1e0efc2..013634744d8 100644 --- a/src/gateway/server.cron.test.ts +++ b/src/gateway/server.cron.test.ts @@ -1337,6 +1337,66 @@ describe("gateway server cron", () => { } }, 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-", + cronEnabled: false, + }); + + const { server, ws } = await startServerWithClient(); + await connectOk(ws); + + try { + cronIsolatedRun.mockResolvedValueOnce({ status: "error", summary: "delivery failed" }); + const addRes = await rpcReq(ws, "cron.add", { + name: "session target failure fallback", + enabled: true, + schedule: { kind: "every", everyMs: 60_000 }, + sessionTarget: "session:agent:avery:feishu:direct:ou_founder", + wakeMode: "next-heartbeat", + payload: { kind: "agentTurn", message: "test" }, + delivery: { + mode: "announce", + channel: "feishu", + to: "ou_founder", + }, + }); + const jobId = expectCronJobIdFromResponse(addRes); + + const updateRes = await rpcReq(ws, "cron.update", { + id: jobId, + patch: { + sessionKey: "agent:avery:feishu:group:oc_group:sender:ou_founder", + }, + }); + expect(updateRes.ok).toBe(true); + + const finished = waitForCronEvent( + ws, + (payload) => payload?.jobId === jobId && payload?.action === "finished", + ); + await runCronJobForce(ws, jobId); + await finished; + + expect(sendFailureNotificationAnnounceMock).toHaveBeenCalledTimes(1); + expect(sendFailureNotificationAnnounceMock).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.any(String), + jobId, + { + channel: "feishu", + to: "ou_founder", + accountId: undefined, + sessionKey: "agent:avery:feishu:direct:ou_founder", + }, + '⚠️ Cron job "session target failure fallback" failed: unknown error', + ); + } finally { + await cleanupCronTestRun({ ws, server, prevSkipCron }); + } + }, 45_000); + test("ignores non-string cron.webhookToken values without crashing webhook delivery", async () => { const { prevSkipCron } = await setupCronTestRun({ tempPrefix: "openclaw-gw-cron-webhook-secretinput-",