a5b3cc98b0
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The routine scheduler lets agents and users run work on cron schedules; every 30s a tick computes each trigger's next occurrence > - `computeNextRun`/`nextCronTickInTimeZone` find that occurrence by stepping forward one minute at a time (capped at 2.6M iterations), constructing a fresh `Intl.DateTimeFormat` on every step — ~1ms of ICU work each > - Sparse schedules (monthly ≈ 43k steps ≈ 40s) and never-matching crons (the #7529 midnight bug → full 2.6M steps ≈ 45 min) block the Node event loop for the whole scan, every tick > - In production this pegs the server at 100% CPU, health checks time out, and Paperclip Desktop repeatedly shows "the embedded server is no longer responding" (diagnosed via a CPU sample: 74% of samples inside `Builtin_DateTimeFormatConstructor`) > - This pull request caches the formatter per timezone, since `Intl.DateTimeFormat` instances are immutable and reusable > - The benefit is each minute-step pays only `formatToParts` (~1µs): a 43k-step scan drops from ~40s of blocked event loop to under a second, and the server stays responsive while routines are active ## Linked Issues or Issue Description - Fixes: #8033 - Refs #7529 — a never-matching midnight cron forces the minute-stepper through its full 2.6M-iteration cap, which is the worst-case trigger for this perf bug; the two compound - Refs #7922 — in-flight fix for #7529 touching the same function (`getZonedMinuteParts`); the changes are compatible (this PR changes formatter construction, that PR changes hour normalization) ## What Changed - `server/src/services/routines.ts`: added a per-timezone `Intl.DateTimeFormat` cache (`getZonedMinuteFormatter`) used by `getZonedMinuteParts` and `assertTimeZone`, replacing per-call construction - Exported `nextCronTickInTimeZone` so the behavior is testable (same export PR #7922 makes) - Added `server/src/services/routines-formatter-cache.test.ts`: verifies a sparse monthly cron resolves to the correct next occurrence across a DST-bearing timezone, and asserts at most one formatter construction for a ~43k-minute-step scan (and zero on a warm cache) ## Verification - `npx vitest run server/src/services/routines-formatter-cache.test.ts` — 2 tests pass - `pnpm --filter @paperclipai/server exec tsc --noEmit` — clean - Live validation: applied the same patch to the server bundled in Paperclip Desktop 3.2.9, which was hitting 99.3% CPU with 5s health-check timeouts within 60s of boot on a real workload; after the patch, CPU idles at 0–3% across scheduler ticks and `/api/health` answers in ~1ms (observed over multiple 30s ticks) ## Risks - Low risk. `Intl.DateTimeFormat` instances are immutable and safe to reuse; the cache key is the timezone string, and entries are small and bounded by the number of distinct timezones in use - Invalid timezones still throw in the constructor before anything is cached, so `assertTimeZone` semantics are unchanged - Does not change cron matching semantics; minute-stepping remains (replacing it with cron-field arithmetic is noted in #8033 as a follow-up) ## Model Used - Claude (Anthropic), model ID `claude-fable-5` (Fable 5), via Claude Code CLI with extended thinking and tool use (profiling, patching, and live verification performed by the model under user supervision) ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — server-only change) - [ ] I have updated relevant documentation to reflect my changes (N/A — internal perf fix, no doc surface) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
52 lines
2.0 KiB
TypeScript
52 lines
2.0 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import { nextCronTickInTimeZone } from "./routines.js";
|
|
|
|
describe("nextCronTickInTimeZone formatter caching", () => {
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
it("computes the next occurrence of a sparse monthly cron correctly", () => {
|
|
const next = nextCronTickInTimeZone(
|
|
"30 6 1 * *",
|
|
"America/New_York",
|
|
new Date("2026-06-02T00:00:00Z"),
|
|
);
|
|
expect(next).not.toBeNull();
|
|
// 06:30 America/New_York on July 1st is 10:30 UTC (EDT, UTC-4).
|
|
expect(next!.toISOString()).toBe("2026-07-01T10:30:00.000Z");
|
|
});
|
|
|
|
it("does not construct a new Intl.DateTimeFormat per minute-step", () => {
|
|
const RealDateTimeFormat = Intl.DateTimeFormat;
|
|
let constructions = 0;
|
|
// Must be a `function` (not an arrow) so the mock stays constructable
|
|
// when the code under test calls `new Intl.DateTimeFormat(...)`.
|
|
vi.spyOn(Intl, "DateTimeFormat").mockImplementation(function (
|
|
...args: ConstructorParameters<typeof Intl.DateTimeFormat>
|
|
) {
|
|
constructions += 1;
|
|
return new RealDateTimeFormat(...args);
|
|
} as unknown as typeof Intl.DateTimeFormat);
|
|
|
|
// A monthly cron scanned from just after the previous firing walks tens of
|
|
// thousands of minute-steps before it matches. Use a timezone that nothing
|
|
// else in this test file has warmed so cache misses are observable.
|
|
const next = nextCronTickInTimeZone(
|
|
"0 12 1 * *",
|
|
"Pacific/Kiritimati",
|
|
new Date("2026-06-01T12:01:00Z"),
|
|
);
|
|
expect(next).not.toBeNull();
|
|
|
|
// One construction to populate the per-timezone cache; the ~43k subsequent
|
|
// minute-steps must reuse it. Before the cache this was one construction
|
|
// per step, which blocked the event loop for minutes (#8033).
|
|
expect(constructions).toBeLessThanOrEqual(1);
|
|
|
|
constructions = 0;
|
|
nextCronTickInTimeZone("0 12 1 * *", "Pacific/Kiritimati", new Date("2026-06-01T12:01:00Z"));
|
|
expect(constructions).toBe(0);
|
|
});
|
|
});
|