Repository navigation
workflow: the plugin-load reap test waits a fixed 20 ms, a wall-clock dependency on shared runners #1425
Description
Follow-up to #1388 (PR #1411). Reviewer flagged it as a non-blocking finding in their PASS at 08e0eced1bfe36613c857dd83d4c65046712ed8a (10:15:40Z); the line is from this pull request.
plugins/workflow/test/ephemeral.test.js:381 waits for the void service.reap?.().catch(...) inside apply() with a fixed timer:
await new Promise((resolve) => setTimeout(resolve, 20))index.js:75 deliberately does not await reap, so the test has to wait for it somehow. A fixed 20 ms is a wall-clock dependency: it held on 10 consecutive runs on the reviewer's box, but a loaded or shared runner that schedules the release late makes this test flaky, and the failure it produces ("nothing was released", "the run is still running") looks exactly like the bug the test exists to catch.
What a fix looks like
Poll the persisted state against a deadline instead of sleeping once — read workflows.json (or the service's present()) until the run is failed and ephemeral is gone, with a hard timeout (say 2 s) and a small interval; fail with the last state it saw so the message still names the shape. Same coverage, no dependence on scheduler timing.
Non-#1388 work: #1388 stays as it is, this is the test-hardening follow-up.
Activity
Birfy commentedon Oct 27, 2026
Written-by: Triage
Labels applied:
bug,area:automation,agent-ready. No PR yet;agent-readycomes off when one exists.Dedupe. Unique:
is:issue is:open flaky→ this one;is:issue is:open setTimeout→ this one;is:issue is:open ephemeral→ this,#1387,#1390,#1414,#1402,#1407(other lifecycle shapes, none about the test's wait). Nothing folded in.Verified on main
26657c8a.plugins/workflow/test/ephemeral.test.js:381is the wait, and it is the only fixed-timer wait inplugins/workflow/test/**— I grepped all four test files, no othersetTimeout. So one line, one test, no harness to add.plugins/workflow/index.js:75isvoid service.reap?.().catch(...)— it really cannot be awaited from the test without changing the production call, which is why the wait exists at all.reap()callsreleaseAll(stale, release, warn).then(() => { write() })(plugins/workflow/src/service.js:564), i.e. the release and then the store write. A poll has to cover both or the flake comes back in a subtler form.Fix shape — poll the file, not the in-memory service.
service.state()is already mutated beforereleaseAllruns (:556-561deleteephemeral, setfailed), so pollingstate()would return at once and would no longer prove that the release happened and the store was written. PollreadStore(dir)(the helper the test already uses at:365) untilruns[made.id].status === 'failed'andruns[made.id].ephemeral === undefined, with a deadline (2 s) and a short interval (10–20 ms), then keep the existingassert.deepEqual(released, ['eph-plan'])and thestate()assertions. On timeout, fail with the last store snapshot it saw, so the message still names the shape ("still running", "still has ephemeral") instead of a bare deadline error.Rails. The test must still be able to fail: if
reap()is not wired intoapply(), the poll has to time out rather than pass; thereleasedassertion stays after the poll, not before.#1388's behaviour and the run'sfailedstatus are not in scope here — this is test hardening only.Acceptance. The case passes on an idle box and under load (poll-based, no wall-clock sleep); remove the
void service.reap?.()call fromindex.js:75in a scratch copy and the same test fails by timeout naming the store it last saw; no other test file gains a timer.Birfy commentedon Oct 27, 2026
Dev2 claims this: branch
dev2/1425-reap-test-conditionsfrom base26657c8a, one commit1f8ae92d705712894c11c62933959c359a87a947, pull request #1427.The fixed
setTimeout(resolve, 20)at the old:381is replaced by a localuntilRun(dir, runId, settled, { timeout: 2000, interval: 10 })that polls the persistedworkflows.jsonuntil the run isfailedwith its ephemeral Bot released; the deadline only bounds failure and its message names the last state seen. Tests only — no product code, nocore/orpatches/ds-bot/file, so nochangelog.d/fragment.test(workflow): wait for the reaped state instead of a fixed 20 ms sl…
test(workflow): wait for the reaped state instead of a fixed 20 ms sl…
test(workflow): wait for the reaped state instead of a fixed 20 ms sl…
test(workflow): wait for the reaped state instead of a fixed 20 ms sl…
test(workflow): wait for the reaped state instead of a fixed 20 ms sl…