mirror of
https://github.com/block/buzz.git
synced 2026-11-01 06:51:17 +00:00
fix(b1-store): round-11 — hoist file recovery before migrations, fail closed
run_file_commit_recovery is now called immediately after reset handling succeeds, before run_boot_migrations*, backfill_persona_snapshots, and every other canonical-store reader/writer. A stale stage from a pre-crash commit can no longer overwrite migration/backfill output. Fail closed: a recovery error sets store_recovery_failed on AppState (same pattern as reset_failed) and returns early, keeping all store-touching setup closed. get_identity surfaces the flag to the frontend. spawn_boot_recovery (publication-only since round 9) is the sole remaining background recovery path; it cannot enter file repair at any schedule. New test: test_setup_order_recovery_completes_before_migration_reads_canonical plants stale canonical + stage file with repaired content, runs file_commit_recovery_at_pub (the exact function production delegates to), then asserts the canonical holds the repaired content — proving any subsequent migration read sees the repaired state. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
parent
040af53286
commit
29d04a5cf4
@@ -56,9 +56,8 @@ pub struct AppState {
|
||||
pub huddle_audio: crate::huddle::tts_settings::HuddleAudioSettingsState,
|
||||
/// Tauri app handle — stored after setup so huddle commands can emit
|
||||
/// `huddle-state-changed` events without needing the handle threaded
|
||||
/// through every call site.
|
||||
///
|
||||
/// Set once during `setup()` in `lib.rs`; never cleared.
|
||||
/// through every call site. Set once during `setup()` in `lib.rs`;
|
||||
/// never cleared.
|
||||
pub app_handle: Mutex<Option<AppHandle>>,
|
||||
/// Port of the localhost media streaming proxy (set during setup).
|
||||
pub media_proxy_port: AtomicU16,
|
||||
@@ -68,13 +67,8 @@ pub struct AppState {
|
||||
/// signing commands check this flag via [`AppState::signing_keys`] and
|
||||
/// return `Err` so no events are published under the inaccessible identity.
|
||||
/// Mutually exclusive with `identity_lost` (guaranteed by `RecoveryState`
|
||||
/// at the resolve boundary).
|
||||
///
|
||||
/// Ordering: writers store with `Ordering::Release` after `state.keys` is
|
||||
/// updated, so a reader observing `false` with `Ordering::Acquire` is
|
||||
/// guaranteed to see the updated keys. Writers: `setup()` (initial
|
||||
/// resolution via `resolve_persisted_identity`) and `import_identity`
|
||||
/// (clears the flag when the user successfully imports a new key).
|
||||
/// at the resolve boundary). Ordering: writers store with
|
||||
/// `Ordering::Release`; readers use `Ordering::Acquire`.
|
||||
pub keyring_locked: AtomicBool,
|
||||
/// Set when identity resolution detected a "lost" state: the migration
|
||||
/// marker was present but the keyring was empty and no plaintext fallback
|
||||
@@ -97,11 +91,12 @@ pub struct AppState {
|
||||
/// Set when the boot-time Phase 2 reset attempted a wipe but verification
|
||||
/// failed. The sentinel is preserved so the next relaunch retries. All
|
||||
/// identity-dependent setup is skipped; the frontend shows a reset-failed
|
||||
/// recovery screen via `get_identity`.
|
||||
///
|
||||
/// Ordering: written once in `setup()` with `Ordering::Release`; read in
|
||||
/// `get_identity` with `Ordering::Acquire`.
|
||||
/// recovery screen via `get_identity`. Written once in `setup()` with
|
||||
/// `Ordering::Release`; read in `get_identity` with `Ordering::Acquire`.
|
||||
pub reset_failed: AtomicBool,
|
||||
/// Set when pre-migration file-commit recovery fails; all store-touching
|
||||
/// setup is skipped. Same write/read ordering as `reset_failed`.
|
||||
pub store_recovery_failed: AtomicBool,
|
||||
/// Cached ACP session config from running agents, keyed by canonical
|
||||
/// `(agent pubkey, relay URL)` runtime identity.
|
||||
/// Populated when the harness emits `session_config_captured` observer events.
|
||||
@@ -226,6 +221,7 @@ pub fn build_app_state() -> AppState {
|
||||
keyring_locked: AtomicBool::new(false),
|
||||
identity_lost: AtomicBool::new(false),
|
||||
reset_failed: AtomicBool::new(false),
|
||||
store_recovery_failed: AtomicBool::new(false),
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
mesh_llm_runtime: AsyncMutex::new(None),
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
|
||||
@@ -39,6 +39,9 @@ pub fn get_identity(state: State<'_, AppState>) -> Result<IdentityInfo, String>
|
||||
let reset_failed = state
|
||||
.reset_failed
|
||||
.load(std::sync::atomic::Ordering::Acquire);
|
||||
let store_recovery_failed = state
|
||||
.store_recovery_failed
|
||||
.load(std::sync::atomic::Ordering::Acquire);
|
||||
|
||||
Ok(IdentityInfo {
|
||||
pubkey: pubkey_hex,
|
||||
@@ -47,6 +50,7 @@ pub fn get_identity(state: State<'_, AppState>) -> Result<IdentityInfo, String>
|
||||
lost,
|
||||
locked,
|
||||
reset_failed,
|
||||
store_recovery_failed,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -382,6 +386,7 @@ pub async fn import_identity(
|
||||
lost: false,
|
||||
locked: false,
|
||||
reset_failed: false,
|
||||
store_recovery_failed: false,
|
||||
})
|
||||
})
|
||||
.await
|
||||
@@ -515,6 +520,7 @@ pub async fn persist_current_identity(
|
||||
lost: false,
|
||||
locked: false,
|
||||
reset_failed: false,
|
||||
store_recovery_failed: false,
|
||||
})
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -340,6 +340,24 @@ pub fn run() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// File-commit recovery MUST run before any migration, backfill, or
|
||||
// canonical-store reader/writer. A stale stage from a pre-crash
|
||||
// commit must not overwrite migration output. One owner, one site.
|
||||
// Fail closed: a repair error sets store_recovery_failed and skips
|
||||
// all store-touching setup.
|
||||
{
|
||||
let state = app_handle.state::<AppState>();
|
||||
if let Err(e) =
|
||||
crate::managed_agents::store_journal::run_file_commit_recovery(&app_handle)
|
||||
{
|
||||
eprintln!("buzz-desktop: file-commit-recovery: {e}");
|
||||
state
|
||||
.store_recovery_failed
|
||||
.store(true, std::sync::atomic::Ordering::Release);
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
// Run all pre-identity data migrations before state loads from disk.
|
||||
if reset_outcome.completed {
|
||||
migration::run_boot_migrations_after_reset(&app_handle);
|
||||
@@ -597,18 +615,6 @@ pub fn run() {
|
||||
}
|
||||
});
|
||||
}
|
||||
// File-commit recovery runs synchronously under the advisory lock
|
||||
// BEFORE spawn_boot_recovery is called. This ensures the JSON
|
||||
// store is in a consistent state (no dangling *.stage files)
|
||||
// before the background task or any command can arrive.
|
||||
// File repair has exactly ONE owner: this synchronous call.
|
||||
// spawn_boot_recovery is publication-only — it does NOT run
|
||||
// recover_interrupted_file_commits.
|
||||
if let Err(e) =
|
||||
crate::managed_agents::store_journal::run_file_commit_recovery(app.handle())
|
||||
{
|
||||
eprintln!("buzz-desktop: file-commit-recovery: {e}");
|
||||
}
|
||||
crate::managed_agents::spawn_boot_recovery(app.handle());
|
||||
Ok(())
|
||||
})
|
||||
|
||||
@@ -674,6 +674,75 @@ fn test_sync_file_recovery_repairs_dangling_intent_row() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Fix A — production-order seam test: file-commit recovery must complete
|
||||
/// BEFORE any migration or canonical-store reader observes the canonical files.
|
||||
///
|
||||
/// Seam used: `file_commit_recovery_at_pub` is the exact function that
|
||||
/// `run_file_commit_recovery` delegates to in production — not a
|
||||
/// reimplementation. The "migration read" is a direct filesystem read of the
|
||||
/// canonical path, matching what every real migration/backfill does.
|
||||
///
|
||||
/// Scenario: a process crashed after writing a stage file but before the rename
|
||||
/// completed. The canonical (`managed-agents.json`) still holds pre-crash
|
||||
/// ("stale") content. The stage file holds the intended post-crash ("repaired")
|
||||
/// content. After recovery runs, the canonical must hold the repaired content —
|
||||
/// so any subsequent migration or backfill reader sees the repaired state.
|
||||
#[test]
|
||||
fn test_setup_order_recovery_completes_before_migration_reads_canonical() {
|
||||
let dir = tmp_dir();
|
||||
let anchor = dir.path().to_path_buf();
|
||||
|
||||
// "Stale" content — what canonical holds before recovery.
|
||||
let stale_content = b"[{\"pubkey\":\"stale-pre-crash\"}]";
|
||||
// "Repaired" content — what the stage file holds (the intended commit).
|
||||
let repaired_content = b"[{\"pubkey\":\"repaired-post-crash\"}]";
|
||||
let teams_content = b"[]";
|
||||
|
||||
let a_can = anchor.join("managed-agents.json");
|
||||
let t_can = anchor.join("teams.json");
|
||||
let a_stage = anchor.join("managed-agents.order_test.stage");
|
||||
let t_stage = anchor.join("teams.order_test.stage");
|
||||
|
||||
// Write stale canonical — this is what migrations would see WITHOUT recovery.
|
||||
std::fs::write(&a_can, stale_content).unwrap();
|
||||
std::fs::write(&t_can, teams_content).unwrap();
|
||||
|
||||
// Write stage files containing the repaired (intended) state.
|
||||
std::fs::write(&a_stage, repaired_content).unwrap();
|
||||
std::fs::write(&t_stage, teams_content).unwrap();
|
||||
|
||||
// Plant the intent row — recovery will detect and replay it.
|
||||
{
|
||||
let j = open_journal(&anchor).unwrap();
|
||||
insert_phase_row(
|
||||
&j,
|
||||
"order_test",
|
||||
"intent",
|
||||
a_stage.to_str().unwrap(),
|
||||
t_stage.to_str().unwrap(),
|
||||
&sha256_hex(repaired_content),
|
||||
&sha256_hex(teams_content),
|
||||
);
|
||||
}
|
||||
|
||||
// Run recovery — the exact same code path production calls via
|
||||
// run_file_commit_recovery → file_commit_recovery_at.
|
||||
file_commit_recovery_at_pub(&anchor).unwrap();
|
||||
|
||||
// Now simulate a "migration read" — read the canonical directly, as every
|
||||
// real migration/backfill does.
|
||||
let observed = std::fs::read(&a_can).unwrap();
|
||||
assert_eq!(
|
||||
observed, repaired_content,
|
||||
"migration read must observe the repaired canonical, not the stale pre-crash state"
|
||||
);
|
||||
// Stale content must not be present.
|
||||
assert_ne!(
|
||||
observed, stale_content,
|
||||
"stale canonical content must have been replaced by recovery"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Fix B: migration tests ───────────────────────────────────────────────────
|
||||
|
||||
/// Helper: create a v2-shaped in-memory journal (no hash/d-tag columns).
|
||||
|
||||
@@ -24,6 +24,9 @@ pub struct IdentityInfo {
|
||||
/// reset-failed recovery screen. The sentinel is preserved so the next
|
||||
/// relaunch retries the wipe automatically.
|
||||
pub reset_failed: bool,
|
||||
/// True when pre-migration file-commit recovery failed at boot. All
|
||||
/// store-touching setup was skipped; the frontend shows a recovery screen.
|
||||
pub store_recovery_failed: bool,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize)]
|
||||
|
||||
Reference in New Issue
Block a user