Skip to content

fix(host-mode): sync the recovery files before and after they replace the record - #467

Merged
nissa-seru merged 11 commits into
anima-research:mainfrom
nissa-seru:fix/recovery-file-durability
Oct 18, 2026
Merged

nissa-seru merged 11 commits into
anima-research:mainfrom
nissa-seru:fix/recovery-file-durability

Conversation

@nissa-seru

@nissa-seru nissa-seru commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR: The host-mode record and the deferred-write queue are now synced to disk before they replace the old record, and their directory and its parent after the rename, except on Windows or where the filesystem can't sync a directory. A write that fails before the rename removes its temp file, logging one it can't, and its error names the record. Any other directory sync that fails raises an ops alert. On one Linux host, a recovery write now takes 3.5–4.0 ms at the median, against about 0.2 ms before.

Problem

writeRecoveryFile (src/framework.ts) writes the two recovery files that exist so a quiesce, and the context writes it holds back, survive a crash: host-mode.json and deferred-writes.json. It writes the document to a temp file and renames that over the record, and nothing syncs either one. So when the call returns, nothing has asked for the new record to be on disk.

A probe on main (3af13bf, Node 24.22.0, Linux) recorded the node:fs calls quiesce() makes on the recovery directory:

  • writeFileSync of host-mode.json.<pid>.<uuid>.tmp, then renameSync of it over host-mode.json, and no fsyncSync at all.
  • With a directory standing where the record goes, the rename failed, quiesce() rejected with EISDIR, and the recovery directory still held host-mode.json.<pid>.<uuid>.tmp.

Changes

writeRecoveryFile keeps its name and signature, and both recovery files go through it, so both change:

  • The temp file is synced before the rename. It's written through a descriptor (openSync at 0600, writeFileSync, fsyncSync, closeSync), then renamed over the record, so the rename can't publish a record whose data was never asked to reach the disk.
  • The directory and its parent are synced after the rename, so the rename itself is asked to reach the disk, and so is the directory's own entry. That entry is new when a write had to make the directory, this one or an earlier one that failed after making it. Directories aren't synced on win32, where only the file is.
  • A directory sync refused with EINVAL or EBADF is let go the same way. PostgreSQL lets the same two codes go when it syncs a directory, which it also opens read-only, since "Some OSes don't allow us to fsync directories at all" (fsync_fname_ext). On Linux, fsync answers EINVAL for a file that doesn't support synchronization (fsync(2)). An alert for such a refusal would fire on every write, since the refusal comes again each time.
  • A failure before the rename, making the directory included, throws an error that names the record and says it was left as it was, with the original as its cause and its code kept. The fd-based write, sync and close errors name no path on their own. The temp file is removed if this write opened it, and one that can't be removed is logged.
  • Any other directory sync that fails raises an ops alert, not a throw, naming the record and saying it may not survive a crash. Both syncs come after the rename, when the new record is in place, and a throw would leave its callers saying it isn't: persistDeferredWrites marks the queue persisted only after the write returns (framework.ts:5486 at this head), and quiesce() would reject while the record says quiesced.

The comment that called this writer "the same idiom as the Discord awareness outbox" is gone, since the two no longer write alike.

On main, #453's changelog entry and the title of its test in test/host-quiesce.test.ts say that with a recovery file a failed sync is only logged, meaning the store's sync. Both now say "a failed sync of the store", since a recovery file's own syncs now fail the write or raise an alert.

The price. Each recovery write now waits for three syncs, the file's and two directories', all synchronous on the event loop. A benchmark wrote a 1.7 KB queue 100 times with main's calls (writeFileSync, then renameSync) and 100 times with this change's, leaving out the mkdirSync both make first, in two runs on one Linux host (ext4 on a virtual disk, Node 24.22.0, load average about 11). At the median, this change's writes took 3.5–4.0 ms and main's took 0.18–0.19 ms. This change's writes took 10–20 ms at the 90th percentile and 61–67 ms at most. We haven't timed it on other disks or on an idle host.

The host-mode record is written when quiesce() or resume() changes it. persistDeferredWrites rewrites the deferred-writes record each time its queue changes, while the host is quiesced or a queue written then is still on disk (its guard, quiesced || deferredWritesPersisted). Each context write deferred during a quiesce changes the queue, so a burst of them costs one recovery write each.

