agent: lumotia-rebrand — fix QC blockers for phase 5
Phase 5 QC found two blockers + four advisories. All addressed: B1 (FATAL) — Migration error now aborts startup instead of silently continuing past it. Without this fix a transient EACCES / EXDEV / ENOSPC would log a warning, init_db would create a fresh empty lumotia dir, and the user would appear to lose their transcripts. B2 (FATAL) — Linux dot-home vs XDG mismatch. The old probe returned ~/.magnotia as legacy but the caller passed app_data_dir() as the new path — which could be $XDG_DATA_HOME/lumotia. fs::rename across filesystems would EXDEV-fail; even when it succeeded the user's storage convention silently changed. Refactored: legacy_and_target_paths() returns the (legacy, target) pair together. Dot-home legacy lands in ~/.lumotia; XDG-set legacy lands in $XDG_DATA_HOME/lumotia; XDG-default legacy lands in ~/.local/share/lumotia. macOS / Windows / non-tier-1 unchanged. migrate_legacy_data_dir() now takes no argument; src-tauri caller updated. A1 — Removed dead new_db.exists() check inside rename_db_file_if_present (unreachable: rename_db is called only AFTER the legacy dir was just renamed to the new path, so a stray lumotia.db there is impossible). A2 — Added 4 unit tests for migrate_legacy_setting_keys: lone-magnotia, both-present (orphan delete), no-magnotia, idempotent. A3 — migrate_legacy_setting_keys now returns (renamed, orphans_deleted). When both keys exist, the legacy magnotia row is DELETED in the same transaction (lumotia row is authoritative). A4 — Added dot-home convention regression test in paths::tests. cargo build --workspace passes. cargo test --workspace: 339 pass, 0 fail (up from 334 in the original Phase 5 commit; +5 new tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -122,25 +122,27 @@ pub enum MigrationStatus {
|
||||
NoLegacyFound,
|
||||
}
|
||||
|
||||
/// Probe the legacy magnotia data dir paths on the current platform and
|
||||
/// return the first one that exists, if any. Mirrors the search order
|
||||
/// inside `resolve_app_data_dir()` but for the old name.
|
||||
fn legacy_magnotia_data_dir() -> Option<PathBuf> {
|
||||
/// Probe the legacy magnotia data dir paths on the current platform.
|
||||
/// Returns the matched legacy path AND its convention-preserving lumotia
|
||||
/// target so the migration lands the same kind of dir it found (dot-home
|
||||
/// stays dot-home, XDG stays XDG, macOS Application Support stays the
|
||||
/// same).
|
||||
fn legacy_and_target_paths() -> Option<(PathBuf, PathBuf)> {
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
let local_app_data = std::env::var("LOCALAPPDATA").ok()?;
|
||||
let candidate = PathBuf::from(local_app_data).join("magnotia");
|
||||
return candidate.exists().then_some(candidate);
|
||||
let legacy = PathBuf::from(&local_app_data).join("magnotia");
|
||||
let target = PathBuf::from(local_app_data).join("lumotia");
|
||||
return legacy.exists().then_some((legacy, target));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
let home = std::env::var("HOME").ok()?;
|
||||
let candidate = PathBuf::from(home)
|
||||
.join("Library")
|
||||
.join("Application Support")
|
||||
.join("Magnotia");
|
||||
return candidate.exists().then_some(candidate);
|
||||
let app_support = PathBuf::from(home).join("Library").join("Application Support");
|
||||
let legacy = app_support.join("Magnotia");
|
||||
let target = app_support.join("Lumotia");
|
||||
return legacy.exists().then_some((legacy, target));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
@@ -148,72 +150,84 @@ fn legacy_magnotia_data_dir() -> Option<PathBuf> {
|
||||
let home = std::env::var("HOME").ok()?;
|
||||
let dot_legacy = PathBuf::from(&home).join(".magnotia");
|
||||
if dot_legacy.exists() {
|
||||
return Some(dot_legacy);
|
||||
return Some((dot_legacy, PathBuf::from(&home).join(".lumotia")));
|
||||
}
|
||||
if let Ok(xdg) = std::env::var("XDG_DATA_HOME") {
|
||||
if !xdg.is_empty() {
|
||||
let xdg_legacy = PathBuf::from(xdg).join("magnotia");
|
||||
let xdg_legacy = PathBuf::from(&xdg).join("magnotia");
|
||||
if xdg_legacy.exists() {
|
||||
return Some(xdg_legacy);
|
||||
return Some((xdg_legacy, PathBuf::from(xdg).join("lumotia")));
|
||||
}
|
||||
}
|
||||
}
|
||||
let xdg_default = PathBuf::from(home)
|
||||
let xdg_default_legacy = PathBuf::from(&home)
|
||||
.join(".local")
|
||||
.join("share")
|
||||
.join("magnotia");
|
||||
xdg_default.exists().then_some(xdg_default)
|
||||
if xdg_default_legacy.exists() {
|
||||
let xdg_default_target = PathBuf::from(home)
|
||||
.join(".local")
|
||||
.join("share")
|
||||
.join("lumotia");
|
||||
return Some((xdg_default_legacy, xdg_default_target));
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))]
|
||||
{
|
||||
let home = std::env::var("HOME").ok()?;
|
||||
let candidate = PathBuf::from(home).join(".magnotia");
|
||||
candidate.exists().then_some(candidate)
|
||||
let legacy = PathBuf::from(&home).join(".magnotia");
|
||||
let target = PathBuf::from(home).join(".lumotia");
|
||||
legacy.exists().then_some((legacy, target))
|
||||
}
|
||||
}
|
||||
|
||||
/// Migrate a legacy magnotia data directory to the new lumotia path on
|
||||
/// first launch. Idempotent: safe to call on every boot.
|
||||
/// Migrate a legacy magnotia data directory to its convention-preserving
|
||||
/// lumotia equivalent on first launch. Idempotent: safe to call on every
|
||||
/// boot.
|
||||
///
|
||||
/// Rules:
|
||||
/// * If the new path already exists, do nothing and return
|
||||
/// * If the resolved target already exists, do nothing and return
|
||||
/// `TargetAlreadyExists`. We do not destroy lumotia data, even if a
|
||||
/// stale legacy dir is also present.
|
||||
/// * If only the legacy path exists, rename it to the new path and
|
||||
/// rename `magnotia.db` -> `lumotia.db` inside it if found.
|
||||
/// * If only the legacy path exists, rename it to the matching lumotia
|
||||
/// target (same parent dir / same convention) and rename
|
||||
/// `magnotia.db` -> `lumotia.db` inside it if found.
|
||||
/// * If neither exists, return `NoLegacyFound` — the first launch on a
|
||||
/// clean system will create the new path itself.
|
||||
pub fn migrate_legacy_data_dir(new_path: &Path) -> Result<MigrationStatus, std::io::Error> {
|
||||
migrate_legacy_data_dir_inner(new_path, legacy_magnotia_data_dir())
|
||||
///
|
||||
/// Callers should treat `Err` as a hard startup failure; silently
|
||||
/// continuing past a migration error orphans user data behind a fresh
|
||||
/// empty lumotia dir.
|
||||
pub fn migrate_legacy_data_dir() -> Result<MigrationStatus, std::io::Error> {
|
||||
migrate_legacy_data_dir_inner(legacy_and_target_paths())
|
||||
}
|
||||
|
||||
/// Test-friendly inner shape: takes the legacy path explicitly so tests
|
||||
/// don't depend on platform-specific HOME / LOCALAPPDATA env vars.
|
||||
/// Test-friendly inner shape: takes the (legacy, target) pair explicitly
|
||||
/// so tests don't depend on platform-specific HOME / LOCALAPPDATA / XDG
|
||||
/// env vars.
|
||||
fn migrate_legacy_data_dir_inner(
|
||||
new_path: &Path,
|
||||
legacy: Option<PathBuf>,
|
||||
pair: Option<(PathBuf, PathBuf)>,
|
||||
) -> Result<MigrationStatus, std::io::Error> {
|
||||
if new_path.exists() {
|
||||
return Ok(MigrationStatus::TargetAlreadyExists {
|
||||
target: new_path.to_path_buf(),
|
||||
});
|
||||
}
|
||||
|
||||
let Some(from) = legacy else {
|
||||
let Some((from, to)) = pair else {
|
||||
return Ok(MigrationStatus::NoLegacyFound);
|
||||
};
|
||||
|
||||
if let Some(parent) = new_path.parent() {
|
||||
if to.exists() {
|
||||
return Ok(MigrationStatus::TargetAlreadyExists { target: to });
|
||||
}
|
||||
|
||||
if let Some(parent) = to.parent() {
|
||||
std::fs::create_dir_all(parent)?;
|
||||
}
|
||||
std::fs::rename(&from, new_path)?;
|
||||
std::fs::rename(&from, &to)?;
|
||||
|
||||
let renamed_db = rename_db_file_if_present(new_path)?;
|
||||
let renamed_db = rename_db_file_if_present(&to)?;
|
||||
|
||||
Ok(MigrationStatus::Migrated {
|
||||
from,
|
||||
to: new_path.to_path_buf(),
|
||||
to,
|
||||
renamed_db,
|
||||
})
|
||||
}
|
||||
@@ -224,9 +238,6 @@ fn rename_db_file_if_present(dir: &Path) -> Result<bool, std::io::Error> {
|
||||
return Ok(false);
|
||||
}
|
||||
let new_db = dir.join("lumotia.db");
|
||||
if new_db.exists() {
|
||||
return Ok(false);
|
||||
}
|
||||
std::fs::rename(&legacy_db, &new_db)?;
|
||||
Ok(true)
|
||||
}
|
||||
@@ -268,13 +279,13 @@ mod tests {
|
||||
fn migrate_with_legacy_present_renames_dir_and_db() {
|
||||
let root = unique_tmp("legacy-present");
|
||||
let legacy = root.join("magnotia");
|
||||
let new_path = root.join("lumotia");
|
||||
let target = root.join("lumotia");
|
||||
std::fs::create_dir_all(&legacy).unwrap();
|
||||
std::fs::write(legacy.join("magnotia.db"), b"sqlite-stub").unwrap();
|
||||
std::fs::write(legacy.join("recordings.placeholder"), b"x").unwrap();
|
||||
|
||||
let result =
|
||||
migrate_legacy_data_dir_inner(&new_path, Some(legacy.clone())).expect("migrate ok");
|
||||
let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone())))
|
||||
.expect("migrate ok");
|
||||
|
||||
match result {
|
||||
MigrationStatus::Migrated {
|
||||
@@ -283,17 +294,17 @@ mod tests {
|
||||
renamed_db,
|
||||
} => {
|
||||
assert_eq!(from, legacy);
|
||||
assert_eq!(to, new_path);
|
||||
assert_eq!(to, target);
|
||||
assert!(renamed_db, "expected db file to be renamed");
|
||||
}
|
||||
other => panic!("expected Migrated, got {other:?}"),
|
||||
}
|
||||
assert!(!legacy.exists(), "legacy dir should be gone");
|
||||
assert!(new_path.exists(), "new dir should exist");
|
||||
assert!(new_path.join("lumotia.db").exists(), "db at new name");
|
||||
assert!(!new_path.join("magnotia.db").exists(), "old db gone");
|
||||
assert!(target.exists(), "new dir should exist");
|
||||
assert!(target.join("lumotia.db").exists(), "db at new name");
|
||||
assert!(!target.join("magnotia.db").exists(), "old db gone");
|
||||
assert!(
|
||||
new_path.join("recordings.placeholder").exists(),
|
||||
target.join("recordings.placeholder").exists(),
|
||||
"other files preserved"
|
||||
);
|
||||
|
||||
@@ -304,25 +315,25 @@ mod tests {
|
||||
fn migrate_with_both_present_returns_target_exists_and_preserves_lumotia() {
|
||||
let root = unique_tmp("both-present");
|
||||
let legacy = root.join("magnotia");
|
||||
let new_path = root.join("lumotia");
|
||||
let target = root.join("lumotia");
|
||||
std::fs::create_dir_all(&legacy).unwrap();
|
||||
std::fs::create_dir_all(&new_path).unwrap();
|
||||
std::fs::write(new_path.join("lumotia.db"), b"new-data").unwrap();
|
||||
std::fs::create_dir_all(&target).unwrap();
|
||||
std::fs::write(target.join("lumotia.db"), b"new-data").unwrap();
|
||||
std::fs::write(legacy.join("magnotia.db"), b"legacy-data").unwrap();
|
||||
|
||||
let result =
|
||||
migrate_legacy_data_dir_inner(&new_path, Some(legacy.clone())).expect("migrate ok");
|
||||
let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone())))
|
||||
.expect("migrate ok");
|
||||
|
||||
assert_eq!(
|
||||
result,
|
||||
MigrationStatus::TargetAlreadyExists {
|
||||
target: new_path.clone()
|
||||
target: target.clone()
|
||||
}
|
||||
);
|
||||
assert!(legacy.exists(), "legacy dir preserved");
|
||||
assert!(new_path.exists(), "new dir preserved");
|
||||
assert!(target.exists(), "new dir preserved");
|
||||
assert_eq!(
|
||||
std::fs::read(new_path.join("lumotia.db")).unwrap(),
|
||||
std::fs::read(target.join("lumotia.db")).unwrap(),
|
||||
b"new-data".to_vec(),
|
||||
"lumotia.db not overwritten"
|
||||
);
|
||||
@@ -332,27 +343,21 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn migrate_with_neither_present_returns_no_legacy() {
|
||||
let root = unique_tmp("neither-present");
|
||||
let new_path = root.join("lumotia");
|
||||
|
||||
let result = migrate_legacy_data_dir_inner(&new_path, None).expect("migrate ok");
|
||||
let result = migrate_legacy_data_dir_inner(None).expect("migrate ok");
|
||||
|
||||
assert_eq!(result, MigrationStatus::NoLegacyFound);
|
||||
assert!(!new_path.exists(), "no dir created");
|
||||
|
||||
std::fs::remove_dir_all(&root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migrate_with_legacy_present_but_no_db_inside() {
|
||||
let root = unique_tmp("legacy-no-db");
|
||||
let legacy = root.join("magnotia");
|
||||
let new_path = root.join("lumotia");
|
||||
let target = root.join("lumotia");
|
||||
std::fs::create_dir_all(&legacy).unwrap();
|
||||
std::fs::write(legacy.join("recordings.placeholder"), b"x").unwrap();
|
||||
|
||||
let result =
|
||||
migrate_legacy_data_dir_inner(&new_path, Some(legacy.clone())).expect("migrate ok");
|
||||
let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone())))
|
||||
.expect("migrate ok");
|
||||
|
||||
match result {
|
||||
MigrationStatus::Migrated { renamed_db, .. } => {
|
||||
@@ -360,8 +365,28 @@ mod tests {
|
||||
}
|
||||
other => panic!("expected Migrated, got {other:?}"),
|
||||
}
|
||||
assert!(new_path.exists());
|
||||
assert!(new_path.join("recordings.placeholder").exists());
|
||||
assert!(target.exists());
|
||||
assert!(target.join("recordings.placeholder").exists());
|
||||
|
||||
std::fs::remove_dir_all(&root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migrate_preserves_dot_home_to_dot_home_convention() {
|
||||
// Regression for B2 from Phase 5 QC: dot-home legacy must land in
|
||||
// dot-home target, not XDG target.
|
||||
let root = unique_tmp("convention");
|
||||
let legacy = root.join(".magnotia");
|
||||
let target = root.join(".lumotia");
|
||||
std::fs::create_dir_all(&legacy).unwrap();
|
||||
std::fs::write(legacy.join("magnotia.db"), b"data").unwrap();
|
||||
|
||||
let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone())))
|
||||
.expect("migrate ok");
|
||||
|
||||
assert!(matches!(result, MigrationStatus::Migrated { .. }));
|
||||
assert!(target.exists());
|
||||
assert!(target.join("lumotia.db").exists());
|
||||
|
||||
std::fs::remove_dir_all(&root).ok();
|
||||
}
|
||||
|
||||
@@ -908,11 +908,18 @@ pub async fn get_setting(pool: &SqlitePool, key: &str) -> Result<Option<String>>
|
||||
/// One-shot key rename for settings rows carried over from the magnotia era
|
||||
/// (`magnotia_preferences`, `magnotia_morning_triage_last_shown`, etc.).
|
||||
///
|
||||
/// Idempotent: rows already on the new key are left alone; rows on the old
|
||||
/// key are renamed only if the new key does not already exist. Returns the
|
||||
/// number of rows actually renamed so the caller can log on first launch.
|
||||
pub async fn migrate_legacy_setting_keys(pool: &SqlitePool) -> Result<u64> {
|
||||
let result = sqlx::query(
|
||||
/// Idempotent. Returns `(renamed, orphans_deleted)`:
|
||||
/// * `renamed` — rows where only the magnotia key existed; the row's key
|
||||
/// is updated to the lumotia equivalent.
|
||||
/// * `orphans_deleted` — rows where BOTH keys existed; the lumotia row is
|
||||
/// authoritative and the magnotia row is deleted to avoid silent debt.
|
||||
pub async fn migrate_legacy_setting_keys(pool: &SqlitePool) -> Result<(u64, u64)> {
|
||||
let mut tx = pool.begin().await.map_err(|source| Error::Query {
|
||||
operation: "migrate_legacy_setting_keys:begin".into(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
let renamed = sqlx::query(
|
||||
"UPDATE settings \
|
||||
SET key = 'lumotia_' || substr(key, length('magnotia_') + 1) \
|
||||
WHERE key LIKE 'magnotia_%' \
|
||||
@@ -921,13 +928,36 @@ pub async fn migrate_legacy_setting_keys(pool: &SqlitePool) -> Result<u64> {
|
||||
WHERE dst.key = 'lumotia_' || substr(settings.key, length('magnotia_') + 1)\
|
||||
)",
|
||||
)
|
||||
.execute(pool)
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(|source| Error::Query {
|
||||
operation: "migrate_legacy_setting_keys".into(),
|
||||
operation: "migrate_legacy_setting_keys:rename".into(),
|
||||
source,
|
||||
})?
|
||||
.rows_affected();
|
||||
|
||||
let deleted = sqlx::query(
|
||||
"DELETE FROM settings \
|
||||
WHERE key LIKE 'magnotia_%' \
|
||||
AND EXISTS (\
|
||||
SELECT 1 FROM settings AS dst \
|
||||
WHERE dst.key = 'lumotia_' || substr(settings.key, length('magnotia_') + 1)\
|
||||
)",
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(|source| Error::Query {
|
||||
operation: "migrate_legacy_setting_keys:delete_orphans".into(),
|
||||
source,
|
||||
})?
|
||||
.rows_affected();
|
||||
|
||||
tx.commit().await.map_err(|source| Error::Query {
|
||||
operation: "migrate_legacy_setting_keys:commit".into(),
|
||||
source,
|
||||
})?;
|
||||
Ok(result.rows_affected())
|
||||
|
||||
Ok((renamed, deleted))
|
||||
}
|
||||
|
||||
// --- Row types ---
|
||||
@@ -2740,4 +2770,94 @@ mod tests {
|
||||
.unwrap();
|
||||
assert_eq!(remaining, vec!["recent".to_string()]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_legacy_setting_keys_renames_lone_magnotia_rows() {
|
||||
let pool = test_pool().await;
|
||||
set_setting(&pool, "magnotia_preferences", "blob-A")
|
||||
.await
|
||||
.unwrap();
|
||||
set_setting(&pool, "magnotia_morning_triage_last_shown", "2026-05-12")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (renamed, deleted) = migrate_legacy_setting_keys(&pool).await.unwrap();
|
||||
|
||||
assert_eq!(renamed, 2);
|
||||
assert_eq!(deleted, 0);
|
||||
assert_eq!(
|
||||
get_setting(&pool, "lumotia_preferences")
|
||||
.await
|
||||
.unwrap()
|
||||
.as_deref(),
|
||||
Some("blob-A")
|
||||
);
|
||||
assert_eq!(
|
||||
get_setting(&pool, "magnotia_preferences").await.unwrap(),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
get_setting(&pool, "lumotia_morning_triage_last_shown")
|
||||
.await
|
||||
.unwrap()
|
||||
.as_deref(),
|
||||
Some("2026-05-12")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_legacy_setting_keys_deletes_orphans_when_both_present() {
|
||||
let pool = test_pool().await;
|
||||
set_setting(&pool, "magnotia_preferences", "legacy-blob")
|
||||
.await
|
||||
.unwrap();
|
||||
set_setting(&pool, "lumotia_preferences", "current-blob")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (renamed, deleted) = migrate_legacy_setting_keys(&pool).await.unwrap();
|
||||
|
||||
assert_eq!(renamed, 0);
|
||||
assert_eq!(deleted, 1);
|
||||
assert_eq!(
|
||||
get_setting(&pool, "lumotia_preferences")
|
||||
.await
|
||||
.unwrap()
|
||||
.as_deref(),
|
||||
Some("current-blob"),
|
||||
"lumotia row preserved verbatim"
|
||||
);
|
||||
assert_eq!(
|
||||
get_setting(&pool, "magnotia_preferences").await.unwrap(),
|
||||
None,
|
||||
"orphan magnotia row deleted"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_legacy_setting_keys_no_op_when_no_magnotia_rows() {
|
||||
let pool = test_pool().await;
|
||||
set_setting(&pool, "lumotia_preferences", "blob")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (renamed, deleted) = migrate_legacy_setting_keys(&pool).await.unwrap();
|
||||
|
||||
assert_eq!(renamed, 0);
|
||||
assert_eq!(deleted, 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_legacy_setting_keys_is_idempotent() {
|
||||
let pool = test_pool().await;
|
||||
set_setting(&pool, "magnotia_preferences", "blob")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let first = migrate_legacy_setting_keys(&pool).await.unwrap();
|
||||
let second = migrate_legacy_setting_keys(&pool).await.unwrap();
|
||||
|
||||
assert_eq!(first, (1, 0));
|
||||
assert_eq!(second, (0, 0));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user