Skip to content

fix(agents): execute same-cycle due schedules in deterministic order - #1

Open
Glavin001 wants to merge 1 commit into
mainfrom
fix/deterministic-due-schedule-order
Open

fix(agents): execute same-cycle due schedules in deterministic order#1
Glavin001 wants to merge 1 commit into
mainfrom
fix/deterministic-due-schedule-order

Conversation

@Glavin001

@Glavin001 Glavin001 commented Aug 13, 2026

Copy link
Copy Markdown

Summary

The alarm() due-schedule scan had no ORDER BY:

SELECT * FROM cf_agents_schedules WHERE time <= ${now}

When multiple schedule rows are due in the same alarm cycle (common after a DO restart, a delayed alarm, or accumulated overdue rows), their relative execution order was an accident of the storage engine's scan order. Any interaction between same-cycle schedules — e.g. an app-level watchdog schedule racing the SDK's _chatRecoveryContinue continuation — was racy and unreproducible: whichever row happened to scan first changed the outcome.

This orders the scan deterministically:

SELECT * FROM cf_agents_schedules WHERE time <= ${now}
ORDER BY time ASC, id ASC

so earlier-due schedules always dispatch first and same-time ties break stably by id. The fiber-recovery scan over cf_agents_runs (_runFiberRecoveryPass) is aligned the same way (ORDER BY created_at ASC, id ASC) so recovery of orphaned runs is also processed oldest-first and reproducibly.

Kept deliberately minimal (two ORDER BY clauses) so it rebases cleanly onto the AgentScheduler extraction in cloudflare#1897.

Regression test

schedule.test.ts › "same-cycle deterministic ordering": a TestScheduleAgent helper inserts already-due rows in an order that contradicts (time, id) order, drives alarm() once in-instance, and asserts the callbacks executed in (time, id) order. Both cases fail without the ORDER BY (dispatch follows insertion/scan order) and pass with it.

Includes a patch changeset for agents.

Link to Devin session: https://app.devin.ai/sessions/1745b6a41ce947c69f12ffcd9c1dd9d9
Requested by: @Glavin001


Open in Devin Review

The alarm() due-schedule scan (SELECT * FROM cf_agents_schedules WHERE
time <= now) had no ORDER BY, so when multiple schedule rows were due in
the same alarm cycle (e.g. after a DO restart or a delayed alarm) their
relative execution order was an accident of the storage engine's scan
order. Interactions between same-cycle schedules were racy and
unreproducible.

Order the scan by (time ASC, id ASC) so earlier-due schedules always run
first and ties break deterministically. Align the fiber-recovery scan
over cf_agents_runs the same way (created_at ASC, id ASC).

Adds a workers-pool regression test that inserts due rows in an order
contradicting (time, id) order and asserts dispatch follows the
deterministic order (fails without the ORDER BY).

Co-Authored-By: glavin@coframe.com <glavin.wiechert@gmail.com>
@Glavin001 Glavin001 self-assigned this Aug 13, 2026
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown

Tested end-to-end via the workers-pool test suite (@cloudflare/vitest-pool-workers).

Results

  • ✅ Both new "same-cycle deterministic ordering" tests pass on this branch
  • ✅ Full schedule.test.ts suite: 54/54 passed, run 3× with no flakes
  • ✅ Failing-first verified: with src/index.ts reverted to main (no ORDER BY time ASC, id ASC), both new tests fail deterministically (3 retries each), then pass again after restoring the fix
Failing-first evidence (fix reverted to main)
 × should execute schedules due at the same time in id order (retry x3)
   → expected [ 'c-third', 'b-second', 'a-first' ] to deeply equal [ 'a-first', 'b-second', 'c-third' ]
 × should execute schedules due in the same cycle in time order (retry x3)
   → expected [ 'a-due-later', 'b-due-earlier' ] to deeply equal [ 'b-due-earlier', 'a-due-later' ]
 Tests  2 failed | 52 skipped (54)
Passing run on this branch
 ✓ schedule.test.ts > same-cycle deterministic ordering > should execute schedules due at the same time in id order
 ✓ schedule.test.ts > same-cycle deterministic ordering > should execute schedules due in the same cycle in time order
 Test Files  1 passed (1)
 Tests  54 passed (54)

Note: the fiber-recovery scan ordering (ORDER BY created_at ASC, id ASC on cf_agents_runs) has no dedicated failing-first regression test; it's covered only by the suite passing overall.

Tested by Devin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant