fix: atomic claim prevents scheduled tasks from executing twice

[gabi-simons]

Summary

Fixes the scheduler race condition where tasks execute twice because enqueueTask() only deduplicates against pendingTasks — a running task has already been shifted out, so the next poll re-queues it.

Closes #138, #211, #300, #578, #669

The fix

3-line fix in group-queue.ts: Add runningTaskId to GroupState. enqueueTask() now checks both the pending array and the currently-running task ID, rejecting duplicates at the queue layer where the state lives.

Drift fix in task-scheduler.ts: computeNextRun() anchors interval tasks to scheduled_time + N*interval instead of Date.now() + interval, preventing cumulative timing drift. Includes a guard against zero/negative interval values.

Why not "atomic claim" (previous approach in this PR)

The first commits implemented claimDueTasks() — a SQLite transaction that advanced next_run before execution. Per review, this had real downsides:

• Once-task crash risk: process dies after claim → task orphaned forever
• No retry on failure: next_run already skipped ahead
• Misleading transaction: Node.js is single-threaded, better-sqlite3 is synchronous — no interleaving possible

The dedup belongs in GroupQueue, not the DB layer.

Test plan

• 334/334 tests pass (npx vitest run)
• New regression test: rejects duplicate enqueue of a currently-running task
• Drift test: interval anchored to scheduled time, not wall clock
• Deploy to staging and verify no duplicate task runs over 24h

Credits

Drift logic from PR #601 by @taslim. Issues reported by @baijunjie (#138), @Michaelliv (#300), and @kk17 (#669).

🤖 Generated with Claude Code

[gavrielc]

The drift fix (anchoring to scheduled_time + N*interval) is a solid improvement — worth keeping regardless of approach.

But I think the dedup is being solved at the wrong layer. The GroupQueue already deduplicates pending tasks by ID (group-queue.ts:94), but that check misses currently-running tasks (they've been shifted out of pendingTasks). So the next poll re-discovers the task and re-queues it. That's the actual bug.

Advancing next_run before execution works around it, but has real downsides:

• Once-task crash risk: process dies after claim (next_run = NULL) but before execution → task is orphaned forever (active status, but no poll finds it).
• No retry on failure: execution fails, but next_run already skipped ahead.
• Transaction framing is misleading: Node.js is single-threaded, better-sqlite3 is synchronous — no interleaving is possible. The transaction provides no protection over sequential statements.

The fix is ~3 lines in group-queue.ts — track the running task ID in GroupState and include it in the dedup check:

// Add to GroupState: runningTaskId: string | null
// In enqueueTask: if (state.isTaskContainer && state.runningTaskId === taskId) return;
// Set/clear in runTask()

This keeps next_run accurate (updated post-execution), preserves crash recovery, and avoids coupling scheduler logic into the DB layer. The computeNextRun drift fix can be applied in updateTaskAfterRun where it belongs.

[Koshkoshinsk]

E2E Test Results

@gabi-simons — Full end-to-end testing completed. All checks pass.

Summary

#	Check	Expected	Result
1	Phase 2 (main): duplicate execution	2+ runs	2 runs (10:21:33 & 10:21:48) — bug reproduced
2	Phase 3 (PR): "Task already running, skipping"	Duplicate rejected	3 duplicates rejected
3	Phase 3 (PR): single execution in DB	1 row	1 row (success, 49793ms)
4	Phase 4: npm test	0 failures	351 tests passed (30 files, 0 failures)

Test Details

• Bug reproduction (main): Reduced poll interval to 15s, created an interval task with sleep 25. On main, the task was enqueued twice within 15 seconds — confirmed duplicate execution.
• Fix verification (PR branch): Same setup. Logs showed Task already running, skipping on 3 consecutive polls while the container was active. DB confirmed exactly 1 execution.
• Unit tests: All 351 tests pass including the 4 new tests (computeNextRun anchoring, once-task null return, missed interval skip, duplicate enqueue rejection).
• Edge case review: Verified that different tasks for the same group are not blocked, runningTaskId clears properly in finally on both success and failure paths, and messages queue correctly while tasks run.

No regressions found.

[gavrielc]

Great fix. Thanks!!!