#16542: fix(sessions): use atomic temp+rename write on Windows
channel: discord
stale
size: XS
trusted-contributor
Cluster:
Session Management Enhancements
## Summary
Remove the Windows-specific non-atomic `writeFile` path in `saveSessionStoreUnlocked()`. Use the existing temp-file + rename strategy on all platforms.
## Problem
On Windows, `saveSessionStoreUnlocked()` used `fs.promises.writeFile()` directly, which truncates the file to 0 bytes before writing. When multiple messages arrive in quick succession, concurrent `loadSessionStore()` calls can read the file mid-write (empty/partial JSON), causing `JSON.parse` to fail silently and return an empty store — resulting in session loss.
## Fix
NTFS supports same-volume atomic renames, so the temp-file + rename path (already used on Linux/Mac) works on Windows too. The existing fallback-to-direct-write in the catch block handles edge cases where rename fails.
**-18 lines, +2 lines** — just removes the Windows special case.
## Testing
All 39 session store tests pass (`pnpm vitest run src/config/sessions/store`).
Fixes #16516
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
Prevented empty `channels: {}` object injection during Discord guild entry merging. When both `sourceGuild.channels` and `existing.channels` are undefined, spreading produces an empty object that incorrectly signals a configured (but empty) channel allowlist downstream, potentially causing channel access issues. The fix conditionally assigns `channels` only when `mergedChannels` has keys, using `undefined` otherwise to preserve the "no channel config" semantic.
- Fixed guild merging logic to check `mergedChannels` is non-empty before assignment
- Prevents downstream code from treating empty object as a configured allowlist
- Aligns with existing test expectations (`channels: {}` should behave like `channels: undefined`)
<h3>Confidence Score: 5/5</h3>
- This PR is safe to merge with minimal risk
- The change is a small, well-targeted fix that adds a single conditional check to prevent unintended empty object assignment. The logic is straightforward, follows existing patterns in the codebase, and addresses a specific edge case. The fix aligns with existing test expectations and doesn't introduce new behavior.
- No files require special attention
<sub>Last reviewed commit: 194a0e6</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#4664: fix: per-session metadata files to eliminate lock contention
by tsukhani · 2026-01-30
81.3%
#16061: fix(sessions): tolerate invalid sessionFile metadata
by haoyifan · 2026-02-14
80.1%
#15888: fix: store relative session file paths instead of absolute
by devAnon89 · 2026-02-14
77.1%
#17132: fix: filter out invalid session entries with empty sessionFile
by Limitless2023 · 2026-02-15
76.7%
#16609: fix: resolve session store race condition and contextTokens updates
by battman21 · 2026-02-14
76.4%
#16987: fix(config): add skipCache to updateSessionStoreEntry and updateLas...
by AI-Reviewer-QS · 2026-02-15
76.1%
#3410: fix(sessions): always compute session paths from current environment
by sakunsylvi · 2026-01-28
76.0%
#17463: fix: write config files with explicit 0o600 mode instead of post-wr...
by miclaldogan · 2026-02-15
76.0%
#13881: fix: Address Greptile feedback - test isolation and channel resolution
by trevorgordon981 · 2026-02-11
75.7%
#17887: refactor: consolidate atomic JSON writes into shared writeJsonAtomic
by iyoda · 2026-02-16
75.5%