fix(server): scheduled runs record how their turn ended#23131

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve next_run_at when the terminal event wins the race.

recordRunEnd can write a terminal status after trackedRuns.dispatched(messageId) returns but before markCompleted runs. For terminal input, the suggested WHERE condition becomes 0 = 1 OR last_run_status = 'running', so it rejects the update after recordRunEnd wins. The task remains due and can run again on the next poll.

Match the row by task_id and last_run_at. Preserve the terminal status and error already written by recordRunEnd, and increment run_count only when the previous status was running and the completion is terminal.

✅ Confirmed as addressed by @voida6

Same race as the Macroscope finding above, same fix in 0e9db7228d (outcome is settled only after the dispatch bookkeeping is saved). No deterministic repro of the exact window, so it is fixed structurally.

@voida6 Thanks for clarifying. The supplied code now calls trackedRuns.dispatched(...) only after the dispatch bookkeeping is saved. This ordering addresses the reported race without changing the SQL guard. A deterministic reproduction of the exact window remains unverified.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep the scheduled-task event listener alive and reconcile missed events.

Stream.runForEach(threadManagement.streamDomainEvents, ...) catches a stream failure by logging it, then completes. forkParked uses Effect.forkScoped without restart supervision, so a transient event-store failure can permanently stop outcome processing while the scheduler continues.

streamDomainEvents starts from the latest sequence. A later listener restart can therefore skip terminal events emitted during the outage. The affected rows remain running, and listDueTasks excludes running rows. Supervise the listener and replay or reconcile events from a durable cursor before resuming the live tail. Retrying only the live subscription would not recover events already skipped.

✅ Addressed in commit 2cbe737

Partly addressed in 0e9db7228d. The listener now resumes after a stream failure with a bounded exponential backoff (5 retries), and before each (re)subscribe it catches up still-running rows by reading the message's run from the thread projection (getThreadRecords) and replaying it as a run update. If retries are exhausted, the remaining runs are released as failed ('Run outcomes could no longer be followed.'). Tests cover resume plus catch-up and give-up. Not covered: no durable event cursor, so an event landing in the instant between the catch-up read and the new subscription can still be missed (that run then waits for a later event or the restart recovery); provider error text is not recovered on catch-up, so a failed run reads as 'The run failed.'; the retry budget is per process lifetime, not reset after a healthy period.

@voida6 Thanks for the clarification. The supplied code shows bounded retries, projection catch-up, and failure release after retry exhaustion. These address the listener stopping permanently.

The catch-up/subscription gap remains part of the original finding. If a terminal event lands in that gap, the row can remain running even after the listener recovers. Retry exhaustion does not cover this case because the new subscription can stay healthy.

A durable cursor is not the only solution. Establish the live subscription and buffer events before reading the projection, then process the buffered events. Add a deterministic test that emits a terminal event during this handoff and verifies that the task leaves running without another event or a server restart.

The generic catch-up error and lifetime retry budget are separate limitations. The remaining handoff gap is why this finding stays open.

You are interacting with an AI system.

Fixed in 2cbe737350. The stream is now started (first pull forked) before the projection catch-up, so a terminal event landing in between is delivered by the stream. A run seen by both is settled once because the tracker forgets it when it ends. Test: the run is still running in the projection read and completes during it; it times out with the old order and passes now.

@voida6 The supplied code addresses the remaining handoff gap. It starts the first stream pull before projection catch-up, so the stream captures terminal events that arrive during catch-up.

The reported regression test covers the requested race. I have not run it independently. This addresses the finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Recover each failed outcome write.

If recordRunEnd still fails after retries, logging alone leaves the task row as running. listDueTasks then excludes it, so that task can remain unavailable until service restart. Retrying alone does not handle a failure that persists across all attempts.

Process each outcome independently. After the retries fail, call releaseStuckRun for that run’s task. Catch fallback failures per outcome so one failed write does not prevent later outcomes from being recorded.

✅ Addressed in commit 0e9db72

Fixed in 0e9db7228d. Each outcome is now handled on its own: recordRunEnd is retried twice, and if it still fails the run is released as failed via releaseStuckRun (error 'Could not record how the run ended.'), so the row does not stay running and the task is not blocked. Test forces the outcome write to fail and expects the row to end failed.

@voida6 Thanks for the fix. The supplied code handles each outcome independently, retries twice, and calls releaseStuckRun if persistence still fails. This addresses the finding. I did not run the regression test independently.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

A retry of a cancelled run skips the outcome write and leaves the row running.

settleOutcome retries all of settleEnded, not only recordRunEnd. This fails for a cancelled run (cancelledRunId !== null):

1. On the first attempt, trackedRuns.confirmCancel returns true and deletes the tracked entry.
2. recordRunEnd then fails, for example with a transient SQLite error.
3. On the retry, confirmCancel cannot find the entry and returns false.
4. settleEnded returns without an error, so the retry counts as a success.
5. catchCause never runs, so releaseStuckRun is never called.

