fix: stray java runtime tmps and version files (#724)

* feat: added removal of .tmp files on launcher start after 10 minutes of being untouched; added removal of java runtime files whenever user deletes java runtime inside java settings (before they were only being deleted from the database, not files)

* feat: added modal for removing java runtimes

---------

Co-authored-by: LynithDev <61880709+LynithDev@users.noreply.github.com>

packages/oneclient_app/src/hooks/actions.rs

@@ -482,11 +482,13 @@ impl Actions {
                     events.signal(oneclient_events::Signal::JavaChanged);
                     invalidate_java_queries().await
                 },
-                Err(err) => events
+                Err(err) => {
+					events
                     .notify("Java install failed")
                     .body(err.to_string())
                     .error()
-                    .send(),
+                    .send()
+				}
             }
         });
     }

packages/oneclient_app/src/view/app/settings/java.rs

@@ -1,8 +1,10 @@
+use std::path::Path;
+
 use freya::prelude::*;
-use oneclient_java::{JavaRuntime, JavaVendor};
+use oneclient_java::{JavaRuntime, JavaVendor, is_launcher_managed};
 
 use super::settings_page;
-use crate::components::{Button, Icon, IconType, JavaInstallManager, ScrollArea};
+use crate::components::{Button, Icon, IconType, JavaInstallManager, OverlayPopup, ScrollArea};
 use crate::hooks::{Actions, java_runtimes, use_dispatch, use_java_runtimes};
 use crate::invalidate_java_queries;
 use crate::theme::colors;
