fix(server): Windows saves no longer fail while a scanner holds the file

[yaacovcorcos]

Problem

On Windows, renaming over a file fails with EPERM, EACCES, or EBUSY while another process, such as a virus scanner or the search indexer, briefly has the target open. writeFileStringAtomically (apps/server/src/atomicWrite.ts) renames its temp file over the target once, so a save that lands in that window fails outright. Every caller is affected: server settings, keybindings, provider status and model caches, runtime state, and the other files written through this helper.

Fix

Retry the final rename on Windows when it fails with one of those three codes, with a short exponential backoff capped at 500 ms per wait and 10 retries (about 3 seconds in total). Every other error, and every error on other platforms, still fails on the first attempt. The temp file stays in its private temp directory until a rename succeeds, so the target never shows partial contents, and a lock that does not clear fails the save with the old contents intact.

Why it qualifies

Very small, focused fix for an obvious bug: one rename in one helper, no behaviour change off Windows or for other errors.

How it was checked

New tests in apps/server/src/atomicWrite.test.ts. They wrap the real file system and replace rename, and set HostProcessPlatform to win32. Retry waits run on TestClock; nothing sleeps.

• retries a Windows replacement while another process holds the target: rename fails twice with EBUSY, then the real rename runs. The save must succeed, the file must have the new contents, and no temp entry may be left.
• gives up on a Windows lock that does not clear and keeps the old contents: rename always fails with EPERM. The save must fail after more than one attempt, with the old contents and no temp entry left.
• fails a rename at once outside Windows or for other errors: EPERM on Linux and EXDEV on Windows are each tried exactly once. This guards the scope of the retry and passes before and after.

Before the fix (apps/server/src/atomicWrite.ts reverted, tests kept), the first two fail:

× retries a Windows replacement while another process holds the target
  PlatformError: Unknown: FileSystem.rename (/var/folders/.../t3-atomic-write-n3ZOAR/settings.json)
    cause: Error: EBUSY ... { code: 'EBUSY' }
× gives up on a Windows lock that does not clear and keeps the old contents
  AssertionError: expected 1 to be above 1
Tests  2 failed | 8 passed (10)

After the fix:

cd apps/server && ../../node_modules/.bin/vp test run src/atomicWrite.test.ts
Tests  10 passed (10)        (run 5 times in a row, all passed)

Other checks:

• cd apps/server && ../../node_modules/.bin/vp run typecheck: no new errors. The same 10 errors in src/process/externalLauncher.test.ts also appear on unmodified main 9bd1d8009a.
• vp lint --report-unused-disable-directives apps/server/src: exit 0, no findings in the touched files.
• vp fmt --check apps/server/src: clean.

Not checked

This was not run on real Windows. The tests simulate the lock by failing rename with the Node error codes Windows reports. They do not prove which codes a given scanner produces, or that about 3 seconds is always long enough.

Found and fixed in a downstream fork (Scient), then ported to current main. Ported and verified with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: a16b3412-11a6-4869-b3a6-a971d84171aa

📥 Commits

Reviewing files that changed from the base of the PR and between 3dfe373 and f2f463b.

📒 Files selected for processing (2)

• apps/server/src/atomicWrite.test.ts
• apps/server/src/atomicWrite.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Atomic file writes now retry selected rename failures on Windows. The retry policy uses exponential delays and a limit of 10 retries. Tests cover transient failures, persistent failures, and errors that do not trigger retries.

Changes

Atomic file writes

Layer / File(s)	Summary
Windows rename retry behavior apps/server/src/atomicWrite.ts, apps/server/src/atomicWrite.test.ts	The write function retries up to 10 times, starting at 25 ms and capping delays at 500 ms, when Windows rename errors have cause code EPERM, EACCES, or EBUSY. Tests check retry success, persistent failure without changing the original file, and fail-fast behavior for Linux EPERM and Windows EXDEV.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f2f46

The Windows retry path matches the filesystem provider’s error shape. No issue identified here prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f2f46

The change is bounded to selected Windows errors and does not add an entrypoint or increase filesystem privileges. However, a delayed retry can overwrite a newer settings update from another process, potentially restoring an older security-relevant configuration. This requires concurrent writers with existing access to the same file.

Retained concerns

• Medium · security · inferred: A Windows rename retry can commit an older settings document after a newer update succeeds. The theme command checks for concurrent changes before calling the helper, but the helper retries the same prepared contents without repeating that check. This widens an existing cross-process race and can revert security-relevant settings; the server watcher accepts the final document rather than restoring the newer update.

Security review details

Security Blast Radius

• inferred — The retained settings concern is bounded to concurrent writes targeting the same environment settings file under existing filesystem authority. The inspected path does not demonstrate a new remote entrypoint, privilege gain, or cross-environment access.

Security Findings and Attack Paths

• inferred — A theme edit can capture an older configuration, pass its comparison, encounter a Windows lock, and later overwrite a newer server update after the lock clears. The settings schema includes agent-browser enablement, so lost updates can affect security-relevant configuration rather than appearance alone. This is a source-supported concurrency concern, not a verified attacker exploit; downstream browser enforcement was not fully inspected.

Trust Boundaries and Controls

• observed — Provider status caches have separate instance-keyed paths, schema decoding, and instance-and-driver correlation before hydration. Disabled or enabled-state-mismatched cached providers fall back to current configuration. Unbounded persistence across different instances alone therefore does not establish a same-target race or cross-instance authority bypass.

Resilience and Maintainability Implications

• observed — Retries are finite and retain rename-based replacement rather than falling back to direct target writes or weakening permissions. Scoped cleanup remains unchanged, but physical cleanup during real Windows interruption and blocked removal is not established by the simulated tests.

Hardening Proposals

• proposed — Coordinate settings read-modify-write operations across participating processes, or move retry recovery to the document-owning layer so it can reload and merge current settings. An isolated pre-rename comparison is not an atomic concurrency control. Exercise a delayed older write racing a newer security-relevant settings update.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: Windows saves retry when a scanner or another process temporarily holds the target file.
Description check	✅ Passed	The description covers the problem, the fix, why the change qualifies without prior approval, and focused verification results. It also states the limits of the Windows simulation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[macroscopeapp]

Approvability

Verdict: Approved at f2f463b

Macroscope's review found this PR approvable — This is a small, localized Windows reliability fix: existing atomic saves retry only transient rename-lock errors, while other platforms and failures remain unchanged. The added tests cover success, exhaustion, and retry boundaries without affecting production behavior.

^{You can add or adjust custom eligibility rules. Learn more.}