Skip to content

test: retry transient Windows removals ourselves (Bun ignores fs.rm maxRetries) - #9026

Closed
code-yeongyu wants to merge 4 commits into
devfrom
fix/win-ci-worker-compile-teardown
Closed

code-yeongyu wants to merge 4 commits into
devfrom
fix/win-ci-worker-compile-teardown

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

Test cleanups across the repo passed maxRetries / retryDelay to fs.rm / fs.rmSync to absorb Windows EBUSY on temp-dir teardown. Bun 1.4.2 parses those options and never retries (oven-sh/bun#41480, still open), so every one of those retries was inert. This PR routes them through one helper that does the retry itself.

Root cause

  • The compiled-worker test failed teardown with EBUSY (integration run 36340211078 attempt 3). A first fix that raised the cleanup to maxRetries: 10, retryDelay: 500 failed again in a focused soak (run 36343784552, iteration 16). The whole test took 1382 ms, so the 5 s retry window never ran.
  • Bun's fs.rm ignores both options. The same inert pattern is in 66 cleanups across 65 test files (memory-core, omo-senpi memory commands, isolation-core's shared fixture, script tests), feeding the rotating Windows teardown failures in ci(windows): dev push CI fails a rotating Windows test by timeout on every run, blocking the publish gate #8324.

Fix

  • test-support/remove-tree.ts: removeTree / removeTreeSync call rm(path, { recursive: true, force: true }) and retry EBUSY, EPERM, EMFILE, ENFILE and ENOTEMPTY with linear backoff. When the budget runs out they rethrow the original error. Non-transient errors rethrow immediately. The sync path uses Bun.sleepSync, falling back to Atomics.wait under node.
  • Every .ts call site keeps its existing retry numbers. Existing .catch(() => undefined) wrappers are unchanged.
  • Not converted, with the reason: packages/omo-codex/plugin/scripts/sync-skills.mjs and packages/omo-senpi/plugin/scripts/sync-skills.mjs are shipped build scripts, and two packages/omo-senpi/scripts/qa/*-e2e.mjs drivers run under node and cannot import a TS helper. Their options are equally inert and are left as-is.

QA & Evidence

  • New unit tests for the helper cover transient-then-success, transient-until-exhausted (rethrows the same error), and non-transient (no retry), for both async and sync.
  • bun test test-support packages/isolation-core packages/memory-core script/senpi-worker-compile.test.ts: 1212 pass, 11 skip, 0 fail.
  • bun test packages/omo-senpi/src/components/memory: 1644 pass, 0 fail.
  • tsgo --noEmit for memory-core, omo-senpi, isolation-core and script is clean, and biome is clean.
  • git grep -E "(rm|rmSync)\\([^)]*maxRetries" over .ts returns only the helper.
  • Windows focused soaks (3 x 20 iterations) are listed in a comment.

Fixes #9025
Refs #8324

…ase window (#9025)

Deleting the scratch dir right after running the relocated omo.exe raced Windows' post-exit image lock and failed with EBUSY although the test passed. Use the same bounded retry window the isolation-core fixtures use for this EBUSY family.
…9025)

Bun parses fs.rm maxRetries/retryDelay but never retries (oven-sh/bun#41480). removeTree/removeTreeSync retry EBUSY, EPERM, EMFILE, ENFILE and ENOTEMPTY with linear backoff and rethrow the real error when the budget runs out.
66 cleanups in 65 test files passed maxRetries/retryDelay to fs.rm, which Bun ignores, so their Windows EBUSY retries never ran. Each now keeps its retry budget through removeTree/removeTreeSync. The compiled-worker test's buildRoot removal is included.
@code-yeongyu code-yeongyu changed the title test(build): give the compiled-worker teardown the Windows image-release window test: retry transient Windows removals ourselves (Bun ignores fs.rm maxRetries) Sep 27, 2026
@github-actions github-actions Bot added isolation-core Changes under packages/isolation-core omo-senpi Changes under packages/omo-senpi memory-core Changes under packages/memory-core labels Sep 27, 2026
@code-yeongyu

Copy link
Copy Markdown
Owner Author

Focused Windows soaks of this head (windows-latest, 20 iterations each; script/senpi-worker-compile.test.ts + packages/isolation-core/src/git + test-support + packages/memory-core/src/git): 36345727979 success, 36345729528 success, 36345731033 success.

… test (#9025)

The relocated-worker job copies the worker test away from the repository to prove the compiled worker needs no source tree. The test now imports test-support/remove-tree (node builtins only), so the job mirrors the script/ and test-support/ layout in its scratch directory.
@code-yeongyu

Copy link
Copy Markdown
Owner Author

Superseded by #8961, which lands this fix together with the other win-ci fixes as one merge (all commits of this branch kept, bundles regenerated once over the merged sources). The evidence in this PR (RED/GREEN, focused Windows soaks) still applies; the issue is closed by #8961's Fixes line.

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

Labels

ci:full-matrix Force the full 3-OS CI matrix on this PR isolation-core Changes under packages/isolation-core memory-core Changes under packages/memory-core omo-senpi Changes under packages/omo-senpi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows CI: test cleanups rely on fs.rm maxRetries, which Bun ignores (EBUSY teardown failures)

1 participant