The row stays running. listDueTasks filters out running rows, so the task stops firing until a manual run or a server restart. This is the same blocked-task state that the fallback is meant to prevent.

The fix is to run the confirmCancel decision once and retry only the persistence step.

✅ Addressed in commit 2cbe737

Fixed in 2cbe737350. The cancel decision (confirmCancel) is now made once in settleOutcome and only recordRunEnd is retried, so a failed first write no longer turns the retry into a silent no-op. Added a test that fails the first write on a cancelled run; it times out on the previous code and passes now.

@voida6 Thanks for the fix. The supplied code confirms that confirmCancel runs once and only recordRunEnd is retried. This prevents the silent no-op after a failed first write. The reported regression test covers that failure path.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

After the stream retries are used up, every later run stays running until restart.

When the 5 retries are exhausted, abandonAll releases only the runs tracked at that moment. After that, the listener fiber is gone, but runTask still dispatches new runs:

• markCompleted writes last_run_status = 'running' for each new run.
• trackedRuns.track adds the run to a tracker that no event will ever update.
• No later event or recovery path settles these runs.

listDueTasks filters out running rows. So after the budget is spent, each scheduled task fires once more and then stops until the server restarts. Nothing tells the user. The budget is never reset after a healthy period, so 5 drops spread over a long uptime are enough to cause this.

This is separate from the earlier listener-supervision comment. That comment covered runs already in flight; this covers runs dispatched after the give-up.

Choose one of these fixes:

• Preferred: keep resubscribing forever with a capped backoff, for example Schedule.exponential("1 second").pipe(Schedule.either(Schedule.spaced("1 minute"))) without times. The catch-up at each resubscribe already reconciles missed ends.
• Alternative: set an "outcomes unavailable" flag in the give-up branch. When the flag is set, runTask records accepted dispatches as terminal (the previous succeeded behavior) and does not leave them running.

✅ Addressed in commit 2cbe737

Fixed in 2cbe737350, taking the suggested approach: the listener resumes forever with an exponential backoff capped at 1 minute, no retry limit. Runs dispatched while it is down stay tracked and are caught up on the next subscribe. The give-up branch and abandonAll are deleted. Test: the stream fails 8 times, then the run is recorded.

@voida6 Thanks for the fix. The supplied code retries indefinitely with backoff capped at one minute and preserves catch-up on each resubscription. This addresses the listener-exhaustion finding. The reported eight-failure test covers the previous retry limit.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Forget only the unreadable run, not every run of the task.

catchUpRuns iterates over a snapshot from trackedRuns.pending(). Each item's read retries take about 3.5 seconds in total (500 ms, 1 s and 2 s of backoff) before the failure branch runs. During that time, a user can press Run now. runTask has already released activeRuns, and the UI keeps Run now available while a run is running.

trackedRuns.track for the new message calls forgetTask(taskId). This removes the old run from the tracker. The stale pending item still holds the old message.

If the old thread read then fails, line 1199 calls trackedRuns.forgetTask(pending.taskId). This removes the new run from the tracker. releaseRun correctly skips the row, because startedAtIso no longer matches. But nothing now settles the new run. The row stays running and listDueTasks skips it until the server restarts.

Add a per-message removal method to makeTrackedRuns. Use it here.

✅ Confirmed as addressed by @voida6

Confirmed and fixed in f5ae53f9b4. The catch-up failure branch now calls a new TrackedRuns.forgetMessage(messageId) instead of forgetTask, so it drops only the run it gave up on. Audit of the other forgetTask callers: all three are in runTask for the run it just dispatched, where the task row is still running for that same run, so a newer run cannot have started yet. Test: the catch-up read keeps failing, Run now is pressed during the retries, and the newer run still settles from its event; it times out on the previous code and passes now.

@voida6 Thanks for confirming. The supplied code now uses forgetMessage(pending.messageId), so stale catch-up cleanup cannot remove the newer run. Your reported regression test covers the exact race described in this finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Trigger catch-up before waiting for a failed read.

If the listener completes its initial catch-up before runNow dispatches, the healthy event stream never starts another catch-up. failedReads stays empty, so Queue.take(harness.failedReads) waits indefinitely. Make the harness drop and resubscribe after the first dispatch, then wait for the failed read.

✅ Addressed in commit beceabd

Confirmed, fixed in beceabdfb0 (test only). Without a stream drop the test only saw a failed read because the listener's first catch-up happened to start after the dispatch. That is fiber ordering, as you said. The test now drops the stream after the first dispatch, which guarantees a resubscribe catch-up that reads the dispatched run, and only the first message's projection reads fail, so the newer run's read and its event succeed whichever catch-up handles it (my earlier attempt with a plain stream drop let a later catch-up release the newer run). It still times out on the previous forgetTask code and passes on the fix; it passed 5 of 5 repeated runs.

@voida6 Thanks for the confirmation. The supplied code addresses the race: it drops the stream after dispatch to trigger catch-up before waiting for the failed read. Restricting read failures to the first message also protects the newer run from unintended release.

Your reported regression check and five repeated passes support the fix.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}