fix(cron): deliver other due jobs while one remains unacked - #100
Conversation
Deferred ack kept the earliest due row at the head of the queue, so a failed send or agent run blocked every later reminder until that job succeeded.
84a781f to
0e5bc51
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e5bc51. Configure here.
There was a problem hiding this comment.
Summary
This fixes a real head-of-line block. After a failed delivery the earliest cron row stays next_run <= now, and cron_job_get_next_due used to return only that row, so every later reminder waited on it. The poller now remembers recent offers and walks a (next_run, id) cursor until it finds a due job outside that window. Ack behavior is unchanged, the single-job re-offer wait still holds, and make test_cron passes here (linked with -lm; this image has no libcurl).
The backoff does not survive more than 16 in-flight offers. Past that, a stuck channel tight-loops agent_run. GitHub rejects a changes-requested review from this account on its own PR, so this is a comment. The ring bug should land before merge.
Must Fix
src/tools/cron.c:CRON_OFFER_TRACKis 16, and a full ring overwrites the oldest id.cron_offer_is_hottreats a missing id as cold, so the next poll returns that job with nocron_wait_remaining. I drove the built objects with 17 due jobs and an 800ms timeout: the first pass returnsj00throughj16immediately (correct), then the next two polls returnj00andj01with elapsed 0. The same driver with 2 jobs waits out the timeout on the third poll.POLL_TIMEOUT_MSis 1000 (src/core/main.c), and a systemdcli_pollof/dev/nullreturns immediately, so this sleep is the only gap between provider calls. Create is uncapped and the list API already returns 64 rows, so a down channel with a backlog of reminders hits this. Reuse a slot only after its recorded age is past the timeout. If every slot is still inside the window, take the existing fallback and wait on the earliest due job.
Should Fix
tests/test_cron.c: the new test locks "offerjob_bwhilejob_ais hot" and that neithernext_runmoves. It does not lock the fallback once every due job is hot (the next poll should wait, then returnjob_a), or the 17-job case above.s_offersis process-global, and neithercron_cleanupnormemory_cleanupclears it. A second scenario in the same process skippedj00on a fresh database because an earlier poll had marked that id. Later cron poll tests in this binary can inherit a hot id. Zero the table when the channel shuts down, and at the start of tests that callpoll.
Nice to Have
cron_pick_due_rowissues one query per skipped hot row. A singleORDER BY next_run, idscan would do the same walk in memory.cron_job_createstill accepts an id longer than 127 bytes.fill_cron_job_rowtruncates intoid[128], the cursor is that prefix, and the monotonicity check then stops the walk, so an overlong head id hides later jobs again. The tool andPOST /api/cronalreadysnprintfwith%.127s. Reject the long id at insert.cron_job_listis stillORDER BY next_runonly, so ties do not match the poller.
Positive Highlights
- The due-row predicate is bound parameters, and
ORDER BY next_run, idmatches the cursor (next_run > ? OR (next_run = ? AND id > ?)). - The guard in
cron_pick_due_rowthat bails when a fetched row is not strictly after the cursor prevents a repeat row from spinning the walk. - The regression checks the actual contract:
job_bcomes back in well under the 2000ms timeout, and both rows stay at their originalnext_run.
Sent by Cursor Automation: Code review
A full ring used to drop the oldest id, and the next poll treated that job as cold, so a down channel tight-looped agent_run. Reuse a slot only after its recorded window has elapsed, and wait on the earliest due job when every slot is still hot.



Deferred ack left a failed cron job at the head of the due queue.
cron_job_get_next_duealways returns that earliest row, so every later reminder waits until the stuck job is delivered.Bug and impact
Two jobs are due. Job A is earlier. Its channel send fails, or
agent_runreturns an error (send_then_maybe_ack_cronskipscron_ack_delivery). Job A staysnext_run <= now. Every poll offers A again and never reaches job B. A permanently broken recipient (missing channel, provider error) stops the rest of the schedule.Root cause
dcf193bmoved delete/reschedule to after a successful delivery, but the poller still selectsORDER BY next_run ASC LIMIT 1with no way to pass a job that is still inside its re-offer window.Fix
cron_pollremembers recent offers and, while the earliest job is still inside that window, offers the next due job. If it is the only due job, the existing re-offer wait is unchanged. Ack semantics are unchanged: the stuck job stays due and is retried.Validation
make test_cron,make test_dispatch, andmake test_memorypassed. The new regression offers job B immediately while job A is still unacked, and the single-job re-offer wait test still holds.Note
Medium Risk
Changes cron channel polling and due-job selection logic that affects reminder delivery order and timing, though ack/reschedule behavior is preserved and regression tests were added.
Overview
Fixes a case where an unacked earliest due job blocked every later reminder:
cron_pollno longer always takesORDER BY next_run LIMIT 1when that job is still inside its re-offer window.The poller now keeps a 16-slot offer table (per-job timeout) and
cron_pick_due_rowscans due jobs via newcron_job_list_due, preferring jobs that are returnable while the head job is “hot.” If only the earliest job is due, behavior is unchanged—it still waits before re-offering.cron_ack_deliverysemantics are unchanged; stuck jobs stay due and retry.The memory layer adds stable ordering
next_run ASC, id ASC,cron_job_get_next_due_afterfor cursor-style due selection, and rejects ids ≥ 128 chars on create so truncated ids cannot break tie-breaking. Tests cover sibling delivery, a full offer table, and tie ordering.Reviewed by Cursor Bugbot for commit e09b2e9. Bugbot is set up for automated code reviews on this repo. Configure here.