fix(fs): preserve symlinks in atomicWrite and writeFileAtomicDurable#894
fix(fs): preserve symlinks in atomicWrite and writeFileAtomicDurable#894bj456736 wants to merge 1 commit into
Conversation
When atomicWrite() or writeFileAtomicDurable() wrote to a path that was a symlink, the rename() call replaced the symlink itself with a regular file. This broke setups where config.toml was a symlink (e.g., to iCloud Drive or a dotfiles repo). Now both functions resolve symlinks via realpath() before creating the temp file and performing the rename, so the symlink remains intact and only the target file's content is updated. Fixes MoonshotAI#751
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fc9d6f8e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try { | ||
| const stats = await lstat(filePath); | ||
| if (stats.isSymbolicLink()) { | ||
| return await realpath(filePath); |
There was a problem hiding this comment.
When filePath is a symlink whose target does not yet exist, lstat succeeds but realpath(filePath) throws ENOENT; the catch then returns the original symlink path, so the later rename(tmpPath, targetPath) replaces the symlink instead of creating/updating its target. This still breaks first-run config/dotfile symlinks to not-yet-created files—the preservation path this change is meant to fix—so resolve the link text rather than falling back to filePath on this ENOENT.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #751 — when
atomicWrite()orwriteFileAtomicDurable()wrote to a symlink path, therename()call replaced the symlink itself with a regular file. This broke setups whereconfig.tomlwas managed as a symlink (e.g., to iCloud Drive or a dotfiles repo).Root Cause
Both functions created a temp file next to the given path and then called
rename(tmp, path). On POSIX,renamereplaces the directory entry, so whenpathwas a symlink, the symlink got overwritten.Solution
Added a
resolveSymlinkTarget()helper that useslstat+realpathto detect symlinks and resolve them to their underlying target before creating the temp file. BothatomicWriteandwriteFileAtomicDurablenow write to the resolved target path, leaving the symlink intact.Changes
packages/agent-core/src/utils/fs.ts: addedresolveSymlinkTargethelper, applied to both atomic write functionspackages/agent-core/test/utils/fs.test.ts: added 6 tests covering:Testing
All 6 new tests pass. Existing config and per-id-json-store tests also pass (52 tests).