Summary
SuggestionStore.SaveUnlocked performs File.WriteAllText to a temp path, then File.Move(..., overwrite: true). There is no fsync/Flush(flushToDisk: true) between the write and the rename, so on power loss the post-crash on-disk file can be empty or short, and the previous suggestions list is silently lost.
The class also writes a .bak on JSON parse error — but a zero-byte file parses as empty, not as corrupt, so no .bak is written and history is silently dropped.
Evidence
src/CodeIndex/Cli/SuggestionStore.cs:236-246:
File.WriteAllText(tempPath, json);
File.Move(tempPath, _filePath, overwrite: true);
On macOS/Linux the rename is atomic with respect to crashes only if the data blocks are durable on disk first; without an fsync the post-crash visible state can be: temp file truncated/zero bytes, then rename made it visible at the canonical path.
Impact
A single power loss / kernel panic / forced shutdown can wipe the local suggestions list (including suggestions not yet submitted to GitHub) with no .bak to recover.
Proposed direction
- Use
using var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write, FileShare.None); JsonSerializer.Serialize(fs, records, _jsonOptions); fs.Flush(flushToDisk: true); then File.Move.
- Treat zero-byte (vs whitespace-only) as suspect when the file existed before, and preserve as
.bak.
- Optionally
fsync the parent directory after the rename on POSIX to guarantee the rename is durable, not just the file contents.
Repro env
- Branch:
main @ 2ee912d (release v1.21.0)
Summary
SuggestionStore.SaveUnlockedperformsFile.WriteAllTextto a temp path, thenFile.Move(..., overwrite: true). There is nofsync/Flush(flushToDisk: true)between the write and the rename, so on power loss the post-crash on-disk file can be empty or short, and the previous suggestions list is silently lost.The class also writes a
.bakon JSON parse error — but a zero-byte file parses as empty, not as corrupt, so no.bakis written and history is silently dropped.Evidence
src/CodeIndex/Cli/SuggestionStore.cs:236-246:On macOS/Linux the rename is atomic with respect to crashes only if the data blocks are durable on disk first; without an
fsyncthe post-crash visible state can be: temp file truncated/zero bytes, then rename made it visible at the canonical path.Impact
A single power loss / kernel panic / forced shutdown can wipe the local suggestions list (including suggestions not yet submitted to GitHub) with no
.bakto recover.Proposed direction
using var fs = new FileStream(tempPath, FileMode.Create, FileAccess.Write, FileShare.None); JsonSerializer.Serialize(fs, records, _jsonOptions); fs.Flush(flushToDisk: true);thenFile.Move..bak.fsyncthe parent directory after the rename on POSIX to guarantee the rename is durable, not just the file contents.Repro env
main@ 2ee912d (release v1.21.0)