This repository was archived by the owner on May 13, 2026. It is now read-only.
chore(persistence): atomic-write AppleSyncConfig::save - #913
Merged
Conversation
shiba4life
enabled auto-merge
May 6, 2026 12:24
PR #868 introduced `write_atomic_0600` (tmpfile + fsync + rename); a follow-up landed the helper as `crate::utils::fs_atomic::write_atomic` with an explicit `mode: Option<u32>` and converted three of the four remaining daemon-runtime config saves (`save_node_config`, `IngestionConfig::save_to_file`, `IngestionConfig::write_saved_to_disk`). `AppleSyncConfig::save` was the only production save site still using plain `std::fs::write`. A power loss or OOM-kill mid-write would leave a half-written / zero-byte JSON file, and `load()` silently returns `Default::default()` — every successful or failed Apple auto-sync passes through this path, so the user's auto-sync schedule + last-error state could vanish without an error. Route it through `fs_atomic::write_atomic(path, data, None)` to match the pattern the other three plaintext writers use (umask applies; the helper's docs explicitly say "plaintext config writers pass `mode = None`"). Test: two new regression tests in `apple_import::sync_config::tests` assert that `save()` leaves no stale `<path>.tmp` sibling on success, and that a leftover tmpfile from a prior crash is silently overwritten on retry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shiba4life
force-pushed
the
chore/atomic-write-config-saves
branch
from
May 6, 2026 12:28
bb547fa to
fc7c6b7
Compare
shiba4life
disabled auto-merge
May 6, 2026 12:29
shiba4life
enabled auto-merge
May 6, 2026 12:29
4 tasks
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
PR #868 introduced
write_atomic_0600(tmpfile + fsync + rename); a follow-up landed the helper ascrate::utils::fs_atomic::write_atomicwith an explicitmode: Option<u32>and converted three of the four remaining daemon-runtime config saves (save_node_config,IngestionConfig::save_to_file,IngestionConfig::write_saved_to_disk).AppleSyncConfig::savewas the only production save site still using plainstd::fs::write. A power loss or OOM-kill mid-write would leave a half-written / zero-byte JSON file, andload()silently returnsDefault::default()— every successful or failed Apple auto-sync passes through this path, so the user's auto-sync schedule + last-error state could vanish without an error.This PR routes it through
fs_atomic::write_atomic(path, data, None)to match the pattern the other three plaintext writers use. The helper's docs (src/utils/fs_atomic.rs:11) explicitly say "plaintext config writers passmode = None", and thewrite_atomic_without_mode_honors_umasktest there guards against the regression of using 0o600 on plaintext.Sites converted
src/ingestion/apple_import/sync_config.rs:176—AppleSyncConfig::save(every Apple auto-sync attempt, success or failure)Out of scope (deferred to follow-up)
src/handlers/auth.rs:1101—write_bootstrap_status(one-shot during onboarding)src/bin/folddb/restore.rs:160,194— identity-restore writessrc/bin/folddb/commands/setup.rs— fresh-setup writesrc/server/routes/file_upload.rs— multipart upload tempfile (different concern)Tests
Two new regression tests in
ingestion::apple_import::sync_config::tests:save_uses_atomic_write_no_stale_tmpfile— aftersave()succeeds, no<path>.tmpsibling remains and the file parses cleanly viaload().save_recovers_from_stale_tmpfile— a leftover<path>.tmpfrom a prior crash does not block the nextsave(); the tmpfile is opened withtruncate(true), so stale staged bytes are silently overwritten.Both serialize on a
NODE_CONFIG_LOCKmutex since they mutate the process-wideNODE_CONFIGenv var to redirectconfig_path()at a tempdir.Test plan
cargo clippy --workspace --all-targets -- -D warnings(clean apart from a pre-existingapple_import.rsmacOS-only dead-code warning that does not fire in Linux CI)cargo check --workspacecargo test --workspace --lib -- --test-threads=1— 980 passed🤖 Generated with Claude Code