What each part is for

  • The temp file's sync before the rename. It asks for the new record's data to reach the disk before the rename makes it the record. On main nothing asks for that order. It costs one sync on every write, one of the three in the price above. Nothing simpler asks for the data first.
  • The directory syncs after the rename. The record's directory is synced so the rename itself, the record's entry in that directory, is asked to reach the disk. Its parent is synced for the directory's own entry, which is new when a write had to make the directory, this one or an earlier one that failed after making it. They cost the other two syncs on every write outside win32. Syncing the parent only when this write made the directory would miss a directory an earlier failed write made, so both are synced on every write, one rule for both cases. Levels above the parent are left to the filesystem, as Not verified says.
  • Removing the temp file on a failure. On main, a write that fails after making its temp file leaves it in the recovery directory, as the probe in Problem showed for a failed rename, so the directory gains one file per such failure while a fault lasts. Removing it costs an unlink on the failure path, and a log line when the unlink fails too.
  • The EINVAL/EBADF skip. Where a filesystem can't sync a directory, the refusal comes again on every write, so an alert for it would fire on every write, and no operator could clear it. Letting the two codes go, as PostgreSQL does, costs one check in the catch. The alternatives are worse: alerting on every failure buries a real I/O failure among alerts nobody can act on, and alerting on none hides it.
  • The ops alert for a directory sync that fails after the rename. The new record is in place by then, so a throw would have callers record a failure the file contradicts: persistDeferredWrites wouldn't mark the queue persisted, and quiesce() would reject while the record says quiesced. The alert tells an operator that the record may not survive a crash, and reaches them where a log line alone wouldn't. It costs one alert path.
  • Left out: the two other temp-and-rename writers, under Out of scope. The Discord awareness outbox's writer is the one feat(awareness): make Discord awareness marks an explicit, one-shot choice that never gates MCPL #250 replaces. tool-presentation.ts's has its own lock and checks, so its durability is a question of its own.

Tests

test/recovery-file-durability.test.ts drives both records through the framework with node:fs's openSync, fsyncSync, closeSync, renameSync and unlinkSync recorded, in twelve tests:

  • quiesce() syncs the host-mode record's temp file before renaming it over the record, and the directory and its parent after, closing each descriptor it opened, and the record is published at 0600;
  • each of two context writes deferred while quiesced rewrites the deferred-writes record, synced the same way;
  • a directory a failed write had to make is synced into its parent by the next write;
  • a write whose directory can't be made, with a file where it goes, fails with an error that names the record and keeps mkdirSync's code, and opens and removes no temp file;
  • a record that can't be replaced, with a directory where it goes, leaves no temp file behind, and quiesce() fails with the rename's EISDIR, naming the record;
  • a quiesce whose temp file can't be synced closes it and rejects with the host still serving, the record as it was and no temp file;
  • a temp file that can't be removed after a failed write is named in the log;
  • with process.platform set to win32, the temp file is still synced and no directory is opened;
  • a sync of the record's directory, or of its parent, that fails after the rename leaves the new record in place and raises an ops alert naming it;
  • both directories' syncs refused with EINVAL, or with EBADF, are let go with no ops alert, each descriptor closed and the new record in place.

Run on Linux (x86_64) with Node 24.22.0 and npm 11.19.1, at 3f8f1e9, this branch merged with main (86a3683):

  • The build and typecheck are clean, and the new tests pass with host-quiesce.test.ts, 56 of 56.
  • With main's src/framework.ts, all twelve new tests fail.
  • Each part, taken out in turn at the commit named, fails at least one of the new tests as they stood there. At 8b8e96a: letting a directory sync's EINVAL and EBADF go, letting EBADF go, and the alert for any other code. At 4e7b984: the temp file's close moved out of its finally. At 89ecd77: the temp file's 0600 mode, its close, and a directory's close. At 8060125: the parent synced after the rename, and the ops alert for a failed directory sync. At f404ed9: making the directory inside the try, and the win32 check. At bd592ae: removing the temp file only if this write opened it, the log of one that can't be removed, and the directory synced after the rename. At 7633bd2: the temp file's sync and the error naming the record.
  • Opening a directory read-only and syncing it works under Node 24.22.0 and Bun 1.4.3 on Linux.

The full suite is left to CI.

Not verified

  • No crash or power loss was simulated. The tests check which calls are made, and in what order.
  • Windows: the win32 branch is tested by setting process.platform, not on Windows.
  • A filesystem that can't sync a directory wasn't tried. The tests inject EINVAL and EBADF into the directories' syncs.
  • macOS runs in CI only.
  • The ops alert is checked by standing in for opsAlert and reading its call, not by what it sends.
  • Directories above the record's own: only the record's directory and its parent are synced, so where a write had to make more than one level of directory, the levels above are left to the filesystem.

Out of scope

Overlaps

#453, now on main and merged here, has runQuiesce write the record before it marks the host quiesced. So a quiesce whose write fails here rejects with the host still serving and the record as it was, which a test pins. #453's own test wraps writeRecoveryFile by name, which this keeps.

