fix(fs): write files atomically to avoid truncated concurrent reads

[lazerg]

writeFile opened the target with O_TRUNC and then streamed the data in, so a getItem/getItemRaw running against the same key during a write could observe the file while it was still empty or half-written and return a truncated value. Writing to a temp file in the same directory and renaming it over the target makes the swap atomic, so a concurrent read sees either the old value or the new one.

Fixes #797

Summary by CodeRabbit

• Bug Fixes
  • Improved raw file writing to be atomic: writes go to a uniquely named temp file first, then safely finalize to the destination.
  • Prevented spurious temp entries from appearing in listings by skipping temp files during recursive directory scans.
  • Preserved existing file permissions on overwrite and tightened error handling to only ignore missing destinations.
• Tests
  • Strengthened concurrency tests to ensure reads never return truncated or mixed-byte buffers during overlapping writes.
  • Added regression coverage that key listings never include temp markers while temp keys are being written.
  • Added a platform-gated overwrite permission test (skipped on Windows).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 74556239-ef7b-4d0b-8d6f-f4a90cf19775

📥 Commits

Reviewing files that changed from the base of the PR and between 6d85e18 and 504014a.

📒 Files selected for processing (2)

• src/drivers/utils/node-fs.ts
• test/drivers/fs.test.ts

---

📝 Walkthrough

Walkthrough

Changes

The shared Node filesystem writer now writes through uniquely named temporary files, preserves existing permissions, renames atomically, and hides temporary entries. The fs and fs-lite suites validate complete concurrent reads, key listings, permissions, and stat-error handling.

Atomic filesystem writes

Layer / File(s)	Summary
Atomic write helper src/drivers/utils/node-fs.ts	writeFile uses PID- and UUID-based temporary paths, preserves existing permissions, renames files atomically, cleans up failures, rethrows non-ENOENT stat errors, and filters temporary entries from recursive listings.
Concurrent write validation test/drivers/fs.test.ts, test/drivers/fs-lite.test.ts	Both drivers verify complete uniform buffers, hide temporary filenames during concurrent writes, preserve permissions when overwriting files, and retain contents when stat fails.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Writer
  participant writeFile
  participant Filesystem
  participant Reader
  Writer->>writeFile: write data
  writeFile->>Filesystem: write temporary file
  writeFile->>Filesystem: rename temporary file to target
  Reader->>Filesystem: read target
  Filesystem-->>Reader: complete old or new value

Loading

Suggested reviewers: pi0

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: making fs writes atomic to prevent truncated concurrent reads.
Linked Issues check	✅ Passed	The changes implement atomic temp-file writes for fs and fs-lite, preserve same-directory renames, and hide temp files from recursive key listing.
Out of Scope Changes check	✅ Passed	The extra permission-preservation and stat error handling changes are supporting details for the atomic overwrite fix, not unrelated scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 20-23: Update readdirRecursive() to exclude only temporary files
matching the generated writeFile() pattern `*.<pid>.<UUID>.tmp` during recursive
key enumeration, while preserving user keys that merely end in `.tmp`;
alternatively, add a concurrent setItem/getKeys test that verifies these
in-progress files are omitted.

In `@test/drivers/fs.test.ts`:
- Around line 30-31: Update the concurrency read assertions in
test/drivers/fs.test.ts lines 30-31 and test/drivers/fs-lite.test.ts lines 29-30
to verify each buffer’s complete content is either all `a` or all `b`, rather
than checking length only; preserve the existing read loop and ensure
mixed-content buffers fail.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a8641cb7-2430-40c0-a50b-7c3f2b658e75

📥 Commits

Reviewing files that changed from the base of the PR and between e6be613 and 3227ba7.

📒 Files selected for processing (3)

• src/drivers/utils/node-fs.ts
• test/drivers/fs-lite.test.ts
• test/drivers/fs.test.ts

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/drivers/utils/node-fs.ts (1)

22-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Preserve destination permissions when replacing files atomically.

writeFile() creates the temporary path with the default mode (0o666 minus the umask), then rename() replaces the existing destination. An existing restrictive file such as 0600 can end up as roughly 0644 after an overwrite. Capture and restore the destination’s mode before renaming, and cover this with a regression test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/drivers/utils/node-fs.ts` around lines 22 - 25, Update the atomic write
flow using the temporary path and rename operation to capture the existing
destination file mode and apply it to the temporary file before renaming,
preserving restrictive permissions during replacement. Handle destinations that
do not yet exist with the current creation behavior, and add a regression test
verifying an overwrite retains the original mode.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 22-25: Update the atomic write flow using the temporary path and
rename operation to capture the existing destination file mode and apply it to
the temporary file before renaming, preserving restrictive permissions during
replacement. Handle destinations that do not yet exist with the current creation
behavior, and add a regression test verifying an overwrite retains the original
mode.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b02ad41-0c88-4f5e-9fb5-9f82923f0903

📥 Commits

Reviewing files that changed from the base of the PR and between 3227ba7 and 4eb91cc.

📒 Files selected for processing (3)

• src/drivers/utils/node-fs.ts
• test/drivers/fs-lite.test.ts
• test/drivers/fs.test.ts

[lazerg]

Thanks, good catch. Fixed in 6d85e18 by reading the destination file's mode and applying it to the temp file before the rename, so an existing 0600 file keeps its permissions after an overwrite. Added a regression test to both the fs and fs-lite suites (skipped on Windows).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 25-31: The destination mode lookup should suppress only an ENOENT
error. Update the fsPromises.stat(path) catch in the destination-mode flow to
rethrow all other filesystem errors, while retaining the existing undefined
result and chmod behavior when the destination is absent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d9ee1632-d4ff-400f-b28e-b45eff602c5d

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb91cc and 6d85e18.

📒 Files selected for processing (3)

• src/drivers/utils/node-fs.ts
• test/drivers/fs-lite.test.ts
• test/drivers/fs.test.ts