@@ -15,9 +17,11 @@ pub struct SettingsJava;
 impl Component for SettingsJava {
     fn render(&self) -> impl IntoElement {
         let dispatch = use_dispatch();
+        let removing_dispatch = dispatch.clone();
         let runtimes_query = use_java_runtimes();
         let runtimes = java_runtimes(&runtimes_query);
         let mut show_manager = use_state(|| false);
+        let pending_remove = use_state(|| None::<PendingRemove>);
 
         fn invalidate_runtimes(dispatch: Actions) {
             spawn(async move {
@@ -35,25 +39,8 @@ impl Component for SettingsJava {
         let mut shell = settings_page()
             .child(section_header("ADD RUNTIME"))
             .child(AddRow { show_manager }.into_element())
-            .child(
-                rect()
-                    .width(Size::Fill)
-                    .direction(Direction::Horizontal)
-                    .main_align(Alignment::SpaceBetween)
-                    .cross_align(Alignment::Center)
-                    .child(section_header("INSTALLED RUNTIMES"))
-                    .child(
-                        Button::new()
-                            .secondary()
-                            .small()
-                            .enabled(false) // disabled for now
-                            .on_press(move |_| {
-                                invalidate_runtimes(refresh_dispatch.clone());
-                            })
-                            .child(label().text("Refresh"))
-                    )
-            )
-            .child(runtimes_table(runtimes));
+            .child(section_header("INSTALLED RUNTIMES"))
+            .child(runtimes_table(runtimes, pending_remove));
 
         if *show_manager.read() {
             shell = shell.child(
@@ -67,6 +54,11 @@ impl Component for SettingsJava {
             );
         }
 
+        let pending = pending_remove.read().clone();
+        if let Some(target) = pending {
+            shell = shell.child(confirm_remove_modal(removing_dispatch, pending_remove, target))
+        }
+
         shell.into_element()
     }
 }
@@ -115,7 +107,10 @@ impl Component for AddRow {
     }
 }
 
-fn runtimes_table(runtimes: Vec<JavaRuntime>) -> impl IntoElement {
+fn runtimes_table(
+    runtimes: Vec<JavaRuntime>,
+    pending_remove: State<Option<PendingRemove>>,
+) -> impl IntoElement {
     if runtimes.is_empty() {
         return rect()
             .width(Size::fill())
@@ -146,6 +141,7 @@ fn runtimes_table(runtimes: Vec<JavaRuntime>) -> impl IntoElement {
             RuntimeRow {
                 runtime,
                 last: idx + 1 == count,
+                pending_remove
             }
             .into_element(),
         );
@@ -195,11 +191,11 @@ fn table_header() -> impl IntoElement {
 struct RuntimeRow {
     runtime: JavaRuntime,
     last: bool,
+    pending_remove: State<Option<PendingRemove>>
 }
 
 impl Component for RuntimeRow {
     fn render(&self) -> impl IntoElement {
-        let dispatch = use_dispatch();
         let runtime = &self.runtime;
         let path = runtime.absolute_path.clone();
 
@@ -271,7 +267,7 @@ impl Component for RuntimeRow {
                             ),
                     ),
             )
-            .child(remove_button(dispatch, path))
+            .child(remove_button(self.pending_remove, path))
     }
 }
 
@@ -279,11 +275,108 @@ fn path_content_width(path: &str) -> f32 {
     (path.chars().count() as f32 * 7.0).max(1.0)
 }
 
-fn remove_button(dispatch: Actions, path: String) -> impl IntoElement {
+#[derive(Clone, PartialEq)]
+struct PendingRemove {
+    path: String,
+    /// Whether the files live in OneClient's own java dir, which is the only
+    /// case where removal takes them off disk
+    managed: bool,
+}
+
+fn confirm_remove_modal(
+    dispatch: Actions,
+    mut pending: State<Option<PendingRemove>>,
+    target: PendingRemove,
+) -> impl IntoElement {
+    let PendingRemove { path, managed } = target;
+    let remove_path = path.clone();
+
+    let consequence = if managed {
+        "This runtime is managed by OneClient. It's files WILL be deleted."
+    } else {
+        "This runtime is not managed by OneClient. It's files will not be deleted."
+    };
+
+    let confirm = if managed {
+        Button::new().danger().text("Remove and delete files")
+    } else {
+        Button::new().primary().text("Remove from list")
+    };
+
+     OverlayPopup::new()
+        .on_close(move |()| pending.set(None))
+        .child(
+            rect()
+                .width(Size::window_percent(100.))
+                .height(Size::window_percent(100.))
+                .center()
+                .child(
+                    rect()
+                        .vertical()
+                        .width(Size::px(440.))
+                        .max_width(Size::window_percent(90.))
+                        .spacing(14.)
+                        .padding(Gaps::new_all(20.))
+                        .corner_radius(CornerRadius::new_all(14.))
+                        .background(colors::page_elevated())
+                        .child(
+                            label()
+                                .text("Remove Java Runtime?")
+                                .font_size(16.)
+                                .font_weight(FontWeight::SEMI_BOLD)
+                                .color(colors::fg_primary()),
+                        )
+                        .child(
+                            label()
+                                .text(consequence)
+                                .font_size(12.)
+                                .max_lines(4)
+                                .width(Size::fill())
+                                .color(colors::fg_secondary()),
+                        )
+                        .child(
+                            label()
+                                .text(path)
+                                .font_size(12.)
+                                .max_lines(3)
+                                .width(Size::fill())
+                                .color(colors::fg_secondary()),
+                        )
+                        .child(
+                            rect()
+                                .horizontal()
+                                .width(Size::fill())
+                                .main_align(Alignment::End)
+                                .spacing(8.)
+                                .child(
+                                    Button::new()
+                                        .secondary()
+                                        .on_press(move |_| pending.set(None))
+                                        .text("Cancel"),
+                                )
+                                .child(confirm.on_press(move |_| {
+                                    dispatch.remove_java_runtime(remove_path.clone());
+                                    pending.set(None);
+                                })),
+                        ),
+                )
+        )
+        .into_element()
+}
+
+fn remove_button(
+    mut pending_remove: State<Option<PendingRemove>>,
+    path: String,
+) -> impl IntoElement {
     Button::new()
         .ghost()
         .small()
-        .on_press(move |_| dispatch.remove_java_runtime(path.clone()))
+        .on_press(move |_| {
+            pending_remove.set(Some(PendingRemove {
+                managed: is_launcher_managed(Path::new(&path)),
+                path: path.clone(),
+            }));
+        })
         .child(
             Icon::new(IconType::Trash01)
                 .size(14.)

packages/oneclient_core/src/state.rs

@@ -108,9 +108,27 @@ impl LauncherState {
 	}
 }
 
+/// Java archives are the only downloads big enough for a leaked scratch file to
+/// matter and they land flat in the java dir so this needs no recursion
+async fn sweep_java_scratch_files() {
+	let dir = match paths::java_dir() {
+		Ok(dir) => dir,
+		Err(err) => {
+			tracing::warn!("could not resolve the java dir to sweep: {err:#}");
+			return;
+		}
+	};
+
+	if let Err(err) = polyio::sweep_temp_files(&dir).await {
+		tracing::warn!("java scratch file sweep failed: {err:#}");
+	}
+}
+
 pub fn run_startup_tasks(state: &Arc<LauncherState>) {
 	let background = Arc::clone(state);
 	tokio::spawn(async move {
+			sweep_java_scratch_files().await;
+
 			let recovery = match crate::recovery::reconstruct_from_disk(&background).await {
 				Ok(report) => report,
 				Err(err) => {

packages/oneclient_java/src/install.rs

@@ -92,6 +92,60 @@ pub async fn install_package(
 	Ok(executable)
 }
 
+/// The directory this crate extracted for `executable`, or `None` when the
+/// runtime sits outside the launcher's java dir which is where a folder the
+/// user added themselves points
+#[tracing::instrument(level = "debug")]
+fn managed_install_root(executable: &Path) -> JavaResult<Option<PathBuf>> {
+	let java_dir = paths::java_dir()?;
+
+	let Ok(root) = polyio::canonicalize(&java_dir) else {
+		return Ok(None);
+	};
+
+	// Refuses to resolve once the executable is gone which is the safe answer
+	// there is no way left to prove what the path pointed at
+	let canon = match polyio::ensure_under(executable, [&root]) {
+		Ok(Some(canon)) => canon,
+		Ok(None) => return Ok(None),
+		Err(err) => {
+			tracing::debug!("could not resolve the java runtime path: {err}");
+			return Ok(None);
+		}
+	};
+
+	let Ok(relative) = canon.strip_prefix(&root) else {
+		return Ok(None);
+	};
+
+	match relative.components().next() {
+		Some(std::path::Component::Normal(name)) => Ok(Some(root.join(name))),
+		_ => Ok(None),
+	}
+}
+
+/// Whether [`remove_installed_package`] would take this runtime's files with
+/// it so the UI can say up front what removing it costs
+#[must_use]
+pub fn is_launcher_managed(executable: &Path) -> bool {
+	managed_install_root(executable).is_ok_and(|root| root.is_some())
+}
+
+/// Only ever touches OneClient's own java dir a runtime the user added from
+/// their own folder is left on disk untouched
+#[tracing::instrument(level = "debug")]
+pub async fn remove_installed_package(executable: &Path) -> JavaResult<bool> {
+	let Some(install_root) = managed_install_root(executable)? else {
+		tracing::debug!("java runtime is not launcher-managed leaving its files alone");
+		return Ok(false);
+	};
+
+	polyio::remove_dir_all(&install_root).await?;
+
+	tracing::info!(path = %install_root.display(), "removed installed Java runtime files");
+	Ok(true)
+}
+
 #[tracing::instrument(level = "debug")]
 fn resolve_installed_executable(extract_root: &Path, package: &JavaPackage) -> PathBuf {
     let mut base_path = extract_root.to_path_buf();
@@ -163,3 +217,105 @@ fn stem_without_archive(name: &str) -> String {
 		stem.to_string()
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use std::sync::atomic::{AtomicU32, Ordering};
+
+	/// Keeps the whole suite inside a temp tree the override is a `OnceLock` so
+	/// the first caller wins and every case works under one java dir
+	fn java_dir() -> PathBuf {
+		static ONCE: std::sync::Once = std::sync::Once::new();
+		ONCE.call_once(|| {
+			paths::set_launcher_dir(
+				std::env::temp_dir().join(format!("oneclient-java-test-{}", std::process::id())),
+			);
+		});
+
+		let dir = paths::java_dir().unwrap();
+		std::fs::create_dir_all(&dir).unwrap();
+		dir
+	}
+
+	fn install_runtime(root: &Path) -> (PathBuf, PathBuf) {
+		static N: AtomicU32 = AtomicU32::new(0);
+		let install = root.join(format!("zulu21-{}", N.fetch_add(1, Ordering::Relaxed)));
+		let bin = install.join("zulu21.0.12").join("bin");
+		std::fs::create_dir_all(&bin).unwrap();
+
+		let executable = bin.join("java");
+		std::fs::write(&executable, b"").unwrap();
+		(install, executable)
+	}
+
+	#[test]
+	fn resolves_the_extracted_directory_not_the_executable_parent() {
+		let java_dir = java_dir();
+		let (install, executable) = install_runtime(&java_dir);
+
+		let resolved = managed_install_root(&executable).unwrap().unwrap();
+
+		assert_eq!(resolved, polyio::canonicalize(&install).unwrap());
+	}
+
+	#[tokio::test]
+	async fn removing_a_managed_runtime_deletes_its_files() {
+		let java_dir = java_dir();
+		let (install, executable) = install_runtime(&java_dir);
+
+		assert!(remove_installed_package(&executable).await.unwrap());
+
+		assert!(!install.exists());
+		assert!(java_dir.exists(), "the java dir itself must survive");
+	}
+
+	#[tokio::test]
+	async fn a_runtime_outside_the_java_dir_keeps_its_files() {
+		let java_dir = java_dir();
+		let elsewhere = java_dir.parent().unwrap().join("user-picked-jdk");
+		let (_, executable) = install_runtime(&elsewhere);
+
+		assert_eq!(managed_install_root(&executable).unwrap(), None);
+		assert!(!remove_installed_package(&executable).await.unwrap());
+		assert!(executable.exists(), "a folder the user added is not ours to delete");
+
+		std::fs::remove_dir_all(&elsewhere).unwrap();
+	}
+
+	#[test]
+	fn a_traversal_back_out_of_the_java_dir_is_not_managed() {
+		let java_dir = java_dir();
+		let elsewhere = java_dir.parent().unwrap().join("traversal-jdk");
+		let (_, executable) = install_runtime(&elsewhere);
+
+		let sneaky = java_dir
+			.join("..")
+			.join("traversal-jdk")
+			.join(executable.strip_prefix(&elsewhere).unwrap());
+
+		assert_eq!(managed_install_root(&sneaky).unwrap(), None);
+
+		std::fs::remove_dir_all(&elsewhere).unwrap();
+	}
+
+	#[test]
+	fn the_managed_flag_matches_what_removal_would_actually_delete() {
+		let java_dir = java_dir();
+		let (_, ours) = install_runtime(&java_dir);
+		let elsewhere = java_dir.parent().unwrap().join("flagged-jdk");
+		let (_, theirs) = install_runtime(&elsewhere);
+
+		assert!(is_launcher_managed(&ours));
+		assert!(!is_launcher_managed(&theirs));
+
+		std::fs::remove_dir_all(&elsewhere).unwrap();
+	}
+
+	#[test]
+	fn the_java_dir_itself_is_never_managed() {
+		let java_dir = java_dir();
+
+		assert_eq!(managed_install_root(&java_dir).unwrap(), None);
+	}
+}

packages/oneclient_java/src/lib.rs

@@ -16,7 +16,7 @@ pub mod vendors;
 pub use checker::{JavaCheckInfo, check_java_runtime};
 pub use data::{JavaPackage, JavaRuntime, PackageArchive, java_executable_relative_path};
 pub use error::{JavaError, JavaResult};
-pub use install::install_package;
+pub use install::{install_package, is_launcher_managed};
 pub use locate::{LocatedJava, best_for_major, locate_java};
 pub use platform::{HostArch, HostOs, HostTarget};
 pub use service::{

packages/oneclient_java/src/service.rs

@@ -155,7 +155,17 @@ impl JavaService {
 	#[tracing::instrument(skip(self))]
 	pub async fn remove_runtime(&self, absolute_path: &str) -> JavaResult<()> {
 		self.store.delete_by_path(absolute_path).await?;
-		tracing::info!("removed Java runtime");
+
+		let removed_files =
+			match crate::install::remove_installed_package(Path::new(absolute_path)).await {
+				Ok(removed) => removed,
+				Err(err) => {
+					tracing::warn!("could not remove the installed Java files: {err:#}");
+					false
+				}
+			};
+
+		tracing::info!(removed_files, "removed Java runtime");
 		Ok(())
 	}
 

packages/polyio/src/file.rs

@@ -1,4 +1,5 @@
 use std::sync::atomic::{AtomicU64, Ordering};
+use std::time::Duration;
 use std::{fs::Metadata, path::{Path, PathBuf}};
 
 use async_tempfile::{TempDir, TempFile};
@@ -318,8 +319,94 @@ fn temp_sibling(path: &Path) -> PathBuf {
 	}
 }
 
-/// Readers see either the old contents or the complete new ones
-/// The fsync before the rename stops a crash leaving a correctly-named zero-length file
+// Time for a file to be flagged as abandoned
+const STALE_TEMP_AGE: Duration = Duration::from_secs(15 * 60);
+
+fn temp_sibling_pid(name: &str) -> Option<u32> {
+	let inner = name.strip_prefix('.')?.strip_suffix(".tmp")?;
+	let (head, counter) = inner.rsplit_once('.')?;
+	let (stem, pid) = head.rsplit_once('.')?;
+
+	counter.parse::<u64>().ok()?;
+	if stem.is_empty() {
+		return None;
+	}
+
+	pid.parse().ok()
+}
+
+/// Returns the number of bytes reclaimed non-recursive and never an error for a
+/// missing directory
+#[tracing::instrument(
+    level = "debug",
+    skip(dir),
+    fields(dir = %dir.as_ref().display())
+)]
+pub async fn sweep_temp_files(dir: impl AsRef<Path>) -> PolyIOResult<u64> {
+	let dir = dir.as_ref();
+	let path_err = |source| IOError::PathIOError {
+		source,
+		path: dir.to_string_lossy().to_string(),
+	};
+
+	let mut entries = match tokio::fs::read_dir(dir).await {
+		Ok(entries) => entries,
+		// Nothing has ever been written here so there is nothing to sweep
+		Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(0),
+		Err(err) => return Err(path_err(err)),
+	};
+
+	let current_pid = std::process::id();
+	let mut files = 0usize;
+	let mut bytes = 0u64;
+
+	while let Some(entry) = entries.next_entry().await.map_err(path_err)? {
+		let name = entry.file_name();
+		let Some(pid) = name.to_str().and_then(temp_sibling_pid) else {
+			continue;
+		};
+
+		if pid == current_pid {
+			continue;
+		}
+
+		let Ok(metadata) = entry.metadata().await else {
+			continue;
+		};
+		if !metadata.is_file() {
+			continue;
+		}
+
+		let stale = metadata
+			.modified()
+			.ok()
+			.and_then(|modified| modified.elapsed().ok())
+			.is_some_and(|age| age >= STALE_TEMP_AGE);
+		if !stale {
+			continue;
+		}
+
+		let size = metadata.len();
+		match tokio::fs::remove_file(entry.path()).await {
+			Ok(()) => {
+				files += 1;
+				bytes += size;
+			}
+			// Another process may have just renamed it out from under us
+			Err(err) => tracing::debug!(
+				path = %entry.path().display(),
+				"could not remove stale scratch file: {err}"
+			),
+		}
+	}
+
+	if files > 0 {
+		tracing::info!(files, bytes, dir = %dir.display(), "removed stale scratch files");
+	}
+
+	Ok(bytes)
+}
+
 #[tracing::instrument(
     level = "debug",
     skip(path, data),
@@ -383,10 +470,6 @@ pub async fn write_json_atomic<T: Serialize>(
 	write_atomic(path, bytes).await
 }
 
-/// Traversal guard for externally supplied paths
-/// `Ok(None)` when the path resolves outside every root
-/// `Err` only when `path` cannot be canonicalised
-/// Roots that cannot be canonicalised (e.g. not yet created) are skipped
 #[tracing::instrument(
     level = "debug",
     skip(path, roots),
@@ -396,8 +479,6 @@ pub fn ensure_under<R: AsRef<Path>>(
 	path: impl AsRef<Path>,
 	roots: impl IntoIterator<Item = R>,
 ) -> PolyIOResult<Option<PathBuf>> {
-	// Not `std::fs::canonicalize` its Windows `\\?\` UNC output would never
-	// `starts_with` the plain-path roots so everything would look like an escape
 	let canon = crate::canonicalize(path)?;
 
 	for root in roots {
@@ -411,9 +492,6 @@ pub fn ensure_under<R: AsRef<Path>>(
 	Ok(None)
 }
 
-/// `exclude_top` applies only at the top level a nested directory of the same
-/// name is still copied
-/// Symlinks are followed and copied as their contents
 #[tracing::instrument(level = "debug", skip(exclude_top))]
 pub async fn copy_dir(src: &Path, dst: &Path, exclude_top: &[&str]) -> PolyIOResult<()> {
 	let mut stack: Vec<(PathBuf, PathBuf, bool)> =
@@ -723,6 +801,69 @@ mod tests {
 		std::fs::remove_dir_all(&dir).unwrap();
 	}
 
+	/// Ages a file past [`STALE_TEMP_AGE`] so the sweep treats it as abandoned
+	fn backdate(path: &Path) {
+		let stale = std::time::SystemTime::now() - (STALE_TEMP_AGE + Duration::from_secs(60));
+		let file = std::fs::File::options().write(true).open(path).unwrap();
+		file.set_times(std::fs::FileTimes::new().set_modified(stale))
+			.unwrap();
+	}
+
+	#[test]
+	fn temp_sibling_names_round_trip() {
+		let name = temp_sibling(Path::new("/java/zulu21.zip"));
+		let name = name.file_name().unwrap().to_str().unwrap();
+
+		assert_eq!(temp_sibling_pid(name), Some(std::process::id()));
+
+		// Files polyio did not write
+		assert_eq!(temp_sibling_pid("zulu21.zip"), None);
+		assert_eq!(temp_sibling_pid(".vimrc.tmp"), None);
+		assert_eq!(temp_sibling_pid(".cache.notapid.7.tmp"), None);
+		assert_eq!(temp_sibling_pid(".cache.4242.notacounter.tmp"), None);
+	}
+
+	#[tokio::test]
+	async fn sweep_removes_only_abandoned_scratch_files() {
+		let dir = scratch("sweep");
+
+		let abandoned = dir.join(format!(".zulu21.zip.{}.0.tmp", std::process::id() + 1));
+		std::fs::write(&abandoned, b"half a runtime").unwrap();
+		backdate(&abandoned);
+
+		// Same shape but young enough to still have a writer behind it
+		let in_flight = dir.join(format!(".zulu17.zip.{}.0.tmp", std::process::id() + 2));
+		std::fs::write(&in_flight, b"downloading").unwrap();
+
+		// Ours, however old the launcher has been up
+		let ours = temp_sibling(&dir.join("zulu8.zip"));
+		std::fs::write(&ours, b"mine").unwrap();
+		backdate(&ours);
+
+		// Not a scratch file at all
+		let keep = dir.join("zulu21.zip");
+		std::fs::write(&keep, b"a real archive").unwrap();
+		backdate(&keep);
+
+		let freed = sweep_temp_files(&dir).await.unwrap();
+
+		assert_eq!(freed, "half a runtime".len() as u64);
+		assert!(!abandoned.exists());
+		assert!(in_flight.exists());
+		assert!(ours.exists());
+		assert!(keep.exists());
+
+		std::fs::remove_dir_all(&dir).unwrap();
+	}
+
+	#[tokio::test]
+	async fn sweep_ignores_a_missing_directory() {
+		let dir = scratch("sweep-missing");
+		std::fs::remove_dir_all(&dir).unwrap();
+
+		assert_eq!(sweep_temp_files(&dir).await.unwrap(), 0);
+	}
+
 	fn failing_stream(
 		chunks: Vec<&'static [u8]>,
 	) -> impl futures_lite::Stream<Item = Result<bytes::Bytes, IOError>> + Unpin + Send {