fix(atomic-json): write through symlinked destinations - #266
Open
Moret84 wants to merge 1 commit into
Open
Conversation
writeTextAtomic ended in rename(tmp, path), which replaces the path itself rather than following it. When the destination was a symlink - the usual shape for a config file linked into a dotfiles repository - the rename detached the link and left a plain file behind, so every later edit stopped reaching the original target. The retryable-rename fallback made it worse by unlinking the destination first. Resolve the destination through its symlink chain before choosing the temp path and renaming. The temp file stays in the real target's directory, so the write remains atomic on the same filesystem. A missing destination, a dangling link, or a cycle resolves to the last path reached and is written as a regular file. Add real-filesystem regression tests; three of the four fail without this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
writeTextAtomic()finished withrename(tmp, path).rename(2)replaces the path itself rather than following it, so a symlinked destination was destroyed and replaced by a regular file. Sinceinit-installer.tswritesopencode.jsonthroughwriteJsonAtomic, any config write silently detached anopencode.jsonthat was symlinked into a dotfiles repository, and every later edit stopped reaching the original target. The retryable-rename fallback compounded it by callingsafeRm(path)on the link first.Temp-file-plus-rename is the right pattern; the defect was only the missing symlink resolution. This change resolves the destination through its symlink chain before choosing the temp path and renaming, so the temp file stays in the real target's directory and the write remains atomic on the same filesystem. A missing destination, a dangling link, or a cycle beyond
MAX_SYMLINK_DEPTHresolves to the last path reached and is written as a regular file, matching the previous behavior for those cases.Scope:
src/lib/atomic-json.tsonly, plus tests. No public signature changes, no behavior change for non-symlinked destinations.Tests
tests/lib.atomic-json.symlink.test.ts(new) exercises the real filesystem rather than mocks, since the defect is precisely about inode identity: write-through, a symlink chain, a dangling link, and a plain missing destination as a control. Three of the four fail onmain; the control passes, which is what makes it a regression test rather than a behavior change.tests/lib.atomic-json.test.tsgainslstat/readlinkin its existingfs/promisesmock. Its assertions are unchanged.Linked Issue
Fixes #265
OpenCode Validation
@slkiser/opencode-quota@latestto 4.4.1, where I hit the bug on a real symlinked~/.config/opencode/opencode.json. I also reproduced it directly against that published 4.4.1 artifact, and confirmed the patched code writes through the link. The fix is in the plugin's own file writer and does not depend on OpenCode internals.Quality Checklist
pnpm run typecheckpnpm run buildpnpm testpnpm verifypasses end to end (Biome, pinned TypeScript check, history/privacy, typecheck, build, 2284 tests, four-surface parity, package contents) on Node 24.18.1, macOS.