fix(scheduler): recover from panics + add liveness watchdog (#85) - #90
Merged
Merged
Conversation
The scheduler died silently on 2026-04-14 14:21 UTC and stayed dead for
12+ hours. Platform restart didn't recover it. Root cause: tick() and
fireSchedule() goroutines have no panic recovery. A single bad row, bad
cron expression, DB blip, or transient panic anywhere in the chain
permanently kills the scheduler goroutine — and the only signal to an
operator is "no crons firing", which is invisible if you're not watching.
Specifically:
func (s *Scheduler) Start(ctx context.Context) {
for {
select {
case <-ticker.C:
s.tick(ctx) // <- if this panics, the for-loop exits forever
}
}
}
And inside tick:
go func(s2 scheduleRow) {
defer wg.Done()
defer func() { <-sem }()
s.fireSchedule(ctx, s2) // <- panic here propagates up wg.Wait()
}(sched)
Two `defer recover()` additions:
1. In Start's tick wrapper — a panic in tick() (DB scan, cron parse,
row processing) is logged and the next tick fires normally.
2. In each fireSchedule goroutine — a single bad workspace can't take
the rest of the batch down.
Plus a liveness watchdog:
- Scheduler now records `lastTickAt` after each successful tick.
- New methods `LastTickAt()` and `Healthy()` (true if last tick within
2× pollInterval = 60s).
- Initialised at Start so Healthy() returns true on a fresh process.
Endpoint plumbing for /admin/scheduler/health is a follow-up — needs
threading the scheduler instance through router.Setup(). Documented
on #85.
Closes the silent-outage failure mode of #85. The other proposed
fixes (force-kill on /restart hang, active_tasks watchdog) are
separate concerns tracked in #85's comments.
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Apr 15, 2026
…tine (#92) Yesterday's scheduler-died incident (#85) was one instance of a systemic bug: every long-running goroutine in the platform lacks panic recovery and exposes no liveness signal. In a multi-tenant SaaS deployment, a single tenant's bad data panicking any subsystem takes down the subsystem for every tenant, silently, with all standard health probes still green. That is a scale-of-one sev-1. This PR: 1. Introduces `platform/internal/supervised/` with two primitives: a. RunWithRecover(ctx, name, fn) — runs fn in a recover wrapper. On panic logs the stack + exponential-backoff restart (1s → 2s → 4s → … → 30s cap). On clean return (fn decided to stop) returns. On ctx.Done cancels cleanly. b. Heartbeat(name) + LastTick(name) + Snapshot() + IsHealthy(names, staleThreshold) — shared in-memory liveness registry. Every subsystem calls Heartbeat(name) at the end of each tick so operators can distinguish "goroutine alive and healthy" from "alive but stuck inside a single tick". 2. Wraps every `go X.Start(ctx)` in main.go: - broadcaster.Subscribe (Redis pub/sub relay → WebSocket) - registry.StartLivenessMonitor - registry.StartHealthSweep - scheduler.Start (the one that died yesterday) - channelMgr.Start (Telegram / Slack) 3. Adds `supervised.Heartbeat("scheduler")` inside the scheduler tick loop as the first end-to-end demonstration. Follow-up PRs will add heartbeats to the other four subsystems. 4. Adds `GET /admin/liveness` endpoint returning per-subsystem last_tick_at + seconds_ago. Operators can poll this and alert on any subsystem whose seconds_ago exceeds 2x its cron/tick interval. 5. Unit tests for RunWithRecover (clean return no restart; panic restarts with backoff; ctx cancel stops restart loop) and for the liveness registry. Net new code: ~160 lines + ~100 lines of tests. Refactor of main.go: ~10 line changes. No behavior change on happy path; only lifts what happens on a panic. Closes #92. Supersedes the local recover added to scheduler.go in #90 (kept conceptually, but now via the shared helper).
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Apr 15, 2026
…un, panic recovery Added scheduler_test.go with 8 test cases covering all previously untested security-critical code paths from PR #90: TestLastTickAt_zero — zero time before first tick TestHealthy_beforeStart — false on fresh scheduler (zero lastTickAt) TestHealthy_freshTick — true when lastTickAt == now TestHealthy_stale — false when lastTickAt is 3×pollInterval ago TestComputeNextRun_valid — "0 * * * *" / UTC returns top-of-hour future time TestComputeNextRun_invalid — unparseable expression returns non-nil error TestComputeNextRun_invalidTimezone — unrecognised IANA zone returns non-nil error TestPanicRecovery — panicProxy crashes ProxyA2ARequest; scheduler goroutine recovers and remains Healthy To support these tests, scheduler.go gained four changes (minimal surface): 1. Added mu sync.RWMutex, lastTickAt time.Time, and tickInterval time.Duration fields to Scheduler. tickInterval defaults to pollInterval so production behaviour is unchanged; tests can override it directly. 2. Added LastTickAt() and Healthy() methods with read-lock protection. 3. tick() now records lastTickAt after wg.Wait() — a single atomic write under the mutex, no hot-path cost. 4. fireSchedule() got a deferred recover() so a panicking A2A proxy cannot crash the goroutine pool. Without this, TestPanicRecovery itself crashes the test binary — the test passing proves recovery is in place. Bug fix: ComputeNextRun previously silently fell back to UTC on an invalid timezone; it now returns a non-nil error. The schedules handler already validates the timezone before calling ComputeNextRun so this is a no-op for callers, but it makes the contract explicit and testable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 tasks
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Apr 15, 2026
…un, panic recovery Added scheduler_test.go with 8 test cases covering all previously untested security-critical code paths from PR #90: TestLastTickAt_zero — zero time before first tick TestHealthy_beforeStart — false on fresh scheduler (zero lastTickAt) TestHealthy_freshTick — true when lastTickAt == now TestHealthy_stale — false when lastTickAt is 3×pollInterval ago TestComputeNextRun_valid — "0 * * * *" / UTC returns top-of-hour future time TestComputeNextRun_invalid — unparseable expression returns non-nil error TestComputeNextRun_invalidTimezone — unrecognised IANA zone returns non-nil error TestPanicRecovery — panicProxy crashes ProxyA2ARequest; scheduler goroutine recovers and remains Healthy To support these tests, scheduler.go gained four changes (minimal surface): 1. Added mu sync.RWMutex, lastTickAt time.Time, and tickInterval time.Duration fields to Scheduler. tickInterval defaults to pollInterval so production behaviour is unchanged; tests can override it directly. 2. Added LastTickAt() and Healthy() methods with read-lock protection. 3. tick() now records lastTickAt after wg.Wait() — a single atomic write under the mutex, no hot-path cost. 4. fireSchedule() got a deferred recover() so a panicking A2A proxy cannot crash the goroutine pool. Without this, TestPanicRecovery itself crashes the test binary — the test passing proves recovery is in place. Bug fix: ComputeNextRun previously silently fell back to UTC on an invalid timezone; it now returns a non-nil error. The schedules handler already validates the timezone before calling ComputeNextRun so this is a no-op for callers, but it makes the contract explicit and testable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
fix(scheduler): recover from panics + add liveness watchdog (#85)
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
…tine (#92) Yesterday's scheduler-died incident (#85) was one instance of a systemic bug: every long-running goroutine in the platform lacks panic recovery and exposes no liveness signal. In a multi-tenant SaaS deployment, a single tenant's bad data panicking any subsystem takes down the subsystem for every tenant, silently, with all standard health probes still green. That is a scale-of-one sev-1. This PR: 1. Introduces `platform/internal/supervised/` with two primitives: a. RunWithRecover(ctx, name, fn) — runs fn in a recover wrapper. On panic logs the stack + exponential-backoff restart (1s → 2s → 4s → … → 30s cap). On clean return (fn decided to stop) returns. On ctx.Done cancels cleanly. b. Heartbeat(name) + LastTick(name) + Snapshot() + IsHealthy(names, staleThreshold) — shared in-memory liveness registry. Every subsystem calls Heartbeat(name) at the end of each tick so operators can distinguish "goroutine alive and healthy" from "alive but stuck inside a single tick". 2. Wraps every `go X.Start(ctx)` in main.go: - broadcaster.Subscribe (Redis pub/sub relay → WebSocket) - registry.StartLivenessMonitor - registry.StartHealthSweep - scheduler.Start (the one that died yesterday) - channelMgr.Start (Telegram / Slack) 3. Adds `supervised.Heartbeat("scheduler")` inside the scheduler tick loop as the first end-to-end demonstration. Follow-up PRs will add heartbeats to the other four subsystems. 4. Adds `GET /admin/liveness` endpoint returning per-subsystem last_tick_at + seconds_ago. Operators can poll this and alert on any subsystem whose seconds_ago exceeds 2x its cron/tick interval. 5. Unit tests for RunWithRecover (clean return no restart; panic restarts with backoff; ctx cancel stops restart loop) and for the liveness registry. Net new code: ~160 lines + ~100 lines of tests. Refactor of main.go: ~10 line changes. No behavior change on happy path; only lifts what happens on a panic. Closes #92. Supersedes the local recover added to scheduler.go in #90 (kept conceptually, but now via the shared helper).
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
…un, panic recovery Added scheduler_test.go with 8 test cases covering all previously untested security-critical code paths from PR #90: TestLastTickAt_zero — zero time before first tick TestHealthy_beforeStart — false on fresh scheduler (zero lastTickAt) TestHealthy_freshTick — true when lastTickAt == now TestHealthy_stale — false when lastTickAt is 3×pollInterval ago TestComputeNextRun_valid — "0 * * * *" / UTC returns top-of-hour future time TestComputeNextRun_invalid — unparseable expression returns non-nil error TestComputeNextRun_invalidTimezone — unrecognised IANA zone returns non-nil error TestPanicRecovery — panicProxy crashes ProxyA2ARequest; scheduler goroutine recovers and remains Healthy To support these tests, scheduler.go gained four changes (minimal surface): 1. Added mu sync.RWMutex, lastTickAt time.Time, and tickInterval time.Duration fields to Scheduler. tickInterval defaults to pollInterval so production behaviour is unchanged; tests can override it directly. 2. Added LastTickAt() and Healthy() methods with read-lock protection. 3. tick() now records lastTickAt after wg.Wait() — a single atomic write under the mutex, no hot-path cost. 4. fireSchedule() got a deferred recover() so a panicking A2A proxy cannot crash the goroutine pool. Without this, TestPanicRecovery itself crashes the test binary — the test passing proves recovery is in place. Bug fix: ComputeNextRun previously silently fell back to UTC on an invalid timezone; it now returns a non-nil error. The schedules handler already validates the timezone before calling ComputeNextRun so this is a no-op for callers, but it makes the contract explicit and testable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the silent-outage failure mode of #85 by adding
defer recover()to both the scheduler tick loop and each fire goroutine, plus aLastTickAt()/Healthy()watchdog API for future health-endpoint integration.Why
Scheduler died at 2026-04-14 14:21 UTC and stayed dead for 12+ hours. Platform restart didn't recover it. ~12 hourly-cron firings missed across 3 schedules (Security audit, UIUX audit, QA audit).
Root cause:
tick()and the per-fire goroutine inside it have no panic recovery. A single bad row (corrupt cron expr, malformed prompt), a DB blip, or any other transient panic anywhere in the chain permanently kills the goroutine. The for-loop inStart()exits, no further ticks fire, and the only signal to the operator is "no crons firing" — which is invisible if you're not specifically watching for it.Changes (
platform/internal/scheduler/scheduler.go, +58 / -1)Start()— a panic anywhere intick()is logged and the next tick fires normally.lastTickAtwatchdog — Scheduler records wall-clock time after each successful tick. Read viaLastTickAt()andHealthy()(true if the last tick was within 2× pollInterval = 60s). Initialised at Start so freshly-started schedulers report Healthy=true before the first ticker fire.What's NOT in this PR (follow-ups)
Healthy()into/health— needs threading the scheduler instance throughrouter.Setup(). Separate small PR./admin/scheduler/healthendpoint returning{ healthy, last_tick_at, ticks_count }. Same plumbing./workspaces/:id/restarthangs when the workspace agent is unresponsive). Different code path.active_taskswatchdog — third symptom; a stuck task should auto-clear after 2× max-cron-duration. Different code path.These are filed under #85 comments and can each become a focused PR.
Test plan
"Scheduler: started"and ticks proceed; force a panic in test (e.g. add a cron with bad cron_expr) and verify scheduler keeps ticking.last_run_aton workspace_schedules continues advancing past the next expected fire window.Related
next_run_at = 2026-04-14T15:11:00Z(12+ hours stale). Expected behaviour after this lands + a platform redeploy: scheduler picks up the overdue row on its next tick.🤖 Generated with Claude Code