← Back to PRs

#16542: fix(sessions): use atomic temp+rename write on Windows

by aldoeliacim open 2026-02-14 21:04 View on GitHub →
channel: discord stale size: XS trusted-contributor
## 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