Written by Basil-1007 (Claude Opus 5.5). Hilda-2681 (Claude Opus 5.5) reviewed its code at 3f8f1e9.

Basil-1007 and others added 11 commits October 17, 2026 17:51
… the record

writeRecoveryFile, which writes the host-mode record and the deferred-write
queue, wrote a temp file and renamed it over the record with nothing
synced, so a crash soon after a write could lose the new record or leave
it empty, where these files exist to be the record. A write or a rename
that failed also left its temp file beside the record.

It now syncs the temp file before the rename and the directory after it
(not on win32), and when anything before the rename fails it removes the
temp file and rethrows. A directory sync that fails after the rename is
logged rather than thrown: the new record is already in place, and a
throw would leave the callers saying it isn't, with the deferred-write
queue not marked persisted and quiesce() rejecting while the record says
quiesced.

Tests drive the host-mode record through quiesce() with node:fs's calls
recorded: the temp file is synced before the rename and the directory
after it; a record that can't be replaced leaves no temp file; on win32
the directory isn't opened; and a failed directory sync leaves the new
record in place and logs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a directory the write made

A review of the change found:

- The fd-based write, fsync and close of the temp file throw errors that
  name no path, so a failed recovery write said only "EIO: i/o error,
  fsync". A failure before the rename now throws an error that names the
  record and says it was left as it was, with the original as its cause
  and its code kept.
- The temp file's removal swallowed every error. One it can't remove,
  for anything but a missing file, is now logged with its path.
- When the write had to make the record's directory, nothing synced the
  directory's new entry in its parent. The parent is now synced too.
- The tests failed only the rename and the directory's sync. They now
  also fail the temp file's sync, which this change adds, and its
  removal; drive the deferred-writes record through a context write
  deferred while quiesced; write to a fresh directory; and check that
  the rename was tried, with its error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ting, and raise a failed directory sync as an ops alert

A review of the round before found:

- The parent sync covered only the record directory's own parent, and
  only when this very write made it. A write that made two levels left
  the upper one's entry unsynced, and a directory made by a write that
  then failed was never synced into its parent. mkdirSync answers the
  first directory it made, in the form it was given (probed under Node
  24.22.0 and Bun 1.4.3), so each directory up to that one is now synced
  into its parent right after it's made, before anything is written.
- mkdirSync ran before the try, so a directory that couldn't be made
  threw unwrapped. It now runs inside it, and its failure names the
  record like any other before the rename. The temp file is removed
  only if this write opened it, so the unlink no longer needs to ignore
  ENOENT.

A failed directory sync was only logged. A failing directory sync
usually means a failing disk, so it now raises an ops alert naming the
record and saying it may not survive a crash. Directory syncs go
through one helper, syncRecoveryDirectory, which skips win32.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rather than each directory a write makes

A review of the round before found two gaps in syncing the directories
a write makes, each before anything was written:

- A sync of one that failed raised an ops alert and the write went on,
  though nothing was yet in place, so the reason for not throwing
  didn't hold for it.
- A directory made by a write that then failed, or made in part by a
  mkdirSync that then threw, was never synced into its parent, since a
  later write's mkdirSync makes nothing.

Every directory sync now comes after the rename: the record's
directory, for the record's entry, and its parent, for the directory's
own, on every write. So a directory any earlier write made is synced
into its parent by the next write that succeeds, and every alert is
raised with the new record in place. Levels above the parent are left
to the filesystem.

With main merged, quiesce() writes its record before it marks the host
quiesced, so a quiesce whose temp file can't be synced rejects with
the host still serving. The test of that failure now checks that too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…deferred-writes record's parent

A review of the round before found:

- Only the record's own directory had its sync failed in a test. The
  alert test now runs for the directory and for its parent, each
  expecting the new record in place and one ops alert naming it.
- The deferred-writes test, named for syncing that record "the same
  way", didn't check its parent. It does now.
- The writer's comment didn't say how far up it syncs. It now says the
  directories above the record's directory's parent are left to the
  filesystem.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lose, and say which sync stays only logged

A review of the round before found:

- No test checked the 0600 mode the record is published with, which
  this change moved from writeFileSync's options into openSync, or that
  the write closes each descriptor it opens. The first test now checks
  both.
- The directory-sync helper's comment gave a diagnosis, a failing disk,
  that nothing here checks. It now says only that a failure is raised
  as an ops alert naming the record.
- The changelog entry for the quiesce-writes-first fix says that with a
  recovery file "a failed sync is still only logged", meaning the
  store's sync. Since a recovery file's own syncs now fail the write or
  raise an alert, it now says "a failed sync of the store".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sync fails

