From ff8dda06d0bb2f0446da92598b39b59f0ca337ca Mon Sep 17 00:00:00 2001 From: Jake Date: Thu, 14 May 2026 13:59:08 +0100 Subject: [PATCH] =?UTF-8?q?agent:=20lumotia=20=E2=80=94=20Phase=20A.7=20fi?= =?UTF-8?q?x=20startup-order=20race=20that=20silently=20orphaned=20legacy?= =?UTF-8?q?=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical bug surfaced by the dogfood drill: every upgrading Magnotia user would silently keep a fresh empty Lumotia install while their Magnotia data sat orphaned next to it. Drill caught it on the first real run under sandboxed HOME. ROOT CAUSE src-tauri/src/lib.rs::run() previously called the migrations from inside the Tauri setup hook (post `tauri::Builder::default()`). But three sequential actions BEFORE the setup hook had already created the destination directories: 1. init_tracing() -> logs_dir() -> create_dir_all(app_data_dir/logs) creates the lumotia/ root. 2. install_panic_hook() -> crashes_dir() -> create_dir_all() ditto. 3. Tauri's WebKitGTK runtime / plugin chain creates the bundle-id-keyed consulting.corbel.lumotia/ dir eagerly when the WebContext spins up (mediakeys, storage, WebKitCache subdirs appeared even without our hook explicitly creating them). By the time the setup-hook migrations fired, every legacy candidate returned `TargetAlreadyExists` (paths.rs) or `BothExistLegacyPreserved` (tauri_app_data_migration.rs) — both silent no-op codepaths. Legacy data was left untouched, fresh Lumotia install gained no transcripts, settings, or window state. FIX Migrate BEFORE any other code touches app_data_dir(). src-tauri/src/tauri_app_data_migration.rs: - NEW_BUNDLE_ID const ("consulting.corbel.lumotia"). MUST agree with tauri.conf.json#identifier; reviewer-enforced invariant. - Renamed private `legacy_tauri_app_data_dir_for` -> public `tauri_app_data_dir_for(identifier)`. Function is parameterised by bundle id; the "legacy" name was misleading after this change. - New `current_tauri_app_data_dir()` resolves the NEW bundle path from platform env vars (same convention Tauri 2 uses), so the pre-runtime migration can address its destination without needing an AppHandle. src-tauri/src/lib.rs: - New `migrate_user_data_pre_runtime()` orchestrates the two migrations + ambiguity guard. Uses `eprintln!` for surface events (tracing not yet initialised at this stage; stderr lands in journald / foreground terminal which is the right transport for boot-phase output). FATAL errors call process::exit(1) — the setup-hook version returned Err from the closure, equivalent effect. - run() now calls migrate_user_data_pre_runtime() as its first line, BEFORE init_tracing(), install_panic_hook(), and the Tauri builder. - Setup-hook migration blocks deleted (~90 lines). Setup hook now starts with a one-line comment pointing at the pre-runtime fn. VERIFICATION Re-ran the dogfood drill (scripts/dogfood-rebrand-drill.sh) — 8/8 probes pass after the fix (was 4/8). Both stderr lines fire: [lumotia-startup] migrated legacy magnotia data dir to lumotia: .../magnotia -> .../lumotia (renamed_db=true, elapsed_ms=0) [lumotia-startup] migrated Tauri app_data_dir from legacy bundle identifier: .../uk.co.corbel.magnotia -> .../consulting.corbel.lumotia (elapsed_ms=0) On-disk post-state confirms: magnotia/ gone, lumotia/ has migrated db + recordings, uk.co.corbel.magnotia/ preserved as backup, consulting.corbel.lumotia/localStorage/leveldb/ has migrated data. - cargo fmt --check: clean - cargo clippy --workspace --all-targets -- -D warnings: clean - cargo test --workspace: 409/0 (no regression) --- src-tauri/src/lib.rs | 247 ++++++++++++---------- src-tauri/src/tauri_app_data_migration.rs | 37 +++- 2 files changed, 168 insertions(+), 116 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index e42d5b5..360c001 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -298,8 +298,136 @@ fn init_tracing() { }); } +/// One-shot data migration that runs BEFORE anything else in `run()`. +/// +/// CRITICAL ORDERING: this must come before `init_tracing`, +/// `install_panic_hook`, AND `tauri::Builder::default()`. Each of those +/// either calls `create_dir_all` on a child of `app_data_dir()` or causes +/// Tauri/WebKitGTK to create its own bundle-identifier-keyed dir for +/// plugin state. Either way, by the time the migration would otherwise +/// run from inside the Tauri setup hook, the destination already exists +/// and the migration short-circuits via `TargetAlreadyExists` / +/// `BothExistLegacyPreserved` — silently leaving the user's legacy +/// Magnotia data orphaned next to a fresh empty Lumotia install. The +/// `scripts/dogfood-rebrand-drill.sh` integration probe is what surfaced +/// this race; see commit history for the original buggy ordering. +/// +/// Tracing isn't initialised yet, so migration outcomes are written to +/// stderr via `eprintln!`. The format mirrors the structured fields the +/// setup-hook tracing layer would have emitted — same content, different +/// transport. systemd-journald + a foreground terminal both capture +/// stderr, which is the audit surface that matters at boot. +/// +/// Fatal failures (data-dir migration error, ambiguous lumotia paths on +/// disk) call `process::exit(1)` rather than panicking. We refuse to +/// start with the wrong path resolved — silently continuing would +/// orphan user data, which is the failure mode this fix is closing. +fn migrate_user_data_pre_runtime() { + // 1. Hand-rolled data-dir migration: ~/.local/share/magnotia (and + // macOS / Windows equivalents) -> ~/.local/share/lumotia. Drives + // every legacy candidate independently so multi-legacy Linux + // users (`~/.magnotia` AND `~/.local/share/magnotia` from + // different historical builds) get all of them migrated, not + // just the first one probed. + let t = std::time::Instant::now(); + match migrate_legacy_data_dir() { + Ok(statuses) => { + for status in &statuses { + match status { + MigrationStatus::Migrated { + from, + to, + renamed_db, + } => { + eprintln!( + "[lumotia-startup] migrated legacy magnotia data dir to lumotia: \ + {from} -> {to} (renamed_db={renamed_db}, elapsed_ms={ms})", + from = from.display(), + to = to.display(), + renamed_db = *renamed_db, + ms = t.elapsed().as_millis(), + ); + } + MigrationStatus::TargetAlreadyExists { .. } + | MigrationStatus::NoLegacyFound => { + // Steady state on the second-or-later boot. Chatty + // logging here would dwarf the genuine first-boot + // event, so we stay silent. + } + } + } + } + Err(e) => { + eprintln!( + "[lumotia-startup] FATAL: legacy data dir migration failed — refusing \ + to start (would orphan user data): {e}" + ); + std::process::exit(1); + } + } + + // 2. Tauri app_data_dir migration: ~/.local/share/uk.co.corbel.magnotia + // -> ~/.local/share/consulting.corbel.lumotia. Copy-via-staging so + // a half-written destination cannot appear on disk. Legacy is + // preserved as a backup. + use crate::tauri_app_data_migration::{ + current_tauri_app_data_dir, legacy_tauri_app_data_dir, + migrate_tauri_app_data_dir_with_paths, AppDataMigrationStatus, + }; + let t = std::time::Instant::now(); + if let (Some(legacy), Some(current)) = + (legacy_tauri_app_data_dir(), current_tauri_app_data_dir()) + { + match migrate_tauri_app_data_dir_with_paths(&legacy, ¤t) { + AppDataMigrationStatus::Migrated { old, new } => { + eprintln!( + "[lumotia-startup] migrated Tauri app_data_dir from legacy bundle \ + identifier: {old} -> {new} (elapsed_ms={ms})", + old = old.display(), + new = new.display(), + ms = t.elapsed().as_millis(), + ); + } + AppDataMigrationStatus::BothExistLegacyPreserved { old, new } => { + // Reachable on the second-or-later boot OR if the user + // ran a side-by-side install. Either is benign — we + // preserve legacy, use new — so log at INFO not WARN. + eprintln!( + "[lumotia-startup] Tauri app_data_dir present at new path; legacy \ + preserved: old={old} new={new}", + old = old.display(), + new = new.display(), + ); + } + AppDataMigrationStatus::NoLegacyFound => {} + } + } + + // 3. Ambiguity guard. After migrations, if more than one lumotia + // target candidate exists on disk (`~/.lumotia` AND + // `~/.local/share/lumotia` from split runs), refuse to start + // rather than silently pick one. The user must consolidate + // manually. + if let Err(amb) = check_target_ambiguity() { + eprintln!( + "[lumotia-startup] FATAL: ambiguous lumotia data directory — refusing to \ + start: {amb}" + ); + std::process::exit(1); + } +} + #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { + // Step 1: migrate legacy magnotia data BEFORE anything else touches + // the lumotia data dir. init_tracing, install_panic_hook, and + // tauri::Builder::default() all lazily create directories under + // app_data_dir on first use; if any of those run before migration, + // every migrate_one() probe returns TargetAlreadyExists / both-exist + // and the legacy data is silently orphaned. + migrate_user_data_pre_runtime(); + + // Step 2: structured logging on the (now correctly migrated) logs dir. init_tracing(); #[cfg(target_os = "linux")] @@ -345,117 +473,14 @@ pub fn run() { builder .setup(|app| { - // Tauri 2 keys both `app_data_dir` and the webview's data - // store (localStorage, IndexedDB, cookies, service worker - // storage, cache) plus all Tauri plugin state files - // (window-state geometry, autostart enable flag) by the - // bundle identifier. Commit `14313cf` renamed the bundle - // from `uk.co.corbel.magnotia` to `consulting.corbel.lumotia`, - // which would silently orphan every existing user's - // webview state under the new identifier. Migrate the - // legacy identifier-keyed app_data_dir first thing in - // setup, before the webview loads its first page and - // touches the on-disk store. Failure is logged but does - // NOT block startup: the hand-rolled `paths.rs` migration - // below covers the user's transcripts and models, so a - // total failure here only loses webview-keyed state. - // - // This is a separate concern from the - // `~/.local/share/magnotia` → `~/.local/share/lumotia` - // migration just below — that one lives under a - // hand-rolled path the app picks itself; this one lives - // under Tauri's identifier-keyed convention. - let t_app_data = Instant::now(); - match app.path().app_data_dir() { - Ok(new_app_data) => { - use crate::tauri_app_data_migration::{ - legacy_tauri_app_data_dir, migrate_tauri_app_data_dir_with_paths, - AppDataMigrationStatus, - }; - if let Some(legacy) = legacy_tauri_app_data_dir() { - match migrate_tauri_app_data_dir_with_paths(&legacy, &new_app_data) { - AppDataMigrationStatus::Migrated { old, new } => { - tracing::info!( - target: "lumotia_startup", - elapsed_ms = t_app_data.elapsed().as_millis(), - old = %old.display(), - new = %new.display(), - "migrated Tauri app_data_dir from legacy bundle identifier; legacy preserved as backup" - ); - } - AppDataMigrationStatus::BothExistLegacyPreserved { old, new } => { - tracing::warn!( - target: "lumotia_startup", - old = %old.display(), - new = %new.display(), - "both legacy and new Tauri app_data_dir exist; using new, legacy preserved" - ); - } - AppDataMigrationStatus::NoLegacyFound => {} - } - } - } - Err(e) => { - tracing::warn!( - target: "lumotia_startup", - error = %e, - "could not resolve Tauri app_data_dir; skipping bundle-identifier migration" - ); - } - } - - // One-shot legacy data-dir migration: rename ~/.local/share/magnotia - // (and macOS/Windows equivalents) to the convention-preserving - // lumotia path on first launch after the rebrand. Idempotent — - // safe to call on every boot. A migration error is fatal: silently - // continuing past it would orphan the user's transcripts and - // settings behind a fresh empty lumotia dir. - let t_migrate = Instant::now(); - match migrate_legacy_data_dir() { - Ok(statuses) => { - // Drive every legacy candidate independently: on Linux a - // user may have both `~/.magnotia` and - // `~/.local/share/magnotia`, and migrating only one - // would orphan the other forever. - for status in &statuses { - match status { - MigrationStatus::Migrated { from, to, renamed_db } => { - tracing::info!( - target: "lumotia_startup", - elapsed_ms = t_migrate.elapsed().as_millis(), - from = %from.display(), - to = %to.display(), - renamed_db = *renamed_db, - "migrated legacy magnotia data dir to lumotia" - ); - } - MigrationStatus::TargetAlreadyExists { .. } => {} - MigrationStatus::NoLegacyFound => {} - } - } - } - Err(e) => { - tracing::error!( - target: "lumotia_startup", - error = %e, - "legacy data dir migration failed — refusing to start (would orphan user data)" - ); - return Err(Box::new(e) as Box); - } - } - - // After migration, refuse to start if more than one lumotia - // target candidate exists on disk (e.g. both `~/.lumotia` AND - // `~/.local/share/lumotia`). Silently picking one would point - // the app at the wrong half of a split data directory. - if let Err(amb) = check_target_ambiguity() { - tracing::error!( - target: "lumotia_startup", - error = %amb, - "ambiguous lumotia data directory — refusing to start" - ); - return Err(Box::new(amb) as Box); - } + // Data migrations + ambiguity guard ran in + // `migrate_user_data_pre_runtime()` BEFORE tauri::Builder + // was constructed — see the doc comment on that function + // for why setup-hook timing is too late. By the time we get + // here every app_data_dir path is the post-rebrand one and + // any legacy magnotia state has either been migrated or + // preserved as a backup. + let _ = app; // Initialise database and startup settings in one runtime entry. let db_path = database_path(); diff --git a/src-tauri/src/tauri_app_data_migration.rs b/src-tauri/src/tauri_app_data_migration.rs index 12f9ecb..836c80f 100644 --- a/src-tauri/src/tauri_app_data_migration.rs +++ b/src-tauri/src/tauri_app_data_migration.rs @@ -30,6 +30,14 @@ use lumotia_core::paths::copy_dir_recursive; /// (commit `14313cf`). pub const OLD_BUNDLE_ID: &str = "uk.co.corbel.magnotia"; +/// Bundle identifier in use post-rebrand. MUST match the `identifier` +/// field in `src-tauri/tauri.conf.json` — if those two ever drift, +/// the pre-runtime migration would copy data into a path Tauri never +/// looks at. There is no compile-time check for this invariant; +/// reviewer's job to catch a tauri.conf.json edit that doesn't update +/// this const. +pub const NEW_BUNDLE_ID: &str = "consulting.corbel.lumotia"; + /// Outcome of an `app_data_dir` migration attempt. Mostly informational /// for the setup-hook tracing layer; the function never aborts startup. #[derive(Debug, Clone, PartialEq, Eq)] @@ -47,9 +55,10 @@ pub enum AppDataMigrationStatus { Migrated { old: PathBuf, new: PathBuf }, } -/// Resolve the OLD Tauri `app_data_dir` from platform conventions. The -/// AppHandle no longer knows the legacy identifier, so this is -/// hand-rolled per platform. +/// Resolve the OLD Tauri `app_data_dir` from platform conventions. +/// Used by the pre-runtime migration; the AppHandle isn't available +/// yet at that stage AND the AppHandle is keyed by the NEW identifier +/// anyway, so it couldn't surface the legacy path even if we had it. /// /// Returns `None` when the environment variable required to anchor the /// path is missing or empty — `HOME` on Unix, `APPDATA` on Windows. In @@ -57,10 +66,28 @@ pub enum AppDataMigrationStatus { /// fallback (the new path also depends on the same env vars and would /// be equally unrooted). pub fn legacy_tauri_app_data_dir() -> Option { - legacy_tauri_app_data_dir_for(OLD_BUNDLE_ID) + tauri_app_data_dir_for(OLD_BUNDLE_ID) } -fn legacy_tauri_app_data_dir_for(identifier: &str) -> Option { +/// Resolve the CURRENT (post-rebrand) Tauri `app_data_dir` from +/// platform conventions. Used by the pre-runtime migration as the +/// destination for the copy — pre-runtime means we run before +/// `tauri::Builder::default()` so we can't call `app.path().app_data_dir()`. +/// +/// CRITICAL: this resolver MUST agree with Tauri 2's own resolution at +/// runtime, otherwise the migration copies data into a path Tauri never +/// reads. Both sides use the same XDG / Library / APPDATA conventions +/// keyed by the bundle identifier, so they agree by construction as +/// long as `NEW_BUNDLE_ID` matches `tauri.conf.json#identifier`. +pub fn current_tauri_app_data_dir() -> Option { + tauri_app_data_dir_for(NEW_BUNDLE_ID) +} + +/// Platform-aware resolver shared by [`legacy_tauri_app_data_dir`] and +/// [`current_tauri_app_data_dir`]. Public so integration tests in +/// sibling crates can substitute an arbitrary identifier — production +/// callers should prefer the two wrappers above. +pub fn tauri_app_data_dir_for(identifier: &str) -> Option { // Exactly one of the four cfg blocks below is present per target // compile. Each is a tail expression that becomes the function's // return value. Avoiding explicit `return` keeps clippy's