fix(scheduler): prevent next_run_at=NULL permanently silencing schedules (#722) - #723
Closed
molecule-ai[bot] wants to merge 2 commits into
Closed
fix(scheduler): prevent next_run_at=NULL permanently silencing schedules (#722)#723molecule-ai[bot] wants to merge 2 commits into
molecule-ai[bot] wants to merge 2 commits into
Conversation
…les (#722) Three-part fix for the scheduler null next_run_at regression observed on the Documentation Specialist workspace (all 3 schedules created 2026-04-16, never fired, next_run_at=NULL, run_count=0). Bug 1 (fireSchedule) + Bug 2 (recordSkipped): when ComputeNextRun fails, the UPDATE wrote NULL to next_run_at (nextRunPtr=nil → $2=NULL → next_run_at=NULL). Fix: COALESCE($2, next_run_at) — preserves existing value when $2 is NULL. Also adds a WARN log so ComputeNextRun failures are no longer silent. Bug 3 (Start): no startup repair for rows already silenced. tick() filters WHERE next_run_at IS NOT NULL, so a NULL row is permanently skipped. Fix: repairNullNextRunAt() called once at Start() before the first tick. Queries enabled schedules with next_run_at IS NULL, recomputes next_run_at, patches the row. Schedules with unparseable cron expressions are left alone (logged as a warning) — they need operator intervention anyway. org.go importer: ComputeNextRun error was discarded with _, passing zero time.Time{} (0001-01-01) to INSERT. Fix: capture the error, pass *time.Time (nil=NULL) — the startup repair covers NULL rows on next boot. Tests: TestRepairNullNextRunAt_repairsRows, _noRows, _badCronSkipped. Closes #722. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CLAUDE.md convention: always check rows.Err() after iterating result sets. hibernateIdleWorkspaces() in the same file does this correctly; repairNullNextRunAt was missing it — a connection drop mid-iteration would silently treat the partial scan as complete, leaving some NULL-next_run_at schedules permanently silenced. Add the rows.Err() check after the for loop and a new test TestRepairNullNextRunAt_rowsErrAborts that uses sqlmock.RowError to exercise the early-return path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
There was a problem hiding this comment.
APPROVED — Fix is correct. rows.Err() check added after the loop in repairNullNextRunAt() with a clear comment explaining the risk (partial repair silently treated as complete). New test TestRepairNullNextRunAt_rowsErrAborts covers the error path with a simulated mid-iteration connection drop. Meets codebase convention. Ready to merge.
6 tasks
Contributor
Author
|
Closing as superseded by PR #728 (), which already merged to main at ae7df68 with the same scheduler null fix for issue #722. Both PRs fix the same bugs (COALESCE in fireSchedule/recordSkipped, repairNullNextRunAt, org.go error capture, rows.Err() check). PR #728's implementation is on main; PR #723's additional rows.Err() check was also included in #728's scope. |
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
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.
Problem
Three related bugs all lead to
next_run_at=NULLonworkspace_schedulesrows, permanently silencing enabled schedules. Thetick()poll loop filtersWHERE next_run_at IS NOT NULL, so any row with NULL is skipped forever — no error, no log, no history entry.Observed: Documentation Specialist workspace — all 3 schedules created 2026-04-16, never fired,
next_run_at=NULL,run_count=0. Fixed only by manual PATCH.Bug 1 —
fireSchedulewrites NULL onComputeNextRunfailureFix:
COALESCE($2, next_run_at)— preserves existing value when$2is NULL. Added WARN log so the failure is no longer silent.Bug 2 —
recordSkippedsame issueIdentical pattern to Bug 1 in
recordSkipped. Same fix.Bug 3 — No startup repair for rows already NULL
tick()filtersWHERE next_run_at IS NOT NULL AND next_run_at <= now()— rows already silenced are never seen. No repair existed.Fix:
repairNullNextRunAt()called once inStart()before the first tick. Queries enabled schedules withnext_run_at IS NULL, recomputes viaComputeNextRun, patches them. Schedules with unparseable cron expressions are logged and left alone — they need operator intervention.Bug 4 —
org.goimporter discardsComputeNextRunerrorFix: capture error, use
*time.Time(nil=NULL). The startup repair covers NULL rows on next boot.Files changed
platform/internal/scheduler/scheduler.gorepairNullNextRunAt()+ Bug 1/2 COALESCE + WARN logsplatform/internal/scheduler/scheduler_test.goplatform/internal/handlers/org.goTests added
TestRepairNullNextRunAt_repairsRows— one null row, expects UPDATETestRepairNullNextRunAt_noRows— empty result, no UPDATETestRepairNullNextRunAt_badCronSkipped— unparseable cron, no UPDATECloses #722.