Skip to content

fix(scheduler): detect and recover when a job's timer stops firing - #875

Merged
joryirving merged 1 commit into
mainfrom
fix/scheduler-liveness
Aug 27, 2026
Merged

fix(scheduler): detect and recover when a job's timer stops firing#875
joryirving merged 1 commit into
mainfrom
fix/scheduler-liveness

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

  • Log every scheduled run, not just failures.
  • Track last-run per job and expose it on /api/health.
  • Re-arm a job that ran and then stopped.

What happened

The in-app scheduler stopped running its jobs on a pod that kept serving requests normally.

pr-followup did not fire for at least 15 hours. Three PRs in misospace/llmkube-images sat at CHANGES_REQUESTED with nothing acting on them, and /api/pr-fix-queue/history returned 404 for each — the items had never been created. A manual POST to the same endpoint returned:

{"reposScanned":9,"prsScanned":4,"enqueued":35,"skipped":28,"rateLimited":false}

So the endpoint, the auth, the tracked-repo list and the ingestion gates were all fine. Only the timer was not.

Confirmed rather than inferred: the enqueue path writes a prFixHistory row on every observation, even when the evidence is already known. A single firing would have moved the count. Measured across a 17-minute window with a 15-minute interval, it stayed at 11 with the newest entry still stamped from the manual call.

Why nobody noticed

runJob logged only failures, so silence meant either "ran fine" or "never ran". An empty PR-fix queue is indistinguishable from having no work to do, and a scheduler feeding every queue in the system can stop without producing a single line of output.

Two existing tests asserted that silence explicitly (expect(deps.logs).toHaveLength(0); // 200 -> quiet). Both are updated, because that property is precisely what hid this.

What this does not do

It does not claim to fix the root cause. Why the interval stops in the standalone runtime is not established — the timers are armed with the real setInterval, the loopback base is http://127.0.0.1:${port}, auth is a bearer token whose failure would log, and nothing deletes the rows involved. Rather than guess, this detects the state and recovers from it.

The supervisor deliberately ignores a job that has never run, so a slow start is not mistaken for a dead timer; only a job that ran and then went quiet for more than two intervals is re-armed.

Verification

  • npx vitest run2412 passed, 3 skipped.
  • Two new specs: a stalled job is reported overdue and re-armed with a log line, and a never-run job is left alone.
  • npm run lint clean; tsc clean apart from the pre-existing pr-fix-queue/history/route.ts error also on main.

Notes

  • The boot line scheduling "X" every Yms prints before the interval is armed inside a setTimeout, so it never proved a timer existed. Commented accordingly — the supervisor's output is the real evidence.
  • /api/health now carries scheduler and schedulerOverdue, which makes this alertable rather than only greppable.
  • The pod was restarted to restore service before this was written; the fix is what stops it recurring silently.

The in-app scheduler stopped running its jobs on a pod that kept serving
requests normally. pr-followup did not fire for at least 15 hours: three PRs
sat on reviewer feedback with nothing acting on it, and a manual POST to the
same endpoint enqueued 35 items immediately, so the endpoint was healthy and
the timer was not.

It was invisible because runJob only logged failures. Silence meant either
"ran fine" or "never ran" and nothing distinguished them, so an empty queue
looked exactly like having no work to do. Two tests asserted that silence
explicitly; both are updated, since that was the property that hid this.

Three changes:

- Log every run, not just failures, so silence is diagnosable.
- Track last completed run per job and expose it on /api/health, including a
  schedulerOverdue list, so liveness is answerable without reading logs.
- Add a supervisor that re-arms a job which ran and then went quiet for more
  than two intervals, and says so loudly.

Root cause in the standalone runtime is not established, so this deliberately
does not claim to prevent the stall. It detects it and recovers. A job that
has never run is left alone, so a slow start is not mistaken for a dead timer.

Also corrects the boot log's implication: "scheduling X" prints before the
interval is armed inside a setTimeout, so it never proved a timer existed.

2412 passed.

Claude-Session: https://claude.ai/code/session_01YSuDvZq9ncvyX85Uzx3cQh

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — primary route

PR Review: fix(scheduler): detect and recover when a job's timer stops firing

Recommendation: Approve

This PR addresses a scheduler liveness bug where jobs could silently stop firing while the app continued serving requests normally. The changes add scheduler state tracking, supervisor re-arming logic, and enhanced /api/health visibility. All CI checks passed.


Required Checks

✅ verify route access controls are in place

The only route file modified is src/app/api/health/route.ts. This endpoint:

  • Does not enforce authentication — intentionally, as health endpoints must be accessible to load balancers, Kubernetes probes, and external monitoring systems without credentials.
  • The change adds scheduler and schedulerOverdue fields to the existing public GET response, which is consistent with the endpoint's existing behavior (already unauthenticated before this PR).
  • No new authentication requirements or changes to auth enforcement are introduced.

Conclusion: Auth is correctly absent on this health endpoint. This matches repository convention and operational requirements.

✅ check for unintended public endpoints

The /api/health endpoint was already public. This PR:

  • Adds two read-only informational fields (scheduler, schedulerOverdue) containing operational state.
  • Does not enable any new mutating operations.
  • Does not expose secrets or credentials.
  • The scheduler field exposes job names, last-run timestamps, intervals, and overdue flags — all operational/monitoring data appropriate for a health endpoint.

Conclusion: No unintended public endpoints introduced. The health endpoint is appropriately public for monitoring purposes.


Standards Compliance

The AGENTS.md standards file does not define specific requirements for health endpoint access controls. The implementation follows existing patterns in the codebase where health routes are unauthenticated (as evidenced by the existing /api/health implementation).


Changes Summary

File Change Notes
src/app/api/health/route.ts Added scheduler and schedulerOverdue to GET response Operational telemetry for alerting
src/lib/scheduler.ts Added schedulerState(), supervisor interval, and improved logging Detect-and-recover for stopped timers
src/lib/scheduler.test.ts Updated existing tests and added supervisor regression tests 2 new test cases

Tool Harness Findings

No tool harness output was provided in the corpus.


CI Status

All checks passed: Lint, Tests (2412 passed), Database integration, Coverage, Database migrations, Typecheck, Docker Build (MCP), smoke, Build, npm audit, Docker Build.


Unknowns / Needs Verification

None. The corpus contains sufficient evidence to verify all required checks.

@joryirving
joryirving merged commit 2de79de into main Aug 27, 2026
12 checks passed
@joryirving
joryirving deleted the fix/scheduler-liveness branch August 27, 2026 14:19
@its-miso its-miso Bot mentioned this pull request Aug 27, 2026
joryirving added a commit that referenced this pull request Aug 27, 2026
/api/health reported "scheduler":{"jobs":[]} on 0.5.46 while the jobs were
demonstrably running — the logs added in #875 show sync, groomer, pr-followup
and stale-work all firing.

startScheduler runs from instrumentation.ts, which Turbopack standalone
isolates into its own chunk graph. The module-level Maps it populates are a
different instance from the ones /api/health imports, so the endpoint read
empty maps. This is the same isolation instrumentation.ts documents for lane
config, and the reason the scheduler fires loopback HTTP rather than calling
job functions directly. I reintroduced the trap the file warns about.

Hold the state on globalThis, which both graphs share. Same pattern as the
Prisma singleton in src/lib/prisma.ts.

The supervisor was unaffected — it lives in the same instance as the timers,
so recovery worked; only the reporting was blind.

Claude-Session: https://claude.ai/code/session_01YSuDvZq9ncvyX85Uzx3cQh

Co-authored-by: Jory Irving <jory.irving@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant