feat(scheduler): in-app scheduler; internalize scheduled sync#515
Conversation
Adds an opt-in in-process scheduler (DISPATCH_SCHEDULER_ENABLED) wired from instrumentation.ts register(), so periodic work runs inside the server instead of external Kubernetes cronjobs. First job: scheduled issue sync (/api/sync/scheduled), DISPATCH_SYNC_INTERVAL_MS (default 15m). Jobs fire as loopback HTTP POSTs to the app's own endpoints, not direct function calls: this routes through the real route module graph (avoiding the Turbopack standalone chunk-graph isolation that instrumentation.ts/lane-config warn about) and reuses each endpoint's auth + DB lock (concurrent fires 409). The sync endpoint is globally DB-locked, so it's safe to internalize now. Node runtime only (dynamic import so the edge bundle skips it). runJob never throws — a transient failure can't kill the interval; 409 is treated as expected. Scheduler logic is pure + dependency-injected (fetch/timers) so it's unit-tested without a server (10 tests). groomer/pr-followup/prune-closed follow in #503/#504 (they need their own concurrency guards first). Closes #502
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
PR Review: PR 515 — feat(scheduler): in-app scheduler; internalize scheduled sync
Summary
This PR adds an opt-in in-process scheduler wired from instrumentation.ts register(), enabling Dispatch to self-host the periodic issue sync instead of relying on an external Kubernetes cronjob. The implementation fires loopback HTTP POSTs to existing endpoints, reuses existing auth and DB locking, and is properly guarded for edge/Node runtime differences.
Change-by-Change Findings
AGENTS.md — Two new optional environment variables added to the table:
DISPATCH_SCHEDULER_ENABLED(off by default; confine to single replica)DISPATCH_SYNC_INTERVAL_MS(default 900000 = 15m)- These match the environment variable naming convention (
DISPATCH_*) and the documented semantics from PR 502.
src/instrumentation.ts — Scheduler wired into register():
NEXT_RUNTIME !== "nodejs"guard excludes edge runtime (no timers/loopback in edge)- Dynamic import (
await import("@/lib/scheduler")) prevents the edge bundle from pulling in Node-only code - Passes
fetch,setInterval,setTimeout, and a typed logger as dependencies
src/lib/scheduler.ts (new) — Pure scheduler implementation:
schedulerConfigFromEnv: readsPORTfor loopback base URL (defaults 3000),DISPATCH_AGENT_TOKEN,DISPATCH_SYNC_INTERVAL_MS, andDISPATCH_SCHEDULER_STARTUP_DELAY_MSrunJob: fires one HTTP POST with bearer auth; treats 409 as non-error (DB lock held); swallows fetch rejections so a transient failure cannot kill the intervalstartScheduler: no-ops when disabled or token missing; schedules each job once afterstartupDelayMs, then arms an interval- Design rationale (chunk-graph isolation, locking reuse) is documented inline
src/lib/scheduler.test.ts (new) — 10 unit tests covering:
- Config parsing: disabled unless
DISPATCH_SCHEDULER_ENABLED=true; defaults to 15m; honorsDISPATCH_SYNC_INTERVAL_MS; falls back on garbage input; uses PORT for baseUrl runJob: POSTs correct JSON with bearer auth; 409 is silent; non-ok logs without throwing; fetch rejection is swallowedstartScheduler: disabled → no-op; enabled+no-token → no-op with warning; enabled+token → fires startup timer then arms interval
Standards Compliance
- ✅ Environment variables:
DISPATCH_SCHEDULER_ENABLEDandDISPATCH_SYNC_INTERVAL_MSfollow existingDISPATCH_*naming pattern; both marked optional in AGENTS.md - ✅ Error handling:
runJobuseserror instanceof Errorpattern via try/catch; meaningful log messages on failure - ✅ No commit of secrets:
DISPATCH_AGENT_TOKENis referenced but never logged or persisted - ✅ Node runtime only: guarded with
NEXT_RUNTIMEcheck and dynamic import
Linked Issue Fit (PR 502)
| Requirement from PR 502 | Status |
|---|---|
Opt-in via DISPATCH_SCHEDULER_ENABLED (default off) |
✅ Implemented |
Interval via DISPATCH_SYNC_INTERVAL_MS (~900000) |
✅ Implemented with 15m default |
Reuse DISPATCH_AGENT_TOKEN for loopback auth |
✅ Implemented |
| Startup delay until server is listening | ✅ 5s default (DISPATCH_SCHEDULER_STARTUP_DELAY_MS) |
| Swallow/log fetch errors so failure can't kill interval | ✅ runJob swallows all errors; logs failure |
| Loopback HTTP POST (not direct function call) | ✅ Deliberate design; avoids chunk-graph isolation |
| DB lock makes concurrent fires safe (409 → expected) | ✅ 409 explicitly treated as non-error; documented in code and tests |
| Single replica confinement until per-job locks exist | ✅ Documented; enabled gate is the confinement mechanism |
Evidence Provider Findings
No evidence providers configured for this PR.
Tool Harness Findings
All tool reads succeeded:
src/lib/scheduler.ts: Implementation matches the PR diff; dependencies are correctly typed and injectedsrc/instrumentation.ts: Registration logic matches the PR diff; runtime guard and dynamic import are correctgit log/git grep: Confirmed this is the first commit on the feature branch;409handling matches existing sync-lock semantics insrc/lib/sync-lock.ts
CI Results
All checks passed: Typecheck, Lint, Docker Build, Tests, Build — all at commit f82b0db.
Unknowns / Needs Verification
- The PR body states a follow-up (PR 504) will internalize groomer/pr-followup/prune-closed once their DB locks exist. This is tracked as out-of-scope for this PR and consistent with the design constraint documented in
scheduler.ts.
Recommendation
Approve. The implementation is clean, well-tested, matches all linked issue acceptance criteria, and follows repository conventions. The design decision to use loopback HTTP POSTs (not direct function calls) is explicitly documented and correctly reasoned.
Summary
DISPATCH_SCHEDULER_ENABLED) wired frominstrumentation.tsregister(), replacing the external cronjob that POSTs/api/sync/scheduled. Interval viaDISPATCH_SYNC_INTERVAL_MS(default 15m).src/lib/scheduler.ts— pure + dependency-injected (fetch/timers), unit-tested without a server.Design
instrumentation.ts/lane-config.tscomments describe) and reuses each endpoint's auth + DB lock for free./api/sync/scheduledis globally DB-locked, so internalizing it is safe now (concurrent fires just 409, whichrunJobtreats as expected).runJobnever throws; opt-in so dev/CI don't spin timers and it can be confined to one replica.Scope / follow-ups
pr-followup/sync+prune-closedinternalize in feat(scheduler): internalize groomer + pr-followup/sync + prune-closed loops #504.Verification
vitest run src/lib/scheduler.test.ts→ 10 passing (config parsing, POST+bearer shape, 409-is-fine, non-ok logs, fetch-rejection swallowed, disabled/no-token no-ops, startup-delay→interval).tsc --noEmit→ 0.eslint→ clean.Closes #502