Skip to content

fix(selfhost): phase-align the cron scheduler to wall-clock boundaries - #8061

Merged
JSONbored merged 1 commit into
mainfrom
selfhost-cron-wallclock-alignment
Jul 22, 2026
Merged

fix(selfhost): phase-align the cron scheduler to wall-clock boundaries#8061
JSONbored merged 1 commit into
mainfrom
selfhost-cron-wallclock-alignment

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Cloudflare's own */2 * * * * cron trigger fires exactly on wall-clock 2-minute boundaries, which every minute-gated job in enqueueScheduledJobs (minute % 10 === 0, minute === 0, minute % 30 === 0 -- all even) depends on to ever run.
  • The self-host entrypoint's plain setInterval instead ticked every CRON_INTERVAL_MS from whatever moment the container booted, with no relation to wall-clock boundaries. Since the interval evenly divides an hour, that locks every tick to a fixed minute parity for the container's entire lifetime -- a container that boots in an odd minute ticks only on odd minutes forever, so refresh-registry, ops-alerts, sweep-liveness-watchdog, backfill-registered-repos, and both reconciliation sweeps silently never fire.
  • Confirmed live on our own self-hosted box (edge-nl-01): the app container booted at :49 (odd) and ran 3+ hours of on-schedule ~2-minute ticks with zero occurrences of any minute-gated job, while the unconditional every-tick sweep ran normally the whole time.

What Changed

  • New src/selfhost/cron-alignment.ts: pure delayToNextWallClockBoundaryMs(nowMs, intervalMs) helper, 100% branch-covered.
  • src/server.ts: the cron setup now phase-aligns its first tick to the next true wall-clock boundary with a one-shot setTimeout, then hands off to setInterval from that aligned moment so every subsequent tick keeps landing on the same boundaries Cloudflare's cron would.

Test plan

  • Added test/unit/selfhost-cron-alignment.test.ts covering both branches (mid-cycle boot, exact-boundary boot) plus a real-timestamp regression case matching the observed production boot moment.
  • npm run test:ci -- green (one unrelated, pre-existing flaky test in agent-sdk-driver.test.ts timed out under parallel load; confirmed passing in isolation, unrelated to this change).
  • npm run typecheck -- clean.
  • npm audit --audit-level=moderate -- same 13 pre-existing transitive vulnerabilities as before this change (sharp/miniflare/wrangler chain), unrelated.

Cloudflare's own */2 * * * * cron trigger fires exactly on wall-clock
2-minute boundaries (:00, :02, :04, ...), which every minute-gated job
in enqueueScheduledJobs (minute % 10 === 0, minute === 0, minute % 30
=== 0 -- all even) depends on to ever run. The self-host entrypoint's
plain setInterval instead ticked every CRON_INTERVAL_MS from whatever
moment the container booted, with no relation to wall-clock
boundaries -- and since the interval evenly divides an hour, that
locks every tick to a FIXED minute parity for the container's entire
lifetime. A container booting in an odd minute then ticks ONLY on odd
minutes forever, so refresh-registry, ops-alerts, sweep-watchdog,
backfill-registered-repos, and both reconciliation sweeps silently
NEVER fire.

Confirmed live on edge-nl-01: the app container booted at :49 (odd)
and ran 3+ hours of on-schedule ~2-minute ticks with zero occurrences
of any minute-gated job, while the unconditional every-tick sweep ran
normally the whole time.

Phase-align the first tick to the next true wall-clock boundary
(computed from epoch, itself minute-aligned) with a one-shot
setTimeout, then hand off to setInterval from that aligned moment.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 22, 2026
@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-22 17:44:01 UTC

3 files · 1 AI reviewer · 2 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This fixes a real bug: the self-host cron used a plain setInterval from container-boot time, which for an interval that evenly divides an hour locks every tick to the boot minute's parity forever, silently starving all minute-gated jobs (refresh-registry, ops-alerts, watchdog sweeps, reconciliation) whenever the container happens to boot on an odd minute. The fix computes delay-to-next-wall-clock-boundary via `nowMs % intervalMs` (epoch-anchored, so it matches Cloudflare's minute-aligned cron) and uses a one-shot setTimeout before handing off to setInterval, with the mutable `cron` variable correctly still visible to the later `clearInterval(cron)` in the shutdown handler. The new helper is small, pure, and has dedicated branch-covering tests including a real production boot timestamp.

Nits — 5 non-blocking
  • cron-alignment.ts:11-12 waits a full intervalMs when already exactly on a boundary rather than firing immediately — correct per the stated setInterval-parity rationale, but worth a one-line comment closer to the return statement itself rather than only in the file-header doc.
  • The `let cron: NodeJS.Timeout` reassignment inside the setTimeout callback (server.ts) is correct but slightly subtle — a reader skimming just the `clearInterval(cron)` call in shutdown might not immediately realize `cron` starts as a Timeout from setTimeout and later refers to the interval; a short inline comment at the reassignment site would help.
  • The lengthy header comment in server.ts (lines ~1094-1105) restates nearly the entire PR description inline; consider trimming to a shorter pointer plus a reference to cron-alignment.ts's own doc comment to avoid duplication drift if the rationale changes.
  • Consider extracting the repeated wall-clock-boundary rationale into a single location (e.g. only in cron-alignment.ts) and have server.ts's comment simply reference it, to avoid two prose copies of the same explanation drifting apart over time.
  • No migration or config-as-code changes are needed here since this is a pure self-host timer fix, not a schema/gate change — correctly scoped to server.ts and the new pure helper only.

Concerns raised — review before merging

  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 16 registered-repo PR(s), 14 merged, 290 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 16 PR(s), 290 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 16 PR(s), 290 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 22, 2026
@JSONbored
JSONbored merged commit eb17890 into main Jul 22, 2026
13 checks passed
@JSONbored
JSONbored deleted the selfhost-cron-wallclock-alignment branch July 22, 2026 17:55
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b63fa69) to head (9f10cf2).
⚠️ Report is 14 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #8061      +/-   ##
===========================================
+ Coverage   91.81%   100.00%   +8.18%     
===========================================
  Files         731         1     -730     
  Lines       74862         2   -74860     
  Branches    22973         1   -22972     
===========================================
- Hits        68733         2   -68731     
+ Misses       5034         0    -5034     
+ Partials     1095         0    -1095     
Flag Coverage Δ
rees ?
shard-1 100.00% <100.00%> (+40.95%) ⬆️
shard-2 0.00% <0.00%> (-48.60%) ⬇️
shard-3 0.00% <0.00%> (-54.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/cron-alignment.ts 100.00% <100.00%> (ø)

... and 731 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant