diff --git a/core/archipelago/src/api/rpc/package/async_lifecycle.rs b/core/archipelago/src/api/rpc/package/async_lifecycle.rs index 17dd0d51..52d86302 100644 --- a/core/archipelago/src/api/rpc/package/async_lifecycle.rs +++ b/core/archipelago/src/api/rpc/package/async_lifecycle.rs @@ -114,6 +114,16 @@ impl RpcHandler { Err(e) => { error!("package.install {} failed: {:#}", package_id_spawn, e); install_log(&format!("INSTALL FAIL: {} — {:#}", package_id_spawn, e)).await; + // handle_package_install saves the catalog-provided + // dynamic app config to /var/lib/archipelago/app-configs + // BEFORE the install pipeline runs, so a failure can + // strand that file (and the optimistic state entry) with + // no container behind it. Probe once here; both cleanup + // branches below only fire when the app has no footprint. + // A retry re-saves the config (the frontend sends + // containerConfig on every install), so removal is safe. + let left_container = + failed_install_left_container(&handler, &package_id_spawn).await; // Dependency-gate rejections happen BEFORE any resource // (container/image/data dir) exists for this package, so // keeping the optimistic entry would leave a phantom @@ -123,30 +133,47 @@ impl RpcHandler { // surface the reason as a notification instead. if let Some(gate) = e.downcast_ref::() { - let (mut data, _) = handler.state_manager.get_snapshot().await; - data.package_data.remove(&package_id_spawn); - data.notifications.push(crate::data_model::Notification { - id: format!("install-deps-{package_id_spawn}"), - level: crate::data_model::NotificationLevel::Error, - title: format!("Could not install {package_id_spawn}"), - message: gate.to_string(), - timestamp: chrono::Utc::now().to_rfc3339(), - app_id: Some(package_id_spawn.clone()), - }); - while data.notifications.len() > 20 { - data.notifications.remove(0); + if !left_container { + remove_dynamic_app_config(&package_id_spawn).await; } - handler.state_manager.update_data(data).await; + remove_entry_with_notification( + &handler, + &package_id_spawn, + "install-deps", + &gate.to_string(), + ) + .await; return; } - // Don't remove the entry — that's what made the card + // A failed install that left NO container behind has no + // real footprint either — keeping the entry would leave + // the same phantom "Stopped" tile in My Apps (and the + // scanner-side absence eviction takes 3 scans to catch + // it). Remove the saved config + entry and surface the + // failure as a notification, exactly like the gate case. + if !left_container { + remove_dynamic_app_config(&package_id_spawn).await; + remove_entry_with_notification( + &handler, + &package_id_spawn, + "install-failed", + &format!("Install failed: {:#}", e), + ) + .await; + return; + } + // A container exists (crash-after-start kept for + // visibility, retry over an existing install, upgrade) — + // don't remove the entry, that's what made the card // vanish from My Apps mid-install / between retry-loop // attempts (e.g. tailscale's entrypoint failure). Leave // the entry visible with state=Stopped + the install // error in install_progress.message so the user can see // what went wrong and decide whether to retry or // uninstall. clear_install_progress would erase the - // message, so we set it explicitly here instead. + // message, so we set it explicitly here instead. The + // phase is cleared (None) so no stale InstallPhase + // lingers on the card. let err_msg = format!("Install failed: {:#}", e); let (mut data, _) = handler.state_manager.get_snapshot().await; if let Some(entry) = data.package_data.get_mut(&package_id_spawn) { @@ -384,6 +411,77 @@ async fn flip_to_installing(state_manager: &StateManager, package_id: &str) { state_manager.update_data(data).await; } +/// True when the failed install still has a real footprint: any container +/// belonging to `package_id` exists (any state — created/exited count too; +/// the install-crash path deliberately keeps the exited container visible), +/// or the app carries a user-stopped marker (Quadlet units run with `--rm`, +/// so a cleanly user-stopped app legitimately has no podman record). Errors +/// from the podman probe count as "exists" — never clean up on an uncertain +/// reading. +async fn failed_install_left_container(handler: &RpcHandler, package_id: &str) -> bool { + if crate::crash_recovery::load_user_stopped(&handler.config.data_dir) + .await + .contains(package_id) + { + return true; + } + match super::config::get_containers_for_app(package_id).await { + Ok(containers) => !containers.is_empty(), + Err(e) => { + warn!( + "install cleanup {}: container probe failed ({:#}); keeping saved config", + package_id, e + ); + true + } + } +} + +/// Remove the catalog-provided dynamic app config that +/// `handle_package_install` saved before the pipeline ran (mirror of the +/// write in install.rs). Only called when the app has no container — for an +/// existing install (retry/upgrade) the file is still the app's live runtime +/// config and must be kept. +async fn remove_dynamic_app_config(package_id: &str) { + let config_path = format!("/var/lib/archipelago/app-configs/{}.json", package_id); + match tokio::fs::remove_file(&config_path).await { + Ok(()) => info!( + "Removed dynamic app config for {} after failed install (no container)", + package_id + ), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => warn!( + "Failed to remove dynamic app config for {}: {}", + package_id, e + ), + } +} + +/// Remove the package's optimistic state entry (clearing any pending install +/// phase with it) so the card reverts to installable, and surface the failure +/// reason as an error notification instead. +async fn remove_entry_with_notification( + handler: &RpcHandler, + package_id: &str, + id_prefix: &str, + message: &str, +) { + let (mut data, _) = handler.state_manager.get_snapshot().await; + data.package_data.remove(package_id); + data.notifications.push(crate::data_model::Notification { + id: format!("{id_prefix}-{package_id}"), + level: crate::data_model::NotificationLevel::Error, + title: format!("Could not install {package_id}"), + message: message.to_string(), + timestamp: chrono::Utc::now().to_rfc3339(), + app_id: Some(package_id.to_string()), + }); + while data.notifications.len() > 20 { + data.notifications.remove(0); + } + handler.state_manager.update_data(data).await; +} + /// Flip an existing entry's state and return the pre-flip value (or None if /// no entry existed). Used for revert-on-failure. async fn flip_package_state(