Skip to content

fix: await progress-log writes before finishing CLI download - #1082

Open
EhabY wants to merge 1 commit into
mainfrom
fix/flaky-cli-concurrent-download-test
Open

fix: await progress-log writes before finishing CLI download#1082
EhabY wants to merge 1 commit into
mainfrom
fix/flaky-cli-concurrent-download-test

Conversation

@EhabY

@EhabY EhabY commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

What

Fixes a flaky test: test/unit/core/cliManager.concurrent.test.ts > redownloads when version mismatch is detected concurrently.

Root cause

performBinaryDownload writes the progress log in two ways:

  1. Fire-and-forget "downloading" writes from the onProgress callback (not awaited).
  2. An awaited clearProgress in the finally block.

The final "downloading" write could land after clearProgress ran, re-creating .progress.log and leaving it behind. The ordering between the last writeStream.write callback and readStream.on("close") is non-deterministic, so the test's vi.waitFor occasionally timed out waiting for the file to disappear.

Fix

Track the latest progress write and await it before resolving/rejecting the download promise (on both the success close path and the error paths), so clearProgress always runs after the final write.

Why this is safe

  • The progress log is a plain fs.writeFile — no lock, no rename, no ownership change. It never goes through renameWithRetry, so there's no Windows rename/backoff cost.
  • The write is already .catch-wrapped and logged as a warning, so a slow/failing write never rejects the download; the promise just resolves once the write settles.
  • writeStream.close() flushes pending writes, firing the pending callback and resolving the tracked promise — no deadlock.

Testing

  • pnpm typecheck
  • pnpm exec eslint src/core/cliManager.ts
  • pnpm test:extension ./test/unit/core/cliManager.concurrent.test.ts ./test/unit/core/cliManager.test.ts (67 passing, concurrent test run 15x locally)

🤖 Generated by Coder Agents.

@EhabY EhabY self-assigned this Aug 17, 2026
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