#1271 Extract the duplicated atomic-write sequence into one helper - #1306
Merged
Conversation
Adds `writeAtomic` at `packages/kb/src/filesystem/write-atomic.ts`, holding the temp-file-plus-rename sequence that `note-io/write-note.ts` and `taxonomy/write-taxonomy.ts` each spell out for themselves. The helper is internal: `filesystem/index.ts` does not re-export it, keeping it off the `@williamthorsen/kb/filesystem` entry point. Its `__tests__` sibling covers the failure paths neither call site's suite reaches: a `rename` failure removes the staged temp file and rethrows, and a cleanup failure on top of that still surfaces the rename error. The suite also pins the temp file as a sibling of the target, since a cross-directory rename can cross filesystems, where it is not atomic.
Reads the temp path `writeAtomic` staged by narrowing the recorded `writeFile` argument to a string, so a `FileHandle` or `URL` argument fails the check rather than reaching an assertion as `[object Object]`.
Points `writeNote` and `writeTaxonomy` at `filesystem/write-atomic.ts` and deletes the two copies of the temp-file-plus-rename sequence they each carried. kb spells the atomic-write contract once, so a change to it can no longer be made in one copy and missed in the other. `writeNote` keeps rendering the note and hands the rendered content to the helper.
Fixes an orphan `.tmp` file left beside the target when a `writeAtomic` write fails partway, as on a full disk. Cleanup guarded the `rename` alone, so a failed `writeFile` had nothing to reap what it had already created. The `writeFile` call now sits inside the same `try`, and both `writeNote` and `writeTaxonomy` inherit the fix.
williamthorsen
added
refactoring
Dependency auditProduction dependency audit passed. |
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
1 participant
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.
What
Extracts duplicated code into a shared
writeAtomicfunction that atomically writes content to a temp file and then renames it. Also fixes an issue where a temporary file could be left on disk after a failed write.Why
A change to kb's atomic-write contract had to be applied twice, once per copy, and could be applied once by mistake; the most recent such change was made identically in each. Neither call site's suite reached the failure paths, so the cleanup behavior went unguarded wherever it lived.
Details
🐛 Bug fixes
writeAtomic'swriteFilecall sits inside thetry, so a write that fails partway takes the same unlink-and-rethrow path as a failed rename;writeNoteandwriteTaxonomyboth inherit it.♻️ Refactoring
filesystem/write-atomic.tscarries the sequence: arandomBytes(8)temp suffix beside the target,writeFile,rename, best-effortunlinkon failure, rethrow. Its docblock records why the temp file is a sibling, which is that the rename is atomic only within one filesystem.filesystem/index.tsis untouched, so the module reaches no published subpath.note-io/write-note.tskeepsrenderNoteand delegates the write;taxonomy/write-taxonomy.tsloses its privatewriteAtomiccopy along with the "matchingnote-io" framing in its docblock.🧪 Tests
filesystem/__tests__/write-atomic.unit.test.tscovers five cases: content lands at the target, the temp file is a sibling of the target, and three failure paths, namely a failed write, a failed rename, and a failed rename whose cleanup also fails, where the rename error is the one that reaches the caller.vi.mock('node:fs/promises')that overridesrenameandunlinkper test, leaving every other operation live.Closes #1271