The close of the temp file sits in a finally so it runs on every path,
but only the successful write checked it. The test of a temp file whose
sync fails now checks it too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st's title

The test's title said that with a recovery file "a failed sync is only
logged", as its changelog entry did. Its body fails only the store's
sync, and a recovery file's own syncs now fail the write or raise an
alert, so the title now says "a failed sync of the store", as the entry
does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… as on win32

fsync answers EINVAL for a file that doesn't support synchronization, and
some systems answer EBADF for a file opened read-only, as these
directories are. syncRecoveryDirectory raised either as a recovery-sync
ops alert, so where a directory can't be synced, every recovery write
raised one, for a refusal that comes again on every write. Those two
codes are now let go, as win32 is, the way PostgreSQL's fsync_fname_ext
lets them go for a directory. Any other failure of a directory's sync, or
of opening it, still raises the alert. The doc comments and the changelog
entry say so.

Tests: both directories' syncs refused with EINVAL, and with EBADF, are
let go with no alert, the new record in place and each descriptor closed.
The deferred-writes test now defers two context writes and checks that
each rewrites the record, synced the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at a crash leaves

The writer's doc comment and the changelog entry said that a crash
leaves the record as it was or the new one, whole. No test crashes the
host, so the comment now says what the syncs do: the new record's data
is asked to reach the disk before the rename publishes it. The changelog
entry drops the sentence, since the ones before it say what is synced.
The rest of the comment is rewrapped, unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 18, 2026

Copy link
Copy Markdown

Required label not found on this PR.

nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
@nissa-seru
nissa-seru merged commit e670363 into anima-research:main Oct 18, 2026
6 checks passed
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
main gained anima-research#457, anima-research#458, anima-research#459, anima-research#454, anima-research#460, anima-research#464, anima-research#412 and anima-research#467 since
86a3683. One block conflicts, in restorePersistedDeferredWrites: anima-research#454
narrowed its try to reading the recovery file, so that a file it can't
read or take fails create(), and moved the parsing and numbering after
it, while this branch changed that numbering (boot numbers the restored
queue 1..N). The resolution takes main's side of the block, then makes
this branch's change to main's numbering where it now stands: the queue
is ordered by valid seq, then the rest in file order, numbered 1..N
after the already-landed entries are left out, and the counter goes on
from N; the file is rewritten for an entry without a valid seq, not for
the new numbers. main's check for writes that already landed, which now
refuses create() when a context can't be read, is kept as main has it,
over the ordered entries.

Everything else merged without a conflict. The only other change of
main's within forty lines of one of this branch's is anima-research#457's to an
ephemeral run's idle watchdog, which now rejects a run that stop() has
begun to end as stopped; it sits above the run's finally, which this
branch changes, and neither depends on the other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
main brings agent-framework anima-research#456 (the workspace file tools' fixes), anima-research#460 (editors' files and nested mounts), anima-research#410, anima-research#455, anima-research#457, anima-research#458, anima-research#459, anima-research#454, anima-research#464, anima-research#412 and anima-research#467.

- write, edit and delete: anima-research#456 answers a change stored in the workspace but not on disk with one frame (storedButUnfinished), which leads with "stored, so don't send it again" and gives the push's reason. This branch had reworded those frames to say where force goes. Its frame goes and anima-research#456's stays: every reason a write's push can return names materialize where it offers force, and so do this branch's mode notes. Each success keeps its modeKept note, and commitToolChange keeps anima-research#456's onCommitted and answers { refused, modeKept }.
- write: an overwrite keeps the entry's mode inside anima-research#456's guarded commit, as an edit does.
- reconcile.ts: an effect that fails untouched names its kind, a chmod step's included, beside anima-research#456's account of what's in the way (inTheWay).
- edit: anima-research#456's edit takes a change on disk in first, with a pass over its path. Where disk changed only a file's mode, that pass takes the mode in, as a listing does, and the edit's note read it as disk's copy having changed and the edit applying to that text. A mode taken in alone isn't a change to the text, so the note no longer counts one. The edit then writes at the mode disk has and leaves nothing owed.
- anima-research#460: a push skips a path in another mount's directory before it plans, so it plans no chmod there either.
- The changelog fragment drops its sentence about the write tools' frame, which anima-research#456's replaces, and no longer names an edit among what says that nothing records which side changed a mode after a refused widening, since an edit's pass takes disk's mode in first.
- Tests: the test of a refused narrowing's frame takes anima-research#456's wording, the test of the force sentence goes with that sentence, and two tests pin an edit after a mode disk changed and after a refused widening.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant