You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
AGENTS.md's atomic-write rule has two halves. The first — flush the temporary file before the rename — is satisfied at every site that stages through a temporary file, after #804. The second is not satisfied anywhere:
sync_all the temporary file before the rename, then open the containing directory and sync_all that too.
Nothing in this crate opens a directory to flush it. Every sync_all in src/ is on a file handle: fs_util.rs:141, fs_util.rs:213, fs_util.rs:317, cert_group.rs:417, commands/trust.rs:74, and the two in commands/init/steps/orchestrator.rs.
Why the second half matters
Flushing the temporary puts its bytes on disk. The rename that publishes it is a directory operation, and until the directory is flushed the entry pointing at those bytes may not be. A crash in that window can leave the destination name still pointing at the old inode — or at nothing — with the fully written new data sitting on disk unreferenced. The file survived; the fact that it is the current one did not.
The same applies to a file published by creation rather than by rename: O_EXCL + write + sync_all puts the bytes down and leaves the new directory entry unflushed exactly as a rename does.
A journaling filesystem narrows the window — on ext4 with data=ordered the rename lands in the journal within the commit interval, and auto_da_alloc carries a heuristic for the replace-by-rename pattern — but nothing here is a guarantee, and none of it is configuration this project controls.
FastPollState::save already documents the stronger property in fast_poll.rs:
it is what the next tick resumes from, so it has to survive a power loss rather than merely replace cleanly
That is the gap this issue closes: a documented intent the code does not yet deliver.
Why this is a decision rather than a fix
The rule itself says so, and says why: a directory sync_all is a disk round trip per write, so it belongs on files that have to survive a crash rather than on every atomic write. The decisions below are made here rather than left to the implementer, so the outcome does not depend on who picks up the issue.
Sites that take the directory sync_all
fs_util::atomic_write_blocking — the shared path, used by fast_poll's state and by trust's rotation-state.json update. Both are what the program reads back to resume.
trust::create_rotation_state — publishes the same file by O_EXCL creation rather than by rename, so atomic_write_blocking alone leaves phase 0 of a rotation undurable. Syncing only the update path would make the file's first record the weakest one.
fs_util::atomic_rewrite_owned_no_symlink and fs_util::create_owned_credential_noclobber — the secret_id writers. Losing the local write after OpenBao has accepted the new credential does not cost a rewrite; it locks the agent out until an operator intervenes.
Sites that record the reasoning and do not sync
cert_group::write_key_file (the rename at cert_group.rs:378; stage_key_file is the helper that creates and flushes the temporary, so the comment belongs at the rename rather than in the helper) — a lost key rename costs a reissue on the next renewal.
fs_util::write_owned_file_replace — eab.json is rewritten on every sync.
Each of these gets a comment saying so, so the absence reads as a decision rather than an omission.
Implementation notes
The directory sync_all goes after the rename (or after the O_EXCL create), never before.
One flush is enough per site: every staged temporary is created with NamedTempFile::new_in(parent), so the temporary and the destination always share a directory.
File::open(dir) + sync_all() is the portable-enough spelling on Linux and macOS, but it can surface EINVAL elsewhere. Put it behind one helper in fs_util rather than open-coding it at five call sites, and give the helper the error context.
What that buys is a Linux guarantee. macOS fsync does not flush the drive's own write cache — F_FULLFSYNC is the call that does — so on a developer machine the helper closes the ordering gap without promising power-loss durability. Linux is the deployment target and is where the guarantee has to hold.
Not in this issue
The first half. It is done everywhere a temporary file is staged.
The write sites that never stage a temporary file at all and truncate in place. These are a separate gap against the same rule and they need their own issue; do not convert them here:
commands/init/steps/orchestrator.rs's write_init_summary_json (sync_all at :292) and write_root_token_file (:334). Both flush their bytes and both publish through create(true).truncate(true), so on a first write they leave a new directory entry unflushed exactly as the others do. They are named here so the two sync_alls in the enumeration above do not read as unclassified.
openbao_unseal::save_unseal_keys — weaker still: tokio::fs::write with no sync_all at all, so it fails the first half too. It holds the unseal keys, which is the one file in this tree that cannot be regenerated from anything else, so its own issue should not be a low-priority one.
FastPollState::save calls create_dir_all on the parent before writing. When that call is what creates the directory, the directory's own entry in its parent is unflushed, and flushing the parent does not cover it. Chasing the chain upward is out of scope here; the helper flushes the directory that holds the published file and no further.
Done when
Every site listed above has its answer about directory durability recorded at the site
The sites named for it sync_all the containing directory after the rename or create, through a single fs_util helper
AGENTS.md's atomic-write rule has two halves. The first — flush the temporary file before the rename — is satisfied at every site that stages through a temporary file, after #804. The second is not satisfied anywhere:Nothing in this crate opens a directory to flush it. Every
sync_allinsrc/is on a file handle:fs_util.rs:141,fs_util.rs:213,fs_util.rs:317,cert_group.rs:417,commands/trust.rs:74, and the two incommands/init/steps/orchestrator.rs.Why the second half matters
Flushing the temporary puts its bytes on disk. The
renamethat publishes it is a directory operation, and until the directory is flushed the entry pointing at those bytes may not be. A crash in that window can leave the destination name still pointing at the old inode — or at nothing — with the fully written new data sitting on disk unreferenced. The file survived; the fact that it is the current one did not.The same applies to a file published by creation rather than by rename:
O_EXCL+ write +sync_allputs the bytes down and leaves the new directory entry unflushed exactly as a rename does.A journaling filesystem narrows the window — on ext4 with
data=orderedthe rename lands in the journal within the commit interval, andauto_da_alloccarries a heuristic for the replace-by-rename pattern — but nothing here is a guarantee, and none of it is configuration this project controls.FastPollState::savealready documents the stronger property infast_poll.rs:That is the gap this issue closes: a documented intent the code does not yet deliver.
Why this is a decision rather than a fix
The rule itself says so, and says why: a directory
sync_allis a disk round trip per write, so it belongs on files that have to survive a crash rather than on every atomic write. The decisions below are made here rather than left to the implementer, so the outcome does not depend on who picks up the issue.Sites that take the directory
sync_allfs_util::atomic_write_blocking— the shared path, used byfast_poll's state and bytrust'srotation-state.jsonupdate. Both are what the program reads back to resume.trust::create_rotation_state— publishes the same file byO_EXCLcreation rather than by rename, soatomic_write_blockingalone leaves phase 0 of a rotation undurable. Syncing only the update path would make the file's first record the weakest one.fs_util::atomic_rewrite_owned_no_symlinkandfs_util::create_owned_credential_noclobber— thesecret_idwriters. Losing the local write afterOpenBaohas accepted the new credential does not cost a rewrite; it locks the agent out until an operator intervenes.Sites that record the reasoning and do not sync
cert_group::write_key_file(the rename atcert_group.rs:378;stage_key_fileis the helper that creates and flushes the temporary, so the comment belongs at the rename rather than in the helper) — a lost key rename costs a reissue on the next renewal.fs_util::write_owned_file_replace—eab.jsonis rewritten on every sync.Each of these gets a comment saying so, so the absence reads as a decision rather than an omission.
Implementation notes
sync_allgoes after the rename (or after theO_EXCLcreate), never before.NamedTempFile::new_in(parent), so the temporary and the destination always share a directory.File::open(dir)+sync_all()is the portable-enough spelling on Linux and macOS, but it can surfaceEINVALelsewhere. Put it behind one helper infs_utilrather than open-coding it at five call sites, and give the helper the error context.fsyncdoes not flush the drive's own write cache —F_FULLFSYNCis the call that does — so on a developer machine the helper closes the ordering gap without promising power-loss durability. Linux is the deployment target and is where the guarantee has to hold.Not in this issue
StateFile::save(state.rs),cert_group::write_cert_file, andeab::write_eab_file—eab.jsonis read bybootroot-agent, which is the shape of the bootroot-agent burns renewal retries when reloaded agent.toml temporarily loses profile #613 race.commands/init/steps/orchestrator.rs'swrite_init_summary_json(sync_allat:292) andwrite_root_token_file(:334). Both flush their bytes and both publish throughcreate(true).truncate(true), so on a first write they leave a new directory entry unflushed exactly as the others do. They are named here so the twosync_alls in the enumeration above do not read as unclassified.openbao_unseal::save_unseal_keys— weaker still:tokio::fs::writewith nosync_allat all, so it fails the first half too. It holds the unseal keys, which is the one file in this tree that cannot be regenerated from anything else, so its own issue should not be a low-priority one.FastPollState::savecallscreate_dir_allon the parent before writing. When that call is what creates the directory, the directory's own entry in its parent is unflushed, and flushing the parent does not cover it. Chasing the chain upward is out of scope here; the helper flushes the directory that holds the published file and no further.Done when
sync_allthe containing directory after the rename or create, through a singlefs_utilhelper