diff --git a/apps/server/src/jobs/service.ts b/apps/server/src/jobs/service.ts index 8a6a372f1..ee3f735b0 100644 --- a/apps/server/src/jobs/service.ts +++ b/apps/server/src/jobs/service.ts @@ -592,16 +592,12 @@ export class JobService { async addJob(input: AddJobInput): Promise { const displayName = input.displayName?.trim() || input.name; const schedule = input.schedule === "" ? null : (input.schedule ?? null); - if (schedule && !validateCronExpression(schedule)) { - throw new Error( - `Job "${displayName}" has an invalid cron expression: "${schedule}"` - ); - } - if (input.enabled && !schedule && !input.continuationEnabled) { - throw new Error( - `Job "${displayName}" needs a schedule or continuation enabled before it can be enabled.` - ); - } + assertScheduleValid({ + label: displayName, + schedule, + enabled: input.enabled, + continuationEnabled: input.continuationEnabled, + }); const agentConfig = applyAgentConfigDefaults(input); @@ -677,18 +673,14 @@ export class JobService { const schedule = input.schedule === "" ? null : input.schedule; const nextSchedule = input.schedule === undefined ? existing.schedule : schedule; - if (nextSchedule && !validateCronExpression(nextSchedule)) { - throw new Error( - `Job "${input.displayName ?? existing.name}" has an invalid cron expression: "${nextSchedule}"` - ); - } const nextContinuationEnabled = input.continuationEnabled ?? existing.continuationEnabled; - if (input.enabled && !nextSchedule && !nextContinuationEnabled) { - throw new Error( - `Job "${input.displayName ?? existing.name}" needs a schedule or continuation enabled before it can be enabled.` - ); - } + assertScheduleValid({ + label: input.displayName ?? existing.name, + schedule: nextSchedule, + enabled: input.enabled, + continuationEnabled: nextContinuationEnabled, + }); const config: Parameters[1] = {}; const displayName = normalizeOptionalString(input.displayName); @@ -1347,6 +1339,30 @@ function formatCompletionCriteria( return criteria.map((criterion) => `- ${criterion}`).join("\n"); } +/** + * Shared schedule guards for addJob and updateJob. Each caller resolves the + * label, schedule and continuation flag with its own policy and passes the + * already-resolved values, so only the two thrown messages are shared. + */ +function assertScheduleValid(params: { + label: string; + schedule: string | null | undefined; + enabled: boolean | undefined; + continuationEnabled: boolean | undefined; +}): void { + const { label, schedule, enabled, continuationEnabled } = params; + if (schedule && !validateCronExpression(schedule)) { + throw new Error( + `Job "${label}" has an invalid cron expression: "${schedule}"` + ); + } + if (enabled && !schedule && !continuationEnabled) { + throw new Error( + `Job "${label}" needs a schedule or continuation enabled before it can be enabled.` + ); + } +} + function normalizeOptionalString( value: string | undefined ): string | undefined { diff --git a/apps/server/test/jobs/service.test.ts b/apps/server/test/jobs/service.test.ts index b3acdc280..a0867560e 100644 --- a/apps/server/test/jobs/service.test.ts +++ b/apps/server/test/jobs/service.test.ts @@ -635,7 +635,9 @@ describe("JobService", () => { prompt: "Test", schedule: "invalid cron", }) - ).rejects.toThrow("invalid cron expression"); + ).rejects.toThrow( + 'Job "bad-cron" has an invalid cron expression: "invalid cron"' + ); await service.shutdown(); }); @@ -759,7 +761,9 @@ describe("JobService", () => { directory: "/tmp/test-upd-cron", schedule: "not a cron", }) - ).rejects.toThrow("invalid cron expression"); + ).rejects.toThrow( + 'Job "upd-bad-cron" has an invalid cron expression: "not a cron"' + ); await service.shutdown(); });