Config: /config/recover stops reporting a write failure the disk has since repaired (F2)

[Broccolito]

Why

Finding F2 (MEDIUM-LOW) of the 2026-09-10 QA run on merged main 7c96d796, a follow-up
to #217. After one genuine write failure — a corrupt config.yaml, chmod 444 on the file and
chmod 555 on its directory — the permissions were restored and the file repaired, and three
consecutive POST /config/recover calls on that healthy, writable, valid config all still
answered persisted: false with the stale Permission denied (os error 13) note. Only an
unrelated POST /config/upsert cleared it (deterministic, 3/3).

Root cause

The record behind Config::last_write_error() had exactly one way to be retired: the clear inside
save_values, on a successful write. recover_config invalidates the cache and reloads — and a
config that loads needs no recovery, so that reload writes nothing. The clear's own comment
described the resulting state: "Without this the record is permanent, and a config directory that
was briefly unwritable … would make /config/recover warn that changes will not persist for the
rest of the process's life." It could only ever be reached by a write, and the one route that
reports the record never makes one on a healthy config.

What changed

crates/biorouter/src/config/base.rs

• Config::outstanding_write_failure() — the record, checked against the disk as it is now.
  last_write_error() is now its string view, so every consumer gets the checked answer, not only
  /config/recover. The rule is clear when every clause of the warning has stopped being true:
  • nothing recorded → no I/O at all (a healthy config answers from memory);
  • the config still does not load → the record stands, however writable the directory has
    become — the values in use really are in memory only, and the next reload runs the recovery
    whose write retires it. Clearing on a writable directory alone would reopen M9;
  • the config loads → probe whether a write would land, and clear if it would.
• probe_config_write does everything save_values does that can fail — the same staging
  path, open, lock, write and fsync, carrying the file's own bytes — and then removes the
  staged file instead of renaming it into place. So config.yaml is never touched: no stamp
  change for catalog::spawn_config_watcher to publish as an outside edit, no backup rotation,
  and no window in which another process's write is reverted by stale bytes. The staging step is
  extracted into stage_config, shared with save_values, so the check fails exactly where a real
  write would.
  • ⚠ On Windows it also treats a read-only config.yaml as unwritable: Rust 1.92's
    std::fs::rename calls MoveFileExW(MOVEFILE_REPLACE_EXISTING) and falls back to
    FileRenameInfoEx without FILE_RENAME_FLAG_IGNORE_READONLY_ATTRIBUTE, so the rename
    every write ends in refuses a read-only destination even when staging succeeds. Not on
    unix, where a 0o444 file in a writable directory is replaced without complaint — checking
    the file's bits there would bring F2 back for every read-only config.
• ConfigWriteFailure (new, re-exported): ValuesInMemoryOnly (M9 — the file is absent or
  will not load) vs NotWritable (the file loads, a write fails now). A probe that still fails
  re-records the error it just got, so the reason reported is the current one.
• The record is a WriteFailureRecord { error, recorded }. The check reads it, probes with no
  lock held, then clears only if recorded has not moved — a failure recorded while it probed is
  one it never tested. last_write_error stays a leaf lock: nothing is acquired while it is held,
  so the guard → values_read → values_cache order is untouched. load() and the
  single-flight gate are not modified; the storm path reaches save_values, whose staging block
  moved verbatim into stage_config.

crates/biorouter-server/src/routes/config_management.rs

• recover_config delegates to run_recovery(&Config) — a seam for the same reason Config: record the failed write on the backup-restore path, and make /config/recover's answer truthful #217 made
  recovery_report pure: the route reads Config::global(), the user's real config directory,
  and the half that depends on the disk can only be tested against a config of its own.
• ⚠ A new sentence for the state between the two ends. A corrupt config repaired while its
  directory stays unwritable now loads, so Config: record the failed write on the backup-restore path, and make /config/recover's answer truthful #217's note — "These values are in memory only … the
  next start will recover again" — would be false in three clauses out of four (before this PR it
  was emitted there anyway, from the stale record). That state now reads "config.yaml loads, but
  it cannot be written right now (…), so a setting changed in this session will not be saved."
  persisted is still exactly write_error.is_none(); its doc now names both ways it can be
  false, and the OpenAPI contract is regenerated for that (doc-only; no shape change).

