mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix: fire a one-time schedule on the exact chosen date and time (#1497)
Simple "Once" schedules compiled to a 5-field cron, which has no year field, so the scheduler computed the next run as the next annual occurrence. A date chosen for a later year ran a year early, and a time already elapsed today ran a year late, contradicting the UI promise that the task fires on the chosen date. One-time schedules now send the chosen absolute timestamp (run_at) and the backend pins next_run_at to it instead of the cron-derived next run, so the run fires on the exact selected instant including the year. The Simple-mode validation now compares the full chosen instant against the current time, so a time earlier today is rejected as past rather than silently deferred a year. run_at is validated as a finite, future epoch-millisecond timestamp on create and update. The enable/disable toggle preserves a one-shot's pinned next_run_at (its yearless cron cannot reconstruct the chosen year), so re-enabling restores the exact instant. Recurring shapes and Advanced mode are unchanged (cron stays authoritative).
This commit is contained in:
@@ -224,6 +224,60 @@ describe('POST /api/scheduled-tasks', () => {
|
||||
expect(res.body.cron_expression).toBe('@daily');
|
||||
});
|
||||
|
||||
it('pins next_run_at to an explicit one-shot run_at instead of the cron-derived next run', async () => {
|
||||
// A one-shot for next year: the cron (0 23 1 7 *) would otherwise resolve to
|
||||
// this year's occurrence. run_at must win so the task fires on the chosen year.
|
||||
const runAt = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: '0 23 1 7 *', delete_after_run: true, run_at: runAt,
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.next_run_at).toBe(runAt);
|
||||
});
|
||||
|
||||
it('falls back to the cron-derived next run when no run_at is supplied', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: '0 3 * * *',
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
// Cron-derived next run is some future instant, not pinned to a passed timestamp.
|
||||
expect(typeof res.body.next_run_at).toBe('number');
|
||||
expect(res.body.next_run_at).toBeGreaterThan(Date.now());
|
||||
});
|
||||
|
||||
it('rejects a run_at in the past with 400', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: '0 23 1 7 *', delete_after_run: true, run_at: Date.now() - 60_000,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/run_at must be in the future/);
|
||||
});
|
||||
|
||||
it('rejects a non-numeric run_at with 400', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: '0 23 1 7 *', run_at: '2027-07-01',
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/run_at must be an epoch-millisecond timestamp/);
|
||||
});
|
||||
|
||||
it('rejects a fractional run_at with 400 (must be an integer epoch-ms)', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: '0 23 1 7 *', run_at: Date.now() + 100000.5,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/run_at must be an epoch-millisecond timestamp/);
|
||||
});
|
||||
|
||||
it('ignores run_at for a disabled task (next_run_at stays null)', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, enabled: false, cron_expression: '0 23 1 7 *', delete_after_run: true,
|
||||
run_at: new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime(),
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.next_run_at).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects unsupported actions', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, action: 'nuke',
|
||||
@@ -417,6 +471,30 @@ describe('PATCH /api/scheduled-tasks/:id/toggle', () => {
|
||||
expect(on.body.enabled).toBe(1);
|
||||
expect(typeof on.body.next_run_at).toBe('number');
|
||||
});
|
||||
|
||||
it('preserves a one-shot pinned next_run_at across a disable/enable cycle', async () => {
|
||||
const now = Date.now();
|
||||
// A one-shot pinned to next year (its yearless cron would resolve to this year).
|
||||
const pinned = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const id = DatabaseService.getInstance().createScheduledTask({
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: pinned, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1,
|
||||
});
|
||||
|
||||
const off = await request(app).patch(`/api/scheduled-tasks/${id}/toggle`).set('Cookie', adminCookie);
|
||||
expect(off.status).toBe(200);
|
||||
expect(off.body.enabled).toBe(0);
|
||||
// The pinned instant survives the disable so it can be restored on re-enable.
|
||||
expect(off.body.next_run_at).toBe(pinned);
|
||||
|
||||
const on = await request(app).patch(`/api/scheduled-tasks/${id}/toggle`).set('Cookie', adminCookie);
|
||||
expect(on.status).toBe(200);
|
||||
expect(on.body.enabled).toBe(1);
|
||||
// Re-enable restores the exact chosen instant, not the cron's annual occurrence.
|
||||
expect(on.body.next_run_at).toBe(pinned);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DELETE /api/scheduled-tasks/:id', () => {
|
||||
@@ -582,6 +660,41 @@ describe('PUT /api/scheduled-tasks/:id - delete_after_run', () => {
|
||||
expect(res2.status).toBe(200);
|
||||
expect(res2.body.delete_after_run).toBe(0);
|
||||
});
|
||||
|
||||
it('pins next_run_at to a re-supplied one-shot run_at on update', async () => {
|
||||
const now = Date.now();
|
||||
const id = DatabaseService.getInstance().createScheduledTask({
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: now + 1000, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1,
|
||||
});
|
||||
|
||||
const runAt = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const res = await request(app)
|
||||
.put(`/api/scheduled-tasks/${id}`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ cron_expression: '0 23 1 7 *', run_at: runAt });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.next_run_at).toBe(runAt);
|
||||
});
|
||||
|
||||
it('rejects an update with a past run_at', async () => {
|
||||
const now = Date.now();
|
||||
const id = DatabaseService.getInstance().createScheduledTask({
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: now + 1000, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1,
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.put(`/api/scheduled-tasks/${id}`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ run_at: Date.now() - 60_000 });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/run_at must be in the future/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PUT /api/scheduled-tasks/:id - stack target validation', () => {
|
||||
|
||||
@@ -163,6 +163,25 @@ function validateCronExpression(cron: unknown): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate the optional explicit fire time. When present it must be a future
|
||||
* epoch-ms integer; absent (undefined/null) leaves the cron-derived next run in
|
||||
* place. By convention only a one-time ('once') schedule populates run_at (the
|
||||
* frontend's getOnceRunAt), because a 5-field cron has no year field and the
|
||||
* cron alone would fire on the next annual occurrence rather than the exact date
|
||||
* the admin chose; this validator itself only checks shape and futureness.
|
||||
*/
|
||||
function validateRunAt(runAt: unknown): string | null {
|
||||
if (runAt === undefined || runAt === null) return null;
|
||||
if (typeof runAt !== 'number' || !Number.isInteger(runAt)) {
|
||||
return 'run_at must be an epoch-millisecond timestamp.';
|
||||
}
|
||||
if (runAt <= Date.now()) {
|
||||
return 'run_at must be in the future.';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export const scheduledTasksRouter = Router();
|
||||
|
||||
scheduledTasksRouter.get('/', (req: Request, res: Response): void => {
|
||||
@@ -201,7 +220,7 @@ scheduledTasksRouter.get('/', (req: Request, res: Response): void => {
|
||||
scheduledTasksRouter.post('/', (req: Request, res: Response): void => {
|
||||
if (!requireAdmin(req, res)) return;
|
||||
try {
|
||||
const { name, target_type, target_id, node_id, action, cron_expression, enabled, prune_targets, target_services, prune_label_filter, delete_after_run } = req.body;
|
||||
const { name, target_type, target_id, node_id, action, cron_expression, enabled, prune_targets, target_services, prune_label_filter, delete_after_run, run_at } = req.body;
|
||||
|
||||
if (!name || typeof name !== 'string' || !name.trim()) {
|
||||
res.status(400).json({ error: 'Name is required' }); return;
|
||||
@@ -227,9 +246,16 @@ scheduledTasksRouter.post('/', (req: Request, res: Response): void => {
|
||||
const cronErr = validateCronExpression(cron_expression);
|
||||
if (cronErr) { res.status(400).json({ error: cronErr }); return; }
|
||||
|
||||
const runAtErr = validateRunAt(run_at);
|
||||
if (runAtErr) { res.status(400).json({ error: runAtErr }); return; }
|
||||
|
||||
const scheduler = SchedulerService.getInstance();
|
||||
const now = Date.now();
|
||||
const nextRun = (enabled !== false) ? scheduler.calculateNextRun(cron_expression) : null;
|
||||
// A one-shot's run_at pins the exact instant (including year); fall back to
|
||||
// the cron-derived next run when no explicit timestamp is supplied.
|
||||
const nextRun = (enabled === false)
|
||||
? null
|
||||
: (typeof run_at === 'number' ? run_at : scheduler.calculateNextRun(cron_expression));
|
||||
const normalizedTargetId = target_type === 'stack' ? target_id : null;
|
||||
const normalizedNodeId = actionRequiresNode(action) ? parsePositiveNodeId(node_id) : null;
|
||||
|
||||
@@ -288,7 +314,7 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => {
|
||||
const existing = db.getScheduledTask(id);
|
||||
if (!existing) { res.status(404).json({ error: 'Scheduled task not found' }); return; }
|
||||
|
||||
const { name, target_type, target_id, node_id, action, cron_expression, enabled, prune_targets, target_services, prune_label_filter, delete_after_run } = req.body;
|
||||
const { name, target_type, target_id, node_id, action, cron_expression, enabled, prune_targets, target_services, prune_label_filter, delete_after_run, run_at } = req.body;
|
||||
|
||||
if (target_type !== undefined && !(VALID_TARGET_TYPES as readonly string[]).includes(target_type)) {
|
||||
res.status(400).json({ error: 'Invalid target_type' }); return;
|
||||
@@ -322,6 +348,9 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => {
|
||||
if (cronErr) { res.status(400).json({ error: cronErr }); return; }
|
||||
}
|
||||
|
||||
const runAtErr = validateRunAt(run_at);
|
||||
if (runAtErr) { res.status(400).json({ error: runAtErr }); return; }
|
||||
|
||||
const updates: Partial<Omit<ScheduledTask, 'id'>> = { updated_at: Date.now() };
|
||||
if (name !== undefined) {
|
||||
if (typeof name !== 'string' || !name.trim()) {
|
||||
@@ -359,7 +388,11 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => {
|
||||
const finalCron = cron_expression || existing.cron_expression;
|
||||
const finalEnabled = enabled !== undefined ? enabled : existing.enabled;
|
||||
if (finalEnabled) {
|
||||
updates.next_run_at = SchedulerService.getInstance().calculateNextRun(finalCron);
|
||||
// A re-supplied one-shot run_at pins the exact instant; otherwise recompute
|
||||
// from the (possibly updated) cron.
|
||||
updates.next_run_at = typeof run_at === 'number'
|
||||
? run_at
|
||||
: SchedulerService.getInstance().calculateNextRun(finalCron);
|
||||
} else {
|
||||
updates.next_run_at = null;
|
||||
}
|
||||
@@ -406,7 +439,19 @@ scheduledTasksRouter.patch('/:id/toggle', (req: Request, res: Response): void =>
|
||||
if (!existing) { res.status(404).json({ error: 'Scheduled task not found' }); return; }
|
||||
|
||||
const newEnabled = existing.enabled ? 0 : 1;
|
||||
const nextRun = newEnabled ? SchedulerService.getInstance().calculateNextRun(existing.cron_expression) : null;
|
||||
// A one-shot's exact instant cannot be reconstructed from its yearless cron,
|
||||
// so preserve its pinned next_run_at across a disable/enable cycle (the
|
||||
// enabled=0 gate stops a disabled task from firing). Recurring tasks clear
|
||||
// next_run_at while disabled and recompute it from the cron on enable.
|
||||
const isOneShot = existing.delete_after_run === 1 && existing.next_run_at != null;
|
||||
let nextRun: number | null;
|
||||
if (isOneShot) {
|
||||
// The pinned instant is kept in both directions: disabling leaves it set
|
||||
// (the enabled=0 gate stops it firing) so re-enabling restores it.
|
||||
nextRun = existing.next_run_at;
|
||||
} else {
|
||||
nextRun = newEnabled ? SchedulerService.getInstance().calculateNextRun(existing.cron_expression) : null;
|
||||
}
|
||||
|
||||
db.updateScheduledTask(id, {
|
||||
enabled: newEnabled,
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
buildCron,
|
||||
parseCron,
|
||||
getSimpleScheduleError,
|
||||
getOnceRunAt,
|
||||
type SimpleSchedule,
|
||||
} from '@/lib/scheduling';
|
||||
import { ScheduleSimplePanel } from './ScheduleSimplePanel';
|
||||
@@ -277,6 +278,11 @@ export default function ScheduledOperationsView({ filterNodeId, onClearFilter, p
|
||||
// backend stores; Advanced mode sends the raw expression as-is.
|
||||
const cronExpression = scheduleMode === 'simple' ? buildCron(simpleSchedule) : formCron;
|
||||
|
||||
// A one-time ('once') Simple schedule pins its exact run instant (including
|
||||
// year) via run_at, because the 5-field cron cannot encode a year. null for
|
||||
// every recurring shape and for Advanced mode, where the cron is authoritative.
|
||||
const runAt = scheduleMode === 'simple' ? getOnceRunAt(simpleSchedule) : null;
|
||||
|
||||
const body: Record<string, unknown> = {
|
||||
name: formName,
|
||||
target_type: actionDef.targetType,
|
||||
@@ -284,6 +290,7 @@ export default function ScheduledOperationsView({ filterNodeId, onClearFilter, p
|
||||
cron_expression: cronExpression,
|
||||
enabled: formEnabled,
|
||||
delete_after_run: formDeleteAfterRun,
|
||||
run_at: runAt,
|
||||
target_id: actionDef.requiresStack ? formTargetId : null,
|
||||
node_id: actionDef.requiresNode && formNodeId ? parseInt(formNodeId, 10) : null,
|
||||
prune_targets: formAction === 'prune' && formPruneTargets.length > 0 ? formPruneTargets : null,
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
buildCron,
|
||||
parseCron,
|
||||
getSimpleScheduleError,
|
||||
getOnceRunAt,
|
||||
type SimpleSchedule,
|
||||
} from './scheduling';
|
||||
|
||||
@@ -140,6 +141,24 @@ describe('parseCron', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getOnceRunAt', () => {
|
||||
it('returns the exact picked instant (date + chosen time) for a once schedule', () => {
|
||||
const runAt = getOnceRunAt(schedule({ frequency: 'once', minute: 30, hour: 23, date: new Date(2027, 6, 1) }));
|
||||
expect(runAt).toBe(new Date(2027, 6, 1, 23, 30, 0, 0).getTime());
|
||||
});
|
||||
|
||||
it('preserves the selected year so a future-year once does not collapse to this year', () => {
|
||||
const runAt = getOnceRunAt(schedule({ frequency: 'once', minute: 0, hour: 9, date: new Date(2030, 0, 15) }));
|
||||
expect(new Date(runAt as number).getFullYear()).toBe(2030);
|
||||
});
|
||||
|
||||
it('returns null for a once schedule with no date and for recurring frequencies', () => {
|
||||
expect(getOnceRunAt(schedule({ frequency: 'once', date: null }))).toBeNull();
|
||||
expect(getOnceRunAt(schedule({ frequency: 'daily' }))).toBeNull();
|
||||
expect(getOnceRunAt(schedule({ frequency: 'monthly', dayOfMonth: 5 }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getSimpleScheduleError', () => {
|
||||
const now = new Date(2026, 5, 28);
|
||||
|
||||
@@ -162,13 +181,24 @@ describe('getSimpleScheduleError', () => {
|
||||
|
||||
it('blocks once with a past date', () => {
|
||||
expect(getSimpleScheduleError(schedule({ frequency: 'once', date: new Date(2026, 5, 1) }), now))
|
||||
.toBe('The selected date is in the past and this schedule would never fire.');
|
||||
.toBe('The selected date and time are in the past and this schedule would never fire.');
|
||||
});
|
||||
|
||||
it('passes once with a future date', () => {
|
||||
expect(getSimpleScheduleError(schedule({ frequency: 'once', date: new Date(2026, 11, 25) }), now)).toBeNull();
|
||||
});
|
||||
|
||||
it('blocks once for a time earlier today that has already passed', () => {
|
||||
const noon = new Date(2026, 5, 28, 12, 0);
|
||||
expect(getSimpleScheduleError(schedule({ frequency: 'once', date: new Date(2026, 5, 28), hour: 1, minute: 0 }), noon))
|
||||
.toBe('The selected date and time are in the past and this schedule would never fire.');
|
||||
});
|
||||
|
||||
it('passes once for a later time today that has not yet passed', () => {
|
||||
const noon = new Date(2026, 5, 28, 12, 0);
|
||||
expect(getSimpleScheduleError(schedule({ frequency: 'once', date: new Date(2026, 5, 28), hour: 23, minute: 0 }), noon)).toBeNull();
|
||||
});
|
||||
|
||||
it('blocks an invalid time', () => {
|
||||
expect(getSimpleScheduleError(schedule({ frequency: 'daily', minute: Number.NaN }), now)).toBe('Enter a valid time.');
|
||||
});
|
||||
|
||||
@@ -70,6 +70,19 @@ export function buildCron(s: SimpleSchedule): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Absolute epoch-ms fire time for a one-time ('once') schedule: the picked date
|
||||
* at the chosen hour and minute. Returns null when the schedule is not 'once' or
|
||||
* has no date. A 5-field cron cannot encode a year, so a one-shot sends this
|
||||
* explicit timestamp to the backend to pin the exact run (year and time of day);
|
||||
* relying on the cron alone fires on the next annual occurrence, which can be a
|
||||
* different year than the date the admin selected.
|
||||
*/
|
||||
export function getOnceRunAt(s: SimpleSchedule): number | null {
|
||||
if (s.frequency !== 'once' || !s.date) return null;
|
||||
return new Date(s.date.getFullYear(), s.date.getMonth(), s.date.getDate(), s.hour, s.minute, 0, 0).getTime();
|
||||
}
|
||||
|
||||
function parseIntField(field: string, min: number, max: number): number | null {
|
||||
if (!/^\d+$/.test(field)) return null;
|
||||
const n = Number(field);
|
||||
@@ -167,9 +180,14 @@ export function getSimpleScheduleError(s: SimpleSchedule, now: Date = new Date()
|
||||
}
|
||||
if (s.frequency === 'once') {
|
||||
if (!s.date) return 'Select a date.';
|
||||
const picked = new Date(s.date.getFullYear(), s.date.getMonth(), s.date.getDate()).getTime();
|
||||
const today = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime();
|
||||
if (picked < today) return 'The selected date is in the past and this schedule would never fire.';
|
||||
// Compare the full chosen instant (date + time), not just the day: a time
|
||||
// earlier today has already passed. The backend also rejects a past run_at
|
||||
// with a 400; this is the friendlier, save-blocking guard surfaced before
|
||||
// the request is sent.
|
||||
const when = getOnceRunAt(s);
|
||||
if (when !== null && when <= now.getTime()) {
|
||||
return 'The selected date and time are in the past and this schedule would never fire.';
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user