chore: clean up redundant code comments

This commit is contained in:
Alex Knight
2026-05-01 21:47:05 +10:00
committed by clawsweeper
parent 24d3da402f
commit f11859e759
4 changed files with 30 additions and 112 deletions

View File

@@ -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<typeof startHeartbeatRunner>[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:0017: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:0017: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:0017: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:0020: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:0017:00 America/New_York (EDT = UTC-4 in June).
// So active hours in UTC = 13:0021:00.
// Interval: 4h. Start at 21:30 UTC (17:30 ET = outside window).
// 09:0017:00 ET (EDT = UTC-4 in June) → 13:0021: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:0021: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:0017: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:0017: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:0017: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:0010: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:0020: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();
});

View File

@@ -226,7 +226,6 @@ type HeartbeatAgentState = {
type ActiveHoursWindow = NonNullable<HeartbeatConfig>["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);

View File

@@ -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:0017: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:0006: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:0023: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:0017: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:0017:00. The next candidate would be
// 8 days later which is past the 7-day horizon. Falls back to startMs.
expect(result).toBe(startMs);
});

View File

@@ -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;
}