diff --git a/crates/biorouter-server/src/routes/config_management.rs b/crates/biorouter-server/src/routes/config_management.rs index 02aa556e2..1af25ff53 100644 --- a/crates/biorouter-server/src/routes/config_management.rs +++ b/crates/biorouter-server/src/routes/config_management.rs @@ -9,7 +9,7 @@ use axum::{ use biorouter::config::declarative_providers::LoadedProvider; use biorouter::config::paths::Paths; use biorouter::config::ExtensionEntry; -use biorouter::config::{Config, ConfigError}; +use biorouter::config::{Config, ConfigError, ConfigWriteFailure}; use biorouter::model::ModelConfig; use biorouter::privacy::ProviderTier; use biorouter::providers::auto_detect::{detect_provider_from_api_key, detectable_providers}; @@ -1296,13 +1296,20 @@ pub struct ConfigRecoveryReport { pub message: String, /// The config keys this process is now running on. pub recovered_keys: Vec, - /// Whether `config.yaml` on disk holds what this report describes. + /// Whether this process's settings are persisting: `config.yaml` holds what + /// this report describes, and a write to it lands. /// - /// `false` means the recovery could not write what it recovered: the keys - /// above live only in this process, the file on disk is unchanged — still - /// absent, or still the contents that would not load — nothing changed in - /// this session survives exit, and the next start runs the same recovery - /// again. + /// `false` in one of two ways, which `message` spells out: + /// - the recovery could not write what it recovered: the keys above live + /// only in this process, the file on disk is unchanged — still absent, or + /// still the contents that would not load — and the next start runs the + /// same recovery again; + /// - `config.yaml` loads, so the keys above are the file's, but it cannot + /// be written right now. + /// + /// Either way a setting changed in this session will not be saved. Checked + /// against the disk on every call, so a failure that has since been + /// repaired is not reported. pub persisted: bool, /// The write error, verbatim, whenever `persisted` is false. pub write_error: Option, @@ -1318,7 +1325,7 @@ pub struct ConfigRecoveryReport { /// user's own config directory. fn recovery_report( recovered_keys: Vec, - write_error: Option, + failure: Option, ) -> ConfigRecoveryReport { let recovered = if recovered_keys.is_empty() { "Config recovery completed, but no data was recoverable. Starting with empty \ @@ -1332,25 +1339,33 @@ fn recovery_report( ) }; - // A recovery that could not WRITE what it recovered leaves the app running - // on values that vanish at exit. That was silent on the arm a corrupted - // config with a usable backup actually takes, so this route answered - // "Recovered 23 keys" for a file it had just failed to write, while the - // corrupt bytes were still on disk. - let message = match write_error.as_deref() { + let message = match &failure { None => recovered, - Some(err) => format!( + // A recovery that could not WRITE what it recovered leaves the app + // running on values that vanish at exit. That was silent on the arm a + // corrupted config with a usable backup actually takes, so this route + // answered "Recovered 23 keys" for a file it had just failed to write, + // while the corrupt bytes were still on disk. + Some(ConfigWriteFailure::ValuesInMemoryOnly(err)) => format!( "{recovered} ⚠ These values are in memory only — the config file could not be \ written ({err}). config.yaml on disk is unchanged, so nothing changed in this \ session will persist and the next start will recover again." ), + // ⚠ Not the sentence above. The file loads, so the values in use ARE + // on disk and the next start will not recover anything; only a change + // made now is at risk. Saying "in memory only" here would be the same + // kind of false note F2 was. + Some(ConfigWriteFailure::NotWritable(err)) => format!( + "{recovered} ⚠ config.yaml loads, but it cannot be written right now ({err}), so \ + a setting changed in this session will not be saved." + ), }; ConfigRecoveryReport { message, recovered_keys, - persisted: write_error.is_none(), - write_error, + persisted: failure.is_none(), + write_error: failure.map(ConfigWriteFailure::into_error), } } @@ -1363,8 +1378,24 @@ fn recovery_report( ) )] pub async fn recover_config() -> Result, StatusCode> { - let config = Config::global(); + match run_recovery(Config::global()) { + Ok(report) => Ok(Json(report)), + Err(e) => { + tracing::error!("Config recovery failed: {}", e); + Err(StatusCode::INTERNAL_SERVER_ERROR) + } + } +} +/// The recovery itself, against whichever config it is handed. +/// +/// A seam, and the reason it exists is the one `recovery_report` gives for +/// being pure: the route reads `Config::global()`, which is the user's real +/// `~/.config/biorouter`. What this adds over `recovery_report` is the part +/// that depends on the DISK — the reload, and what the config layer says about +/// writing afterwards — and a test can only reach that against a config of its +/// own. +fn run_recovery(config: &Config) -> Result { // This endpoint IS a forced re-read, so it has to force one: the config // layer serves a parsed `config.yaml` until the file's stamp moves, and a // caller who reaches for "recover" is asking to go back to the disk @@ -1372,20 +1403,17 @@ pub async fn recover_config() -> Result, StatusCode> config.invalidate_values_cache(); // Force a reload which will trigger recovery if needed - match config.all_values() { - Ok(values) => { - // Read AFTER the reload, never before: the write this reports on is - // one the reload itself has just attempted. - Ok(Json(recovery_report( - values.keys().cloned().collect(), - config.last_write_error(), - ))) - } - Err(e) => { - tracing::error!("Config recovery failed: {}", e); - Err(StatusCode::INTERNAL_SERVER_ERROR) - } - } + let values = config.all_values()?; + + // Asked AFTER the reload, never before: the write this reports on may be + // one the reload itself has just attempted. And asked of the disk, not of + // a record — a config that loads needs no recovery, so this reload writes + // nothing, and a failure recorded by an earlier one would otherwise be + // reported for a file that has since been repaired (finding F2). + Ok(recovery_report( + values.keys().cloned().collect(), + config.outstanding_write_failure(), + )) } #[utoipa::path( @@ -1720,7 +1748,9 @@ mod tests { "BIOROUTER_MODEL".to_string(), "BIOROUTER_PROVIDER".to_string(), ], - Some("Config file I/O failed: Permission denied (os error 13)".to_string()), + Some(ConfigWriteFailure::ValuesInMemoryOnly( + "Config file I/O failed: Permission denied (os error 13)".to_string(), + )), ); assert!( @@ -1761,8 +1791,9 @@ mod tests { /// The other half of the requirement, and the one that is easy to lose: a /// note that outlives its cause tells the user their settings are being /// lost while they are being saved, which is worse than saying nothing. - /// `Config::save_values` clears the record on success; this pins that the - /// route says nothing once it is clear. + /// The config layer retires a failure once it stops being true (a write + /// succeeds, or the file loads and a write would land); this pins that the + /// route says nothing once it has. #[test] fn a_recovery_that_persisted_carries_no_warning() { let report = recovery_report(vec!["BIOROUTER_MODEL".to_string()], None); @@ -1782,7 +1813,12 @@ mod tests { /// which is exactly how one of two branches comes to lose a later edit. #[test] fn a_recovery_with_nothing_to_recover_still_reports_that_it_could_not_write() { - let report = recovery_report(vec![], Some("No space left on device".to_string())); + let report = recovery_report( + vec![], + Some(ConfigWriteFailure::ValuesInMemoryOnly( + "No space left on device".to_string(), + )), + ); assert!(!report.persisted); assert!(report.recovered_keys.is_empty()); @@ -1801,17 +1837,222 @@ mod tests { ); } - /// The flag and the error are one fact, and cannot disagree. + /// The flag and the error are one fact, and cannot disagree — for either + /// shape a failure can take. #[test] fn persisted_is_exactly_the_absence_of_a_write_error() { - for write_error in [None, Some("any failure at all".to_string())] { - let expected = write_error.is_none(); - let report = recovery_report(vec!["K".to_string()], write_error); + for failure in [ + None, + Some(ConfigWriteFailure::ValuesInMemoryOnly( + "any failure at all".to_string(), + )), + Some(ConfigWriteFailure::NotWritable( + "any failure at all".to_string(), + )), + ] { + let expected = failure.is_none(); + let report = recovery_report(vec!["K".to_string()], failure); assert_eq!( report.persisted, expected, "a report may never claim to have persisted while carrying the error that \ says it did not, nor the reverse" ); + assert_eq!(report.write_error.is_none(), expected); + } + } + + /// A config that loads but cannot be written is NOT "in memory only". + /// + /// The file loads, so the values in use are the ones on disk and the next + /// start will not recover anything — M9's sentence would be false in three + /// clauses out of four. What is true is narrower: a setting changed now + /// will not be saved. Reached when a corrupt config is repaired but its + /// directory is left unwritable; before F2's fix this state carried the + /// stale M9 note instead. + #[test] + fn a_config_that_loads_but_cannot_be_written_is_not_called_in_memory_only() { + let report = recovery_report( + vec!["BIOROUTER_MODEL".to_string()], + Some(ConfigWriteFailure::NotWritable( + "Config file I/O failed: Permission denied (os error 13)".to_string(), + )), + ); + + assert!(!report.persisted, "a change made now will not be saved"); + assert_eq!( + report.write_error.as_deref(), + Some("Config file I/O failed: Permission denied (os error 13)") + ); + assert_eq!( + report.message, + "Config recovery completed. Recovered 1 keys: BIOROUTER_MODEL ⚠ 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." + ); + for false_here in ["in memory only", "on disk is unchanged", "recover again"] { + assert!( + !report.message.contains(false_here), + "{false_here:?} is not true of a config that loads; got {:?}", + report.message + ); + } + } + + /// Finding F2: once the config is healed, recovery stops warning. + /// + /// Measured on `7c96d796`: after one genuine write failure the permissions + /// were restored and the file repaired, and three consecutive `POST + /// /config/recover` calls on the healthy, writable, valid config all + /// answered `persisted: false` with the stale `Permission denied` — a + /// config that loads needs no recovery, so the reload wrote nothing, and a + /// write was the only thing that cleared the record. Three calls here + /// because three is what was measured. + /// + /// Portable: the config's parent is a FILE, which fails the write the same + /// way on every platform. `each_recovery_describes_the_config_as_it_is_now` + /// below is the literal `chmod` sequence. + #[test] + fn a_recovery_after_the_config_was_healed_reports_persisted_with_no_note() { + let dir = tempfile::TempDir::new().unwrap(); + let blocked = dir.path().join("blocked"); + std::fs::write(&blocked, "not a directory").unwrap(); + let config_path = blocked.join("config.yaml"); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + + let refused = + run_recovery(&config).expect("an unwritable config still recovers into memory"); + assert!( + !refused.persisted && refused.write_error.is_some(), + "the premise: the first recovery could not write, and said so; got {:?}", + refused.message + ); + + // Healed from outside this process: writable, and holding a valid config. + std::fs::remove_file(&blocked).unwrap(); + std::fs::create_dir(&blocked).unwrap(); + std::fs::write(&config_path, "BIOROUTER_MODEL: gpt-5.5\n").unwrap(); + + for call in 1..=3 { + let report = run_recovery(&config).expect("a healthy config recovers"); + assert!( + report.persisted, + "call {call}: the config loads and can be written, so the recovery persisted; \ + got {:?}", + report.message + ); + assert_eq!(report.write_error, None, "call {call}"); + assert_eq!( + report.message, "Config recovery completed. Recovered 1 keys: BIOROUTER_MODEL", + "call {call}: no note, byte-identical to the healthy answer" + ); + } + } + + /// The F2 measurement, step for step, with the step between the two ends + /// that neither the finding nor #217 measured. + /// + /// 1. A corrupt `config.yaml` beside a usable `.bak`, the file `0o444` and + /// the directory `0o555`: the recovery cannot write what it recovered + /// (M9, fixed by #217). + /// 2. The file repaired, the directory still `0o555`: the config loads, so + /// the values are the file's, but a change still cannot be saved. Both + /// halves of that have to be said, and "in memory only" would be false. + /// 3. The directory restored: nothing to warn about, three times over. + /// + /// unix-only because a mode is how the finding made the directory + /// unwritable; the portable assertion of step 3 is the test above. + #[cfg(unix)] + #[test] + fn each_recovery_describes_the_config_as_it_is_now() { + use std::os::unix::fs::PermissionsExt; + + /// A `TempDir` still at `0o555` cannot delete its own contents, so the + /// modes are restored whatever happens. + struct RestoreModes(std::path::PathBuf, std::path::PathBuf); + impl Drop for RestoreModes { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o755)); + let _ = std::fs::set_permissions(&self.1, std::fs::Permissions::from_mode(0o644)); + } + } + let mode = |path: &std::path::Path, bits: u32| { + std::fs::set_permissions(path, std::fs::Permissions::from_mode(bits)).unwrap() + }; + + let dir = tempfile::TempDir::new().unwrap(); + let config_path = dir.path().join("config.yaml"); + // The exact 27 bytes of the measurement. + std::fs::write(&config_path, "BIOROUTER_MODEL: [unclosed\n").unwrap(); + std::fs::write( + dir.path().join("config.yaml.bak"), + "BIOROUTER_MODEL: gpt-5.5\n", + ) + .unwrap(); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + + let _restore = RestoreModes(dir.path().to_path_buf(), config_path.clone()); + mode(&config_path, 0o444); + mode(dir.path(), 0o555); + // Root ignores the mode, and then every premise below is false. + if std::fs::write(dir.path().join("writability-probe"), "x").is_ok() { + eprintln!("skipped: this process can write a 0o555 directory (running as root?)"); + return; + } + + // 1. M9 — #217's half, re-asserted so the steps after it mean something. + let unwritable = run_recovery(&config).unwrap(); + assert!(!unwritable.persisted, "{:?}", unwritable.message); + assert!( + unwritable.message.contains("in memory only"), + "the corrupt bytes are still on disk, so the values really are in memory only; \ + got {:?}", + unwritable.message + ); + + // 2. The file repaired in place; the directory still refuses writes. + mode(&config_path, 0o644); + std::fs::write(&config_path, "BIOROUTER_MODEL: gpt-5.5\n").unwrap(); + let read_only = run_recovery(&config).unwrap(); + assert!( + !read_only.persisted, + "a change made now still cannot be saved; got {:?}", + read_only.message + ); + assert!( + read_only + .write_error + .as_deref() + .is_some_and(|e| e.contains("Permission denied")), + "and the reason is the one that holds NOW; got {:?}", + read_only.write_error + ); + assert!( + !read_only.message.contains("in memory only") + && !read_only.message.contains("recover again"), + "the file loads and holds these values, so neither \"in memory only\" nor \"the \ + next start will recover again\" is true any more; got {:?}", + read_only.message + ); + assert!( + read_only + .message + .contains("config.yaml loads, but it cannot be written right now"), + "what IS true has to be said instead of nothing; got {:?}", + read_only.message + ); + + // 3. The directory restored: F2. + mode(dir.path(), 0o755); + for call in 1..=3 { + let healed = run_recovery(&config).unwrap(); + assert!(healed.persisted, "call {call}: got {:?}", healed.message); + assert_eq!(healed.write_error, None, "call {call}"); + assert_eq!( + healed.message, "Config recovery completed. Recovered 1 keys: BIOROUTER_MODEL", + "call {call}" + ); } } diff --git a/crates/biorouter/src/config/base.rs b/crates/biorouter/src/config/base.rs index fad692a26..2397a8abf 100644 --- a/crates/biorouter/src/config/base.rs +++ b/crates/biorouter/src/config/base.rs @@ -201,12 +201,16 @@ pub struct Config { // a lock that `save_values` also needs would deadlock against `load`'s own // create branch, which writes. values_read: Mutex<()>, - // The last failure to write a default config file. Reported out of band — - // `load` stays infallible about it on purpose, because making an unwritable - // config directory a hard failure would turn the storm path this layer - // spent two PRs calming down into a start-up crash. See - // `record_config_write_error`. - last_write_error: Mutex>, + // The outstanding failure to write the config file, if any. Reported out of + // band — `load` stays infallible about it on purpose, because making an + // unwritable config directory a hard failure would turn the storm path this + // layer spent two PRs calming down into a start-up crash. See + // `record_config_write_error`, and `outstanding_write_failure` for how a + // record is retired. + // + // A leaf lock: taken and released with nothing else acquired while it is + // held, so it cannot join the `guard` → `values_read` → `values_cache` order. + last_write_error: Mutex, // Test-only replacement for the OS credential store, so cache and // chunking behavior can be exercised without touching a real keyring // (which would show authorization prompts on macOS). @@ -276,7 +280,7 @@ impl Default for Config { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), #[cfg(test)] test_keyring_store: None, #[cfg(test)] @@ -458,6 +462,44 @@ struct CachedConfig { values: Arc, } +/// An outstanding failure to write `config.yaml`, as it stands against the file +/// on disk right now. See [`Config::outstanding_write_failure`]. +/// +/// Two variants because one write error means two different things for the +/// settings in use, and a report that says either one while the other is true +/// is wrong. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ConfigWriteFailure { + /// `config.yaml` is absent or does not load, and could not be written: the + /// values this process runs on exist only in memory, the file on disk is + /// unchanged, and the next start will run the same recovery again. + ValuesInMemoryOnly(String), + /// `config.yaml` loads, so the settings in use are the file's, but a write + /// to it fails right now: a setting changed in this session will not be + /// saved. + NotWritable(String), +} + +impl ConfigWriteFailure { + /// The write error, verbatim. + pub fn into_error(self) -> String { + match self { + Self::ValuesInMemoryOnly(error) | Self::NotWritable(error) => error, + } + } +} + +/// The write failure this process has recorded and not yet retired. +#[derive(Default)] +struct WriteFailureRecord { + error: Option, + /// How many failures have ever been recorded. A check that found the file + /// writable again clears the record only while this still reads what it + /// read before probing: a failure recorded while it probed is one it never + /// tested, and clearing that would be the silence M9 was about. + recorded: u64, +} + /// Attempts a filesystem operation gets before its failure is believed. /// /// Eight, with the 1 ms → 16 ms backoff below, is ~63 ms in the worst case — @@ -540,6 +582,15 @@ fn retry_up_to( struct IoFaults { failing_read_attempts: std::sync::atomic::AtomicUsize, failing_rename_attempts: std::sync::atomic::AtomicUsize, + /// Fails the open that creates a staging file — the step every write and + /// every check that a write would land share. Arming the whole budget is a + /// config directory that refuses new files, on every platform, which a mode + /// only is on unix. + failing_stage_attempts: std::sync::atomic::AtomicUsize, + /// A write failure recorded the instant a writability check's staged write + /// succeeds: the one interleaving in which that check would otherwise clear + /// a failure it never tested. + failure_recorded_mid_probe: Mutex>, /// Config content a "sibling" installs the instant an injected rename fault /// fires. That is the race the fault stands in for — our rename lost /// because another writer got there first — and it is the only way to reach @@ -566,6 +617,15 @@ impl IoFaults { .load(std::sync::atomic::Ordering::Relaxed) } + fn failing_stages(&self) -> usize { + self.failing_stage_attempts + .load(std::sync::atomic::Ordering::Relaxed) + } + + fn take_failure_recorded_mid_probe(&self) -> Option { + self.failure_recorded_mid_probe.lock().unwrap().take() + } + /// Land the sibling's config, once, at the moment a rename fault fires. fn land_sibling_config(&self, at: &Path) { if let Some(content) = self.sibling_lands_on_rename_fault.lock().unwrap().take() { @@ -640,7 +700,7 @@ impl Config { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), #[cfg(test)] test_keyring_store: None, #[cfg(test)] @@ -668,7 +728,7 @@ impl Config { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), #[cfg(test)] test_keyring_store: None, #[cfg(test)] @@ -821,22 +881,54 @@ impl Config { *self.values_cache.lock().unwrap_or_else(|e| e.into_inner()) = ValuesCache::default(); } - /// An **outstanding** failure to write the config file, if any. + /// The error of the outstanding write failure, if any — the string view of + /// [`Self::outstanding_write_failure`], and checked against the disk the + /// same way. Use that one to learn what the failure means for the values + /// in use. + pub fn last_write_error(&self) -> Option { + self.outstanding_write_failure() + .map(ConfigWriteFailure::into_error) + } + + /// An **outstanding** failure to write the config file, checked against + /// the file as it is now. + /// + /// Every arm of the recovery in [`Self::load_uncached`] that writes records + /// its failure through [`Self::record_config_write_error`] — creating a + /// config that is missing, replacing one that will not parse, and + /// **restoring a backup over one that will not parse**. The last of those + /// is the arm a corrupted `config.yaml` with a usable `.bak` beside it + /// actually takes, and it was the one that said nothing: `POST + /// /config/recover` answered "Recovered 23 keys" for a file it had just + /// failed to write, while the corrupt bytes were still on disk. /// - /// Every arm of the recovery in [`Self::load_uncached`] that writes reports - /// through here — creating a config that is missing, replacing one that - /// will not parse, and **restoring a backup over one that will not parse**. - /// The last of those is the arm a corrupted `config.yaml` with a usable - /// `.bak` beside it actually takes, and it was the one that said nothing: - /// `POST /config/recover` answered "Recovered 23 keys" for a file it had - /// just failed to write, while the corrupt bytes were still on disk. + /// Outstanding, not historical, and a record is retired two ways. + /// [`Self::save_values`] clears it the moment a write succeeds, and this + /// clears it once the failure has stopped being true WITHOUT anything + /// having written: the config loads, and a write to it would land. /// - /// Outstanding, not historical: [`Self::save_values`] clears it the moment - /// a write succeeds. A record that only ever accumulated would go on - /// claiming that changes will not persist long after they had started - /// persisting again — a config directory can be unwritable at start-up (an - /// unmounted volume, a full disk) and fine a minute later, and a false - /// version of this message is worse than none. + /// ⚠ The second way is not a refinement of the first. A config that loads + /// needs no recovery, so the reload `POST /config/recover` forces writes + /// nothing, and a record that waited for a write outlived its cause — + /// finding F2 of the 2026-09-10 QA run: permissions restored and the file + /// repaired, three consecutive recoveries still answered `persisted: false` + /// with the stale `Permission denied`. A config directory can be unwritable + /// at start-up (an unmounted volume, a full disk) and fine a minute later, + /// and a false version of this message is worse than none. + /// + /// What the check costs, and why it takes this shape: + /// - **No record, no I/O.** A healthy config answers from memory, so any + /// surface may ask as often as it likes. + /// - **A config that does not load keeps its record, however writable its + /// directory has become.** What the record says is still true — the + /// values in use exist only in this process — and the next reload runs + /// the recovery that is the actual fix, whose write then clears it. + /// Clearing on a writable directory alone would make M9 silent again. + /// - **A config that loads is probed** — see [`Self::probe_config_write`], + /// which does everything a write does except change the file. A probe + /// that still fails re-records what failed NOW, as + /// [`ConfigWriteFailure::NotWritable`]: the values in use are the file's, + /// and only a change made in this session is at risk. /// /// Reported here rather than returned, because [`Self::load`] must keep /// answering: a config directory that cannot be written is an environment @@ -845,11 +937,48 @@ impl Config { /// was that it was *silent* — the write error was logged per attempt at /// best and otherwise discarded, so an install running entirely on /// in-memory defaults looked identical to a healthy one. - pub fn last_write_error(&self) -> Option { - self.last_write_error + pub fn outstanding_write_failure(&self) -> Option { + let (recorded_error, recorded) = { + let slot = self + .last_write_error + .lock() + .unwrap_or_else(|e| e.into_inner()); + (slot.error.clone()?, slot.recorded) + }; + + let Some(on_disk) = self + .read_config_file() + .ok() + .filter(|content| parse_yaml_content(content).is_ok()) + else { + return Some(ConfigWriteFailure::ValuesInMemoryOnly(recorded_error)); + }; + + if let Err(still_failing) = self.probe_config_write(on_disk.as_bytes()) { + let error = still_failing.to_string(); + tracing::debug!("config.yaml loads but still cannot be written: {}", error); + let mut slot = self + .last_write_error + .lock() + .unwrap_or_else(|e| e.into_inner()); + slot.error = Some(error.clone()); + slot.recorded += 1; + return Some(ConfigWriteFailure::NotWritable(error)); + } + + // The file loads and a write to it would land, so every clause of the + // warning has stopped being true — but only for the failure this + // checked. One recorded while the probe ran was never tested, so it + // stands, and is reported as a failure to write a file that loads, + // which is what was just observed. + let mut slot = self + .last_write_error .lock() - .unwrap_or_else(|e| e.into_inner()) - .clone() + .unwrap_or_else(|e| e.into_inner()); + if slot.recorded == recorded { + slot.error = None; + } + slot.error.clone().map(ConfigWriteFailure::NotWritable) } /// Record a failed config write, logging the first one loudly. @@ -862,14 +991,16 @@ impl Config { /// /// Loud once, not per call: before the cache above this was reached on /// every `get_param`, and an error line per settings lookup is noise that - /// buries itself. + /// buries itself. Counted every time, so a check that found the file + /// writable again can tell whether the failure it would clear is still + /// the one it checked. fn record_config_write_error(&self, error: &ConfigError, what: &str) { let message = error.to_string(); let mut slot = self .last_write_error .lock() .unwrap_or_else(|e| e.into_inner()); - if slot.is_none() { + if slot.error.is_none() { tracing::error!( "Failed to write {} to {}: {}. Biorouter will run on in-memory values; \ settings changed in this session will not persist.", @@ -880,7 +1011,8 @@ impl Config { } else { tracing::debug!("Failed to write {} again: {}", what, message); } - *slot = Some(message); + slot.error = Some(message); + slot.recorded += 1; } fn load(&self) -> Result { @@ -1259,6 +1391,115 @@ impl Config { self.config_path.with_file_name(name) } + /// Stage `content` at `temp_path` the way every config write does: created + /// fresh, exclusively locked, written, and synced to disk before anything + /// is renamed into place. + /// + /// Shared by [`Self::save_values`] and [`Self::probe_config_write`], so the + /// check that decides whether a recorded write failure still holds fails + /// exactly where a real write would. + fn stage_config(&self, temp_path: &Path, content: &[u8]) -> Result<(), ConfigError> { + #[cfg(test)] + let mut faults = self.io_faults.failing_stages(); + // Retried for the same reason the rename in `save_values` is: a virus + // scanner or indexer holding a freshly created file open makes this + // fail with the same transient "Access is denied." on Windows, and the + // staging name is ours alone, so a failure here is never contention + // with another Biorouter writer. + let mut file = retry_while_transiently_unavailable(|| { + #[cfg(test)] + { + if faults > 0 { + faults -= 1; + return Err(IoFaults::access_denied()); + } + } + OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(temp_path) + })?; + + // Acquire an exclusive lock + file.lock_exclusive() + .map_err(|e| ConfigError::LockError(e.to_string()))?; + + // Write the contents using the same file handle + file.write_all(content)?; + file.sync_all()?; + + // Unlock is handled automatically when file is dropped + Ok(()) + } + + /// Whether a write to `config.yaml` would land right now — found out + /// without making one. + /// + /// Everything [`Self::save_values`] does that can fail is done for real, + /// up to the step that would change the file: the same staging path, the + /// same open, lock, write and sync, carrying the file's own bytes. Then the + /// staged file is REMOVED instead of renamed into place. On unix that rename + /// asks the directory's permission, not the file's, and staging has just + /// exercised exactly that. What it cannot see — an immutable flag, a sticky + /// directory owned by someone else — is not modelled; a write refused for + /// one of those still fails loudly to its own caller. + /// + /// ⚠ Never renamed, though installing a staged copy of the file's own + /// bytes would be the more literal test. That moves `config.yaml`'s stamp, + /// which `catalog::spawn_config_watcher` publishes to the app as an outside + /// change, and a write from another process landing between our read and + /// our rename would be reverted by bytes that never saw it. A check has no + /// business doing either. + /// + /// ⚠ On Windows the step left out CAN fail when this passes: a read-only + /// `config.yaml` refuses to be replaced — `MoveFileExW` answers + /// `ERROR_ACCESS_DENIED`, and std's `FileRenameInfoEx` fallback does not + /// ask to ignore the attribute — so there the attribute is checked too. Not + /// on unix, where a `0o444` file in a writable directory is replaced + /// without complaint and the check would report a failure `save_values` + /// does not have. + fn probe_config_write(&self, content: &[u8]) -> Result<(), ConfigError> { + #[cfg(windows)] + if std::fs::metadata(&self.config_path)? + .permissions() + .readonly() + { + return Err(ConfigError::FileError(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "config.yaml is read-only", + ))); + } + + let temp_path = self.staging_path(); + let staged = self.stage_config(&temp_path, content); + + #[cfg(test)] + if let Some(failure) = self.io_faults.take_failure_recorded_mid_probe() { + self.record_config_write_error( + &ConfigError::DirectoryError(failure), + "a config (injected while a writability check probed)", + ); + } + + // Removed however far staging got. One that cannot be removed is litter + // rather than a wrong answer — the question was whether a write would + // land, and the part of one that can fail has already answered it. + let removed = + retry_while_transiently_unavailable(|| match std::fs::remove_file(&temp_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + removed => removed, + }); + if let Err(e) = removed { + tracing::warn!( + "Could not remove {} after checking that the config can be written: {}", + temp_path.display(), + e + ); + } + staged + } + fn save_values(&self, values: Mapping) -> Result<(), ConfigError> { #[cfg(test)] self.io_probe.note_config_write(); @@ -1278,30 +1519,7 @@ impl Config { let temp_path = self.staging_path(); let staged = (|| -> Result<(), ConfigError> { - { - // Retried for the same reason the rename below is: a virus - // scanner or indexer holding a freshly created file open makes - // this fail with the same transient "Access is denied." on - // Windows, and the staging name is ours alone, so a failure - // here is never contention with another Biorouter writer. - let mut file = retry_while_transiently_unavailable(|| { - OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(&temp_path) - })?; - - // Acquire an exclusive lock - file.lock_exclusive() - .map_err(|e| ConfigError::LockError(e.to_string()))?; - - // Write the contents using the same file handle - file.write_all(yaml_value.as_bytes())?; - file.sync_all()?; - - // Unlock is handled automatically when file is dropped - } + self.stage_config(&temp_path, yaml_value.as_bytes())?; // Replace the original file. Atomic on unix; on Windows the // destination NAME is briefly unopenable while it happens, and a @@ -1335,16 +1553,17 @@ impl Config { if staged.is_ok() { // A write just succeeded, so "the config file could not be written" - // has stopped being true. Without this the record is permanent, and - // a config directory that was briefly unwritable at start-up — an - // unmounted volume, a full disk, a first-run permissions problem — - // would make `/config/recover` warn that changes will not persist - // for the rest of the process's life, after they had started - // persisting again. - *self - .last_write_error + // has stopped being true. One of the two ways a record is retired; + // `outstanding_write_failure` is the other, for a failure that stops + // being true with NO write — a config repaired from outside loads, + // needs no recovery, and so is never written by the reload + // `/config/recover` forces. Waiting for this clear alone is how a + // config directory that was briefly unwritable came to be warned + // about for the rest of the process's life (finding F2). + self.last_write_error .lock() - .unwrap_or_else(|e| e.into_inner()) = None; + .unwrap_or_else(|e| e.into_inner()) + .error = None; } staged @@ -3714,6 +3933,403 @@ mod tests { ); } + /// A recorded write failure is cleared once the config loads and can be + /// written again — even though nothing has written it since. + /// + /// Finding F2 of the 2026-09-10 QA run on `7c96d796`: after one genuine + /// write failure the permissions were restored and the file repaired, and + /// three `POST /config/recover` calls on that healthy, writable, valid + /// config all still answered `persisted: false` with the stale + /// `Permission denied`. The clear lived only in `save_values`, and a config + /// that loads needs no recovery, so the reload the route forces writes + /// nothing and the record could only be cleared by an unrelated write. + /// + /// Portable on purpose: the parent directory is a FILE, as in + /// `a_write_that_succeeds_clears_an_earlier_recorded_failure`, rather than a + /// mode, which Windows does not have. + #[test] + fn a_recorded_write_failure_is_cleared_once_the_config_loads_and_can_be_written_again() { + let dir = tempfile::tempdir().unwrap(); + let blocked = dir.path().join("blocked"); + std::fs::write(&blocked, "not a directory").unwrap(); + let config_path = blocked.join("config.yaml"); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + + let _ = config.all_values(); + assert!( + config.last_write_error().is_some(), + "the premise: a write failure has been recorded" + ); + + // Repaired the way F2 was, from OUTSIDE this process: the directory can + // be written again and holds a config that loads. + std::fs::remove_file(&blocked).unwrap(); + std::fs::create_dir(&blocked).unwrap(); + std::fs::write(&config_path, "A_KEY_ON_DISK: 7\n").unwrap(); + let writes_before = config.io_probe.config_writes(); + + // What `POST /config/recover` does: drop the cache and reload. + config.invalidate_values_cache(); + assert_eq!( + config.all_values().unwrap().get("A_KEY_ON_DISK"), + Some(&serde_json::json!(7)) + ); + assert_eq!( + config.io_probe.config_writes(), + writes_before, + "the premise of F2: a config that loads needs no recovery, so the reload writes \ + nothing — and a clear that waits for a write never comes" + ); + + assert_eq!( + config.last_write_error(), + None, + "the config loads and can be written, so nothing may still claim it cannot" + ); + } + + /// A config whose write failed and was recorded, then was repaired from + /// outside the process: its directory writable again and holding a config + /// that loads. F2's state, reached portably — the config's parent starts + /// out as a FILE, which fails the write the same way on every platform. + fn a_config_that_could_not_be_written_and_was_repaired() -> (tempfile::TempDir, PathBuf, Config) + { + let dir = tempfile::tempdir().unwrap(); + let blocked = dir.path().join("blocked"); + std::fs::write(&blocked, "not a directory").unwrap(); + let config_path = blocked.join("config.yaml"); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + + let _ = config.all_values(); + assert!( + config.last_write_error.lock().unwrap().error.is_some(), + "the premise: a write failure has been recorded" + ); + + std::fs::remove_file(&blocked).unwrap(); + std::fs::create_dir(&blocked).unwrap(); + std::fs::write(&config_path, "A_KEY_ON_DISK: 7\n").unwrap(); + (dir, config_path, config) + } + + /// A config that still does not load keeps its record, however writable + /// its directory has become. + /// + /// ⚠ The plausible wrong fix for F2 — "the directory accepts a new file + /// now, so clear the record" — fails here, and it would reopen M9: the + /// corrupt bytes are still on disk and every value in use is the backup's, + /// held in this process alone, which is exactly what the record says. What + /// retires it is the reload that retries the restore, by writing. + #[test] + fn a_config_that_still_does_not_load_keeps_its_record_however_writable_its_directory() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("config.yaml"); + std::fs::write(&config_path, "BIOROUTER_MODEL: [unclosed\n").unwrap(); + std::fs::write( + dir.path().join("config.yaml.bak"), + "A_KEY_THE_BACKUP_HAS: 7\n", + ) + .unwrap(); + let mut config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + config.io_faults.failing_rename_attempts = + std::sync::atomic::AtomicUsize::new(TRANSIENT_IO_ATTEMPTS); + let _ = config.all_values(); + + // The condition lifts, and nothing has retried the restore. + config.io_faults.failing_rename_attempts = std::sync::atomic::AtomicUsize::new(0); + + let failure = config.outstanding_write_failure(); + assert!( + matches!(failure, Some(ConfigWriteFailure::ValuesInMemoryOnly(_))), + "the corrupt bytes are still on disk, so the values in use are in memory only, \ + whatever the directory would accept now; got {failure:?}" + ); + assert_eq!( + std::fs::read_to_string(&config_path).unwrap(), + "BIOROUTER_MODEL: [unclosed\n", + "and checking must not have touched them" + ); + + // What does retire it: the reload `/config/recover` forces retries the + // restore, and this time its write lands. + config.invalidate_values_cache(); + assert_eq!( + config.all_values().unwrap().get("A_KEY_THE_BACKUP_HAS"), + Some(&serde_json::json!(7)) + ); + assert_eq!(config.outstanding_write_failure(), None); + } + + /// A config that loads but still cannot be written is reported as a file + /// that cannot be written — not as values held in memory — and the check + /// agrees with the write it stands in for. + /// + /// ⚠ The other plausible wrong fix — "the config loads, so clear" — fails + /// here: nothing about a config loading says a change to it would be saved. + #[test] + fn a_config_that_loads_but_still_cannot_be_written_is_reported_as_not_writable() { + let (_dir, _config_path, mut config) = + a_config_that_could_not_be_written_and_was_repaired(); + // The directory still refuses new files. + config.io_faults.failing_stage_attempts = + std::sync::atomic::AtomicUsize::new(TRANSIENT_IO_ATTEMPTS); + + assert_eq!( + config.outstanding_write_failure(), + Some(ConfigWriteFailure::NotWritable( + "Config file I/O failed: Access is denied.".to_string() + )), + "the file loads, so the values in use are the file's, and the failure reported is \ + the one that holds NOW rather than the directory error recorded before the repair" + ); + assert!( + config.set_param("A_KEY_SET_NOW", 1).is_err(), + "the check must agree with the write it stands in for" + ); + + config.io_faults.failing_stage_attempts = std::sync::atomic::AtomicUsize::new(0); + assert_eq!( + config.outstanding_write_failure(), + None, + "and once a write would land, nothing is reported" + ); + } + + /// Checking whether the config can be written leaves it exactly as it was. + /// + /// The check stages and removes; it never renames. A rename would move + /// `config.yaml`'s stamp — which the app's config watcher publishes as an + /// outside change — and could revert a write another process landed + /// between the read and the rename. So: the same bytes, the same stamp, no + /// backup rotated, no staging file left behind, no write counted. + #[test] + fn checking_whether_the_config_can_be_written_leaves_it_exactly_as_it_was() { + let (_dir, config_path, config) = a_config_that_could_not_be_written_and_was_repaired(); + let siblings = || -> Vec { + let mut names: Vec = std::fs::read_dir(config_path.parent().unwrap()) + .unwrap() + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names + }; + let bytes = std::fs::read(&config_path).unwrap(); + let stamp = FileStamp::of(&config_path); + let listed = siblings(); + let writes = config.io_probe.config_writes(); + + assert_eq!( + config.outstanding_write_failure(), + None, + "the premise: the check ran, and passed" + ); + + assert_eq!(std::fs::read(&config_path).unwrap(), bytes); + assert_eq!( + FileStamp::of(&config_path), + stamp, + "the stamp must not move — the app would be told the config changed" + ); + assert_eq!(siblings(), listed, "no backup rotated, no staging litter"); + assert_eq!( + config.io_probe.config_writes(), + writes, + "and nothing went through save_values" + ); + } + + /// With nothing recorded there is nothing to check, and nothing is read. + /// + /// Every surface that reports on persistence asks this, so it has to be + /// free while the config is healthy: the check exists for the failure + /// case and must not become a cost of the success one. + #[test] + fn a_config_with_nothing_recorded_is_not_checked_at_all() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("config.yaml"); + std::fs::write(&config_path, "A_KEY_THAT_IS_SET: 1\n").unwrap(); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + assert_eq!(config.get_param::("A_KEY_THAT_IS_SET").unwrap(), 1); + let reads = config.io_probe.read_attempts(); + + for _ in 0..8 { + assert_eq!(config.outstanding_write_failure(), None); + assert_eq!(config.last_write_error(), None); + } + + assert_eq!( + config.io_probe.read_attempts(), + reads, + "a healthy config must be answered from memory" + ); + assert_eq!( + std::fs::read_dir(dir.path()).unwrap().count(), + 1, + "and nothing staged" + ); + } + + /// A failure recorded while the check was probing is not the one it + /// clears. + /// + /// The check reads the record, probes with no lock held — probing is I/O, + /// and holding `last_write_error` across it would stop that being a leaf + /// lock — and clears afterwards. A failure recorded in between is one the + /// probe never tested, and clearing it would make it silent, which is M9 + /// again. Injected, because the interleaving is microseconds wide and no + /// test can arrange it. + #[test] + fn a_failure_recorded_while_the_check_probed_is_not_the_one_it_clears() { + let (_dir, _config_path, config) = a_config_that_could_not_be_written_and_was_repaired(); + *config.io_faults.failure_recorded_mid_probe.lock().unwrap() = + Some("No space left on device".to_string()); + + assert_eq!( + config.outstanding_write_failure(), + Some(ConfigWriteFailure::NotWritable( + "Failed to create config directory: No space left on device".to_string() + )), + "the failure that landed mid-probe stands, and is reported" + ); + assert_eq!( + config.outstanding_write_failure(), + None, + "and the next check, which does test it, retires it" + ); + } + + /// The check agrees with the write it stands in for, in both directions, + /// on the platform where a mode is how a directory becomes unwritable. + /// + /// ⚠ The second half is the one a tidy-looking check gets wrong. A `0o444` + /// `config.yaml` in a writable directory is REPLACED by `save_values` — + /// the rename needs the directory, not the file — so a check that looked + /// at the file's own permission bits would report a failure the real write + /// does not have, and F2 would be back for every read-only config. + #[cfg(unix)] + #[test] + fn the_check_agrees_with_the_write_it_stands_in_for() { + use std::os::unix::fs::PermissionsExt; + + /// A `TempDir` still at `0o555` cannot delete its own contents. + struct RestoreModes(PathBuf, PathBuf); + impl Drop for RestoreModes { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o755)); + let _ = std::fs::set_permissions(&self.1, std::fs::Permissions::from_mode(0o644)); + } + } + let mode = |path: &Path, bits: u32| { + std::fs::set_permissions(path, std::fs::Permissions::from_mode(bits)).unwrap() + }; + + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("config.yaml"); + std::fs::write(&config_path, "BIOROUTER_MODEL: [unclosed\n").unwrap(); + std::fs::write( + dir.path().join("config.yaml.bak"), + "A_KEY_THE_BACKUP_HAS: 7\n", + ) + .unwrap(); + let config = + Config::new_with_file_secrets(&config_path, dir.path().join("secrets.yaml")).unwrap(); + + let _restore = RestoreModes(dir.path().to_path_buf(), config_path.clone()); + mode(dir.path(), 0o555); + // Root ignores the mode, and then every premise below is false. + if std::fs::write(dir.path().join("writability-probe"), "x").is_ok() { + eprintln!("skipped: this process can write a 0o555 directory (running as root?)"); + return; + } + let _ = config.all_values(); + + // Repaired in place; the directory still refuses new files. + std::fs::write(&config_path, "A_KEY_ON_DISK: 7\n").unwrap(); + let failure = config.outstanding_write_failure(); + assert!( + matches!(&failure, Some(ConfigWriteFailure::NotWritable(e)) if e.contains("Permission denied")), + "the file loads but the directory still refuses a write; got {failure:?}" + ); + assert!( + config.set_param("A_KEY_SET_NOW", 1).is_err(), + "and the write it stands in for fails too" + ); + + // The directory writable again, the FILE read-only. + mode(dir.path(), 0o755); + mode(&config_path, 0o444); + assert_eq!( + config.outstanding_write_failure(), + None, + "a read-only file in a writable directory is no obstacle to the rename every write \ + ends in" + ); + config + .set_param("A_KEY_SET_NOW", 1) + .expect("and the write it stands in for lands"); + assert_eq!(config.get_param::("A_KEY_SET_NOW").unwrap(), 1); + } + + /// On Windows a read-only `config.yaml` is not called writable. + /// + /// The one platform where the step the check leaves out can fail when the + /// rest passes: the rename every write ends in refuses a read-only + /// destination. The premise is asserted rather than assumed, so a std that + /// learns to replace read-only files turns this red instead of leaving the + /// check reporting a failure that no longer exists. + #[cfg(windows)] + #[test] + // Windows only, where `set_readonly(false)` clears the read-only attribute + // and nothing else; the lint is about unix, where it means world-writable. + #[allow(clippy::permissions_set_readonly_false)] + fn a_read_only_config_is_not_called_writable_on_windows() { + fn set_readonly(path: &Path, readonly: bool) { + let mut permissions = std::fs::metadata(path).unwrap().permissions(); + permissions.set_readonly(readonly); + std::fs::set_permissions(path, permissions).unwrap(); + } + /// A read-only file left behind would outlive the `TempDir` around it. + struct Writable(PathBuf); + impl Drop for Writable { + fn drop(&mut self) { + if let Ok(meta) = std::fs::metadata(&self.0) { + let mut permissions = meta.permissions(); + permissions.set_readonly(false); + let _ = std::fs::set_permissions(&self.0, permissions); + } + } + } + + let (_dir, config_path, config) = a_config_that_could_not_be_written_and_was_repaired(); + let _writable = Writable(config_path.clone()); + set_readonly(&config_path, true); + + let replacement = config_path.with_file_name("replacement.yaml"); + std::fs::write(&replacement, "A_KEY_ON_DISK: 8\n").unwrap(); + let replaced = std::fs::rename(&replacement, &config_path); + let _ = std::fs::remove_file(&replacement); + assert!( + replaced.is_err(), + "the premise of the read-only check in `probe_config_write`: a read-only config.yaml \ + cannot be replaced here. If std can now replace one, that check reports a failure \ + `save_values` no longer has, and it should go" + ); + + let failure = config.outstanding_write_failure(); + assert!( + matches!(failure, Some(ConfigWriteFailure::NotWritable(_))), + "got {failure:?}" + ); + + set_readonly(&config_path, false); + assert_eq!(config.outstanding_write_failure(), None); + } + /// 1,000 `get_param` lookups against a warm config file on disk. /// /// Ignored because it measures rather than asserts. Run it with @@ -4209,7 +4825,7 @@ mod tests { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), test_keyring_store: Some(std::sync::Arc::new(PanicsOnRead)), io_faults: IoFaults::default(), io_probe: IoProbe::default(), @@ -4377,7 +4993,7 @@ mod tests { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), test_keyring_store: Some(store.clone()), io_faults: IoFaults::default(), io_probe: IoProbe::default(), @@ -4416,7 +5032,7 @@ mod tests { secrets_read: Mutex::new(()), values_cache: Mutex::new(ValuesCache::default()), values_read: Mutex::new(()), - last_write_error: Mutex::new(None), + last_write_error: Mutex::new(WriteFailureRecord::default()), test_keyring_store: Some(store.clone()), io_faults: IoFaults::default(), io_probe: IoProbe::default(), diff --git a/crates/biorouter/src/config/mod.rs b/crates/biorouter/src/config/mod.rs index 6444d64c0..6b8696373 100644 --- a/crates/biorouter/src/config/mod.rs +++ b/crates/biorouter/src/config/mod.rs @@ -11,7 +11,7 @@ pub mod signup_tetrate; pub mod usage; pub use crate::agents::ExtensionConfig; -pub use base::{with_config_overrides, Config, ConfigError}; +pub use base::{with_config_overrides, Config, ConfigError, ConfigWriteFailure}; pub use biorouter_mode::BioRouterMode; pub use declarative_providers::DeclarativeProviderConfig; pub use experiments::ExperimentManager; diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 1a66a7468..cc6888530 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -6904,7 +6904,7 @@ }, "persisted": { "type": "boolean", - "description": "Whether `config.yaml` on disk holds what this report describes.\n\n`false` means the recovery could not write what it recovered: the keys\nabove live only in this process, the file on disk is unchanged — still\nabsent, or still the contents that would not load — nothing changed in\nthis session survives exit, and the next start runs the same recovery\nagain." + "description": "Whether this process's settings are persisting: `config.yaml` holds what\nthis report describes, and a write to it lands.\n\n`false` in one of two ways, which `message` spells out:\n- the recovery could not write what it recovered: the keys above live\nonly in this process, the file on disk is unchanged — still absent, or\nstill the contents that would not load — and the next start runs the\nsame recovery again;\n- `config.yaml` loads, so the keys above are the file's, but it cannot\nbe written right now.\n\nEither way a setting changed in this session will not be saved. Checked\nagainst the disk on every call, so a failure that has since been\nrepaired is not reported." }, "recovered_keys": { "type": "array", diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 68b657939..b35605218 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -645,13 +645,20 @@ export type ConfigRecoveryReport = { */ message: string; /** - * Whether `config.yaml` on disk holds what this report describes. + * Whether this process's settings are persisting: `config.yaml` holds what + * this report describes, and a write to it lands. * - * `false` means the recovery could not write what it recovered: the keys - * above live only in this process, the file on disk is unchanged — still - * absent, or still the contents that would not load — nothing changed in - * this session survives exit, and the next start runs the same recovery - * again. + * `false` in one of two ways, which `message` spells out: + * - the recovery could not write what it recovered: the keys above live + * only in this process, the file on disk is unchanged — still absent, or + * still the contents that would not load — and the next start runs the + * same recovery again; + * - `config.yaml` loads, so the keys above are the file's, but it cannot + * be written right now. + * + * Either way a setting changed in this session will not be saved. Checked + * against the disk on every call, so a failure that has since been + * repaired is not reported. */ persisted: boolean; /**