diff --git a/src/cron/legacy-delivery.ts b/src/cron/legacy-delivery.ts index 8dcc6ecda5d..0474f5d7b95 100644 --- a/src/cron/legacy-delivery.ts +++ b/src/cron/legacy-delivery.ts @@ -42,6 +42,102 @@ export function buildDeliveryFromLegacyPayload( return next; } +export function buildDeliveryPatchFromLegacyPayload(payload: Record) { + const deliver = payload.deliver; + const channelRaw = + typeof payload.channel === "string" && payload.channel.trim() + ? payload.channel.trim().toLowerCase() + : typeof payload.provider === "string" && payload.provider.trim() + ? payload.provider.trim().toLowerCase() + : ""; + const toRaw = typeof payload.to === "string" ? payload.to.trim() : ""; + const next: Record = {}; + let hasPatch = false; + + if (deliver === false) { + next.mode = "none"; + hasPatch = true; + } else if ( + deliver === true || + channelRaw || + toRaw || + typeof payload.bestEffortDeliver === "boolean" + ) { + next.mode = "announce"; + hasPatch = true; + } + if (channelRaw) { + next.channel = channelRaw; + hasPatch = true; + } + if (toRaw) { + next.to = toRaw; + hasPatch = true; + } + if (typeof payload.bestEffortDeliver === "boolean") { + next.bestEffort = payload.bestEffortDeliver; + hasPatch = true; + } + + return hasPatch ? next : null; +} + +export function mergeLegacyDeliveryInto( + delivery: Record, + payload: Record, +) { + const patch = buildDeliveryPatchFromLegacyPayload(payload); + if (!patch) { + return { delivery, mutated: false }; + } + + const next = { ...delivery }; + let mutated = false; + + if ("mode" in patch && patch.mode !== next.mode) { + next.mode = patch.mode; + mutated = true; + } + if ("channel" in patch && patch.channel !== next.channel) { + next.channel = patch.channel; + mutated = true; + } + if ("to" in patch && patch.to !== next.to) { + next.to = patch.to; + mutated = true; + } + if ("bestEffort" in patch && patch.bestEffort !== next.bestEffort) { + next.bestEffort = patch.bestEffort; + mutated = true; + } + + return { delivery: next, mutated }; +} + +export function normalizeLegacyDeliveryInput(params: { + delivery?: Record | null; + payload?: Record | null; +}) { + if (!params.payload || !hasLegacyDeliveryHints(params.payload)) { + return { + delivery: params.delivery ?? undefined, + mutated: false, + }; + } + + const nextDelivery = params.delivery + ? mergeLegacyDeliveryInto(params.delivery, params.payload) + : { + delivery: buildDeliveryFromLegacyPayload(params.payload), + mutated: true, + }; + stripLegacyDeliveryFields(params.payload); + return { + delivery: nextDelivery.delivery, + mutated: true, + }; +} + export function stripLegacyDeliveryFields(payload: Record) { if ("deliver" in payload) { delete payload.deliver; diff --git a/src/cron/normalize.ts b/src/cron/normalize.ts index fe06eaf2f46..5a6c66ff356 100644 --- a/src/cron/normalize.ts +++ b/src/cron/normalize.ts @@ -1,10 +1,6 @@ import { sanitizeAgentId } from "../routing/session-key.js"; import { isRecord } from "../utils.js"; -import { - buildDeliveryFromLegacyPayload, - hasLegacyDeliveryHints, - stripLegacyDeliveryFields, -} from "./legacy-delivery.js"; +import { normalizeLegacyDeliveryInput } from "./legacy-delivery.js"; import { parseAbsoluteTimeMs } from "./parse.js"; import { migrateLegacyCronPayload } from "./payload-migration.js"; import { inferLegacyName } from "./service/normalize.js"; @@ -469,14 +465,20 @@ export function normalizeCronJobInput( const isIsolatedAgentTurn = sessionTarget === "isolated" || (sessionTarget === "" && payloadKind === "agentTurn"); const hasDelivery = "delivery" in next && next.delivery !== undefined; - const hasLegacyDelivery = payload ? hasLegacyDeliveryHints(payload) : false; - if (!hasDelivery && isIsolatedAgentTurn && payloadKind === "agentTurn") { - if (payload && hasLegacyDelivery) { - next.delivery = buildDeliveryFromLegacyPayload(payload); - stripLegacyDeliveryFields(payload); - } else { - next.delivery = { mode: "announce" }; - } + const normalizedLegacy = normalizeLegacyDeliveryInput({ + delivery: isRecord(next.delivery) ? next.delivery : null, + payload, + }); + if (normalizedLegacy.mutated && normalizedLegacy.delivery) { + next.delivery = normalizedLegacy.delivery; + } + if ( + !hasDelivery && + !normalizedLegacy.delivery && + isIsolatedAgentTurn && + payloadKind === "agentTurn" + ) { + next.delivery = { mode: "announce" }; } } diff --git a/src/cron/service.jobs.test.ts b/src/cron/service.jobs.test.ts index ae77f488f0a..053ea8764de 100644 --- a/src/cron/service.jobs.test.ts +++ b/src/cron/service.jobs.test.ts @@ -589,19 +589,6 @@ describe("createJob delivery defaults", () => { expect(job.delivery).toEqual({ mode: "none" }); }); - it("preserves legacy payload deliver=false when explicit delivery is omitted", () => { - const state = createMockState(now); - const job = createJob(state, { - name: "isolated-legacy-no-deliver", - enabled: true, - schedule: { kind: "every", everyMs: 60_000 }, - sessionTarget: "isolated", - wakeMode: "now", - payload: { kind: "agentTurn", message: "hello", deliver: false } as never, - }); - expect(job.delivery).toEqual({ mode: "none" }); - }); - it("does not set delivery for main systemEvent jobs without explicit delivery", () => { const state = createMockState(now, { defaultAgentId: "main" }); const job = createJob(state, { diff --git a/src/cron/service/initial-delivery.ts b/src/cron/service/initial-delivery.ts index 8aff8ef4452..9dc2eb908d5 100644 --- a/src/cron/service/initial-delivery.ts +++ b/src/cron/service/initial-delivery.ts @@ -1,17 +1,9 @@ -import { buildDeliveryFromLegacyPayload, hasLegacyDeliveryHints } from "../legacy-delivery.js"; import type { CronDelivery, CronJobCreate } from "../types.js"; export function resolveInitialCronDelivery(input: CronJobCreate): CronDelivery | undefined { if (input.delivery) { return input.delivery; } - const payloadRecord = - input.payload && typeof input.payload === "object" - ? (input.payload as Record) - : undefined; - if (payloadRecord && hasLegacyDeliveryHints(payloadRecord)) { - return buildDeliveryFromLegacyPayload(payloadRecord) as CronDelivery; - } if (input.sessionTarget === "isolated" && input.payload.kind === "agentTurn") { return { mode: "announce" }; } diff --git a/src/cron/service/ops.ts b/src/cron/service/ops.ts index 14758c5df34..2b86b7253c2 100644 --- a/src/cron/service/ops.ts +++ b/src/cron/service/ops.ts @@ -1,3 +1,4 @@ +import { normalizeCronJobCreate } from "../normalize.js"; import type { CronJob, CronJobCreate, CronJobPatch } from "../types.js"; import { applyJobPatch, @@ -234,7 +235,11 @@ export async function add(state: CronServiceState, input: CronJobCreate) { return await locked(state, async () => { warnIfDisabled(state, "add"); await ensureLoaded(state); - const job = createJob(state, input); + const normalizedInput = normalizeCronJobCreate(input); + if (!normalizedInput) { + throw new Error("invalid cron job input"); + } + const job = createJob(state, normalizedInput); state.store?.jobs.push(job); // Defensive: recompute all next-run times to ensure consistency diff --git a/src/cron/service/store.ts b/src/cron/service/store.ts index 0a52197bf81..2c40ac50643 100644 --- a/src/cron/service/store.ts +++ b/src/cron/service/store.ts @@ -1,9 +1,5 @@ import fs from "node:fs"; -import { - buildDeliveryFromLegacyPayload, - hasLegacyDeliveryHints, - stripLegacyDeliveryFields, -} from "../legacy-delivery.js"; +import { normalizeLegacyDeliveryInput } from "../legacy-delivery.js"; import { parseAbsoluteTimeMs } from "../parse.js"; import { migrateLegacyCronPayload } from "../payload-migration.js"; import { coerceFiniteScheduleNumber } from "../schedule.js"; @@ -14,69 +10,6 @@ import { recomputeNextRuns } from "./jobs.js"; import { inferLegacyName, normalizeOptionalText } from "./normalize.js"; import type { CronServiceState } from "./state.js"; -function buildDeliveryPatchFromLegacyPayload(payload: Record) { - const deliver = payload.deliver; - const channelRaw = - typeof payload.channel === "string" ? payload.channel.trim().toLowerCase() : ""; - const toRaw = typeof payload.to === "string" ? payload.to.trim() : ""; - const next: Record = {}; - let hasPatch = false; - - if (deliver === false) { - next.mode = "none"; - hasPatch = true; - } else if (deliver === true || toRaw) { - next.mode = "announce"; - hasPatch = true; - } - if (channelRaw) { - next.channel = channelRaw; - hasPatch = true; - } - if (toRaw) { - next.to = toRaw; - hasPatch = true; - } - if (typeof payload.bestEffortDeliver === "boolean") { - next.bestEffort = payload.bestEffortDeliver; - hasPatch = true; - } - - return hasPatch ? next : null; -} - -function mergeLegacyDeliveryInto( - delivery: Record, - payload: Record, -) { - const patch = buildDeliveryPatchFromLegacyPayload(payload); - if (!patch) { - return { delivery, mutated: false }; - } - - const next = { ...delivery }; - let mutated = false; - - if ("mode" in patch && patch.mode !== next.mode) { - next.mode = patch.mode; - mutated = true; - } - if ("channel" in patch && patch.channel !== next.channel) { - next.channel = patch.channel; - mutated = true; - } - if ("to" in patch && patch.to !== next.to) { - next.to = patch.to; - mutated = true; - } - if ("bestEffort" in patch && patch.bestEffort !== next.bestEffort) { - next.bestEffort = patch.bestEffort; - mutated = true; - } - - return { delivery: next, mutated }; -} - function normalizePayloadKind(payload: Record) { const raw = typeof payload.kind === "string" ? payload.kind.trim().toLowerCase() : ""; if (raw === "agentturn") { @@ -512,30 +445,25 @@ export async function ensureLoaded( const isIsolatedAgentTurn = sessionTarget === "isolated" || (sessionTarget === "" && payloadKind === "agentTurn"); const hasDelivery = delivery && typeof delivery === "object" && !Array.isArray(delivery); - const hasLegacyDelivery = payloadRecord ? hasLegacyDeliveryHints(payloadRecord) : false; + const normalizedLegacy = normalizeLegacyDeliveryInput({ + delivery: hasDelivery ? (delivery as Record) : null, + payload: payloadRecord, + }); if (isIsolatedAgentTurn && payloadKind === "agentTurn") { - if (!hasDelivery) { - raw.delivery = - payloadRecord && hasLegacyDelivery - ? buildDeliveryFromLegacyPayload(payloadRecord) - : { mode: "announce" }; - mutated = true; - } - if (payloadRecord && hasLegacyDelivery) { - if (hasDelivery) { - const merged = mergeLegacyDeliveryInto( - delivery as Record, - payloadRecord, - ); - if (merged.mutated) { - raw.delivery = merged.delivery; - mutated = true; - } - } - stripLegacyDeliveryFields(payloadRecord); + if (!hasDelivery && normalizedLegacy.delivery) { + raw.delivery = normalizedLegacy.delivery; + mutated = true; + } else if (!hasDelivery) { + raw.delivery = { mode: "announce" }; + mutated = true; + } else if (normalizedLegacy.mutated && normalizedLegacy.delivery) { + raw.delivery = normalizedLegacy.delivery; mutated = true; } + } else if (normalizedLegacy.mutated && normalizedLegacy.delivery) { + raw.delivery = normalizedLegacy.delivery; + mutated = true; } } state.store = { version: 1, jobs: jobs as unknown as CronJob[] };