Skip to content

fix: scheduled task can run twice when an approval lands on a tick - #379

Open
Saidheerajgollu wants to merge 1 commit into
andrewyng:mainfrom
Saidheerajgollu:fix/scheduler-stale-tick-double-run
Open

fix: scheduled task can run twice when an approval lands on a tick#379
Saidheerajgollu wants to merge 1 commit into
andrewyng:mainfrom
Saidheerajgollu:fix/scheduler-stale-tick-double-run

Conversation

@Saidheerajgollu

Copy link
Copy Markdown

Main's CI has been failing intermittently on test_blocked_run_does_not_stall_other_tasks with assert 2 == 1 — e.g. run 30587970128 (the #356 merge) and 30570317824. It's not a flaky test; the scheduler can actually run a task twice.

The sequence:

  1. A run parks on an approval. Its task stays due in the store, since next_run only advances after completion.
  2. The next tick snapshots due() and spawns a duplicate run_task. The overlap guard is checked inside the spawned coroutine, not at spawn time.
  3. If the approval lands just before that tick, the parked run resumes first, finishes, and clears the guard. The duplicate then takes its first step, finds the guard empty, and runs the task again — two runs of an automation that was waiting on a human answer.

The fix claims the guard synchronously in _tick before spawning (no await between check and add) and skips the spawn when a run is already in flight. run_task keeps the same claim for direct callers, so test_scheduler_skips_overlapping_run behaves as before.

The new test forces the exact interleaving deterministically (tick → park → approve → tick → drain). It fails on current main every time and passes with the fix. The existing test passes unchanged since the duplicate spawn no longer exists. Full suite: 1016 passed, 1 skipped.

…awned run

A tick's due() snapshot still lists a task whose run is parked on an
approval — next_run only advances on completion. The overlap guard lived
inside the spawned coroutine, so when an approval landed just before a
tick, the parked run could finish and clear the guard before the
duplicate spawn took its first step, and the task ran twice.

This is the intermittent 'assert 2 == 1' in
test_blocked_run_does_not_stall_other_tasks on main's CI. The new
regression test forces that interleaving deterministically.
@Saidheerajgollu

Copy link
Copy Markdown
Author

Another hit today, on an unrelated branch in my fork running the same workflow: https://github.com/Saidheerajgollu/openworker/actions/runs/30665681535 — same assert 2 == 1 in test_blocked_run_does_not_stall_other_tasks. The interleaving lands often enough on GitHub's runners that any branch on current main can go red.

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