fix(server): poll with real ticks instead of bare yields in CursorAdapter tests#4536
fix(server): poll with real ticks instead of bare yields in CursorAdapter tests#4536colonelpanic8 wants to merge 1 commit into
Conversation
…pter tests waitForFileContent and waitForJsonLogMatch busy-polled for output from a real ACP mock child process using Effect.yieldNow with no wall-clock delay between attempts, so under CI load the child process could lose the race against the fixed attempt budget and the assertion would see an incomplete log. Sibling adapter test files already use a real delay between polls for this exact cross-process situation; align this file with that convention (TestClock.adjust for the fiber running under the test's virtual clock, Effect.sleep for the plain-async helper running its own live runtime).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Approved Test-only changes in CursorAdapter.test.ts that improve polling behavior by using real ticks instead of bare yields. No production code is affected. You can customize Macroscope's approvability policy. Learn more. |
What Changed
waitForFileContentandwaitForJsonLogMatchinapps/server/src/provider/Layers/CursorAdapter.test.tsnow poll with a real delay (Effect.sleep("25 millis")/TestClock.adjust("25 millis")) between attempts instead of a bareEffect.yieldNow.Why
CI run 30140499630 failed on
mainin the "cancels pending ACP approvals and marks the turn cancelled when interrupted" test withAssertionError: expected false to be true. The commit that CI run built only touchedapps/web/src/components/chat/TraitsPicker.tsx, so this was a pre-existing flake unrelated to that change.Both helpers busy-poll a log file written by a real ACP mock child process (an actual subprocess, not a stub). With no real wall-clock delay between the 40 poll attempts, all attempts could be exhausted before the child process was scheduled and had written its response under CI load, so the assertion would see an incomplete log.
Sibling test files in the same directory (
CursorProvider.test.ts,GrokAdapter.test.ts) already establish the correct pattern for this exact cross-process situation: sleep briefly between polls. This PR alignsCursorAdapter.test.tswith that convention. Since this suite runs under a virtualTestClock(viait.layer), the fix usesTestClock.adjustfor the generator that runs directly on the test's own fiber (matching an existing local precedent in the same file around line 288-302), and plainEffect.sleepfor the other helper, which is a bare async function that runs its own fresh, liveEffect.runPromiseper iteration.Verified locally:
vp test run CursorAdapter.test.tspasses 18/18 consistently across repeated runs.vp lintand a scopedvp run typecheckinapps/serverare clean.Checklist
Note
Low Risk
Test-only polling changes; no production server or adapter behavior is modified.
Overview
Fixes flaky CursorAdapter tests that poll log files written by a real mock ACP child process.
waitForFileContentnow waits 25ms between attempts viaEffect.sleepinstead ofEffect.yieldNow.waitForJsonLogMatchusesTestClock.adjust("25 millis")with a short comment that the mock runs on the real clock, matching the steer-test polling pattern already in this file and sibling adapter tests.Without wall-clock or scheduler hops, all 40 poll attempts could finish before the subprocess wrote its response under CI load (e.g. the cancel-approval test seeing an incomplete NDJSON log).
Reviewed by Cursor Bugbot for commit e88d8ec. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix polling in
CursorAdaptertests to use real ticks instead of bare yieldswaitForFileContent, replaces a bare scheduler yield between poll attempts with a 25ms sleep, introducing real elapsed time between read checks.waitForJsonLogMatch, replaces the yield withTestClock.adjust("25 millis")and adds a comment explaining why the mock process requires real-clock advancement.Macroscope summarized e88d8ec.