Tests

Seven new in config::base (plus one Windows-only), three new in routes::config_management.

Fail-before — the three F2 tests, run against unmodified production code (only the
run_recovery seam and the tests added):

running 1 test
test config::base::tests::a_recorded_write_failure_is_cleared_once_the_config_loads_and_can_be_written_again ... FAILED
assertion `left == right` failed: the config loads and can be written, so nothing may still claim it cannot
  left: Some("Failed to create config directory: File exists (os error 17)")
 right: None
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3802 filtered out

running 9 tests
test routes::config_management::tests::a_recovery_after_the_config_was_healed_reports_persisted_with_no_note ... FAILED
test routes::config_management::tests::each_recovery_describes_the_config_as_it_is_now ... FAILED

---- a_recovery_after_the_config_was_healed_reports_persisted_with_no_note stdout ----
call 1: the config loads and can be written, so the recovery persisted; got "Config recovery completed.
Recovered 1 keys: BIOROUTER_MODEL ⚠ These values are in memory only — the config file could not be written
(Failed to create config directory: File exists (os error 17)). config.yaml on disk is unchanged, …"

---- each_recovery_describes_the_config_as_it_is_now stdout ----
the file loads and holds these values, so neither "in memory only" nor "the next start will recover again"
is true any more; got "Config recovery completed. Recovered 1 keys: BIOROUTER_MODEL ⚠ These values are in
memory only — the config file could not be written (Config file I/O failed: Permission denied (os error 13)). …"

test result: FAILED. 7 passed; 2 failed; 0 ignored; 0 measured; 579 filtered out

The chmod walk-through failed at its step 2 — the repaired-but-unwritable state — so its step 3 (the
literal F2 sequence) was not reached on the old code; the portable test above is F2's step 3, and fails on
call 1 of 3. Step 2's positive assertion (the new sentence is present) was added after this run.

Each guard fails the wrong fix it names — verified against deliberately poisoned trees
(base.rs restored from a checksummed copy after each):

Wrong fix poisoned into base.rs	Fails
Clear once the directory takes a new file, whether or not the config loads	a_config_that_still_does_not_load_keeps_its_record_however_writable_its_directory — got None; and #217's own a_backup_restored_over_a_config_that_could_not_be_written_is_recorded and a_write_that_succeeds_clears_a_failure_the_restore_path_recorded, because last_write_error() is now the checked answer
Clear whenever the config loads, without probing	a_config_that_loads_but_still_cannot_be_written_is_reported_as_not_writable, the_check_agrees_with_the_write_it_stands_in_for (got None), and the route's each_recovery_describes_the_config_as_it_is_now — persisted true for a directory still at 0o555
Clear unconditionally after a passing probe	a_failure_recorded_while_the_check_probed_is_not_the_one_it_clears — left: None
Probe by renaming the staged copy into place (a literal no-op write)	checking_whether_the_config_can_be_written_leaves_it_exactly_as_it_was — the stamp must not move
Honour the file's own read-only bit on unix as well	the_check_agrees_with_the_write_it_stands_in_for — Some(NotWritable("Config file I/O failed: config.yaml is read-only")) for a 0o444 file save_values replaces without complaint
Read the file before looking at the record	a_config_with_nothing_recorded_is_not_checked_at_all — left: 17 reads, right: 1

Every other test stayed green under each poison. (Under the second, the mid-probe test fails too, but
only because its injection hook lives inside the probe that poison skips — not counted.)

Gates

