From f11859e7596bc8c8f4b739da54130a394d19f7f6 Mon Sep 17 00:00:00 2001 From: Alex Knight Date: Fri, 1 May 2026 21:47:05 +1000 Subject: [PATCH] chore: clean up redundant code comments --- ...t-runner.active-hours-schedule.e2e.test.ts | 79 ++++--------------- src/infra/heartbeat-runner.ts | 8 +- src/infra/heartbeat-schedule.test.ts | 28 +------ src/infra/heartbeat-schedule.ts | 27 +++---- 4 files changed, 30 insertions(+), 112 deletions(-) diff --git a/src/infra/heartbeat-runner.active-hours-schedule.e2e.test.ts b/src/infra/heartbeat-runner.active-hours-schedule.e2e.test.ts index e6997e17525..4f90a07d68f 100644 --- a/src/infra/heartbeat-runner.active-hours-schedule.e2e.test.ts +++ b/src/infra/heartbeat-runner.active-hours-schedule.e2e.test.ts @@ -4,14 +4,7 @@ import { startHeartbeatRunner } from "./heartbeat-runner.js"; import { computeNextHeartbeatPhaseDueMs, resolveHeartbeatPhaseMs } from "./heartbeat-schedule.js"; import { resetHeartbeatWakeStateForTests } from "./heartbeat-wake.js"; -/** - * E2E tests for heartbeat active-hours-aware scheduling (#75487). - * - * Verifies that the scheduler seeks forward through phase-aligned slots to - * find the first one that falls within the configured activeHours window, - * rather than arming a timer for a quiet-hours slot and relying solely on - * the runtime execution guard to skip it. - */ +/** Verifies that the scheduler seeks to in-window phase slots (#75487). */ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { type RunOnce = Parameters[0]["runOnce"]; const TEST_SCHEDULER_SEED = "heartbeat-ah-schedule-test-seed"; @@ -58,12 +51,7 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); it("skips quiet-hours slots and fires at the first in-window phase slot", async () => { - // Active window: 09:00–17:00 UTC. Interval: 4h. - // Start the clock at 16:30 UTC — the next raw phase slot will be computed - // from this time. For a 4h interval the phase-aligned slots repeat every - // 4h. We resolve the first raw due, assert it falls outside the active - // window, then verify the runner arms its timer for the first in-window - // slot (which must be >= 09:00 the next day). + // 09:00–17:00 UTC, 4h interval. Start at 16:30 — raw due is after 17:00. const startMs = Date.parse("2026-06-15T16:30:00.000Z"); useFakeHeartbeatTime(startMs); @@ -83,26 +71,17 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { stableSchedulerSeed: TEST_SCHEDULER_SEED, }); - // Compute what the raw (timezone-unaware) first due would be. const rawDueMs = resolveDueFromNow(startMs, intervalMs, "main"); - // Advance past the raw due — the scheduler should NOT fire because that - // slot falls outside active hours (it's after 17:00). + // Advance past the raw due — should NOT fire (quiet hours). await vi.advanceTimersByTimeAsync(rawDueMs - startMs + 1); - // If the scheduler is timezone-aware, it shouldn't have fired at the raw - // quiet-hours slot. It might have already found and armed the first - // in-window slot. - // Now advance to 09:01 on the next day plus enough time for any phase - // offset — the scheduler should fire within the active window. - const nextDay0900 = Date.parse("2026-06-16T09:00:00.000Z"); + // Advance to end of next day's window — should fire within 09:00–17:00. const safeEndOfWindow = Date.parse("2026-06-16T17:00:00.000Z"); await vi.advanceTimersByTimeAsync(safeEndOfWindow - Date.now()); - // The first call must have happened within the active window. expect(runSpy).toHaveBeenCalled(); - const firstCallTime = callTimes[0]!; - const firstCallHourUTC = new Date(firstCallTime).getUTCHours(); + const firstCallHourUTC = new Date(callTimes[0]!).getUTCHours(); expect(firstCallHourUTC).toBeGreaterThanOrEqual(9); expect(firstCallHourUTC).toBeLessThan(17); @@ -110,8 +89,6 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); it("fires immediately when the first phase slot is already within active hours", async () => { - // Active window: 08:00–20:00 UTC. Interval: 4h. - // Start at 10:00 UTC — the first phase slot is within active hours. const startMs = Date.parse("2026-06-15T10:00:00.000Z"); useFakeHeartbeatTime(startMs); @@ -128,8 +105,6 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); const rawDueMs = resolveDueFromNow(startMs, intervalMs, "main"); - - // The raw due should be within active hours — advance to it. await vi.advanceTimersByTimeAsync(rawDueMs - startMs + 1); expect(runSpy).toHaveBeenCalledTimes(1); @@ -137,9 +112,8 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); it("seeks forward correctly with a non-UTC timezone (e.g. America/New_York)", async () => { - // Active window: 09:00–17:00 America/New_York (EDT = UTC-4 in June). - // So active hours in UTC = 13:00–21:00. - // Interval: 4h. Start at 21:30 UTC (17:30 ET = outside window). + // 09:00–17:00 ET (EDT = UTC-4 in June) → 13:00–21:00 UTC. + // Start at 21:30 UTC (17:30 ET = outside window). const startMs = Date.parse("2026-06-15T21:30:00.000Z"); useFakeHeartbeatTime(startMs); @@ -159,15 +133,10 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { stableSchedulerSeed: TEST_SCHEDULER_SEED, }); - // Advance through two full days to capture the first fire. await vi.advanceTimersByTimeAsync(48 * 60 * 60_000); expect(runSpy).toHaveBeenCalled(); - // Verify the first call was within the ET active window. - // 09:00 ET = 13:00 UTC, 17:00 ET = 21:00 UTC (during EDT, June). - const firstCallTime = callTimes[0]!; - const firstCallHourUTC = new Date(firstCallTime).getUTCHours(); - // In ET active hours → UTC 13:00–21:00 + const firstCallHourUTC = new Date(callTimes[0]!).getUTCHours(); expect(firstCallHourUTC).toBeGreaterThanOrEqual(13); expect(firstCallHourUTC).toBeLessThan(21); @@ -175,10 +144,7 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); it("advances to in-window slot after a quiet-hours skip during interval runs", async () => { - // Active window: 09:00–17:00 UTC. Interval: 4h. - // Start at 09:00 UTC — first fire is within window. - // After the first fire, the next raw slot may fall outside the window. - // Verify the scheduler seeks forward past quiet-hours slots. + // 09:00–17:00 UTC, 4h interval. Verify ALL fires over 48h stay in-window. const startMs = Date.parse("2026-06-15T09:00:00.000Z"); useFakeHeartbeatTime(startMs); @@ -198,10 +164,8 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { stableSchedulerSeed: TEST_SCHEDULER_SEED, }); - // Advance through 48 hours — collect all fire times. await vi.advanceTimersByTimeAsync(48 * 60 * 60_000); - // Every single fire must be within 09:00–17:00 UTC. expect(callTimes.length).toBeGreaterThan(0); for (const t of callTimes) { const hour = new Date(t).getUTCHours(); @@ -218,8 +182,7 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }); it("does not loop indefinitely when activeHours window is zero-width", async () => { - // start === end means never-active. The scheduler should arm at the raw - // slot (fallback) and the runtime guard will skip execution. + // start === end → never-active; seek falls back, runtime guard skips. const startMs = Date.parse("2026-06-15T10:00:00.000Z"); useFakeHeartbeatTime(startMs); @@ -234,20 +197,15 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { stableSchedulerSeed: TEST_SCHEDULER_SEED, }); - // Advance 2 hours — the scheduler should not hang or spin. await vi.advanceTimersByTimeAsync(2 * 60 * 60_000); - // The runner fires because start === end returns false from - // isWithinActiveHours, so the seek falls back to the raw slot. - // runOnce returns quiet-hours skip each time. expect(runSpy).toHaveBeenCalled(); runner.stop(); }); it("recomputes schedule when activeHours config changes via hot reload", async () => { - // Start with a narrow window that pushes nextDueMs far ahead. - // Then widen the window via updateConfig — the scheduler should - // recompute from `now` instead of keeping the stale far-future slot. + // Narrow window pushes nextDueMs to tomorrow; widening via updateConfig + // must recompute from `now` so the timer fires today. const startMs = Date.parse("2026-06-15T14:00:00.000Z"); useFakeHeartbeatTime(startMs); @@ -258,8 +216,6 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { return { status: "ran", durationMs: 1 }; }); - // Narrow window: 09:00–10:00 UTC. At 14:00 UTC the next in-window - // slot is tomorrow ~09:xx (19+ hours away). const runner = startHeartbeatRunner({ cfg: heartbeatConfig({ every: "4h", @@ -269,12 +225,10 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { stableSchedulerSeed: TEST_SCHEDULER_SEED, }); - // Advance 1 hour — should NOT fire (next slot is tomorrow). await vi.advanceTimersByTimeAsync(60 * 60_000); expect(runSpy).not.toHaveBeenCalled(); - // Hot-reload: widen window to 08:00–20:00 UTC. - // At 15:00 UTC the next phase slot should now be reachable within hours. + // Widen window — scheduler must recompute, not keep stale tomorrow slot. runner.updateConfig( heartbeatConfig({ every: "4h", @@ -282,15 +236,12 @@ describe("heartbeat scheduler: activeHours-aware scheduling (#75487)", () => { }), ); - // Advance another 8 hours — should fire within the widened window. await vi.advanceTimersByTimeAsync(8 * 60 * 60_000); expect(runSpy).toHaveBeenCalled(); - const firstCallTime = callTimes[0]!; - const firstCallHour = new Date(firstCallTime).getUTCHours(); + const firstCallHour = new Date(callTimes[0]!).getUTCHours(); expect(firstCallHour).toBeGreaterThanOrEqual(8); expect(firstCallHour).toBeLessThan(20); - // Crucially, the first fire should be today (June 15), not tomorrow. - expect(new Date(firstCallTime).getUTCDate()).toBe(15); + expect(new Date(callTimes[0]!).getUTCDate()).toBe(15); // today, not tomorrow runner.stop(); }); diff --git a/src/infra/heartbeat-runner.ts b/src/infra/heartbeat-runner.ts index ec7df6afe06..5b3708b83a4 100644 --- a/src/infra/heartbeat-runner.ts +++ b/src/infra/heartbeat-runner.ts @@ -226,7 +226,6 @@ type HeartbeatAgentState = { type ActiveHoursWindow = NonNullable["activeHours"]; -/** Shallow equality for the three scheduling-relevant activeHours fields. */ function activeHoursConfigMatch(a?: ActiveHoursWindow, b?: ActiveHoursWindow): boolean { if (a === b) return true; if (!a || !b) return false; @@ -1913,11 +1912,8 @@ export function startHeartbeatRunner(opts: { }); intervals.push(intervalMs); const prevState = prevAgents.get(agent.agentId); - // When activeHours config changes, discard the preserved nextDueMs so - // the scheduler recomputes from `now` instead of keeping a stale slot - // that was pushed far ahead by the old window. resolveNextDue only - // compares intervalMs/phaseMs, so we null-out prevState when the - // scheduling-relevant active-hours fields differ. + // resolveNextDue only compares intervalMs/phaseMs, so discard + // prevState when activeHours changed to avoid a stale far-future slot. const ahChanged = prevState && !activeHoursConfigMatch(prevState.heartbeat?.activeHours, agent.heartbeat?.activeHours); diff --git a/src/infra/heartbeat-schedule.test.ts b/src/infra/heartbeat-schedule.test.ts index 126e3c8ab3d..9f01e459c25 100644 --- a/src/infra/heartbeat-schedule.test.ts +++ b/src/infra/heartbeat-schedule.test.ts @@ -90,9 +90,7 @@ describe("seekNextActivePhaseDueMs", () => { }); it("skips quiet-hours slots and returns the first in-window slot", () => { - // Active window: 08:00 – 17:00 UTC - // Interval: 4h, start slot at 19:00 UTC (quiet hours) - // Next slots: 23:00 (quiet), 03:00 (quiet), 07:00 (quiet), 11:00 (active!) + // 08:00–17:00 UTC, 4h interval, start at 19:00 (quiet). const startMs = Date.parse("2026-01-01T19:00:00.000Z"); const intervalMs = 4 * HOUR; const isActive = (ms: number) => { @@ -107,14 +105,11 @@ describe("seekNextActivePhaseDueMs", () => { isActive, }); - // 19:00 + 4h = 23:00 (skip) + 4h = 03:00 (skip) + 4h = 07:00 (skip) + 4h = 11:00 expect(result).toBe(Date.parse("2026-01-02T11:00:00.000Z")); }); it("handles overnight active windows correctly", () => { - // Active window: 22:00 – 06:00 UTC (overnight) - // Interval: 4h, start slot at 10:00 UTC (quiet hours) - // Next: 14:00 (quiet), 18:00 (quiet), 22:00 (active!) + // 22:00–06:00 UTC (overnight), 4h interval, start at 10:00 (quiet). const startMs = Date.parse("2026-01-01T10:00:00.000Z"); const intervalMs = 4 * HOUR; const isActive = (ms: number) => { @@ -133,7 +128,6 @@ describe("seekNextActivePhaseDueMs", () => { }); it("falls back to startMs when no slot is active within the seek horizon", () => { - // All slots are outside active hours (isActive always returns false) const startMs = Date.parse("2026-01-01T10:00:00.000Z"); const result = seekNextActivePhaseDueMs({ startMs, @@ -146,16 +140,12 @@ describe("seekNextActivePhaseDueMs", () => { }); it("seeks across timezone-aware active hours using isWithinActiveHours semantics", () => { - // Simulate Asia/Shanghai: active 08:00-23:00 local = 00:00-15:00 UTC - // Interval: 4h, phase slot at 15:21 UTC (23:21 Shanghai = quiet) - // Next: 19:21 UTC (03:21 Shanghai = quiet), 23:21 UTC (07:21 = quiet), - // 03:21 UTC (11:21 Shanghai = active!) + // Asia/Shanghai (UTC+8): active 08:00–23:00 local. const startMs = Date.parse("2026-01-01T15:21:00.000Z"); const intervalMs = 4 * HOUR; const shanghaiOffsetMs = 8 * HOUR; const isActive = (ms: number) => { - // Simulate Asia/Shanghai = UTC+8, active 08:00-23:00 const shanghaiMs = ms + shanghaiOffsetMs; const shanghaiHour = new Date(shanghaiMs).getUTCHours(); return shanghaiHour >= 8 && shanghaiHour < 23; @@ -168,13 +158,11 @@ describe("seekNextActivePhaseDueMs", () => { isActive, }); - // 15:21 + 4h = 19:21 (skip) + 4h = 23:21 (skip) + 4h = 03:21 UTC = 11:21 Shanghai expect(result).toBe(Date.parse("2026-01-02T03:21:00.000Z")); }); it("handles very short intervals efficiently", () => { - // 30-minute interval, active window 09:00-17:00 - // Start at 17:00 (quiet), should find 09:00 next day + // 30m interval, 09:00–17:00. Start at 17:00 (quiet) → 09:00 next day. const startMs = Date.parse("2026-01-01T17:00:00.000Z"); const intervalMs = 30 * 60_000; const isActive = (ms: number) => { @@ -189,13 +177,10 @@ describe("seekNextActivePhaseDueMs", () => { isActive, }); - // Should skip 32 half-hour slots (17:00 through 08:30) to reach 09:00 next day expect(result).toBe(Date.parse("2026-01-02T09:00:00.000Z")); }); it("caps iterations for pathological sub-second intervals", () => { - // 1ms interval with always-false predicate — without the iteration cap - // this would loop ~604 million times. The cap (10 080) prevents that. const startMs = Date.parse("2026-01-01T12:00:00.000Z"); const t0 = performance.now(); const result = seekNextActivePhaseDueMs({ @@ -206,14 +191,11 @@ describe("seekNextActivePhaseDueMs", () => { }); const elapsedMs = performance.now() - t0; - // Falls back to startMs (runtime guard will handle it). expect(result).toBe(startMs); - // Must complete quickly — without the cap this would take minutes. expect(elapsedMs).toBeLessThan(500); }); it("handles intervalMs larger than the seek horizon", () => { - // 8-day interval — only the startMs candidate is checked within horizon. const startMs = Date.parse("2026-01-01T03:00:00.000Z"); const eightDays = 8 * 24 * HOUR; const result = seekNextActivePhaseDueMs({ @@ -226,8 +208,6 @@ describe("seekNextActivePhaseDueMs", () => { }, }); - // startMs (03:00) is outside 09:00–17:00. The next candidate would be - // 8 days later which is past the 7-day horizon. Falls back to startMs. expect(result).toBe(startMs); }); diff --git a/src/infra/heartbeat-schedule.ts b/src/infra/heartbeat-schedule.ts index d9c3da59b96..6df73e1e0f0 100644 --- a/src/infra/heartbeat-schedule.ts +++ b/src/infra/heartbeat-schedule.ts @@ -60,23 +60,16 @@ export function resolveNextHeartbeatDueMs(params: { /** * Seek forward through phase-aligned slots until one falls within the active - * hours window. Returns the first in-window slot, or falls back to the raw - * next slot when no active hours are configured or no in-window slot is found - * within the seek horizon. + * hours window. Falls back to the raw next slot when no predicate is provided + * or no in-window slot is found within the seek horizon. * - * `isActive` is a predicate that mirrors `isWithinActiveHours` — the caller - * binds the config/heartbeat so this module stays config-agnostic. - * - * `phaseMs` is accepted for call-site symmetry but unused: the caller passes - * a phase-aligned `startMs`, and advancing by `intervalMs` preserves alignment. + * The caller binds config/heartbeat into `isActive` so this module stays + * config-agnostic. `phaseMs` is unused — alignment is preserved because + * `startMs` is already phase-aligned and `intervalMs` addition maintains it. */ -const MAX_SEEK_HORIZON_MS = 7 * 24 * 60 * 60_000; // 7 days - -// Cap iterations so pathological sub-minute intervals (e.g. "1s", "1ms") -// cannot block the event loop. 10 080 = 7 days at 1-minute steps — generous -// enough for any reasonable config. Pathological configs fall back to the raw -// slot where the runtime guard still gates execution. -const MAX_SEEK_ITERATIONS = 10_080; +const MAX_SEEK_HORIZON_MS = 7 * 24 * 60 * 60_000; +// Prevent pathological sub-minute intervals from blocking the event loop. +const MAX_SEEK_ITERATIONS = 10_080; // 7 days at 1-minute steps export function seekNextActivePhaseDueMs(params: { startMs: number; @@ -99,8 +92,6 @@ export function seekNextActivePhaseDueMs(params: { candidateMs += intervalMs; iterations++; } - // All slots within the seek horizon (or iteration cap) fall outside active - // hours — return the raw first slot so the runtime execution guard can - // still gate it. + // No in-window slot found; fall back so the runtime guard can gate it. return params.startMs; }