Skip to content

test: the resume tests wait for the run they resume, not a fixed 30 ms - #131

Open
endrix wants to merge 2 commits into
mainfrom
test/resume-run-race
Open

endrix wants to merge 2 commits into
mainfrom
test/resume-run-race

Conversation

@endrix

@endrix endrix commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

sidecar-toolkit/test/resume-failed-run.test.ts failed in CI on #129 and on #127, with different symptoms each time: one spawn too few or too many, or "There is no failed run to resume". Main was green only by luck.

Why

resumeFailedRun starts the resumed run without awaiting it (cli-run-driver.ts: "a run can outlast a tool call"). The tests waited a fixed 30 ms. On a loaded runner the resumed run spawned later, during the next test, after beforeEach had reset the spawn list. The fake process decided "fail" by spawns.length === 1, so the next test's own run became spawn #2 and did not fail. That test then found no failed run to resume.

Fix (test only)

  • The fake run fails a plain run and passes a resumed one, decided by whether its arguments carry --resume-from. No count is shared across tests.
  • Each test that expects the resumed run waits for the run's own last word, "Workflow run finished.". The driver says it after the diagram refresh and after it forgets the failed run, so the child's close is not the end. The wait uses vi.waitFor, under the default test timeout, so a slow machine fails by name.
  • The temporary directories each run makes are removed after each test. The "not now" test drops a pause that waited for nothing.

Checks

  • With temporary delays (not committed) of 100 ms before every resumed run and 150 ms after every child's close, the file on main fails 2 of 9 and this one passes 9 of 9.
  • Without the delays, 9/9 three times in a row.

https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv

The driver starts a resumed run without awaiting it (a run can outlast a tool
call). The tests waited 30 ms for it; on a loaded CI runner the resumed run
started later, in the next test, where the fake process counted it as that
test's first run and so did not fail it: "no failed run to resume", or one
spawn too many or too few. Seen on #127 and #129, never on main by luck.

- The fake run fails a plain run and passes a resumed one, decided by its
  arguments, not by a count every test shares.
- A test that expects the resumed run waits until it is spawned and has
  ended (`resumedRun`), however long that takes.

With a 100 ms delay put in front of every resumed run, the old file fails 3
of 9 and this one passes 9 of 9.

Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv
The child's `close` is not the end of a run: the driver still refreshes the
diagram, forgets the failed run and says "Workflow run finished." after it,
and that tail could still land in the next test. A test that expects the
resumed run now waits for that last word, under the default test timeout so a
slow machine fails by name. The temporary directories each run makes are
removed after each test, and the "not now" test drops a pause that waited for
nothing (the resume returns before anything is spawned).

With 100 ms added before every resumed run and 150 ms after every child's
close (not committed), main's file fails 2 of 9 and this one passes 9 of 9.

Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv
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