Gate	Result
BIOROUTER_DISABLE_KEYRING=true cargo test -p biorouter --lib -- config:: --test-threads=32, 20×	TALLY: 20 passed, 0 failed out of 20 (test result: ok. 120 passed; 0 failed; 1 ignored)
… cargo test -p biorouter --lib	test result: ok. 3801 passed; 0 failed; 2 ignored
… cargo test -p biorouter-server --lib	test result: ok. 589 passed; 0 failed; 0 ignored
… cargo test -p biorouter-server --lib -- routes::config_management	test result: ok. 18 passed; 0 failed
cargo fmt --all -- --check	clean
./scripts/clippy-lint.sh	strict pass 0 warnings · ✅ clippy::too_many_lines: ok · ✅ All baseline clippy checks passed! · ✅ Done
just check-openapi-schema (regenerates, then diffs against the commit)	✅ OpenAPI schema is up-to-date, rc=0

120 is #217's 113 plus the seven new unix-visible tests. The storm tests
(a_startup_storm_*), #197's retry tests and the deadlock scan
(nothing_on_the_uncached_load_path_reaches_back_through_the_cache, which now also walks
stage_config via save_values) are untouched and green.

⚠ Not runnable on this machine, so verified on this PR's CI instead:
a_read_only_config_is_not_called_writable_on_windows and the #[cfg(windows)] arm of
probe_config_write. cross-check (x86_64-pc-windows-gnu) compiled both, and the
test (windows-latest) job log shows config::base::tests::a_read_only_config_is_not_called_writable_on_windows ... ok
— so its premise (a read-only config.yaml cannot be replaced there) held on a real Windows kernel —
beside the portable guards and a_recovery_after_the_config_was_healed_reports_persisted_with_no_note ... ok
(biorouter lib on Windows: 3721 passed; 0 failed). All 16 checks: 15 pass, cross-build-nightly
skipped (schedule/dispatch only).

Runtime verification

A sandboxed biorouterd agent (BIOROUTER_PATH_ROOT at a scratch directory, its own port and
secret — never the real ~/.config/biorouter), driven over HTTP through the QA sequence:

== A. healthy config
  persisted: True   write_error: None
== B. M9: 27 corrupt bytes, file 0o444, dir 0o555
  persisted: False  write_error: Config file I/O failed: Permission denied (os error 13)
  message: … ⚠ These values are in memory only — the config file could not be written (…). config.yaml
           on disk is unchanged, so nothing changed in this session will persist and the next start
           will recover again.
  on disk: 27 bytes
== C. file repaired in place (0o644), dir still 0o555
  persisted: False  write_error: Config file I/O failed: Permission denied (os error 13)
  message: … ⚠ config.yaml loads, but it cannot be written right now (Config file I/O failed:
           Permission denied (os error 13)), so a setting changed in this session will not be saved.
== D. dir restored (0o755): F2's measurement, three calls
-- call 1   persisted: True   write_error: None   message: Config recovery completed. Recovered 3 keys: …
-- call 2   persisted: True   write_error: None
-- call 3   persisted: True   write_error: None
  config.yaml bytes unchanged by the three checks: yes
  config.yaml mtime unchanged by the three checks: yes
  staging litter in the config dir: 0

Daemon log for B and C:

ERROR biorouter::config::base: Failed to write the restored backup to …/config/config.yaml: …
INFO  biorouter::config::base: Recovered config values from a backup
DEBUG biorouter::config::base: config.yaml loads but still cannot be written: Config file I/O failed: Permission denied (os error 13)

Step D is the finding, fixed; step C is the state neither the finding nor #217 measured, and before
this PR it carried the M9 note. The daemon was stopped by its recorded pid (never pkill, which
matches other worktrees' daemons) and the sandbox deleted.

Not in scope

• The CLI test suite was not run. Nothing in biorouter-cli references last_write_error,
  outstanding_write_failure or recover_config; its writes reach save_values, whose staging
  moved into stage_config unchanged (same open, lock, write, sync, and the handle still dropped
  before the rename), which the config:: suite and the full biorouter lib suite exercise.
• probe_config_write does not model a unix immutable flag (chflags uchg, chattr +i) or a
  sticky directory owned by someone else — states where staging succeeds and the rename would
  not. There the check can clear a note that is still true; a set_param refused in that state
  still returns its error to the caller.

🤖 Generated with Claude Code