diff --git a/CHANGELOG.md b/CHANGELOG.md index e70e370555..99f69ebe4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -599,6 +599,16 @@ close it, and the reason is in the server's and the computer's logs. Clicking or focusing the desktop window while it downloaded local software showed a spinning cursor until the download finished. The window now comes forward at once and keeps the setup screen. +### A reinstall no longer picks up the previous installation's Bot files and sign-ins + +A desktop installation set up again after its folder was deleted reused whatever the previous one +left in the container engine: Bot workspace files, and each Bot's browser profile with its website +sign-ins, under the same Bot ids. Only the database was checked. Start now finds every volume and +stopped container the previous installation left and offers Start fresh, which deletes exactly those +and then starts; nothing is reused without asking. To keep the data, restore the previous folder +instead. A container from it that is still running stops the deletion until it is stopped. An +installation with its saved key keeps its data as before. + ### A new desktop install starts its server again A desktop install of v0.2.0 stopped at start with `server stopped straight away (exit status: 1)` and diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 2d654af943..dff5e434be 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -13,9 +13,10 @@ mod test_support; use openbot_desktop_lib::{ acquire, deployment, deployment_release, engine, env as openbot_env, harness, host_access, - install, local_browser, preparation, problem::Problem, provider, pull_metrics, quiet, - recorder_external, recorder_host, recorder_install, stack, supervise, telemetry, tray, - windows as win, + install, local_browser, preparation, + problem::{LeftoverReset, Problem}, + provider, pull_metrics, quiet, recorder_external, recorder_host, recorder_install, stack, + supervise, telemetry, tray, windows as win, }; const QUIT_CLEANUP_NOTICE_FILE: &str = ".openbot-quit-cleanup-notice"; @@ -58,7 +59,7 @@ struct Shell { last_failure: Mutex>, /// Reading the notification must not make a partially running deployment adoptable again. recovery_required: Mutex>, - /// An explicit reset remains bound to the engine that found the leftover database. + /// An explicit reset remains bound to the engine and the exact volumes it offered. leftover_database: Mutex>, selected_root: Mutex>, root: Mutex>, @@ -101,9 +102,13 @@ struct ContainerDeployment { address: engine::Address, } +#[derive(Clone)] struct LeftoverDatabase { root: PathBuf, - volume: String, + /// Sorted, exactly as offered: the only set a confirmation may remove. + volumes: Vec, + /// Sorted names of the stopped containers removed before those volumes. + containers: Vec, address: engine::Address, } @@ -1107,46 +1112,113 @@ fn require_existing_encryption_key( Ok(()) } +/** +Refuse a Start that would mint a key for, or reattach, a previous installation's data. + +A deleted installation folder takes `.env` and the vault with it, but the engine keeps every +volume. Without this, a new installation reattaches the previous one's Bots' workspace files and +browser profiles by name, and only the database was ever checked. An installation with its usable +key is a returning one and is never probed: it keeps its data. +*/ fn require_existing_encryption_key_with_recovery( root: &Path, secrets: &stack::Secrets, existing_postgres_volume: impl FnOnce() -> Result, - resettable_volume: impl FnOnce() -> Result, Problem>, + leftovers: impl FnOnce() -> Result, ) -> Result<(), Problem> { + if secrets + .get("KEY_ENCRYPTION_KEY") + .is_some_and(|value| openbot_env::usable_encryption_key(value)) + { + return Ok(()); + } let mut volume_exists = false; - require_existing_encryption_key(root, secrets, || { + let guarded = require_existing_encryption_key(root, secrets, || { volume_exists = existing_postgres_volume()?; Ok(volume_exists) - }) - .map_err(|mut problem| { + }); + let mut problem = match guarded { // A failed probe is not proof of an existing volume. Configured roots never reach here. - if volume_exists { - let recovery = fresh_root_without_encryption_key(root, secrets).and_then(|fresh| { - if fresh { - resettable_volume() - } else { - Ok(None) + Err(problem) if !volume_exists => return Err(problem), + Err(problem) => problem, + Ok(()) => { + // No database, so a key would be minted. Anything else left behind would be reused. + let found = leftovers()?; + if found.volumes.is_empty() { + return Ok(()); + } + let mut problem = Problem::plain("OpenBot found data from a previous installation, such as Bots' files and browser sign-ins. OpenBot will not reuse it in a new installation. Restore the previous installation's folder to keep it, or choose Start fresh to delete it."); + match fresh_root_without_encryption_key(root, secrets) { + Ok(true) if found.resettable() => { + problem.leftover = Some(Box::new(LeftoverReset { + containers: found.container_names(), + volumes: found.volumes, + })); } - }); - match recovery { - Ok(volume) => { - if volume.is_some() { - problem.said = "OpenBot found a database from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or reset the leftover database to start fresh.".into(); + Ok(true) => { + if let Some(running) = found.running_said() { + problem.said = format!("{} {running}", problem.said); } - problem.database_reset = volume; - } - Err(verification) => { - problem.detail = Some(match verification.detail { - Some(detail) => format!("{}\n{detail}", verification.said), - None => verification.said, - }); } + Ok(false) => {} + Err(verification) => problem.detail = Some(verification_detail(verification)), + } + return Err(problem); + } + }; + let recovery = fresh_root_without_encryption_key(root, secrets).and_then(|fresh| { + if fresh { + leftovers().map(Some) + } else { + Ok(None) + } + }); + match recovery { + Ok(Some(found)) if found.database.is_some() && found.resettable() => { + problem.said = if found.volumes.len() > 1 { + "OpenBot found a database and other data from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or choose Start fresh to delete it." + } else { + "OpenBot found a database from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or choose Start fresh to delete it." } + .into(); + problem.leftover = Some(Box::new(LeftoverReset { + containers: found.container_names(), + volumes: found.volumes, + })); + problem.database_reset = found.database; } - problem + Ok(Some(found)) if found.database.is_some() => { + if let Some(running) = found.running_said() { + problem.said = format!("{} {running}", problem.said); + } + } + Ok(_) => {} + Err(verification) => problem.detail = Some(verification_detail(verification)), + } + Err(problem) +} + +/// The reset a refused Start offered, bound to the exact containers and volumes it named. +fn leftover_offer( + problem: &Problem, + root: &Path, + address: &engine::Address, +) -> Option { + (!problem.leftover_volumes().is_empty()).then(|| LeftoverDatabase { + root: root.to_path_buf(), + volumes: problem.leftover_volumes().to_vec(), + containers: problem.leftover_containers().to_vec(), + address: address.clone(), }) } +fn verification_detail(verification: Problem) -> String { + match verification.detail { + Some(detail) => format!("{}\n{detail}", verification.said), + None => verification.said, + } +} + /// Destructive recovery needs positive evidence that root metadata is readable and unconfigured. /// The ordinary startup guard keeps its existing behavior when metadata is unknown. fn fresh_root_without_encryption_key( @@ -1160,7 +1232,7 @@ fn fresh_root_without_encryption_key( return Ok(false); } let unknown = || { - Problem::plain("OpenBot could not verify that this is an unconfigured installation. Its leftover database cannot be reset here.") + Problem::plain("OpenBot could not verify that this is an unconfigured installation. Its leftover data cannot be reset here.") }; let settings = openbot_env::read_already_set(&root.join(".env"), &["DATABASE_URL"]) .map_err(|_| unknown())?; @@ -1186,13 +1258,16 @@ fn fresh_root_without_encryption_key( fn reset_leftover_database_with( shell: &Shell, root: &Path, - volume: &str, + volumes: &[String], + containers: &[String], confirmed: bool, read_secrets: impl FnOnce() -> Result, - reset: impl FnOnce(&engine::Address, &stack::Secrets) -> Result<(), Problem>, + reset: impl FnOnce(&LeftoverDatabase, &stack::Secrets) -> Result<(), Problem>, ) -> Result<(), Problem> { if !confirmed { - return Err(Problem::plain("Confirm that you want to permanently delete the leftover database before resetting it.")); + return Err(Problem::plain( + "Confirm that you want to permanently delete the leftover data before resetting it.", + )); } let attempt = StartAttempt::begin(shell)?; let _startup = attempt.lock_current()?; @@ -1200,7 +1275,7 @@ fn reset_leftover_database_with( || shell.root.lock().unwrap().is_some() || !shell.children.lock().unwrap().is_empty() { - return Err(Problem::plain("OpenBot still owns running services. Choose Stop OpenBot before resetting a leftover database.")); + return Err(Problem::plain("OpenBot still owns running services. Choose Stop OpenBot before resetting leftover data.")); } if shell .selected_root @@ -1209,17 +1284,31 @@ fn reset_leftover_database_with( .as_ref() .is_some_and(|selected| selected != root) { - return Err(Problem::plain("The selected installation changed. Try Start again before resetting its leftover database.")); + return Err(Problem::plain( + "The selected installation changed. Try Start again.", + )); } - let address = shell.leftover_database.lock().unwrap().as_ref() - .filter(|offer| offer.root == root && offer.volume == volume) - .map(|offer| offer.address.clone()) - .ok_or_else(|| Problem::plain("The leftover database reset offer is no longer current. Try Start again before confirming a reset."))?; + let sorted = |names: &[String]| { + let mut names = names.to_vec(); + names.sort(); + names + }; + let (volumes, containers) = (sorted(volumes), sorted(containers)); + // The removal receives the offer itself, so it can only remove the set this check matched. + let offer = shell.leftover_database.lock().unwrap().as_ref() + .filter(|offer| { + offer.root == root + && !volumes.is_empty() + && offer.volumes == volumes + && offer.containers == containers + }) + .cloned() + .ok_or_else(|| Problem::plain("The leftover data changed after it was shown. Try Start again to see what is left now."))?; let secrets = read_secrets()?; if !fresh_root_without_encryption_key(root, &secrets)? { - return Err(Problem::plain("This installation is already configured or has its original encryption key. Its database cannot be reset here.")); + return Err(Problem::plain("This installation is already configured or has its original encryption key. Its data cannot be reset here.")); } - reset(&address, &secrets)?; + reset(&offer, &secrets)?; *shell.leftover_database.lock().unwrap() = None; Ok(()) } @@ -1227,7 +1316,8 @@ fn reset_leftover_database_with( #[tauri::command] async fn reset_leftover_database( root: String, - volume: String, + volumes: Vec, + containers: Vec, confirmed: bool, app: tauri::AppHandle, ) -> Result<(), Problem> { @@ -1236,7 +1326,8 @@ async fn reset_leftover_database( reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &containers, confirmed, || { openbot_desktop_lib::vault::already_given_no_ui( @@ -1245,7 +1336,15 @@ async fn reset_leftover_database( &openbot_env::MINTED[..], ) }, - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume), + |offer, secrets| { + stack::reset_leftover_volumes( + &offer.address, + &root, + secrets, + &offer.volumes, + &offer.containers, + ) + }, ) } @@ -1435,18 +1534,10 @@ async fn start_stack_inner( &root, &existing_secrets, || stack::postgres_volume_exists(&found, &root, &existing_secrets), - || stack::leftover_database_volume(&found, &root, &existing_secrets), + || stack::leftover_volumes(&found, &root, &existing_secrets), ) .inspect_err(|problem| { - *shell.leftover_database.lock().unwrap() = - problem - .database_reset - .as_ref() - .map(|volume| LeftoverDatabase { - root: root.clone(), - volume: volume.clone(), - address: found.clone(), - }); + *shell.leftover_database.lock().unwrap() = leftover_offer(problem, &root, &found); })?; // Only this deployment's recorded hosts are reclaimed; its existing containers are reusable. @@ -5495,12 +5586,14 @@ mod tests { &root, &stack::Secrets::new(), || Ok(true), - || Ok(Some("openbot_postgres-data".into())), + || Ok(database_leftovers(&[])), ) .unwrap_err(); + let offered = serde_json::to_value(error).unwrap(); + assert_eq!(offered["database_reset"], "openbot_postgres-data"); assert_eq!( - serde_json::to_value(error).unwrap()["database_reset"], - "openbot_postgres-data" + offered["leftover_reset"], + serde_json::json!(["openbot_postgres-data"]) ); for (file, content) in [ (".env", "DATABASE_URL=postgres://fixture\n"), @@ -5525,10 +5618,9 @@ mod tests { || panic!("unknown or configured roots must not offer deletion"), ) .unwrap_err(); - assert!(serde_json::to_value(error) - .unwrap() - .get("database_reset") - .is_none()); + let refused = serde_json::to_value(error).unwrap(); + assert!(refused.get("database_reset").is_none()); + assert!(refused.get("leftover_reset").is_none()); std::fs::remove_file(root.join(file)).unwrap(); } let secrets = stack::Secrets::from([( @@ -5545,6 +5637,249 @@ mod tests { std::fs::remove_dir_all(root).unwrap(); } + fn database_leftovers(others: &[&str]) -> stack::LeftoverVolumes { + let mut volumes: Vec = others.iter().map(|name| name.to_string()).collect(); + volumes.push("openbot_postgres-data".into()); + volumes.sort(); + stack::LeftoverVolumes { + database: Some("openbot_postgres-data".into()), + database_exists: true, + volumes, + ..Default::default() + } + } + + fn leftovers_without_database(volumes: &[&str]) -> stack::LeftoverVolumes { + stack::LeftoverVolumes { + database: None, + database_exists: false, + volumes: volumes.iter().map(|name| name.to_string()).collect(), + ..Default::default() + } + } + + const BOT_LEFTOVERS: [&str; 3] = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + ]; + + /// Deleting the folder leaves Bots' files and sign-ins in the engine even with no database. + #[test] + fn a_fresh_root_refuses_to_reattach_any_leftover_volume() { + let root = temp_root("leftover-volumes-offer"); + std::fs::create_dir_all(&root).unwrap(); + + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(leftovers_without_database(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert!( + error.said.contains("previous installation"), + "{}", + error.said + ); + let offered = serde_json::to_value(error).unwrap(); + assert!(offered.get("database_reset").is_none()); + assert_eq!(offered["leftover_reset"], serde_json::json!(BOT_LEFTOVERS)); + + // Nothing left behind starts exactly as before. + assert!(require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(stack::LeftoverVolumes::default()), + ) + .is_ok()); + + // A probe that fails is not permission to reattach whatever is there. + assert!(require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Err(Problem::plain("inventory failed")), + ) + .is_err()); + + // A database joins the same confirmation as everything else it would be reset with. + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(true), + || Ok(database_leftovers(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert_eq!( + error.database_reset.as_deref(), + Some("openbot_postgres-data") + ); + assert_eq!( + error.leftover_volumes(), + database_leftovers(&BOT_LEFTOVERS).volumes + ); + + // A database OpenBot cannot verify keeps the existing refusal, with nothing offered. + let mut unverified = leftovers_without_database(&BOT_LEFTOVERS); + unverified.database_exists = true; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(true), + || Ok(unverified), + ) + .unwrap_err(); + assert_eq!(error.database_reset, None); + assert!(error.leftover_volumes().is_empty()); + assert!(error + .said + .contains("OpenBot will not replace the key automatically")); + + // Stopped containers that hold the volumes join the confirmation; running ones block it. + let mut held = leftovers_without_database(&BOT_LEFTOVERS); + held.containers = vec![stack::LeftoverContainer { + name: "openbot-computer-general-assistant".into(), + id: "c-computer".into(), + }]; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(held.clone()), + ) + .unwrap_err(); + assert_eq!(error.leftover_volumes(), BOT_LEFTOVERS); + assert_eq!( + error.leftover_containers(), + ["openbot-computer-general-assistant"] + ); + assert_eq!( + serde_json::to_value(&error).unwrap()["leftover_containers"], + serde_json::json!(["openbot-computer-general-assistant"]) + ); + held.running = vec!["openbot-agent-computer-1".into()]; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(held), + ) + .unwrap_err(); + assert!(error.leftover_volumes().is_empty() && error.leftover_containers().is_empty()); + assert!( + error.said.contains("openbot-agent-computer-1") && error.said.contains("still running"), + "{}", + error.said + ); + + // Unknown root metadata refuses Start but never offers deletion. + std::fs::write( + root.join(openbot_desktop_lib::saved_intent::FILE), + "invalid-settings-secret", + ) + .unwrap(); + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(leftovers_without_database(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert!(error.leftover_volumes().is_empty()); + assert!(!format!("{error:?}").contains("invalid-settings-secret")); + std::fs::remove_file(root.join(openbot_desktop_lib::saved_intent::FILE)).unwrap(); + + // A returning installation with its key keeps its data and is never probed. + let secrets = stack::Secrets::from([( + "KEY_ENCRYPTION_KEY".into(), + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=".into(), + )]); + std::fs::write(root.join(".env"), "DATABASE_URL=postgres://fixture\n").unwrap(); + assert!(require_existing_encryption_key_with_recovery( + &root, + &secrets, + || panic!("original key needs no probe"), + || panic!("original key keeps its volumes") + ) + .is_ok()); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_refused_start_offers_exactly_the_containers_and_volumes_it_named() { + let root = temp_root("leftover-offer"); + let address = engine::Address::new(engine::Engine::Docker, None); + assert!(leftover_offer(&Problem::plain("refused"), &root, &address).is_none()); + let mut problem = Problem::plain("refused"); + problem.leftover = Some(Box::new(LeftoverReset { + volumes: BOT_LEFTOVERS.map(String::from).to_vec(), + containers: vec!["openbot-computer-general-assistant".into()], + })); + let offer = leftover_offer(&problem, &root, &address).unwrap(); + assert_eq!(offer.root, root); + assert_eq!(offer.volumes, BOT_LEFTOVERS); + assert_eq!(offer.containers, ["openbot-computer-general-assistant"]); + assert_eq!(offer.address, address); + } + + #[test] + fn leftover_reset_is_bound_to_the_exact_offered_set() { + let root = temp_root("leftover-volumes-command"); + std::fs::create_dir_all(&root).unwrap(); + let shell = Shell::default(); + let offered: Vec = BOT_LEFTOVERS.map(String::from).to_vec(); + *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { + root: root.clone(), + volumes: offered.clone(), + containers: vec!["openbot-computer-general-assistant".into()], + address: engine::Address::new(engine::Engine::Docker, None), + }); + let mut superset = offered.clone(); + superset.push("other-profile-knowledge".into()); + let containers = ["openbot-computer-general-assistant".to_string()]; + for (stale, stale_containers) in [ + (offered[..2].to_vec(), &containers[..]), + (superset, &containers[..]), + (Vec::new(), &containers[..]), + (offered.clone(), &[][..]), + ( + offered.clone(), + &[containers[0].clone(), "other-computer-knowledge".into()][..], + ), + ] { + assert!(reset_leftover_database_with( + &shell, + &root, + &stale, + stale_containers, + true, + || panic!("a changed set must refuse before reading credentials"), + |_, _| panic!("a changed set must not remove anything") + ) + .is_err()); + } + let mut reordered = offered.clone(); + reordered.reverse(); + reset_leftover_database_with( + &shell, + &root, + &reordered, + &containers, + true, + || Ok(stack::Secrets::new()), + |offer, _| { + assert_eq!(offer.volumes, offered, "removal gets the confirmed volumes"); + assert_eq!(offer.containers, containers, "and the confirmed containers"); + Ok(()) + }, + ) + .unwrap(); + assert!(shell.leftover_database.lock().unwrap().is_none()); + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn leftover_database_reset_rechecks_confirmation_configuration_key_and_ownership() { let root = temp_root("leftover-database-command"); @@ -5553,7 +5888,8 @@ mod tests { let offer = || { *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: "openbot_postgres-data".into(), + volumes: vec!["openbot_postgres-data".into()], + containers: Vec::new(), address: engine::Address::new( engine::Engine::Podman, Some("original-machine".into()), @@ -5564,7 +5900,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], false, || panic!("unconfirmed must not access credentials"), |_, _| panic!("unconfirmed must not delete") @@ -5573,7 +5910,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| Ok(()) @@ -5595,7 +5933,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| panic!("configured or unknown root must not delete") @@ -5607,7 +5946,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| panic!("unreadable settings must not delete") @@ -5621,7 +5961,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(secrets), |_, _| panic!("restored key must prevent deletion") @@ -5631,7 +5972,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("owned hosts must refuse before credential access"), |_, _| panic!("owned hosts must prevent deletion") @@ -5645,7 +5987,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("owned containers must refuse"), |_, _| panic!("owned containers must prevent deletion") @@ -5656,7 +5999,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("concurrent startup must refuse"), |_, _| panic!("concurrent startup must prevent deletion") @@ -5666,7 +6010,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| { @@ -5697,11 +6042,12 @@ mod tests { let shell = Shell::default(); let original = engine::Address::new(engine::Engine::Podman, Some("original-machine".into())); - let volume = "openbot_postgres-data"; + let volume = &["openbot_postgres-data".to_string()][..]; assert!(reset_leftover_database_with( &shell, &root, volume, + &[], true, || panic!("a missing offer must refuse before reading credentials"), |_, _| panic!("a missing offer must not remove anything") @@ -5709,17 +6055,19 @@ mod tests { .is_err()); *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: volume.into(), + volumes: volume.to_vec(), + containers: Vec::new(), address: original.clone(), }); for (selected_root, selected_volume) in [ - (&root, "other-volume"), + (&root, &["other-volume".to_string()][..]), (&root.join("another-root"), volume), ] { assert!(reset_leftover_database_with( &shell, selected_root, selected_volume, + &[], true, || panic!("a mismatched offer must refuse before reading credentials"), |_, _| panic!("a mismatched offer must not remove anything") @@ -5736,10 +6084,11 @@ mod tests { &shell, &root, volume, + &[], true, || Ok(stack::Secrets::new()), - |address, _| { - assert_eq!(address, &original); + |offer, _| { + assert_eq!(offer.address, original); Err(unavailable.clone()) } ), @@ -5749,11 +6098,12 @@ mod tests { &shell, &root, volume, + &[], true, || Ok(stack::Secrets::new()), - |address, _| { - assert_eq!(address, &original); - let command = address.command(); + |offer, _| { + assert_eq!(offer.address, original); + let command = offer.address.command(); let arguments: Vec<_> = command.get_args().collect(); assert_eq!(arguments, ["--connection", "original-machine"]); Ok(()) @@ -5822,23 +6172,33 @@ mod tests { &root, &secrets, || stack::postgres_volume_exists(&address, &root, &secrets), - || stack::leftover_database_volume(&address, &root, &secrets), + || stack::leftover_volumes(&address, &root, &secrets), ) .unwrap_err(); assert_eq!(guarded.database_reset.as_deref(), Some(volume.as_str())); + let volumes = [volume.clone()]; + assert_eq!(guarded.leftover_volumes(), volumes); let shell = Shell::default(); *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: volume.clone(), + volumes: volumes.to_vec(), + containers: Vec::new(), address: address.clone(), }); assert!(reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &[], false, || Ok(secrets.clone()), - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume) + |offer, secrets| stack::reset_leftover_volumes( + &offer.address, + &root, + secrets, + &offer.volumes, + &offer.containers + ) ) .is_err()); assert!(stack::postgres_volume_exists(&address, &root, &secrets).unwrap()); @@ -5847,10 +6207,19 @@ mod tests { reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &[], true, || Ok(secrets.clone()), - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume), + |offer, secrets| { + stack::reset_leftover_volumes( + &offer.address, + &root, + secrets, + &offer.volumes, + &offer.containers, + ) + }, ) .unwrap(); require_existing_encryption_key(&root, &secrets, || { @@ -5877,7 +6246,7 @@ mod tests { String::from_utf8_lossy(&started.stderr) ); assert!( - stack::reset_leftover_database(&address, &root, &secrets, &volume).is_err(), + stack::reset_leftover_volumes(&address, &root, &secrets, &volumes, &[]).is_err(), "the real engine must refuse an attached database volume" ); let ready = address diff --git a/desktop/src-tauri/src/problem.rs b/desktop/src-tauri/src/problem.rs index 285418fd84..16dfa06b4f 100644 --- a/desktop/src-tauri/src/problem.rs +++ b/desktop/src-tauri/src/problem.rs @@ -36,6 +36,21 @@ pub struct Problem { /// The full, verified Compose volume name offered for an explicit fresh-install reset. #[serde(skip_serializing_if = "Option::is_none")] pub database_reset: Option, + /// A verified fresh-install reset of everything a previous installation left behind. + /// Boxed and flattened: the window reads `leftover_reset` and `leftover_containers`. + #[serde(flatten, skip_serializing_if = "Option::is_none")] + pub leftover: Option>, +} + +/// Exactly what one confirmed reset removes: the containers first, then the volumes. +#[derive(Clone, Debug, Default, Serialize, PartialEq, Eq)] +pub struct LeftoverReset { + /// Every verified volume, the database included. + #[serde(rename = "leftover_reset", skip_serializing_if = "Vec::is_empty")] + pub volumes: Vec, + /// Stopped containers that still hold those volumes. + #[serde(rename = "leftover_containers", skip_serializing_if = "Vec::is_empty")] + pub containers: Vec, } impl Problem { @@ -46,6 +61,7 @@ impl Problem { detail: None, connection: None, database_reset: None, + leftover: None, } } @@ -57,9 +73,22 @@ impl Problem { detail: (!detail.trim().is_empty()).then_some(detail), connection: None, database_reset: None, + leftover: None, } } + /// The volumes a confirmed reset removes, or none when no reset is offered. + pub fn leftover_volumes(&self) -> &[String] { + self.leftover.as_deref().map_or(&[], |reset| &reset.volumes) + } + + /// The stopped containers a confirmed reset removes before those volumes. + pub fn leftover_containers(&self) -> &[String] { + self.leftover + .as_deref() + .map_or(&[], |reset| &reset.containers) + } + pub fn connection(mut self, connection: Connection) -> Self { self.connection = Some(connection); self diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index feef07cc88..038a0a3383 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -237,6 +237,12 @@ fn postgres_configuration( } fn named_volume_exists(engine: &Address, root: &Path, volume: &str) -> Result { + Ok(volume_inventory(engine, root)? + .iter() + .any(|name| name == volume)) +} + +fn volume_inventory(engine: &Address, root: &Path) -> Result, Problem> { let inventory = engine .command() .current_dir(root) @@ -251,7 +257,12 @@ fn named_volume_exists(engine: &Address, root: &Path, volume: &str) -> Result) -> Problem { @@ -267,65 +278,120 @@ pub fn leftover_database_volume( root: &Path, secrets: &Secrets, ) -> Result, Problem> { + let config = leftover_configuration(engine, root, secrets)?; + let inventory = volume_inventory(engine, root)?; + owned_database_volume(engine, root, &config, &inventory) +} + +fn leftover_configuration( + engine: &Address, + root: &Path, + secrets: &Secrets, +) -> Result { let configuration = postgres_configuration(engine, root, secrets)?; - let config: serde_json::Value = serde_json::from_slice(&configuration) - .map_err(|_| postgres_volume_problem("Compose config did not return valid JSON."))?; - let Some((source, definition)) = postgres_data_volume(&config) else { + serde_json::from_slice(&configuration) + .map_err(|_| postgres_volume_problem("Compose config did not return valid JSON.")) +} + +fn owned_database_volume( + engine: &Address, + root: &Path, + config: &serde_json::Value, + inventory: &[String], +) -> Result, Problem> { + let Some((source, definition)) = postgres_data_volume(config) else { return Ok(None); }; let Some(project) = config["name"].as_str().filter(|name| !name.is_empty()) else { return Ok(None); }; - let Some(name) = definition["name"].as_str() else { + let Some(name) = ordinary_project_volume(project, source, definition) else { return Ok(None); }; - // Compose's explicit names and external volumes can refer to somebody else's database. - // Permit only ordinary project-scoped, local storage, even if a custom volume has labels. - if name != format!("{project}_{source}") - || !name.bytes().enumerate().all(|(index, byte)| { + // Another service mounting the database volume, under any alias, makes it shared storage. + if config["services"].as_object().is_some_and(|services| { + services.iter().any(|(service, configuration)| { + service != "postgres" + && configuration["volumes"].as_array().is_some_and(|mounts| { + mounts.iter().any(|mount| { + mount["source"].as_str().is_some_and(|other_source| { + other_source == source + || config["volumes"][other_source]["name"].as_str() == Some(name) + }) + }) + }) + }) + }) { + return Ok(None); + } + owned_compose_volume(engine, root, project, source, name, inventory) +} + +/// Compose's explicit names and external volumes can refer to somebody else's data. +/// Permit only ordinary project-scoped, local storage, even if a custom volume has labels. +fn ordinary_project_volume<'a>( + project: &str, + source: &str, + definition: &'a serde_json::Value, +) -> Option<&'a str> { + let name = definition["name"].as_str()?; + (name == format!("{project}_{source}") + && name.bytes().enumerate().all(|(index, byte)| { byte.is_ascii_alphanumeric() || (index > 0 && b"_.-".contains(&byte)) }) - || !matches!( + && matches!( definition.get("external"), None | Some(serde_json::Value::Bool(false)) ) - || definition["driver"] + && !definition["driver"] .as_str() .is_some_and(|driver| driver != "local") - || !empty_volume_options(&definition["driver_opts"]) - || config["services"].as_object().is_some_and(|services| { - services.iter().any(|(service, configuration)| { - service != "postgres" - && configuration["volumes"].as_array().is_some_and(|mounts| { - mounts.iter().any(|mount| { - mount["source"].as_str().is_some_and(|other_source| { - other_source == source - || config["volumes"][other_source]["name"].as_str() - == Some(name) - }) - }) - }) - }) - }) - { + && empty_volume_options(&definition["driver_opts"])) + .then_some(name) +} + +fn owned_compose_volume( + engine: &Address, + root: &Path, + project: &str, + source: &str, + name: &str, + inventory: &[String], +) -> Result, Problem> { + if !inventory.iter().any(|listed| listed == name) { return Ok(None); } - if !named_volume_exists(engine, root, name)? { + let Some(labels) = inspect_local_volume(engine, root, name)? else { return Ok(None); - } + }; + Ok( + (labels["com.docker.compose.project"].as_str() == Some(project) + && labels["com.docker.compose.volume"].as_str() == Some(source)) + .then(|| name.to_owned()), + ) +} + +/// The labels of exactly this local volume, or `None` when the engine describes anything else. +fn inspect_local_volume( + engine: &Address, + root: &Path, + name: &str, +) -> Result, Problem> { let inspect = engine .command() .current_dir(root) .args(["volume", "inspect", name]) .output() .map_err(|error| { - postgres_volume_problem(format!("Could not inspect the database volume: {error}")) + postgres_volume_problem(format!("Could not inspect volume {name}: {error}")) })?; if !inspect.status.success() { - return Err(postgres_volume_problem(format!("Database volume inspection exited with {}. Output omitted because it can contain credentials.", inspect.status))); + return Err(postgres_volume_problem(format!("Inspection of volume {name} exited with {}. Output omitted because it can contain credentials.", inspect.status))); } let inspected: serde_json::Value = serde_json::from_slice(&inspect.stdout).map_err(|_| { - postgres_volume_problem("Database volume inspection did not return valid JSON.") + postgres_volume_problem(format!( + "Inspection of volume {name} did not return valid JSON." + )) })?; let Some(volumes) = inspected.as_array().filter(|volumes| volumes.len() == 1) else { return Ok(None); @@ -335,36 +401,402 @@ pub fn leftover_database_volume( // tied to the full name the person confirmed, never a similarly named backup. Ok((volume["Name"].as_str() == Some(name) && volume["Driver"].as_str() == Some("local") - && empty_volume_options(&volume["Options"]) - && volume["Labels"]["com.docker.compose.project"].as_str() == Some(project) - && volume["Labels"]["com.docker.compose.volume"].as_str() == Some(source)) + && empty_volume_options(&volume["Options"])) + .then(|| volume["Labels"].clone())) +} + +/// A Bot's profile or workspace volume, created by the supervisor in this namespace. +/// Matches `supervisor/src/names.ts` names and `labelsFor` in `supervisor/src/docker.ts`; +/// all three labels are required, so an older or foreign volume is never claimed. +fn owned_supervisor_volume( + engine: &Address, + root: &Path, + namespace: &str, + name: &str, +) -> Result, Problem> { + let Some(bot) = ["profile", "workspace"] + .iter() + .find_map(|kind| name.strip_prefix(&format!("{namespace}-{kind}-"))) + .filter(|bot| supervisor_identifier(bot)) + else { + return Ok(None); + }; + let Some(labels) = inspect_local_volume(engine, root, name)? else { + return Ok(None); + }; + Ok((labels["openbot.supervisor"].as_str() == Some("true") + && labels["openbot.bot-id"].as_str() == Some(bot) + && labels["openbot.namespace"].as_str() == Some(namespace)) .then(|| name.to_owned())) } -pub fn reset_leftover_database( +/// The namespace the selected supervisor will run with, as `supervisor/src/names.ts` reads it. +/// No supervisor, or a namespace it would refuse, owns no volumes. +fn supervisor_namespace(config: &serde_json::Value) -> Option { + let supervisor = config["services"]["supervisor"].as_object()?; + let configured = match supervisor + .get("environment") + .and_then(|environment| environment.get("COMPUTER_NAMESPACE")) + { + None | Some(serde_json::Value::Null) => "", + Some(value) => value.as_str()?, + }; + resolve_namespace(configured).map(str::to_owned) +} + +/// Match supervisor/src/names.ts: trim, default only an empty value, then the same 64-character +/// ASCII identifier grammar. +fn resolve_namespace(configured: &str) -> Option<&str> { + let namespace = match configured.trim() { + "" => "openbot", + value => value, + }; + supervisor_identifier(namespace).then_some(namespace) +} + +fn supervisor_identifier(value: &str) -> bool { + !value.is_empty() + && value.len() <= 64 + && value.as_bytes()[0].is_ascii_alphanumeric() + && value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') +} + +/// What a previous installation of this deployment left on the selected engine. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LeftoverVolumes { + /// The Postgres volume, when Compose and the engine agree this project owns it. + pub database: Option, + /// Whether the volume Compose selects for Postgres exists at all, owned or not. + pub database_exists: bool, + /// Every verified, owned volume, sorted. This is exactly what a reset removes. + pub volumes: Vec, + /// Stopped containers this deployment provably owns, sorted by name. They still hold the + /// volumes, so a reset removes them first. + pub containers: Vec, + /// Owned containers that are not stopped, by name. Any one of them blocks a reset. + pub running: Vec, +} + +/// A stopped container a reset may remove, named for the person and removed by its exact id. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LeftoverContainer { + pub name: String, + pub id: String, +} + +impl LeftoverVolumes { + /// A reset must not leave a database it cannot verify for the new installation to reuse, + /// and never touches a container that is still running. + pub fn resettable(&self) -> bool { + !self.volumes.is_empty() + && (self.database.is_some() || !self.database_exists) + && self.running.is_empty() + } + + pub fn container_names(&self) -> Vec { + self.containers + .iter() + .map(|container| container.name.clone()) + .collect() + } + + /// The sentence for a reset that a running container blocks, or `None` when none is running. + pub fn running_said(&self) -> Option { + (!self.running.is_empty()).then(|| { + format!( + "A container from the previous installation is still running: {}. Stop it in Docker or Podman, then try Start again.", + self.running.join(", ") + ) + }) + } +} + +fn leftover_container_problem(detail: impl Into) -> Problem { + Problem::with( + "OpenBot could not verify which containers a previous installation left. Nothing was changed. Check the selected container engine, then try again.", + detail, + ) +} + +/// The ids carrying every one of these labels. Only a hint: each one is inspected before use. +fn labelled_containers( engine: &Address, root: &Path, - secrets: &Secrets, - confirmed_volume: &str, -) -> Result<(), Problem> { - if leftover_database_volume(engine, root, secrets)?.as_deref() != Some(confirmed_volume) { - return Err(Problem::plain("The leftover database no longer matches the volume you confirmed, or OpenBot could not verify that it owns the volume. Nothing was removed. Try Start again.")); + labels: &[(&str, &str)], +) -> Result, Problem> { + let mut command = engine.command(); + command + .current_dir(root) + .args(["ps", "--all", "--quiet", "--no-trunc"]); + for (key, value) in labels { + command.args(["--filter", &format!("label={key}={value}")]); + } + let listed = command.output().map_err(|error| { + leftover_container_problem(format!("Could not list containers: {error}")) + })?; + if !listed.status.success() { + return Err(leftover_container_problem(format!( + "Container listing exited with {}. Output omitted because it can contain credentials.", + listed.status + ))); } - // Never force this operation: the engine must refuse any container attachment, including a - // stopped container or one created after inspection. Do not stop containers to make it pass. - let removed = engine + let listed = std::str::from_utf8(&listed.stdout) + .map_err(|_| leftover_container_problem("Container listing was not valid UTF-8."))?; + Ok(listed.split_whitespace().map(str::to_owned).collect()) +} + +enum ContainerState { + Stopped, + Running, +} + +/// The name and state of a container whose labels prove this deployment owns it, or `None`. +fn owned_container( + engine: &Address, + root: &Path, + id: &str, + project: Option<&str>, + namespace: Option<&str>, +) -> Result, Problem> { + let inspect = engine .command() .current_dir(root) - .args(["volume", "rm", confirmed_volume]) + .args(["container", "inspect", id]) .output() .map_err(|error| { - Problem::with( - "OpenBot could not reset the leftover database. Nothing else was removed.", - format!("Could not run volume removal: {error}"), - ) + leftover_container_problem(format!("Could not inspect container {id}: {error}")) })?; - if !removed.status.success() { - return Err(Problem::with("OpenBot could not reset the leftover database. It may still be attached to a container. Nothing else was removed; stop the installation using it before trying again.", format!("Volume removal exited with {}. Output omitted because it can contain credentials.", removed.status))); + if !inspect.status.success() { + return Err(leftover_container_problem(format!( + "Inspection of container {id} exited with {}. Output omitted because it can contain credentials.", + inspect.status + ))); + } + let inspected: serde_json::Value = serde_json::from_slice(&inspect.stdout).map_err(|_| { + leftover_container_problem(format!( + "Inspection of container {id} did not return valid JSON." + )) + })?; + let Some(containers) = inspected + .as_array() + .filter(|containers| containers.len() == 1) + else { + return Ok(None); + }; + let container = &containers[0]; + // Exact ids only: an engine resolving a prefix to something else is not this container. + if container["Id"].as_str() != Some(id) { + return Ok(None); + } + // Docker reports `/name`; Podman reports `name`. + let Some(name) = container["Name"] + .as_str() + .map(|name| name.strip_prefix('/').unwrap_or(name)) + .filter(|name| !name.is_empty()) + else { + return Ok(None); + }; + let labels = &container["Config"]["Labels"]; + let compose_owned = project + .is_some_and(|project| labels["com.docker.compose.project"].as_str() == Some(project)); + // Matches `namesFor` in supervisor/src/names.ts and `labelsFor` in supervisor/src/docker.ts. + let supervisor_owned = namespace.is_some_and(|namespace| { + labels["openbot.supervisor"].as_str() == Some("true") + && labels["openbot.namespace"].as_str() == Some(namespace) + && labels["openbot.bot-id"].as_str().is_some_and(|bot| { + supervisor_identifier(bot) && name == format!("{namespace}-computer-{bot}") + }) + }); + if !compose_owned && !supervisor_owned { + return Ok(None); + } + let state = &container["State"]; + // Only states a plain removal accepts count as stopped. Anything else, or anything unknown, + // is treated as running so it is never touched. + let stopped = state["Running"].as_bool() == Some(false) + && state["Status"].as_str().is_some_and(|status| { + matches!(status, "created" | "exited" | "stopped" | "configured") + }); + Ok(Some(( + name.to_owned(), + if stopped { + ContainerState::Stopped + } else { + ContainerState::Running + }, + ))) +} + +fn leftover_containers( + engine: &Address, + root: &Path, + project: Option<&str>, + namespace: Option<&str>, +) -> Result<(Vec, Vec), Problem> { + let mut ids = Vec::new(); + if let Some(project) = project { + ids.extend(labelled_containers( + engine, + root, + &[("com.docker.compose.project", project)], + )?); + } + if let Some(namespace) = namespace { + ids.extend(labelled_containers( + engine, + root, + &[ + ("openbot.supervisor", "true"), + ("openbot.namespace", namespace), + ], + )?); + } + ids.sort(); + ids.dedup(); + let mut stopped = Vec::new(); + let mut running = Vec::new(); + for id in ids { + match owned_container(engine, root, &id, project, namespace)? { + Some((name, ContainerState::Stopped)) => stopped.push(LeftoverContainer { name, id }), + Some((name, ContainerState::Running)) => running.push(name), + None => {} + } + } + stopped.sort_by(|a, b| a.name.cmp(&b.name)); + running.sort(); + Ok((stopped, running)) +} + +/** +Every volume a previous installation of this deployment left behind, and that it provably owns. + +A deleted installation folder removes `.env` and the vault, but not the engine's volumes. Compose +and the supervisor reattach them by name, so a new installation silently inherits the old one's +Bots' workspace files and browser sign-ins. Only three kinds are claimed: the verified Postgres +volume, the other ordinary project-scoped Compose volumes labelled for this project, and the +per-Bot volumes the supervisor labelled for this namespace. Never include config output in +diagnostics: interpolation may have put credentials in it. +*/ +pub fn leftover_volumes( + engine: &Address, + root: &Path, + secrets: &Secrets, +) -> Result { + let config = leftover_configuration(engine, root, secrets)?; + let inventory = volume_inventory(engine, root)?; + let postgres = postgres_data_volume(&config); + let database_exists = postgres + .and_then(|(_, definition)| definition["name"].as_str()) + .is_some_and(|name| inventory.iter().any(|listed| listed == name)); + let database = owned_database_volume(engine, root, &config, &inventory)?; + let mut volumes: Vec = database.iter().cloned().collect(); + let project = config["name"].as_str().filter(|name| !name.is_empty()); + let namespace = supervisor_namespace(&config); + if let (Some(project), Some(definitions)) = (project, config["volumes"].as_object()) { + for (source, definition) in definitions { + // The database has its own, stricter test above. + if postgres.is_some_and(|(postgres_source, _)| postgres_source == source) { + continue; + } + let Some(name) = ordinary_project_volume(project, source, definition) else { + continue; + }; + volumes.extend(owned_compose_volume( + engine, root, project, source, name, &inventory, + )?); + } + } + if let Some(namespace) = &namespace { + for name in &inventory { + volumes.extend(owned_supervisor_volume(engine, root, namespace, name)?); + } + } + volumes.sort(); + volumes.dedup(); + // Stopped containers keep their volumes attached, so the engine refuses to remove them. Only + // worth listing when there is data to reset. + let (containers, running) = if volumes.is_empty() { + Default::default() + } else { + leftover_containers(engine, root, project, namespace.as_deref())? + }; + Ok(LeftoverVolumes { + database, + database_exists, + volumes, + containers, + running, + }) +} + +/// Remove exactly the leftover containers and volumes the person confirmed, containers first, +/// or nothing when either set changed or an owned container is running. +pub fn reset_leftover_volumes( + engine: &Address, + root: &Path, + secrets: &Secrets, + confirmed: &[String], + confirmed_containers: &[String], +) -> Result<(), Problem> { + let leftovers = leftover_volumes(engine, root, secrets)?; + if let Some(said) = leftovers.running_said() { + return Err(Problem::plain(format!("{said} Nothing was removed."))); + } + let sorted = |names: &[String]| { + let mut names = names.to_vec(); + names.sort(); + names + }; + if !leftovers.resettable() + || leftovers.volumes != sorted(confirmed) + || leftovers.container_names() != sorted(confirmed_containers) + { + return Err(Problem::plain("The leftover data no longer matches the containers and volumes you confirmed, or OpenBot could not verify that it owns them. Nothing was removed. Try Start again.")); + } + let mut removed: Vec<&str> = Vec::new(); + let removals = leftovers + .containers + .iter() + .map(|container| ("container", container.id.as_str(), container.name.as_str())) + .chain( + leftovers + .volumes + .iter() + .map(|volume| ("volume", volume.as_str(), volume.as_str())), + ); + for (kind, target, name) in removals { + let so_far = || match removed.as_slice() { + [] => "Nothing was removed.".to_owned(), + removed => format!("Already removed: {}.", removed.join(", ")), + }; + // Never force this operation: the engine must refuse a running container and any + // container attachment, including one created after inspection. Do not stop containers + // to make it pass. + let output = engine + .command() + .current_dir(root) + .args([kind, "rm", target]) + .output() + .map_err(|error| { + Problem::with( + format!("OpenBot could not remove {name}. {}", so_far()), + format!("Could not run {kind} removal: {error}"), + ) + })?; + if !output.status.success() { + let why = if kind == "container" { + "It may have started again." + } else { + "It may still be attached to a container." + }; + return Err(Problem::with( + format!("OpenBot could not remove {name}. {why} {} Stop the installation using it, then try Start again.", so_far()), + format!("{kind} removal exited with {}. Output omitted because it can contain credentials.", output.status), + )); + } + removed.push(name); } Ok(()) } @@ -756,20 +1188,9 @@ fn computer_namespace(engine: &Address, root: &Path) -> Result, S .pointer("/services/supervisor/environment/COMPUTER_NAMESPACE") .and_then(serde_json::Value::as_str) .ok_or("could not resolve computer namespace: supervisor configuration has no namespace")?; - // Match supervisor/src/names.ts: trim, default only an empty value, then the same 64-character - // ASCII identifier grammar. A malformed/missing response never becomes an unscoped filter. - let namespace = match configured.trim() { - "" => "openbot", - value => value, - }; - if namespace.len() > 64 - || !namespace.as_bytes()[0].is_ascii_alphanumeric() - || !namespace - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') - { - return Err("could not resolve computer namespace: supervisor namespace is invalid".into()); - } + // A malformed/missing response never becomes an unscoped filter. + let namespace = resolve_namespace(configured) + .ok_or("could not resolve computer namespace: supervisor namespace is invalid")?; Ok(Some(namespace.to_string())) } @@ -3437,14 +3858,25 @@ mod tests { leftover_database_volume(&address, &root, &Secrets::new()).unwrap(), Some("openbot_postgres-data".into()) ); - assert!( - reset_leftover_database(&address, &root, &Secrets::new(), "unrelated").is_err() - ); + assert!(reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["unrelated".into()], + &[] + ) + .is_err()); assert!(!std::fs::read_to_string(&record) .unwrap() .contains("volume rm")); - reset_leftover_database(&address, &root, &Secrets::new(), "openbot_postgres-data") - .unwrap(); + reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["openbot_postgres-data".into()], + &[], + ) + .unwrap(); assert_eq!( std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(), "unrelated\n" @@ -3557,9 +3989,14 @@ mod tests { if let Some(marker) = marker { std::fs::write(root.join(marker), "").unwrap(); } - let error = - reset_leftover_database(&address, &root, &Secrets::new(), "openbot_postgres-data") - .expect_err(scenario); + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["openbot_postgres-data".into()], + &[], + ) + .expect_err(scenario); assert!( !format!("{error:?}").contains("synthetic-secret"), "{scenario}" @@ -3585,6 +4022,462 @@ mod tests { } } + /// A reinstall's Compose project and supervisor volumes, beside look-alikes it does not own. + fn leftover_volumes_fixture(root: &Path, namespace: &str) { + let config = serde_json::json!({ + "name": "openbot", + "services": { + "postgres": {"volumes": [{ + "type": "volume", "source": "postgres-data", "target": "/var/lib/postgresql/data" + }]}, + "supervisor": {"environment": {"COMPUTER_NAMESPACE": namespace}}, + "agent-computer": {"volumes": [ + {"type": "volume", "source": "agent-profiles", "target": "/profiles"}, + {"type": "volume", "source": "agent-workspace", "target": "/workspace"} + ]} + }, + "volumes": { + "postgres-data": {"name": "openbot_postgres-data"}, + "agent-profiles": {"name": "openbot_agent-profiles"}, + "agent-workspace": {"name": "openbot_agent-workspace"}, + "spire-server-data": {"name": "openbot_spire-server-data"}, + "unlabeled": {"name": "openbot_unlabeled"}, + "foreign": {"name": "openbot_foreign"}, + "shared-cache": {"name": "shared-cache", "external": true}, + "explicit": {"name": "explicitly-named"}, + "remote": {"name": "openbot_remote", "driver_opts": {"device": "/shared"}} + } + }); + std::fs::write(root.join(".fixture-config"), config.to_string()).unwrap(); + std::fs::write( + root.join(".fixture-volumes"), + [ + "unrelated", + "openbot_agent-profiles", + "openbot_agent-workspace", + "openbot_unlabeled", + "openbot_foreign", + "shared-cache", + "explicitly-named", + "openbot_remote", + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot-profile-unlabeled", + "openbot-workspace-elsewhere", + "openbot-profile-legacy", + "other-profile-knowledge", + ] + .map(|name| format!("{name}\n")) + .concat(), + ) + .unwrap(); + let inspect = |name: &str, labels: serde_json::Value| { + std::fs::write( + root.join(format!(".fixture-inspect-{name}")), + serde_json::json!([{ + "Name": name, "Driver": "local", "Options": {}, "Labels": labels + }]) + .to_string(), + ) + .unwrap(); + }; + let compose = |project: &str, source: &str| { + serde_json::json!({ + "com.docker.compose.project": project, "com.docker.compose.volume": source + }) + }; + let supervisor = |bot: &str, namespace: &str| { + serde_json::json!({ + "openbot.supervisor": "true", "openbot.bot-id": bot, "openbot.namespace": namespace + }) + }; + inspect( + "openbot_agent-profiles", + compose("openbot", "agent-profiles"), + ); + inspect( + "openbot_agent-workspace", + compose("openbot", "agent-workspace"), + ); + inspect("openbot_unlabeled", serde_json::json!({})); + inspect("openbot_foreign", compose("another-project", "foreign")); + inspect( + "openbot-profile-general-assistant", + supervisor("general-assistant", "openbot"), + ); + inspect( + "openbot-workspace-general-assistant", + supervisor("general-assistant", "openbot"), + ); + inspect("openbot-profile-unlabeled", serde_json::json!({})); + inspect( + "openbot-workspace-elsewhere", + supervisor("elsewhere", "other"), + ); + inspect( + "openbot-profile-legacy", + serde_json::json!({"openbot.supervisor": "true", "openbot.bot-id": "legacy"}), + ); + inspect("other-profile-knowledge", supervisor("knowledge", "other")); + } + + const OWNED_LEFTOVERS: [&str; 4] = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + "openbot_agent-workspace", + ]; + + #[test] + fn leftover_volumes_are_only_volumes_this_project_or_namespace_provably_owns() { + if crate::test_support::isolated_process( + "stack::tests::leftover_volumes_are_only_volumes_this_project_or_namespace_provably_owns", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + let address = computer_stop_addresses(&path)[0].clone(); + let root = path.bin.join("leftover-volumes"); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + + leftover_volumes_fixture(&root, "openbot"); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert_eq!(found.database, None); + assert!(!found.database_exists); + assert!(found.resettable()); + let log = std::fs::read_to_string(&record).unwrap(); + for never_inspected in ["shared-cache", "explicitly-named", "openbot_remote"] { + assert!( + !log.contains(&format!("volume inspect {never_inspected}")), + "{never_inspected}: {log}" + ); + } + assert!(!log.contains("volume rm")); + + // Another deployment's namespace selects only that namespace's Bots. + leftover_volumes_fixture(&root, "other"); + assert_eq!( + leftover_volumes(&address, &root, &Secrets::new()) + .unwrap() + .volumes, + [ + "openbot_agent-profiles", + "openbot_agent-workspace", + "other-profile-knowledge" + ] + ); + + // The database joins the set when it is verified, and blocks the offer when it is not. + leftover_volumes_fixture(&root, "openbot"); + let mut inventory = std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(); + inventory.push_str("openbot_postgres-data\n"); + std::fs::write(root.join(".fixture-volumes"), inventory).unwrap(); + let database = serde_json::json!([{ + "Name": "openbot_postgres-data", "Driver": "local", "Options": {}, + "Labels": {"com.docker.compose.project":"openbot", "com.docker.compose.volume":"postgres-data"} + }]); + std::fs::write(root.join(".fixture-inspect"), database.to_string()).unwrap(); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.database.as_deref(), Some("openbot_postgres-data")); + assert!(found.database_exists && found.resettable()); + let mut expected = OWNED_LEFTOVERS.map(String::from).to_vec(); + expected.push("openbot_postgres-data".into()); + expected.sort(); + assert_eq!(found.volumes, expected); + + let mut foreign = database.clone(); + foreign[0]["Labels"]["com.docker.compose.project"] = "another-project".into(); + std::fs::write(root.join(".fixture-inspect"), foreign.to_string()).unwrap(); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.database, None); + assert!(found.database_exists); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert!( + !found.resettable(), + "an unverified database must not be left behind by a reset" + ); + std::fs::write(record, "").unwrap(); + assert!(reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &OWNED_LEFTOVERS.map(String::from), + &[] + ) + .is_err()); + assert!(!std::fs::read_to_string(root.join("commands.log")) + .unwrap() + .contains("volume rm")); + } + + #[test] + fn leftover_volume_reset_removes_exactly_the_confirmed_owned_set() { + if crate::test_support::isolated_process( + "stack::tests::leftover_volume_reset_removes_exactly_the_confirmed_owned_set", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + for address in computer_stop_addresses(&path) { + let root = path + .bin + .join(format!("{}-leftover-reset", address.engine.binary())); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + let owned = OWNED_LEFTOVERS.map(String::from).to_vec(); + let mut duplicated = owned.clone(); + duplicated.push(owned[0].clone()); + let mut superset = owned.clone(); + superset.push("other-profile-knowledge".into()); + for stale in [ + owned[..3].to_vec(), + superset, + duplicated, + vec!["unrelated".into()], + Vec::new(), + ] { + let error = reset_leftover_volumes(&address, &root, &Secrets::new(), &stale, &[]) + .expect_err("a changed set must not be removed"); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!std::fs::read_to_string(&record) + .unwrap_or_default() + .contains("volume rm")); + } + + // The engine refuses a volume a container still uses; nothing is forced. + std::fs::write( + root.join(".fixture-attached-openbot-profile-general-assistant"), + "", + ) + .unwrap(); + let error = reset_leftover_volumes(&address, &root, &Secrets::new(), &owned, &[]) + .expect_err("an attached volume is refused"); + assert!( + error.said.contains("openbot-profile-general-assistant"), + "{}", + error.said + ); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!format!("{error:?}").contains("synthetic-secret")); + std::fs::remove_file(root.join(".fixture-attached-openbot-profile-general-assistant")) + .unwrap(); + + let mut reordered = owned.clone(); + reordered.reverse(); + reset_leftover_volumes(&address, &root, &Secrets::new(), &reordered, &[]).unwrap(); + let remaining = std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(); + for name in &owned { + assert!(!remaining.lines().any(|line| line == name), "{name}"); + } + for kept in [ + "unrelated", + "openbot_unlabeled", + "openbot_foreign", + "shared-cache", + "explicitly-named", + "openbot_remote", + "openbot-profile-unlabeled", + "openbot-workspace-elsewhere", + "openbot-profile-legacy", + "other-profile-knowledge", + ] { + assert!(remaining.lines().any(|line| line == kept), "{kept}"); + } + let log = std::fs::read_to_string(&record).unwrap(); + let removals: Vec<_> = log + .lines() + .filter_map(|line| line.split_once("volume rm ")) + .map(|(_, name)| name) + .collect(); + // One refused attempt, then each confirmed volume once. + assert_eq!(removals[0], "openbot-profile-general-assistant"); + assert_eq!(removals[1..], OWNED_LEFTOVERS); + assert!(!log.contains("--force") && !log.contains("prune") && !log.contains("down")); + if address.engine == crate::engine::Engine::Podman { + assert!(log.lines().all(|line| line + .split_once('\t') + .unwrap() + .1 + .starts_with("--connection fixture-machine "))); + } + } + } + + /// The previous installation's stopped computers and Compose containers, beside ones it + /// does not own. Each still mounts the volumes it used, as a real engine reports it. + fn leftover_containers_fixture(root: &Path, computer_status: &str) { + let supervisor = |bot: &str, namespace: &str| { + format!("openbot.supervisor=true,openbot.bot-id={bot},openbot.namespace={namespace}") + }; + let lines = [ + format!( + "c-computer\topenbot-computer-general-assistant\t{computer_status}\t{}\topenbot-profile-general-assistant,openbot-workspace-general-assistant", + supervisor("general-assistant", "openbot") + ), + "c-compose\topenbot-agent-computer-1\texited\tcom.docker.compose.project=openbot,com.docker.compose.service=agent-computer\topenbot_agent-profiles,openbot_agent-workspace".into(), + format!("c-other-namespace\tother-computer-knowledge\texited\t{}\tother-profile-knowledge", supervisor("knowledge", "other")), + "c-unlabeled\topenbot-computer-unlabeled\texited\t\topenbot-profile-unlabeled".into(), + "c-other-project\tanother-project-postgres-1\texited\tcom.docker.compose.project=another-project\t".into(), + format!("c-renamed\topenbot-computer-not-this-bot\texited\t{}\t", supervisor("general-assistant", "openbot")), + "c-legacy\topenbot-computer-legacy\texited\topenbot.supervisor=true,openbot.bot-id=legacy\topenbot-profile-legacy".into(), + ]; + std::fs::write( + root.join(".fixture-containers"), + lines.map(|line| format!("{line}\n")).concat(), + ) + .unwrap(); + } + + const OWNED_CONTAINERS: [&str; 2] = [ + "openbot-agent-computer-1", + "openbot-computer-general-assistant", + ]; + + #[test] + fn leftover_reset_removes_stopped_owned_containers_before_their_volumes() { + if crate::test_support::isolated_process( + "stack::tests::leftover_reset_removes_stopped_owned_containers_before_their_volumes", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + for address in computer_stop_addresses(&path) { + let root = path + .bin + .join(format!("{}-leftover-containers", address.engine.binary())); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + leftover_containers_fixture(&root, "exited"); + + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert_eq!( + found + .containers + .iter() + .map(|container| container.name.as_str()) + .collect::>(), + OWNED_CONTAINERS + ); + assert!(found.running.is_empty() && found.resettable()); + + let volumes = OWNED_LEFTOVERS.map(String::from).to_vec(); + let containers = OWNED_CONTAINERS.map(String::from).to_vec(); + for (stale_volumes, stale_containers) in [ + (volumes.clone(), Vec::new()), + (volumes.clone(), containers[..1].to_vec()), + ( + volumes.clone(), + [containers.clone(), vec!["other-computer-knowledge".into()]].concat(), + ), + (Vec::new(), containers.clone()), + ] { + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &stale_volumes, + &stale_containers, + ) + .expect_err("a changed set must not be removed"); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!std::fs::read_to_string(&record).unwrap().contains(" rm ")); + } + + reset_leftover_volumes(&address, &root, &Secrets::new(), &volumes, &containers) + .unwrap(); + let log = std::fs::read_to_string(&record).unwrap(); + let removals: Vec<_> = log + .lines() + .filter_map(|line| line.split_once('\t')) + .map(|(_, command)| { + command + .strip_prefix("--connection fixture-machine ") + .unwrap_or(command) + }) + .filter(|command| command.contains(" rm ")) + .collect(); + assert_eq!( + removals, + [ + "container rm c-compose", + "container rm c-computer", + "volume rm openbot-profile-general-assistant", + "volume rm openbot-workspace-general-assistant", + "volume rm openbot_agent-profiles", + "volume rm openbot_agent-workspace", + ] + ); + assert!(!log.contains("--force") && !log.contains("prune") && !log.contains("down")); + let kept = std::fs::read_to_string(root.join(".fixture-containers")).unwrap(); + let kept: Vec<_> = kept + .lines() + .map(|line| line.split('\t').next().unwrap()) + .collect(); + assert_eq!( + kept, + [ + "c-other-namespace", + "c-unlabeled", + "c-other-project", + "c-renamed", + "c-legacy" + ] + ); + } + } + + #[test] + fn a_running_owned_container_blocks_the_reset_and_removes_nothing() { + if crate::test_support::isolated_process( + "stack::tests::a_running_owned_container_blocks_the_reset_and_removes_nothing", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + let address = computer_stop_addresses(&path)[0].clone(); + let root = path.bin.join("leftover-running"); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + for status in ["running", "paused", "restarting"] { + leftover_containers_fixture(&root, status); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.running, ["openbot-computer-general-assistant"]); + assert!(!found.resettable(), "{status}"); + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &OWNED_LEFTOVERS.map(String::from), + &OWNED_CONTAINERS.map(String::from), + ) + .expect_err("a running container blocks the reset"); + assert!( + error.said.contains("openbot-computer-general-assistant") + && error.said.contains("still running") + && error.said.contains("Nothing was removed"), + "{}", + error.said + ); + assert!(!format!("{error:?}").contains("synthetic-secret")); + assert!(!std::fs::read_to_string(&record).unwrap().contains(" rm ")); + } + // A container listing that fails is not proof that nothing is running. + leftover_containers_fixture(&root, "exited"); + std::fs::write(root.join(".fixture-ps-failure"), "").unwrap(); + let error = leftover_volumes(&address, &root, &Secrets::new()).unwrap_err(); + assert!(!format!("{error:?}").contains("synthetic-secret")); + } + #[test] fn desktop_approval_transport_credential_reaches_only_the_server() { let secrets = Secrets::from([ @@ -5102,6 +5995,24 @@ fn main() { const FAKE_ENGINE_SOURCE: &str = r#" use std::io::Write; +/// `.fixture-containers`: one container per line, `idnamestatusk=v,k=vvolume,volume`. +struct FixtureContainer { id: String, name: String, status: String, labels: Vec<(String, String)>, mounts: Vec } + +fn fixture_containers() -> Vec { + let Ok(listing) = std::fs::read_to_string(".fixture-containers") else { return Vec::new() }; + listing.lines().filter(|line| !line.is_empty()).map(|line| { + let fields: Vec<&str> = line.split('\t').collect(); + let list = |field: &str| field.split(',').filter(|item| !item.is_empty()).map(str::to_owned).collect::>(); + FixtureContainer { + id: fields[0].into(), + name: fields[1].into(), + status: fields[2].into(), + labels: list(fields[3]).into_iter().map(|pair| { let (k, v) = pair.split_once('=').unwrap(); (k.to_owned(), v.to_owned()) }).collect(), + mounts: list(fields.get(4).copied().unwrap_or("")), + } + }).collect() +} + fn main() { let args: Vec = std::env::args().skip(1).collect(); let joined = args.join(" "); @@ -5123,15 +6034,71 @@ fn main() { (".fixture-volumes", ".fixture-volume-failure") } else if actual == ["volume", "inspect", "openbot_postgres-data"] { (".fixture-inspect", ".fixture-inspect-failure") - } else if actual == ["volume", "rm", "openbot_postgres-data"] { - if std::path::Path::new(".fixture-attached").exists() { + } else if actual.len() == 3 && actual[0] == "volume" && actual[1] == "inspect" { + let file = format!(".fixture-inspect-{}", actual[2]); + if !std::path::Path::new(&file).exists() { + eprintln!("no such volume: {}", actual[2]); + std::process::exit(1); + } + print!("{}", std::fs::read_to_string(file).unwrap()); + return; + } else if actual.len() == 3 && actual[0] == "volume" && actual[1] == "rm" { + let volume = actual[2].as_str(); + if (volume == "openbot_postgres-data" && std::path::Path::new(".fixture-attached").exists()) + || std::path::Path::new(&format!(".fixture-attached-{volume}")).exists() + || fixture_containers().iter().any(|container| container.mounts.iter().any(|mount| mount == volume)) + { eprintln!("synthetic-secret attached container"); std::process::exit(2); } let inventory = std::fs::read_to_string(".fixture-volumes").unwrap(); - let remaining: String = inventory.lines().filter(|name| *name != "openbot_postgres-data").map(|name| format!("{name}\n")).collect(); + let remaining: String = inventory.lines().filter(|name| *name != volume).map(|name| format!("{name}\n")).collect(); std::fs::write(".fixture-volumes", remaining).unwrap(); return; + } else if actual.first().map(String::as_str) == Some("ps") { + assert_eq!(actual[1..4], ["--all", "--quiet", "--no-trunc"], "{actual:?}"); + let wanted: Vec<(&str, &str)> = actual[4..].chunks(2).map(|pair| { + assert_eq!(pair[0], "--filter"); + pair[1].strip_prefix("label=").unwrap().split_once('=').unwrap() + }).collect(); + if std::path::Path::new(".fixture-ps-failure").exists() { + eprintln!("synthetic-secret-must-not-appear-in-diagnostics"); + std::process::exit(17); + } + for container in fixture_containers() { + if wanted.iter().all(|(key, value)| container.labels.iter().any(|(k, v)| k == key && v == value)) { + println!("{}", container.id); + } + } + return; + } else if actual.len() == 3 && actual[0] == "container" && actual[1] == "inspect" { + let Some(container) = fixture_containers().into_iter().find(|container| container.id == actual[2]) else { + eprintln!("no such container: {}", actual[2]); + std::process::exit(1); + }; + // Docker reports names with a leading slash; Podman does not. + let slash = if args.first().map(String::as_str) == Some("--connection") { "" } else { "/" }; + let labels: Vec = container.labels.iter().map(|(k, v)| format!("\"{k}\":\"{v}\"")).collect(); + let mounts: Vec = container.mounts.iter().map(|m| format!("{{\"Type\":\"volume\",\"Name\":\"{m}\"}}")).collect(); + print!( + "[{{\"Id\":\"{}\",\"Name\":\"{slash}{}\",\"State\":{{\"Status\":\"{}\",\"Running\":{}}},\"Config\":{{\"Labels\":{{{}}}}},\"Mounts\":[{}]}}]", + container.id, container.name, container.status, container.status == "running", labels.join(","), mounts.join(",") + ); + return; + } else if actual.len() == 3 && actual[0] == "container" && actual[1] == "rm" { + let containers = fixture_containers(); + let Some(container) = containers.iter().find(|container| container.id == actual[2]) else { + eprintln!("no such container: {}", actual[2]); + std::process::exit(1); + }; + if container.status == "running" { + eprintln!("synthetic-secret cannot remove a running container"); + std::process::exit(2); + } + let inventory = std::fs::read_to_string(".fixture-containers").unwrap(); + let remaining: String = inventory.lines().filter(|line| line.split('\t').next() != Some(actual[2].as_str())).map(|line| format!("{line}\n")).collect(); + std::fs::write(".fixture-containers", remaining).unwrap(); + return; } else { panic!("unexpected volume probe command: {actual:?}"); }; if std::path::Path::new(failure).exists() { eprintln!("synthetic-secret-must-not-appear-in-diagnostics"); diff --git a/desktop/src/App.test.tsx b/desktop/src/App.test.tsx index 98e42fe0ef..0623c05db1 100644 --- a/desktop/src/App.test.tsx +++ b/desktop/src/App.test.tsx @@ -1227,7 +1227,7 @@ function emptyConfiguration() { }; } -test("leftover database recovery requires confirmation and keeps setup ready to retry", async () => { +test("leftover database is offered as a fresh start that carries on into Start", async () => { useCompatibleEndpointSetup({}); const previous = invokeHandler; const reset = deferred(); @@ -1237,6 +1237,7 @@ test("leftover database recovery requires confirmation and keeps setup ready to throw { said: "A previous OpenBot database remains without its encryption key.", database_reset: "fixture_postgres-data", + leftover_reset: ["fixture_postgres-data"], }; if (command === "reset_leftover_database") return reset.promise; return previous(command, args); @@ -1249,49 +1250,97 @@ test("leftover database recovery requires confirmation and keeps setup ready to await userEvent.click(view.getByRole("button", { name: "Start OpenBot" })); const resets = () => invokeCalls.filter((call) => call.command === "reset_leftover_database"); - await userEvent.click( - await view.findByRole("button", { name: "Reset leftover database" }), - ); + const startFresh = await view.findByRole("button", { name: "Start fresh" }); + // A choice, not a failure: nothing announces that Start did not finish. + expect(view.queryByRole("alert")).toBeNull(); + expect(view.getByText(/saved chats, Bots, and settings/)).toBeTruthy(); expect(view.getByText(/permanently deletes/)).toBeTruthy(); + expect(view.getByText(/with its original encryption key/)).toBeTruthy(); + expect(view.getByText("fixture_postgres-data")).toBeTruthy(); expect(resets()).toHaveLength(0); - await userEvent.click(view.getByRole("button", { name: "Keep saved data" })); - expect(resets()).toHaveLength(0); - expect(view.queryByRole("button", { name: "Delete saved data" })).toBeNull(); - await userEvent.click( - view.getByRole("button", { name: "Reset leftover database" }), - ); - await userEvent.click( - view.getByRole("button", { name: "Delete saved data" }), - ); + await userEvent.click(startFresh); expect(resets()).toEqual([ { command: "reset_leftover_database", args: { root: "/tmp/openbot-app-test", - volume: "fixture_postgres-data", + volumes: ["fixture_postgres-data"], + containers: [], confirmed: true, }, }, ]); - expect( - view.getByRole("button", { name: "Deleting saved data…" }), - ).toHaveProperty("disabled", true); + expect(view.getByRole("button", { name: "Starting fresh…" })).toHaveProperty( + "disabled", + true, + ); expect(view.getByRole("button", { name: "Working…" })).toHaveProperty( "disabled", true, ); - await act(async () => reset.resolve()); - expect( - view.queryByRole("button", { name: "Reset leftover database" }), - ).toBeNull(); - expect(view.queryByRole("alert")).toBeNull(); expect(starts).toBe(1); - await userEvent.click(view.getByRole("button", { name: "Start OpenBot" })); + await act(async () => reset.resolve()); await view.findByRole("button", { name: "Ask" }); + expect(starts).toBe(2); expect(installationCalls()).toHaveLength(1); }); -test("database recovery failure is shown without restarting services", async () => { +test("starting fresh names every container and volume and removes only that list", async () => { + useCompatibleEndpointSetup({}); + const previous = invokeHandler; + const volumes = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + ]; + const containers = [ + "openbot-agent-computer-1", + "openbot-computer-general-assistant", + ]; + invokeHandler = async (command, args) => { + if (command === "start_stack") + throw { + said: "OpenBot found data from a previous installation.", + leftover_reset: volumes, + leftover_containers: containers, + }; + if (command === "reset_leftover_database") return null; + return previous(command, args); + }; + const view = await enterCompatibleEndpoint( + "https://model.example/v1", + "synthetic-key", + ); + await userEvent.click(view.getByRole("button", { name: "Continue" })); + await userEvent.click(view.getByRole("button", { name: "Start OpenBot" })); + const startFresh = await view.findByRole("button", { name: "Start fresh" }); + expect(view.queryByText(/saved chats/)).toBeNull(); + expect(view.queryByText(/encryption key/)).toBeNull(); + expect(view.getByText(/browser sign-ins/)).toBeTruthy(); + expect( + view.getByText(/Restore the previous installation’s folder/), + ).toBeTruthy(); + for (const volume of volumes) expect(view.getByText(volume)).toBeTruthy(); + expect(view.getByText(/stopped containers/)).toBeTruthy(); + for (const container of containers) + expect(view.getByText(container)).toBeTruthy(); + await userEvent.click(startFresh); + expect( + invokeCalls.filter((call) => call.command === "reset_leftover_database"), + ).toEqual([ + { + command: "reset_leftover_database", + args: { + root: "/tmp/openbot-app-test", + volumes, + containers, + confirmed: true, + }, + }, + ]); +}); + +test("a fresh start that cannot delete the leftovers is shown without starting", async () => { useCompatibleEndpointSetup({}); const previous = invokeHandler; invokeHandler = async (command, args) => { @@ -1313,12 +1362,10 @@ test("database recovery failure is shown without restarting services", async () await userEvent.click(view.getByRole("button", { name: "Continue" })); await userEvent.click(view.getByRole("button", { name: "Start OpenBot" })); await userEvent.click( - await view.findByRole("button", { name: "Reset leftover database" }), - ); - await userEvent.click( - view.getByRole("button", { name: "Delete saved data" }), + await view.findByRole("button", { name: "Start fresh" }), ); expect(await view.findByText(/database is still in use/)).toBeTruthy(); + expect(view.getByRole("alert")).toBeTruthy(); expect( invokeCalls.filter((call) => call.command === "start_stack"), ).toHaveLength(1); diff --git a/desktop/src/App.tsx b/desktop/src/App.tsx index 073a6f5e0b..b97e86de49 100644 --- a/desktop/src/App.tsx +++ b/desktop/src/App.tsx @@ -2,7 +2,7 @@ import { invoke } from "@tauri-apps/api/core"; import { listen } from "@tauri-apps/api/event"; import { useCallback, useEffect, useRef, useState } from "react"; import { Ask } from "./Ask"; -import { DatabaseReset } from "./DatabaseReset"; +import { StartFresh } from "./StartFresh"; import { DEFAULT_HARNESS, type HarnessChoice, @@ -296,6 +296,13 @@ export function App() { if (problem.connection === "organization") setRefreshOrganization(true); }, []); const displayedFailure = failure ?? recoveryFailure; + // An older shell offered only the database. The list is exactly what a reset may remove. + const leftoverVolumes = displayedFailure?.leftover_reset?.length + ? displayedFailure.leftover_reset + : displayedFailure?.database_reset + ? [displayedFailure.database_reset] + : []; + const leftoverContainers = displayedFailure?.leftover_containers ?? []; const credentialContext = useRef([ root, model, @@ -602,22 +609,27 @@ export function App() { } } - async function resetLeftoverDatabase(volume: string) { + /** The leftovers go first, then Start carries on: starting fresh is one choice, not two. */ + async function startFresh(volumes: string[], containers: string[]) { setBusy(true); try { await invoke("reset_leftover_database", { root: root.trim(), - volume, + volumes, + containers, confirmed: true, }); - setFailure(null); - setRecoveryFailure(null); - setSteps([]); } catch (error) { setFailure(asProblem(error)); + return; } finally { + // `start` sets `busy` itself, and its early return would never clear it. setBusy(false); } + setFailure(null); + setRecoveryFailure(null); + setSteps([]); + await start(); } function modelCanStart() { @@ -982,12 +994,14 @@ export function App() { {/* A failure outranks `running`. The supervisor gives up on a process and sends the window back here, and a heading that still says everything is running while the box underneath names the process that stopped is a screen arguing with itself. */} - {!running && displayedFailure?.database_reset && ( - 0 && ( + )} @@ -1260,7 +1274,10 @@ export function App() { - {displayedFailure && } + {/* Leftover data is a choice, offered above, not a failure to report here. */} + {displayedFailure && leftoverVolumes.length === 0 && ( + + )} {!running && !returningToInstallation && (
diff --git a/desktop/src/DatabaseReset.tsx b/desktop/src/DatabaseReset.tsx deleted file mode 100644 index 8d3dcd46a7..0000000000 --- a/desktop/src/DatabaseReset.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { useState } from "react"; - -/** Only rendered when native setup has identified a recoverable leftover database. */ -export function DatabaseReset({ - busy, - onReset, - volume, -}: { - busy: boolean; - volume: string; - onReset: (volume: string) => Promise; -}) { - const [confirming, setConfirming] = useState(false); - if (!confirming) { - return ( - - ); - } - return ( -
-

- Delete the previous installation’s data? -

-

- This permanently deletes the local OpenBot database, including saved - chats, Bots, and settings. Only continue if you want a fresh - installation and do not need that data. -

-

To keep your data, restore its original encryption key instead.

-
- - -
-
- ); -} diff --git a/desktop/src/Problem.tsx b/desktop/src/Problem.tsx index 723f93edee..72a10b1b9b 100644 --- a/desktop/src/Problem.tsx +++ b/desktop/src/Problem.tsx @@ -13,6 +13,10 @@ export type Problem = { detail?: string | null; connection?: "model" | "intelligence" | "organization" | null; database_reset?: string | null; + /** Every volume a confirmed fresh-install reset removes, the database included. */ + leftover_reset?: string[] | null; + /** Stopped containers that still hold those volumes, removed first by the same reset. */ + leftover_containers?: string[] | null; }; /** Anything thrown, as a problem. A bare string keeps working and reads as it always did. */ diff --git a/desktop/src/StartFresh.tsx b/desktop/src/StartFresh.tsx new file mode 100644 index 0000000000..5d788d0deb --- /dev/null +++ b/desktop/src/StartFresh.tsx @@ -0,0 +1,83 @@ +/** + * Only rendered when native setup has identified leftover data it can remove. + * + * A choice, not a failure. Somebody whose previous installation left data most likely deleted its + * folder to start over, so starting fresh is the main action and it carries on into Start. Keeping + * the data means restoring that folder, which this window cannot do for them, so it is explained + * rather than offered as a button. + */ +export function StartFresh({ + busy, + containers, + database, + onStartFresh, + volumes, +}: { + busy: boolean; + /** Stopped containers that still hold the volumes. Starting fresh removes them first. */ + containers: string[]; + /** Whether the local OpenBot database is one of the volumes. */ + database: boolean; + /** Exactly the volumes native setup verified, and the only ones starting fresh removes. */ + volumes: string[]; + onStartFresh: (volumes: string[], containers: string[]) => Promise; +}) { + const others = volumes.length > (database ? 1 : 0); + const left = + database && others + ? "its database of saved chats, Bots, and settings, and its Bot files and browser sign-ins" + : database + ? "its database of saved chats, Bots, and settings" + : "its Bot files and browser sign-ins"; + return ( +
+

+ A previous OpenBot installation left data here +

+

+ It includes {left}. A new installation does not reuse it. Start fresh + permanently deletes that data, then starts OpenBot. +

+
+ +
+

+ To keep that data instead, do not start. Restore the previous + installation’s folder + {database ? ", with its original encryption key," : ""} and choose it + under Installation options. +

+
+ What Start fresh deletes +
    + {volumes.map((volume) => ( +
  • + {volume} +
    • + ))} +
+ {containers.length > 0 && ( + <> +

+ It first removes these stopped containers from the previous + installation, which still use those volumes: +

+
    + {containers.map((container) => ( +
  • + {container} +
    • + ))} +
+ + )} +
+
+ ); +}