Skip to content

fix: de-flake hook-bus observe-hook test (JSON EOF race) - #168

Merged
saucam merged 1 commit into
mainfrom
fix/flaky-hook-bus-json-race
Jul 12, 2026
Merged

fix: de-flake hook-bus observe-hook test (JSON EOF race)#168
saucam merged 1 commit into
mainfrom
fix/flaky-hook-bus-json-race

Conversation

@saucam

@saucam saucam commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Symptom

main CI (daemon job) is failing intermittently:

SyntaxError: JSON Parse error: Unexpected EOF: src/tests/hook-bus.test.ts:255

Root cause — a pre-existing race, newly exposed

The test "emit() fires observe hooks without blocking the caller" installs an observe hook cat > "${marker}", fires it, then await waitForFile(marker) and JSON.parse(readFileSync(marker)).

waitForFile only checked existsSync. But cat > creates the file empty and writes the JSON asynchronously — so waitForFile returned the instant the empty file appeared, and the read raced the write, hitting an empty (or partial) file → Unexpected EOF.

This race has always been there; it started tipping over in CI only because the integration-test files added in #165/#166 raised bun test's file count/concurrency and widened the window. Reproduced locally at ~1 failure in 5 runs.

Fix

Replace waitForFile with waitForJson<T>(): poll readFile + JSON.parse until the file holds valid JSON (or timeout). Closes both the empty-file and any partial-write race, and hands the parsed object straight back to the test.

Verification

  • hook-bus.test.ts: 10/10 clean (was ~1/5 flaky).
  • Full offline suite: 1347 pass / 9 skip / 0 fail. tsc + biome clean.

🤖 Generated with Claude Code

The "emit() fires observe hooks without blocking the caller" test waited on
waitForFile(), which only checked existsSync. The observe-hook command
(`cat > marker`) CREATES the file empty and writes the JSON asynchronously, so
under load the read raced the write and hit an empty/partial file —
"JSON Parse error: Unexpected EOF" at hook-bus.test.ts:255. Pre-existing race;
it started failing CI once the added integration-test files bumped bun test's
concurrency and widened the window.

Replace waitForFile with waitForJson<T>(): poll readFile + JSON.parse until the
file holds valid JSON (or timeout). Eliminates both the empty-file and any
partial-write race.

Verified: hook-bus.test.ts 10/10 clean (was ~1/5 flaky); tsc + biome clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🤖 Gemini code review

This PR successfully resolves a flaky test in hook-bus.test.ts where a file existence check (waitForFile) raced against the asynchronous completion of the file write (cat > marker). It replaces it with a robust waitForJson helper that polls until the file exists and contains valid JSON, or times out.

Findings: 🔴 0 · 🟠 0 · 🟡 0 · 🟢 0


Tokens spent · ⬆️ Input: 1,174 · ⬇️ Output: 81 · Σ Total: 2,395
Total may be higher due to thinking token counts.

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@05b2b3b). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #168   +/-   ##
=======================================
  Coverage        ?   84.51%           
=======================================
  Files           ?      104           
  Lines           ?    18319           
  Branches        ?        0           
=======================================
  Hits            ?    15482           
  Misses          ?     2837           
  Partials        ?        0           
Flag Coverage Δ
daemon 84.51% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saucam
saucam merged commit b7c9780 into main Jul 12, 2026
5 checks passed
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