Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/schedule-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,25 @@ export interface SchedulerOptions {
}

const activeTasks = new Map<string, ScheduledTask>();
const activeTimers = new Map<string, ReturnType<typeof setTimeout>>();
export const activeTimers = new Map<string, ReturnType<typeof setTimeout>>();
const runningJobs = new Set<string>();

// setTimeout's delay is a 32-bit signed int under the hood — anything past
// this fires almost immediately instead of waiting (Node clamps it to 1ms
// and emits a TimeoutOverflowWarning). Chunk long waits into safe segments.
const MAX_TIMEOUT_MS = 2_147_483_647;

export function scheduleAt(id: string, targetTime: number, onDue: () => void): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scheduleAt is exported and testable in isolation — this function needs unit tests before merge. Jest/Vitest fake timers make it straightforward: advance by MAX_TIMEOUT_MS in the long-delay case and confirm the callback fires exactly once at the right time, not before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test/schedule-timer-overflow.test.ts covering all 4 cases: delay
under the limit (fires once), delay just over it (fires via two chunks,
once, at the right time), an already-past target (fires synchronously),
and cancellation mid-chunk (clearTimeout correctly blocks the callback).

Used the existing node:test + mock.timers setup (matches sdk.test.ts /
telemetry.test.ts) rather than Jest/Vitest, so no new dependency needed.

npm run build and npm test both pass — 31/31, no regressions.

const remaining = targetTime - Date.now();
if (remaining <= 0) {
activeTimers.delete(id);
onDue();
return;
}
const timer = setTimeout(() => scheduleAt(id, targetTime, onDue), Math.min(remaining, MAX_TIMEOUT_MS));
activeTimers.set(id, timer);
}

export async function startScheduler(opts: SchedulerOptions): Promise<void> {
const schedules = await discoverSchedules(opts.agentDir);
let activeCount = 0;
Expand All @@ -33,10 +49,9 @@ export async function startScheduler(opts: SchedulerOptions): Promise<void> {
console.log(dim(`[scheduler] "${schedule.id}" runAt is in the past — skipping`));
continue;
}
const timer = setTimeout(() => {
scheduleAt(schedule.id, new Date(schedule.runAt).getTime(), () => {
executeScheduledJob(schedule, opts, true);
}, delay);
activeTimers.set(schedule.id, timer);
});
const when = new Date(schedule.runAt).toLocaleString();
console.log(dim(`[scheduler] "${schedule.id}" scheduled once at ${when} (in ${Math.round(delay / 1000)}s)`));
activeCount++;
Expand Down
92 changes: 92 additions & 0 deletions test/schedule-timer-overflow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { describe, it, before } from "node:test";
import assert from "node:assert/strict";

let scheduleAt: typeof import("../dist/schedule-runner.js").scheduleAt;
let activeTimers: typeof import("../dist/schedule-runner.js").activeTimers;

before(async () => {
const mod = await import("../dist/schedule-runner.js");
scheduleAt = mod.scheduleAt;
activeTimers = mod.activeTimers;
});

const MAX_TIMEOUT_MS = 2_147_483_647;

describe("scheduleAt (timer overflow guard)", () => {
it("delay < MAX_TIMEOUT_MS: fires in a single setTimeout, callback called exactly once", (t) => {
t.mock.timers.enable({ apis: ["setTimeout", "Date"] });

const start = Date.now();
const delay = 10 * 24 * 60 * 60 * 1000; // 10 days — well under the limit
const target = start + delay;

let fireCount = 0;
scheduleAt("short-job", target, () => fireCount++);

assert.equal(fireCount, 0, "must not fire before the delay elapses");
t.mock.timers.tick(delay);
assert.equal(fireCount, 1, "must fire exactly once once the delay elapses");
});

it("delay slightly above MAX_TIMEOUT_MS: fires via two chunks, callback called once, at the right time", (t) => {
t.mock.timers.enable({ apis: ["setTimeout", "Date"] });

const start = Date.now();
const delay = MAX_TIMEOUT_MS + 1000; // just over the limit — needs a 2nd chunk
const target = start + delay;

let fireCount = 0;
let firedAt = 0;
scheduleAt("two-chunk-job", target, () => {
fireCount++;
firedAt = Date.now();
});

// After the first chunk (exactly MAX_TIMEOUT_MS), it must not have fired yet —
// there's still 1000ms of real delay left.
t.mock.timers.tick(MAX_TIMEOUT_MS);
assert.equal(fireCount, 0, "must not fire after only the first chunk");

// The remaining 1000ms is the second chunk.
t.mock.timers.tick(1000);
assert.equal(fireCount, 1, "must fire exactly once after the second chunk");
assert.equal(firedAt, target, "must fire exactly at the real target time");
});

it("delay <= 0 at call time: callback fires synchronously, no timer scheduled", (t) => {
t.mock.timers.enable({ apis: ["setTimeout", "Date"] });

const now = Date.now();
let fired = false;
scheduleAt("past-job", now - 1000, () => {
fired = true;
});

assert.equal(fired, true, "must fire immediately when the target time is already in the past");
assert.equal(activeTimers.has("past-job"), false, "must not leave a dangling timer entry");
});

it("cancel mid-chunk: clearing the activeTimers entry prevents the callback from firing", (t) => {
t.mock.timers.enable({ apis: ["setTimeout", "Date"] });

const start = Date.now();
const delay = MAX_TIMEOUT_MS + 1000; // needs 2 chunks, so there's a "mid-chunk" to cancel during
const target = start + delay;

let fired = false;
scheduleAt("cancelled-job", target, () => {
fired = true;
});

// Cancel after the first chunk has elapsed, before the second chunk's callback fires.
t.mock.timers.tick(MAX_TIMEOUT_MS);
const timer = activeTimers.get("cancelled-job");
assert.ok(timer, "a pending timer must exist mid-chunk");
clearTimeout(timer);
activeTimers.delete("cancelled-job");

// Advance past the point where it would have fired, if not cancelled.
t.mock.timers.tick(1000);
assert.equal(fired, false, "must not fire after being cancelled mid-chunk");
});
});