Spun out of the #1837 review (codex P2, consciously deferred).
atomicWrite (server/lib/fileUtils.js) writes a temp file then renames it over the target. When the destination path is a symlink, the rename replaces the link with a regular file rather than updating the link's target — whereas a plain writeFile(existingSymlink) follows the link and updates the backing file.
This is an intrinsic, pre-existing property of the canonical atomicWrite helper (temp+rename is deliberately symlink-replacing — following the link would reintroduce the non-atomic in-place truncate the helper exists to avoid). It is shared by every existing atomicWrite caller (auth.js sessions, settings.js, writeJSONLines, createCachedStore, universeBuilder, …), not something the #1837 swap introduced beyond migrating sites to the established pattern.
No PortOS data file is a symlink under any documented single-user setup, so this has no practical impact today. Filed as future so the design question — should the canonical atomic writer follow symlinks, reject them, or keep replacing them? — can be decided independently rather than bolted onto a mechanical swap PR. If we change it, it changes behavior for ALL atomicWrite callers and needs its own tests.
Options to weigh:
- Keep current behavior (replace the link) — standard atomic-write semantics; document it.
- Resolve via
lstat/realpath to the backing file before temp-write/rename when the target is a symlink (preserves the old writeFile follow behavior; partially weakens atomicity for the symlink case).
- Explicitly reject symlinked targets with a clear error.
Spun out of the #1837 review (codex P2, consciously deferred).
atomicWrite(server/lib/fileUtils.js) writes a temp file thenrenames it over the target. When the destination path is a symlink, the rename replaces the link with a regular file rather than updating the link's target — whereas a plainwriteFile(existingSymlink)follows the link and updates the backing file.This is an intrinsic, pre-existing property of the canonical
atomicWritehelper (temp+rename is deliberately symlink-replacing — following the link would reintroduce the non-atomic in-place truncate the helper exists to avoid). It is shared by every existingatomicWritecaller (auth.jssessions,settings.js,writeJSONLines,createCachedStore,universeBuilder, …), not something the #1837 swap introduced beyond migrating sites to the established pattern.No PortOS data file is a symlink under any documented single-user setup, so this has no practical impact today. Filed as
futureso the design question — should the canonical atomic writer follow symlinks, reject them, or keep replacing them? — can be decided independently rather than bolted onto a mechanical swap PR. If we change it, it changes behavior for ALL atomicWrite callers and needs its own tests.Options to weigh:
lstat/realpathto the backing file before temp-write/rename when the target is a symlink (preserves the oldwriteFilefollow behavior; partially weakens atomicity for the symlink case).