Scheduler component plugin: config-declared cron and interval jobs that run once per cluster

[jcohen-hdb]

Closes #951.

Summary

Components can now declare recurring jobs in their config and have core invoke a designated export on that schedule, exactly once per cluster:

scheduler:
  jobs:
    - name: daily-metrics-snapshot
      cron: '0 2 * * *'
      timezone: America/New_York   # optional; defaults to the server timezone
      handler: ./jobs.ts#snapshotMetrics
    - name: sync-exchange-rates
      interval: 15m                # simple cadence for scheduled pulls
      handler: ./jobs.ts#syncExchangeRates

• resources/scheduler/CronExpression.ts — dependency-free five-field cron parser/evaluator (lists, ranges, steps, names, @daily macros, standard day-of-month/day-of-week OR rule), evaluated in an IANA timezone with explicit DST-gap/overlap handling. No new npm dependency.
• resources/scheduler/engine.ts — cluster coordination: deterministic leader election (alphabetical, sticky), heartbeat lease in a new replicating system table hdb_scheduler_state, failover watcher with an escalation ladder (a dead preferred node can't deadlock the cluster), missed-occurrence catch-up sweep, per-job timers with single-flight. Timing thresholds are env-overridable for multi-node testing.
• resources/scheduler/scheduler.ts — the handleApplication plugin (registered in TRUSTED_RESOURCE_PLUGINS): config validation with descriptive errors at load time, handler resolution via scope.import in the component's realm, scope.on('close') teardown. Registration work only under the loader lock; election/scheduling happens async.

The design is a port of the @harperfast/scheduler plugin (HarperFast/scheduler) that has run in production on a two-node Fabric cluster for months — the election/failover/catch-up semantics are battle-tested; the config surface, cron parser, and core integration are new. interval: is included alongside cron: because the motivating use case in #951's comments (summary-tree's periodic maintenance pass) is interval-shaped and inexpressible in five-field cron.

Relationship to #754: schedule computation is deliberately decoupled from timer firing — the setTimeout layer is one seam that the durable timer service can replace later without touching the config surface, election, or catch-up.

Design notes (checked against harper-pro's replication layer)

Before finalizing the election design, we swept harper-pro's replication component for primitives to reuse. Findings, so reviewers don't have to re-derive them:

• No election/consensus primitive and no cross-node CAS exist anywhere (conflict handling is CRDT version convergence). A lease acquired by plain put therefore cannot be race-free by construction — concurrent lease writes both commit and converge by version. Sticky leadership + the heartbeat takeover check + idempotent handlers is the design that matches that substrate, not a workaround.
• The alphabetical-node-name tie-break intentionally mirrors replication's own deterministic failover convention (subscriptionManager.ts sorted-name failover walk).
• The 5-minute stale threshold sits deliberately above replication's LIVENESS_STALE_MS (120s), and inherits the same bounded-clock-skew assumption the CRDT versioning already makes.
• Topology caveat: the lease lives in the system database and assumes it replicates among scheduler-eligible nodes. On constrained/directional topologies (excludeTables, directional replicates), nodes that never see the leader's heartbeat would each self-promote — same class of behavior as any replicated state under partition. Worth documenting; not solvable without consensus.
• Possible future refinement (not in this PR): replication's per-peer connection truth (readConnectionTruth) could serve as a liveness accelerant to detect a dead leader in ~120s instead of 5 minutes. Deferred — it's node-local/one-directional so it can't replace the lease, only shorten failover.

Where to focus review

• Leader-election semantics without CAS (engine.ts) — see design notes above; handlers are documented as needing idempotency.
• New replicating system table hdb_scheduler_state (audit enabled, since replication requires auditing). One lease row + one small row per job.
• promotionWaitMs escalation ladder (engine.ts) — added after review flagged a cold-start liveness gap; each successive roster node waits 2 × watcher interval longer before claiming a leaderless cluster.
• Catch-up policy: at most one missed occurrence fires per job (not every missed occurrence), and a newly deployed job waits for its first scheduled time instead of firing immediately (firstSeenAt baseline).

Open items from review (not resolved in this PR)

• Orphaned job:<component>:<name> state rows are not reaped when jobs are renamed/removed (TODO in runCatchUp); rows are tiny but accumulate.
• Gemini's review findings are addressed (6 fixed in 2b5d2a9, 2 rebutted on-thread with rationale). Codex outside-model review not run (account access); the Harper-domain review pass ran and its findings are fixed.

Verification

• 42 unit tests (unitTests/resources/scheduler/): cron parsing/evaluation incl. DST spring-forward/fall-back pinned to exact instants, leap day, never-fires detection, election, promotion ladder, catch-up decisions, hostile-error handling.
• Single-node integration test + fixture (integrationTests/components/scheduler-jobs.test.ts) — passing via the integration framework.
• Multi-node validation (harper-pro cluster harness): a 2-node replicated cluster test (schedulerFailover.test.mjs, to be contributed to harper-pro once core syncs) passes both assertions — every tick fires from exactly one leader-elected node, and killing the leader promotes the survivor which resumes firing (verified via env-shortened thresholds; the hdb_scheduler_state lease table replicates between nodes as designed).
• Verified live on a dev boot: leader elected, 1s-interval job fired exactly once per second, cron job registered without premature firing, state survived a full restart with cadence resuming and no double-fires.

Docs: companion PR HarperFast/documentation#592 (draft, lands together with this).

Generated by an LLM (Claude Fable 5) pairing with @jcohen-hdb.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a built-in scheduler component plugin that enables components to declare recurring jobs via cron or interval schedules, executing them exactly once per cluster using a leader-elected engine. The implementation includes timezone-aware cron parsing, a state-replicating failover engine, and comprehensive tests. The review feedback is highly constructive, focusing on enhancing the robustness of the scheduling engine. Key recommendations include preventing split-brain scenarios during database read failures, safely handling untrusted error objects with try/catch blocks, snapshotting jobs before asynchronous iteration to avoid concurrent mutation bugs, wrapping database operations in failover checks to prevent unhandled rejections, and enforcing mutually exclusive schema constraints for cron and interval configurations.

[kriszyp]

Really solid work here — the coordination design is thoughtful, the DESIGN.md writeup is exactly the right kind of documentation, and the election/lease core held up impressively well under adversarial verification (see "verified robust" below, which cleared several things that look like bugs). This review ran 8 independent finder angles over the diff, then adversarially verified 12 candidate findings; 8 confirmed (2 empirically, with runnable repros), 4 refuted. Confirmed findings are inline comments — all have cheap, well-understood fixes. The two DST findings and the identity-guard fix are the ones I'd prioritize.

Verified robust (candidates that did NOT survive verification — recorded so nobody re-flags them)

• Cold-boot N-leader race: refuted. server.nodes is rebuilt synchronously from the persisted hdb_nodes table before applications load, so established clusters share a roster at boot. The only duplicate window is initial cluster formation, which the PR documents as by-design. ✅
• Mutual leader step-down → leaderless gap: refuted. Under timestamp-LWW conflict resolution (Table.ts precedesExistingVersion, with its alphabetical node-name tiebreak) the write-order winner always reads its own row as current — only the loser ever steps down. The tiebreaker the lease dance seems to be missing already exists at the replication layer. ✅
• Unbounded audit growth from the 60s heartbeat: refuted — bounded by the default 3d auditRetention purge (~4.3k entries steady-state for the leader row). ✅
• Validation/scope.import under the component load lock: refuted — consistent with existing plugin convention (jsResource, dataLoader, graphql all do awaited I/O under the same lock), and the cron constructor search is coarse-jumping (month/day/hour), sub-millisecond. ✅

Design discussion: the config→code binding (from Kris)

The handler: ./jobs.ts#export path-fragment string introduces a third pattern for connecting declarative config to code. We already have two idioms: (1) JS-initiated slot-filling — the declarative side declares the slot, component code registers the implementation by name (computed fields; MCP quotas in #1821); and (2) named resources from the resources registry (how REST binds names to code — considered for cases like this before). The module#fragment approach needs its own resolution machinery (scope.import + fragment parsing), couples config to file layout (moving a file silently breaks config), and its deploy-time validation promise is exactly what the worker-gate finding below undermines.

Suggestion to consider before the config surface ships (it's the hardest part to change later): keep the job declaration in config (name, cron/interval, timezone — the ops-visible part, which is what #951 asked for), but bind the handler the way computed fields and quotas do — component code registers it against the job name at load (e.g. server.scheduler.handle('daily-metrics-snapshot', fn)), with a post-load check that every declared job got a handler. Handlers stay typed and refactor-safe, the path-resolution machinery disappears, and this becomes the third consecutive feature using the same binding idiom instead of the first of a new one. Open to being argued out of it — if config-only self-containment matters more than consistency here, let's decide that explicitly.

Load distribution (deferred — that's fine)

All jobs currently execute on worker 0 of the leader node — worker 0 is just the first HTTP worker, so a heavy handler competes with HTTP traffic on that thread, on the leader. Acceptable for v1. #754 (sharded hierarchical timing wheel) explicitly owns distribution at the shard-to-node assignment layer, so scheduler firing should migrate onto it when it lands; a one-liner in DESIGN.md noting "handlers run on the leader's worker 0" would make the current constraint visible to operators.

On the latest commits (c1f20c9e0..14648ba07)

The Gemini-review hardening is good but doesn't intersect these findings — two near-misses called out inline: runCatchUp's new snapshot adds exactly the identity check that scheduleNextRun still needs, and the electRole/failoverCheck reads are now guarded but heartbeat()'s (the one that matters for dual-leader) is not.

---

Review by Claude (Fable 5), pairing with @kriszyp — 8 finder angles, 12 adversarially verified candidates, 2 empirical repros (scripts available on request).

[kriszyp]

[confirmed — empirical] 5-attempt DST loop permanently wedges sub-hourly jobs every fall-back transition.

Each attempt advances one occurrence, but converging through the fall-back repeated hour needs up to ~an hour's worth of occurrences. Reproduced against this exact code: */5 * * * * in America/New_York, after = 2026-11-01T06:10:00Z (inside the repeated 1 AM hour) needs 13 iterations; the loop allows 5, so nextDate returns null for the entire 06:00Z–07:10Z window — and this is hit naturally in live sequence (the post-run reschedule of the 06:00Z occurrence lands inside the window).

Downstream, engine.ts scheduleNextRun treats null as "no future occurrence" and returns without arming a timer — and nothing ever retries (the heartbeat catch-up sweep never re-arms timers). The job is permanently degraded to at most one catch-up run per heartbeat (flagged catchUp: true) until failover/restart, recurring every fall-back for every sub-hourly cron job in a DST timezone.

Fix shape: bound the retry by wall-clock distance rather than attempt count (e.g. retry while nextWall - originalWall <= ~3h, or a generous cap like 200 — each iteration is cheap), and make scheduleNextRun treat null as "retry at next heartbeat" rather than warn-and-abandon, so any residual null is self-healing.

[jcohen-hdb]

Fixed in 397e6b3: convergence is now bounded by wall-clock distance (3h horizon, 400-iteration safety cap) instead of 5 attempts — your repro (*/5 at 06:10Z) now converges in ~10 steps and is pinned as a regression test, along with the minutely worst case. Belt-and-braces on the downstream half too: timers clear their handle at fire time and the heartbeat catch-up sweep re-arms any unarmed cron job, so a residual null nextDate self-heals within one heartbeat instead of degrading until failover. (AI-generated response)

[kriszyp]

[confirmed — empirical] Single offset probe maps wall times 02:00–05:59 after fall-back one hour early.

asUtc (the wall time misread as UTC) lands ~4–5h before the real instant, so for the first offset-width hours after the transition the probe still sees the pre-transition offset. Reproduced: NY wall 02:15 on 2026-11-01 → returns 06:15Z (which is wall 01:15 EST); correct is 07:15Z. Round-trip toZonedWallTime(fromZonedWallTime(w)) ≠ w for the whole 02:00–05:59 window.

The instant > after guard in nextDate doesn't mask this — it accepts the wrong instant: a daily 15 2 * * * job fires an hour early at the wrong wall time, and for 30 * * * * one occurrence is silently dropped (wall 05:30 never fires; the retry loop steps past it). The catch-up sweep can't recover the loss because its own previousDate is wrong by the same hour.

Fix shape: the standard second probe iteration — recompute the offset from the first candidate instant and re-resolve (optionally compare both probes to pick the ambiguous-instant side deterministically). Controls confirm the function is exact outside transition windows, so this is strictly additive.

[jcohen-hdb]

Fixed in 397e6b3 with the standard two-probe resolution: re-resolve the offset at the first candidate instant and accept the second candidate only when it round-trips exactly (ambiguous times therefore resolve to their first occurrence deterministically; nonexistent spring-forward times keep the shifted instant, preserving the existing pinned behavior). Your 02:15 repro (07:15Z, was 06:15Z), the dropped hourly occurrence (10:30Z now fires), and exact round-trips across the 02:00-05:59 window are all pinned as regression tests. (AI-generated response)

[kriszyp]

[confirmed] Interval jobs hot-loop when state writes fail but reads succeed.

fireAt anchors solely to the persisted lastRunAt, and putStateRow swallows all write errors. On the canonical partial-failure profile — reads fine, writes throwing (ENOSPC/EDQUOT, we've seen exactly this in the fleet) — each run re-anchors to the frozen lastRunAt → delay <= 0 → Math.max(delay, 0) → immediate refire. The handler executes back-to-back continuously (potentially hundreds/sec for a fast handler) instead of once per interval, and the 2+ warn lines per iteration accelerate filling the disk that caused the failure. job.running doesn't help: it's reset in executeJob's own finally before the reschedule fires.

Fix shape: when the previous persist failed (or unconditionally), floor the reschedule at now + minDelay — or keep an in-memory last-attempt timestamp as a fallback anchor so a failed persist can't time-travel the schedule.

[jcohen-hdb]

Fixed in 397e6b3 with the in-memory anchor variant: RegisteredJob carries lastAttemptAt (set at execution start), and interval scheduling anchors to max(persisted lastRunAt, lastAttemptAt). A frozen persisted row can no longer time-travel the schedule, while the persisted value still drives the overdue-interval catch-up across restarts/failover. (AI-generated response)

[kriszyp]

[confirmed] Name-based re-arm guard → persistent double-fire after component reload.

registerComponentJobs replaces RegisteredJob objects under the same names, so an in-flight executeJob → .finally → scheduleNextRun(oldJob) chain (the window spans the entire handler execution) passes this name-only .has() guard and re-arms the old object — which no map references. unregisterComponentJobs and becomeFollower iterate the current map, so that timer can never be cleared: two independent chains then run every occurrence twice, indefinitely, each .finally re-arming its own object. Trigger is just "redeploy while the leader has a run in flight". (job.running is per-object, so it never dedupes across the two chains.)

Nice part: the fix pattern is already in this file now — runCatchUp's new snapshot check (.get(job.name) !== job) is exactly right. Applying the same identity comparison here, plus re-checking it after the getJobStateRow await (which also closes the concurrent-arm timer leak between registerComponentJobs' fire-and-forget and becomeLeader's loop), closes both legs.

[jcohen-hdb]

Fixed in 397e6b3: extracted the runCatchUp pattern into an isRegistered() helper doing object-identity comparison, applied at scheduleNextRun entry AND re-checked after the getJobStateRow await (closing the concurrent-arm leak you noted between registerComponentJobs' fire-and-forget and becomeLeader's loop), plus at executeJob entry. The stale-object chain now dies at its next step in all three places. (AI-generated response)

[kriszyp]

[confirmed] The load-bearing "this table replicates" invariant is enforced nowhere — and an ordinary config silently breaks it today.

The design comment above requires hdb_scheduler_state to replicate, but no replicate option is passed, and replication is decided by absence from NON_REPLICATING_SYSTEM_TABLES plus harper-pro policy. Concretely: when an operator scopes replication.databases (a documented config), the system database is only replicated if listed or if some system table has replicate === true — this table never sets it, so it can't trigger that fallback. Result: the lease/heartbeat view diverges per node and the scheduler degrades to N independent leaders running every job N times, with no error anywhere.

Fix shape: pass replicate: true explicitly here (a positive declaration the pro subscription check honors), and ideally assert the property at engine start so a future policy change fails loudly instead of silently multiplying job executions.

[jcohen-hdb]

Fixed in 397e6b3: the table now declares replicate: true positively, and becomeLeader asserts the property at promotion — if a future policy change flips it, the leader logs a loud error naming the exact degradation (jobs running on every node) instead of multiplying silently. (AI-generated response)

[kriszyp]

[confirmed] Leader lease renewal is fragile in two ways; both end in dual execution.

(a) Ordering in becomeLeader: one heartbeat is written, then scheduleComponentJobs and the full catch-up pass are awaited — user handlers, serially — before the heartbeat interval is created. If the catch-up pass exceeds STALE_THRESHOLD_MS (5 min total across all missed jobs, not per job), the live leader looks stale, the rung-0 follower promotes, and its own catch-up re-runs the same occurrence — job.running is process-local, so both nodes execute it. This is near-deterministic duplication of exactly the occurrence whose handler caused the stall.

(b) This read is unguarded: the recent hardening wrapped electRole and failoverCheck's reads, but this is the one that matters — a read error here aborts the whole heartbeat via the interval's .catch, skipping both the lease renewal and the takeover step-down check (the path that would heal a dual-leader state). Leader-local read errors > 5 min → follower promotes → both fire, and the healing path is itself the broken path.

Fix shape: start the heartbeat interval before (or concurrently with) the promotion catch-up pass, and wrap this get the same way electRole does — a failed read should still attempt renewal and try the step-down check on the next tick.

[jcohen-hdb]

Both fixed in 397e6b3: (a) the heartbeat interval is created immediately after the first lease write, BEFORE scheduleComponentJobs and the promotion catch-up pass — runCatchUp's single-flight guard keeps the overlapping heartbeat tick from double-sweeping; (b) heartbeat's read is now guarded like electRole's, and on a failed read it renews blind (using an in-memory initializedAt fallback so the row isn't clobbered) rather than aborting the tick — renewal and the takeover step-down check each degrade by one tick instead of failing together. (AI-generated response)

[kriszyp]

[confirmed] The worker-0 gate defeats the deploy-time validation this plugin promises.

Deploy pre-flight validation (components/operations.js → loadComponent with collectScopes) runs in-process on whichever HTTP worker received the deploy operation — worker 0 with probability 1/N. On any other worker this gate returns before any validation: jobs-array shape, duplicate names, cron parse, timezone, interval bounds, and the eager resolveHandler import (whose documented purpose is failing "at deploy time") are all unreachable. A bad config passes pre-flight nondeterministically, deploys and replicates, then the component fails to load cluster-wide at the next restart. Peers re-validate on arbitrary workers too, so a cluster can even split on accepting the same deploy.

(The dataLoader precedent has the same gate but does no validation behind it — the gate is fine as an activation gate; it's the validation behind it that's new here.)

Fix shape: split validation from activation — run the config checks and buildJob/resolveHandler unconditionally (cheap, side-effect-free), and gate only registerComponentJobs/startSchedulerEngine on worker 0.

[jcohen-hdb]

Fixed in 397e6b3 exactly as you outlined: config checks, duplicate-name detection, cron/timezone/interval validation, and the eager resolveHandler import all run unconditionally (every worker, including pre-flight validation loads on arbitrary workers); only registerComponentJobs/startSchedulerEngine sit behind the worker-0 gate. Deploy pre-flight now rejects a bad scheduler config deterministically. (AI-generated response)

[kriszyp]

[confirmed] executeJob never re-checks registration — and deploy-validation loads leak into the live engine under appName 'harper'.

Two related issues with one root cause (liveness checked one await too early):

1. A timer armed after its scope closed (for interval jobs, scheduleNextRun can pass its guard, park at the getJobStateRow await, and arm after unregisterComponentJobs ran) fires into executeJob, whose only gates are role and job.running — the discarded component's handler executes once against production state and writes a state row for a dead component.
2. Sharper, race-free variant: the deploy-validation loadComponent passes no appName, so the validation scope defaults to 'harper' — the same key as root-config jobs. On the leader's worker 0, a deploy validation therefore calls registerComponentJobs('harper', …), which first unregisters any root-config scheduler jobs, and the validation scope's close then deletes the entry entirely — root-level jobs are silently dead until worker restart. No race needed.

Fix shape: add a registration/identity re-check here in executeJob (same pattern as runCatchUp's new snapshot check), and don't register jobs from validation loads at all (e.g. skip when the scope is a collectScopes validation load, or fix the 'harper' appName default for validation scopes — that default collision is worth fixing regardless).

[jcohen-hdb]

Both legs fixed in 397e6b3: (1) executeJob re-checks object-identity registration at entry (and scheduleNextRun re-checks after its await before arming), so a timer that outlives its scope dies at the next step; (2) the loader now marks pre-flight validation scopes with isTransientValidation (one-line marker in componentLoader where collectScopes is handled, declared on Scope), and the plugin validates fully but never registers from them — so a validation load can no longer displace a live component's jobs regardless of the appName collision. Left the 'harper' appName default itself alone to keep this PR's blast radius small, but agreed it's worth fixing on its own — happy to file it. (AI-generated response)

[kriszyp]

Reviewed — the separation between config validation, worker-local activation, replicated run state, and cron calculation is thoughtful, and the DST coverage is strong. A few things worth addressing (flagging as comments):

1. Oversized/non-finite intervals become a ~1ms hot loop (resources/scheduler/scheduler.ts:153, engine.ts:470/487). The interval validation has a lower bound but no finiteness/upper-bound check. YAML .inf / 1e309 parse to Infinity, and large finite values exceed Date range — both pass intervalMs >= 1000. scheduleNextRun then builds an Invalid Date → delay = NaN → Node coerces setTimeout(…, NaN) to 1ms, and re-arms the same way after each run, turning a "disabled/very-long" interval into continuous user-code + state-table I/O. Reject non-finite intervals and cap/chunk to the representable horizon; armTimer should fail closed on a non-finite target. (Confirmed on Node v26: invalid date → NaN → TimeoutNaNWarning → 1ms.)

2. Overlapping scheduling can arm two timers while keeping one handle (engine.ts:198/299/455/487). becomeLeader publishes role='leader' before its awaits and initial scheduling loop, so a component registering in that window starts its own scheduleComponentJobs while becomeLeader also schedules the same job. For interval jobs, both scheduleNextRun calls clear the handle before awaiting getJobStateRow, so both can pass with no timer and call armTimer; the second overwrites the first handle without cancelling it → an uncancellable live timer, double-execution for one fireAt, or persistent competing timers.

3. The once-per-occurrence property is unproved. The integration test passes under the duplicate-worker behavior it claims to detect (single worker, one node), so the load-bearing central coordination isn't actually exercised — worth a real multi-worker / multi-node case.

Nice work on the DST handling.

— Claude

[jcohen-hdb]

All three round-2 findings addressed in aa0aa0a:

1. Interval hot loop: intervals must now be finite and within 1s–365 days (rejects YAML .inf, 1e309, and Date-overflowing values at config load), and armTimer independently fails closed on a non-finite delay instead of letting setTimeout coerce NaN to ~1ms. Cron jobs that decline to arm are picked back up by the heartbeat sweep.

2. Double-timer race: concurrent scheduleNextRun calls for the same job now coalesce onto a single in-flight computation (job.scheduling promise), so the becomeLeader-loop vs. registration fire-and-forget interleaving can no longer arm two timers while retaining one handle.

3. Once-per-occurrence now actually tested: the component integration test runs with threads: { count: 3 } (runtime config overrides the framework's single-worker default — verified http/1..3 in instance logs), so its cadence assertions genuinely exercise the worker-0 activation gate. Cross-NODE dedup remains covered by the 2-node failover test in the harper-pro cluster harness (passes against this branch; to be contributed there once core syncs).

(AI-generated response)

[kriszyp]

Requesting changes on the two High findings from my earlier review comment — flagging as blocking:

1. Oversized/non-finite intervals become a ~1ms hot loop (scheduler.ts:153, engine.ts:470/487): YAML .inf/1e309 → Infinity and over-range finite values both pass intervalMs >= 1000, producing an Invalid Date → NaN delay → setTimeout(…, NaN) coerced to 1ms, re-armed after every run. Reject non-finite intervals + cap/chunk to the representable horizon; armTimer should fail closed on a non-finite target.

2. Overlapping scheduling can arm two timers while keeping one handle (engine.ts:198/299/455/487): becomeLeader publishes role='leader' before its awaits, and both scheduleNextRun calls clear the handle before awaiting getJobStateRow, so two timers can be armed with only one tracked → uncancellable timer / double-fire.

Also worth resolving before merge: the integration test passes under the duplicate-worker behavior it claims to detect (single worker, one node), so the once-per-occurrence coordination is still unproved — a real multi-worker/multi-node case would pin it.

— Claude

[jcohen-hdb]

Ahead of further review we ran our own multipass adversarial audit (two independent deep-review runs per domain — concurrency, data-integrity, config — with lenses derived from the earlier findings). Result: 9 code findings fixed in 3d9cbbb, the notable ones:

• Hemisphere-dependent DST resolution (empirical): the two-probe offset fix was correct for western zones only — a Sydney spring-forward-gap job fired an hour EARLY (before the transition), and Lord Howe ambiguous times resolved to their second occurrence. fromZonedWallTime now probes both sides of a nearby transition and resolves deterministically in every zone (exists → exact; ambiguous → first occurrence; gap → first instant after the gap). Sydney/Lord Howe regression tests added; NY behaviors unchanged.
• Cron catch-up under sustained write failure: the lastAttemptAt hot-loop guard added for interval jobs had not been mirrored to the catch-up baseline — a daily cron would re-execute every heartbeat while state writes failed. Baseline now includes the in-memory anchor (also closing a sweep-straddles-a-fast-run duplicate) and is clamped against clock-skewed future timestamps.
• Destructive first-seen reseed: a transient state-table READ error was treated as "job never seen" and answered with a full-replace put, destroying run history and shifting the catch-up baseline. The sweep now reads strictly (failure skips the job) and the reseed preserves existing fields and skips while a run is in flight.
• Orphaned heartbeat via double promotion: two in-flight failover checks could both promote; the second becomeLeader overwrote the heartbeat handle, leaving an orphaned interval renewing the lease forever with no timers armed (cluster-wide silent stop). Broken three ways: post-await role re-check in failoverCheck, becomeLeader clears any existing interval, heartbeat ticks no-op unless still leader.
• Smaller: 8-year search horizon (leap-day schedules were rejected across the 2096→2104 century gap), unknown job-key rejection (a typo'd timeZone: silently ran in the server timezone), jobs: null (all entries commented out) no longer fails the component, dev-watch scheduler: block removal now triggers a restart, stale-threshold/heartbeat cross-check, occurrence-aware lastRunAt under clock step-back, schema tightening + YAML quoting guidance.

48 unit tests (5 new), single-node + multi-worker integration green; 2-node failover re-validation running. Full audit trail available if useful.

(AI-generated response)

[jcohen-hdb]

@kriszyp All three items in the changes-requested review are already addressed — notably, the review is pinned to commit aa0aa0a, which is the commit that fixed them (the finding text describes the pre-aa0aa0a87 code). Concretely, in the current head:

1. Interval hot loop — fixed in aa0aa0a: intervals must be finite and within 1s–365 days (scheduler.ts:189, MAX_INTERVAL_MS at :28), and armTimer independently fails closed on any non-finite delay (engine.ts:578). Covered for YAML .inf, 1e309, and Date-overflowing finite values.

2. Double-timer race — fixed in aa0aa0a: concurrent scheduleNextRun calls coalesce onto a single in-flight computation per job (job.scheduling promise, engine.ts:529-536), with an object-identity + role re-check after the state-read await before arming.

3. Once-per-occurrence coverage — fixed in aa0aa0a: the integration test boots with threads: { count: 3 } (scheduler-jobs.test.ts:30; runtime config overrides the framework's single-worker default — http/1..3 verified in instance logs), so the cadence assertions exercise the worker-0 gate. Cross-node: a 2-node replicated failover test runs in the harper-pro cluster harness against this branch (single-leader firing + survivor promotion; to be contributed there once core syncs).

Since that commit, a self-audit added further hardening (3d9cbbb — summary in the comment above) and the 2-node harness caught one real engine bug the unit surface couldn't: a state-table operation stalling on a freshly promoted leader during post-failover replication churn could wedge the promotion pipeline behind its await while the heartbeat kept the lease fresh — a healthy-looking leader running zero jobs. Every state-table operation is now bounded by a 10s timeout that fails into the existing degraded paths (bf40342/d7f202b79); statistical re-validation of the failover loop is running now.

Re-requesting review — happy to address anything that survives a look at the current head.

(AI-generated response)

[kriszyp]

Approving. Heads-up: it's currently CONFLICTING against main (went dirty since my earlier review), so it'll need a rebase before it can merge — I can queue one on your word. — Claude

[jcohen-hdb]

Thanks @kriszyp. Resolved the conflict by merging main (428e7ee) — the only collision was DESIGN.md (append vs. append; both sections kept). No rebase/force-push needed. CI is re-running on the merge; scheduler unit + integration coverage unchanged. Ready to merge on your word — I'll hold since the author merges per convention.

(AI-generated response)

[jcohen-hdb]

Thanks for the review, @kriszyp — all findings addressed and the branch is now current with main (0 commits behind; the earlier conflict is resolved), approved, and CI green on 428e7ee4 (MERGEABLE/CLEAN).

Since this merges into core main, I'd rather defer the merge itself to you: do you want me to merge it (author-merge, merge-commit to preserve the commit history), or would you prefer to land it yourself? Happy either way — just say the word.

Companion docs PR HarperFast/documentation#592 is also approved and green whenever you'd like it to go in.

(AI-generated comment on behalf of @jcohen-hdb)

[kriszyp]

Addressed the two actionable review findings directly on this branch:

• Finding 2 (path redaction, Unix-only): widened sanitizeStoredError to also strip Windows paths and /srv, /data, /mnt — b5494e6, with unit coverage.
• Finding 1 (election/heartbeat/failover test coverage): filed Scheduler: add multi-node election/failover integration test coverage in-repo #1866 to track adding real multi-node integration coverage in this repo (the existing schedulerFailover.test.mjs validation lives in the harper-pro cluster harness, not here yet). Not resolving in this PR — agreed with Kris this is out of scope for the initial merge.

🤖 Generated with Claude Code