Merge pull request #696 from dotCooCoo/fix/update-messages

update: say whether an update is installing, ready, installed or failed

crates/update/src/dialog.rs

@@ -35,6 +35,9 @@ pub struct StartupPrompt {
     /// [`StartupPrompt::set_note`] and [`StartupPrompt::set_message`] keep only one.
     pub status_note: Option<Note>,
     pub is_installing: bool,
+    /// The update is ready and waits for the app to close: the main button reads "Restart to
+    /// Update" and answers [`StartupPromptResponse::UpdateAndRestart`].
+    pub is_ready: bool,
     /// Puts the card's words in the interface language ([`crate::translate`]); English by default.
     pub translate: Translate,
 }
@@ -52,6 +55,7 @@ impl StartupPrompt {
             status_message: None,
             status_note: None,
             is_installing: false,
+            is_ready: false,
             translate: english,
         }
     }
@@ -102,7 +106,7 @@ impl StartupPrompt {
         }
 
         let pal = Palette::new(ctx, self.app);
-        let Palette { is_dark, card_bg, border_color, text_primary, text_muted, fill, on_fill } = pal;
+        let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, alert, alert_bg, .. } = pal;
         let t = self.translate;
         let version = self.release.version_label();
 
@@ -126,16 +130,16 @@ impl StartupPrompt {
                         ui.add_space(8.0);
 
                         if let Some(msg) = self.message() {
-                            let alert_bg = if is_dark { Color32::from_rgb(50, 30, 20) } else { Color32::from_rgb(255, 242, 235) };
-                            let alert_color = Color32::from_rgb(225, 95, 30);
                             egui::Frame::new()
                                 .fill(alert_bg)
-                                .stroke(egui::Stroke::new(1.0, alert_color))
+                                .stroke(egui::Stroke::new(1.0, alert))
                                 .corner_radius(egui::CornerRadius::ZERO)
                                 .inner_margin(egui::Margin::symmetric(10, 6))
                                 .show(ui, |ui| {
                                     ui.set_width(ui.available_width());
-                                    ui.label(RichText::new(msg).size(12.0).color(alert_color));
+                                    bounded(ui, "craft_update.prompt_message", |ui| {
+                                        ui.label(RichText::new(msg).size(12.0).color(alert));
+                                    });
                                 });
                             ui.add_space(4.0);
                         }
@@ -190,13 +194,13 @@ impl StartupPrompt {
                                         response = StartupPromptResponse::Later;
                                     }
                                 } else {
-                                    if self.is_self_updating {
-                                        let update_btn = egui::Button::new(
-                                            RichText::new(button(t, "Update and Restart")).size(10.5).strong().monospace().color(on_fill),
-                                        )
-                                        .fill(fill)
-                                        .corner_radius(egui::CornerRadius::ZERO)
-                                        .min_size(Vec2::new(140.0, 26.0));
+                                    if self.is_self_updating || self.is_ready {
+                                        let label = if self.is_ready { "Restart to Update" } else { "Update and Restart" };
+                                        let update_btn =
+                                            egui::Button::new(RichText::new(button(t, label)).size(10.5).strong().monospace().color(on_fill))
+                                                .fill(fill)
+                                                .corner_radius(egui::CornerRadius::ZERO)
+                                                .min_size(Vec2::new(140.0, 26.0));
                                         if ui.add(update_btn).clicked() {
                                             response = StartupPromptResponse::UpdateAndRestart;
                                         }
@@ -243,6 +247,10 @@ struct Palette {
     fill: Color32,
     /// Text on `fill`.
     on_fill: Color32,
+    /// Errors and failure reasons, on the card and on `alert_bg`.
+    alert: Color32,
+    /// The box around a message in the prompt.
+    alert_bg: Color32,
 }
 
 impl Palette {
@@ -257,6 +265,8 @@ impl Palette {
             text_muted: if is_dark { Color32::from_rgb(150, 150, 160) } else { Color32::from_rgb(105, 105, 115) },
             fill,
             on_fill: ink_on(fill),
+            alert: if is_dark { Color32::from_rgb(225, 95, 30) } else { Color32::from_rgb(184, 66, 8) },
+            alert_bg: if is_dark { Color32::from_rgb(44, 26, 18) } else { Color32::from_rgb(255, 242, 235) },
         }
     }
 }
@@ -352,18 +362,24 @@ pub enum CheckResultResponse {
     None,
     /// OK, the close button, Esc or a click outside: the app stops showing the dialog.
     Close,
-    /// Failed state only: the app starts another requested check.
+    /// Failed states only: the app starts another requested check.
     CheckAgain,
+    /// Ready state only: the app calls `Updater::restart_to_update`, which asks it to close.
+    RestartToUpdate,
 }
 
 /// The result of a manual "Check for Updates", in the same card as [`StartupPrompt`]: a spinner
-/// while checking, "newest version" when up to date, the error and a retry when it failed.
+/// while checking, "newest version" when up to date, the error and a retry when it failed, an
+/// update installing or ready, and the launcher's choice when it manages the version. It also
+/// shows what the last update came to at the first start after it.
 ///
 /// An app shows it while a requested check is in view: open it when the user starts a check
-/// (`Updater::check_now`), call [`show`](Self::show) every frame with `updater.status()` until it
-/// returns [`CheckResultResponse::Close`], then stop calling it. On
-/// [`CheckResultResponse::CheckAgain`] start another requested check and keep showing it. An
-/// available update goes to [`StartupPrompt`] instead; the app closes this dialog when the prompt opens.
+/// (`Updater::check_now`) or when `updater.status()` is not [`Status::Idle`] after the check at
+/// start, call [`show`](Self::show) every frame with `updater.status()` until it returns
+/// [`CheckResultResponse::Close`], then call `Updater::dismiss_status` and stop calling it. On
+/// [`CheckResultResponse::CheckAgain`] start another requested check and keep showing it; on
+/// [`CheckResultResponse::RestartToUpdate`] call `Updater::restart_to_update`. An available update
+/// goes to [`StartupPrompt`] instead; the app closes this dialog when the prompt opens.
 #[derive(Clone, Copy, Debug)]
 pub struct CheckResultDialog {
     app: &'static App,
@@ -384,14 +400,15 @@ impl CheckResultDialog {
         self
     }
 
-    /// Draws the modal card for `status` and returns what the user chose. [`Status::Idle`] draws
-    /// nothing and returns [`CheckResultResponse::None`].
+    /// Draws the card for `status` and returns what the user chose. [`Status::Idle`] draws
+    /// nothing and returns [`CheckResultResponse::None`]. The card is modal except for
+    /// [`Status::Updated`], a notice that leaves the app usable behind it.
     pub fn show(&self, ctx: &egui::Context, status: &Status) -> CheckResultResponse {
         if matches!(status, Status::Idle) {
             return CheckResultResponse::None;
         }
         let mut response = CheckResultResponse::None;
-        if backdrop(ctx) {
+        if !matches!(status, Status::Updated(_)) && backdrop(ctx) {
             response = CheckResultResponse::Close;
         }
         if ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) {
@@ -399,7 +416,7 @@ impl CheckResultDialog {
         }
 
         let pal = Palette::new(ctx, self.app);
-        let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, .. } = pal;
+        let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, alert, .. } = pal;
         let t = self.translate;
         egui::Area::new(egui::Id::unique("craft_update.check_result")).anchor(Align2::CENTER_CENTER, Vec2::ZERO).order(egui::Order::Foreground).show(
             ctx,
@@ -424,6 +441,33 @@ impl CheckResultDialog {
                                     ui.label(RichText::new(t("Checking for updates…", &[])).size(12.5).color(text_primary));
                                 });
                             }
+                            Status::Installing => {
+                                ui.horizontal(|ui| {
+                                    ui.spinner();
+                                    ui.label(RichText::new(t("Installing update…", &[])).size(12.5).color(text_primary));
+                                });
+                            }
+                            Status::Ready => {
+                                let text = t(crate::updater::READY_TEXT, &[("app", self.app.name)]);
+                                ui.label(RichText::new(text).size(12.5).color(text_primary));
+                            }
+                            Status::Managed => {
+                                let text = t(
+                                    "{app} {version} follows the version chosen in the launcher, so it does not update itself.",
+                                    &[("app", self.app.name), ("version", self.running_version)],
+                                );
+                                ui.label(RichText::new(text).size(12.5).color(text_muted));
+                            }
+                            Status::Updated(version) => {
+                                let text = t("{app} updated to {version}.", &[("app", self.app.name), ("version", version)]);
+                                ui.label(RichText::new(text).size(12.5).color(text_primary));
+                            }
+                            Status::InstallFailed(reason) => {
+                                let text = t("The update could not be installed: {reason}", &[("reason", reason)]);
+                                bounded(ui, "craft_update.install_failure", |ui| {
+                                    ui.add(egui::Label::new(RichText::new(text).size(12.5).color(alert)).selectable(true));
+                                });
+                            }
                             Status::UpToDate => {
                                 let text = t(
                                     "{app} {version} is the newest version available.",
@@ -432,9 +476,10 @@ impl CheckResultDialog {
                                 ui.label(RichText::new(text).size(12.5).color(text_muted));
                             }
                             Status::Failed(error) => {
-                                let alert_color = Color32::from_rgb(225, 95, 30);
-                                ui.label(RichText::new(t("Could not check for updates.", &[])).size(12.5).strong().color(alert_color));
-                                ui.add(egui::Label::new(RichText::new(error).size(12.0).color(text_muted)).selectable(true));
+                                ui.label(RichText::new(t("Could not check for updates.", &[])).size(12.5).strong().color(alert));
+                                bounded(ui, "craft_update.check_failure", |ui| {
+                                    ui.add(egui::Label::new(RichText::new(error).size(12.0).color(text_muted)).selectable(true));
+                                });
                                 ui.label(
                                     RichText::new(t("All releases are listed at {url}", &[("url", &self.app.releases_url())]))
                                         .size(11.5)
@@ -458,7 +503,7 @@ impl CheckResultDialog {
                                 if ui.add(ok).clicked() {
                                     response = CheckResultResponse::Close;
                                 }
-                                if matches!(status, Status::Failed(_)) {
+                                if matches!(status, Status::Failed(_) | Status::InstallFailed(_)) {
                                     let again = egui::Button::new(RichText::new(button(t, "Check Again")).size(10.5).monospace().color(text_primary))
                                         .corner_radius(egui::CornerRadius::ZERO)
                                         .min_size(Vec2::new(100.0, 26.0));
@@ -466,6 +511,15 @@ impl CheckResultDialog {
                                         response = CheckResultResponse::CheckAgain;
                                     }
                                 }
+                                if matches!(status, Status::Ready) {
+                                    let restart =
+                                        egui::Button::new(RichText::new(button(t, "Restart to Update")).size(10.5).monospace().color(text_primary))
+                                            .corner_radius(egui::CornerRadius::ZERO)
+                                            .min_size(Vec2::new(130.0, 26.0));
+                                    if ui.add(restart).clicked() {
+                                        response = CheckResultResponse::RestartToUpdate;
+                                    }
+                                }
                             });
                         }
                     });
@@ -475,6 +529,16 @@ impl CheckResultDialog {
     }
 }
 
+/// The tallest an error or failure reason draws in a card; a longer one scrolls, so the card's
+/// buttons stay on screen.
+const REASON_MAX_HEIGHT: f32 = 160.0;
+
+/// Draws `add_contents` (an error or a failure reason) at most [`REASON_MAX_HEIGHT`] tall,
+/// scrolling the rest.
+fn bounded(ui: &mut egui::Ui, id: &str, add_contents: impl FnOnce(&mut egui::Ui)) {
+    egui::ScrollArea::vertical().id_salt(id).max_height(REASON_MAX_HEIGHT).auto_shrink([false, true]).show(ui, add_contents);
+}
+
 /// Backgrounds of an unfilled button (LATER, DISMISS) when hovered and when pressed: a step from
 /// the card, so its text keeps the contrast it has at rest.
 fn button_states(is_dark: bool) -> (Color32, Color32) {
@@ -547,7 +611,7 @@ mod tests {
     use egui::{Context, Event, FullOutput, Key, Modifiers, RawInput, Rect};
 
     const DARK_CARD: Color32 = Color32::from_rgb(26, 26, 28);
-    const DARK_STATUS: Color32 = Color32::from_rgb(50, 30, 20);
+    const DARK_STATUS: Color32 = Color32::from_rgb(44, 26, 18);
 
     fn screen() -> Rect {
         Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(640.0, 480.0))
@@ -573,6 +637,7 @@ mod tests {
             status_message: None,
             status_note: None,
             is_installing: false,
+            is_ready: false,
             translate: english,
         }
     }
@@ -860,6 +925,54 @@ mod tests {
         }
     }
 
+    /// Errors and failure reasons reach WCAG AA's 4.5:1 on the card and in the prompt's message
+    /// box, in both themes.
+    #[test]
+    fn failure_text_reaches_aa_in_both_themes() {
+        for theme in [egui::ThemePreference::Dark, egui::ThemePreference::Light] {
+            let ctx = Context::default();
+            ctx.set_theme(theme);
+            let pal = Palette::new(&ctx, &PHOTOCRAFT);
+            for (surface, bg) in [("card", pal.card_bg), ("message box", pal.alert_bg)] {
+                let ratio = contrast(pal.alert, bg);
+                assert!(ratio >= 4.5, "{theme:?} {surface}: {ratio:.2}:1");
+            }
+        }
+    }
+
+    /// A long error or failure reason scrolls inside the card, so the card's buttons stay on
+    /// screen.
+    #[test]
+    fn a_long_reason_keeps_the_buttons_on_screen() {
+        let long = "the installer reported an error. ".repeat(120);
+        let on_screen = |output: &FullOutput, label: &str| text_pos(output, label).is_some_and(|pos| screen().contains(pos));
+        let dialog = CheckResultDialog::new(&PHOTOCRAFT, "0.1.0");
+        for status in [Status::InstallFailed(long.clone()), Status::Failed(long.clone())] {
+            let ctx = dark_context();
+            let mut output = None;
+            // The first frame of an area only sizes it; the second paints.
+            for _ in 0..2 {
+                let mut out = ctx.run_ui(RawInput { screen_rect: Some(screen()), ..RawInput::default() }, |ui| {
+                    dialog.show(ui.ctx(), &status);
+                });
+                out.textures_delta.clear();
+                output = Some(out);
+            }
+            let output = output.unwrap();
+            for label in ["OK", "CHECK AGAIN"] {
+                assert!(on_screen(&output, label), "{label} is off screen for a long {status:?}");
+            }
+        }
+        let ctx = dark_context();
+        let mut prompt = prompt_for(true);
+        prompt.set_message(long);
+        frame(&ctx, &mut prompt, Vec::new());
+        let (_, output) = frame(&ctx, &mut prompt, Vec::new());
+        for label in ["UPDATE AND RESTART", "LATER"] {
+            assert!(on_screen(&output, label), "{label} is off screen under a long message");
+        }
+    }
+
     /// Where the frame painted `text` (inside its galley, so a pointer there is over it).
     fn text_pos(output: &FullOutput, text: &str) -> Option<egui::Pos2> {
         fn find(shape: &egui::Shape, text: &str) -> Option<egui::Pos2> {
@@ -1049,6 +1162,26 @@ mod tests {
         let (_, idle) = run(&ctx, &Status::Idle, Vec::new());
         assert!(painted_text(&idle).is_empty());
 
+        // An update under way, the launcher's choice, and what the last update came to.
+        let ready = crate::translate::fill(crate::updater::READY_TEXT, &[("app", "PhotoCraft")]);
+        let backdrop = Color32::from_black_alpha(150);
+        for (status, wants, modal) in [
+            (Status::Installing, vec!["Installing update…", "OK"], true),
+            (Status::Ready, vec![ready.as_str(), "RESTART TO UPDATE", "OK"], true),
+            (Status::Managed, vec!["PhotoCraft 0.1.0 follows the version chosen in the launcher, so it does not update itself.", "OK"], true),
+            (Status::Updated("0.2.0".into()), vec!["PhotoCraft updated to 0.2.0.", "OK"], false),
+            (Status::InstallFailed("disk full".into()), vec!["The update could not be installed: disk full", "CHECK AGAIN", "OK"], true),
+        ] {
+            let (response, output) = run(&ctx, &status, Vec::new());
+            let text = painted_text(&output);
+            for want in wants {
+                assert!(text.contains(want), "{status:?}: {want} missing in {text}");
+            }
+            assert!(!text.contains("newest version"), "{status:?} is not called newest: {text}");
+            assert_eq!(response, CheckResultResponse::None);
+            assert_eq!(rect_fills(&output).iter().any(|(_, fill)| *fill == backdrop), modal, "{status:?} modal");
+        }
+
         let escape = Event::Key { key: Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE };
         assert_eq!(run(&dark_context(), &Status::UpToDate, vec![escape]).0, CheckResultResponse::Close);
     }
@@ -1070,19 +1203,20 @@ mod tests {
 
     #[test]
     fn the_prompt_draws_its_words_through_the_apps_translator() {
-        for (self_updating, installing) in [(false, false), (true, false), (true, true)] {
+        for (self_updating, installing, ready) in [(false, false, false), (true, false, false), (true, true, false), (true, false, true)] {
             let ctx = dark_context();
-            let mut prompt = StartupPrompt { is_installing: installing, ..prompt_for(self_updating) }.with_translator(pseudo);
+            let mut prompt = StartupPrompt { is_installing: installing, is_ready: ready, ..prompt_for(self_updating) }.with_translator(pseudo);
             frame(&ctx, &mut prompt, Vec::new());
             let (_, output) = frame(&ctx, &mut prompt, Vec::new());
             let painted = painted_text(&output);
             assert_all_translated(&painted, &["P", "PhotoCraft"]);
             // Values reach the translation; buttons are looked up in title case and drawn in capitals.
             assert!(painted.contains("[0.1.0, 0.2.0 | v]") && painted.contains("[ArtCraft | by ]"), "{painted}");
-            let button = match (self_updating, installing) {
-                (_, true) => "[ | DISMISS]",
-                (true, false) => "[ | UPDATE AND RESTART]",
-                (false, false) => "[ | DOWNLOAD]",
+            let button = match (self_updating, installing, ready) {
+                (_, true, _) => "[ | DISMISS]",
+                (_, false, true) => "[ | RESTART TO UPDATE]",
+                (true, false, false) => "[ | UPDATE AND RESTART]",
+                (false, false, false) => "[ | DOWNLOAD]",
             };
             assert!(painted.contains(button) && painted.contains("[ | UPDATE]"), "{painted}");
         }
@@ -1091,7 +1225,16 @@ mod tests {
     #[test]
     fn the_result_dialog_draws_its_words_through_the_apps_translator() {
         let dialog = CheckResultDialog::new(&PHOTOCRAFT, "0.1.0").with_translator(pseudo);
-        for status in [Status::Checking, Status::UpToDate, Status::Failed("no network".into())] {
+        for status in [
+            Status::Checking,
+            Status::UpToDate,
+            Status::Failed("no network".into()),
+            Status::Installing,
+            Status::Ready,
+            Status::Managed,
+            Status::Updated("0.2.0".into()),
+            Status::InstallFailed("no network".into()),
+        ] {
             let ctx = dark_context();
             let mut output = None;
             for _ in 0..2 {

crates/update/src/process.rs

@@ -2,7 +2,8 @@
 
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::{Child, Command, ExitStatus, Stdio};
+use std::sync::mpsc;
 use std::time::{Duration, Instant};
 
 /// A `Command` that won't flash a console window on Windows, and clears AppImage-injected
@@ -62,7 +63,15 @@ pub fn run_command(mut cmd: Command, program: &str, timeout: Duration) -> Result
     let mut child =
         cmd.stdin(Stdio::null()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().map_err(|e| format!("couldn't start {program} ({e})"))?;
     // Read both pipes while it runs, so a chatty helper can't block on a full one.
-    let (stdout, stderr) = (read_all(child.stdout.take()), read_all(child.stderr.take()));
+    let readers = read_all(child.stdout.take()).and_then(|stdout| Ok((stdout, read_all(child.stderr.take())?)));
+    let (stdout, stderr) = match readers {
+        Ok(readers) => readers,
+        Err(e) => {
+            let _ = child.kill();
+            let _ = child.wait();
+            return Err(format!("couldn't read the output of {program} ({e})"));
+        }
+    };
     let start = Instant::now();
     let status = loop {
         match child.try_wait() {
@@ -93,8 +102,8 @@ pub fn run_command(mut cmd: Command, program: &str, timeout: Duration) -> Result
 }
 
 /// Captures at most 8 MB of a child's pipe and drains the rest on its own thread.
-fn read_all<R: std::io::Read + Send + 'static>(pipe: Option<R>) -> std::thread::JoinHandle<Vec<u8>> {
-    std::thread::spawn(move || {
+fn read_all<R: std::io::Read + Send + 'static>(pipe: Option<R>) -> std::io::Result<std::thread::JoinHandle<Vec<u8>>> {
+    std::thread::Builder::new().name("helper-output".into()).spawn(move || {
         let mut out = Vec::new();
         if let Some(mut pipe) = pipe {
             let _ = std::io::Read::read_to_end(&mut std::io::Read::take(&mut pipe, 8 << 20), &mut out);
@@ -106,13 +115,61 @@ fn read_all<R: std::io::Read + Send + 'static>(pipe: Option<R>) -> std::thread::
     })
 }
 
-/// Starts `cmd` without waiting for it, and reaps it when it exits.
+/// Starts `cmd` without waiting for it.
+///
+/// On Unix the process is reaped when it exits: a thread that waits for the process starts
+/// first and starts the process itself. When no thread can be started, the call returns an error
+/// and does not start the process. On Windows the process starts on the calling thread, and no
+/// thread waits for it.
 pub fn spawn_detached(mut cmd: Command) -> Result<(), String> {
-    let mut child = cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn().map_err(|e| format!("couldn't start it ({e})"))?;
-    std::thread::spawn(move || {
-        let _ = child.wait();
-    });
-    Ok(())
+    cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null());
+    if cfg!(windows) {
+        return cmd.spawn().map(drop).map_err(|e| format!("couldn't start it ({e})"));
+    }
+    spawn_detached_with(
+        move || cmd.spawn(),
+        |wait| {
+            std::thread::Builder::new()
+                .name("reap-child".into())
+                .spawn(move || {
+                    let _ = wait();
+                })
+                .map(drop)
+        },
+    )
+}
+
+/// The thread that [`spawn_detached`] starts on Unix: `start_thread` starts a thread that runs
+/// the closure it is given, and that closure calls `spawn` and then waits for the process.
+/// `spawn` is called on that thread only, so when `start_thread` fails nothing is started.
+#[cfg_attr(windows, allow(dead_code))]
+fn spawn_detached_with(
+    spawn: impl FnOnce() -> std::io::Result<Child> + Send + 'static,
+    start_thread: impl FnOnce(Box<dyn FnOnce() -> Option<ExitStatus> + Send>) -> std::io::Result<()>,
+) -> Result<(), String> {
+    let (started, start_result) = mpsc::channel();
+    start_thread(Box::new(move || start_and_wait(spawn, &started))).map_err(|e| format!("couldn't start a thread to wait for it ({e})"))?;
+    match start_result.recv() {
+        Ok(Ok(())) => Ok(()),
+        Ok(Err(e)) => Err(format!("couldn't start it ({e})")),
+        Err(_) => Err("couldn't start it (its thread ended before starting it)".into()),
+    }
+}
+
+/// Calls `spawn`, sends whether that worked on `started`, and waits for the process to exit.
+/// Returns its exit status, or `None` when it didn't start or couldn't be waited for.
+#[cfg_attr(windows, allow(dead_code))]
+fn start_and_wait(spawn: impl FnOnce() -> std::io::Result<Child>, started: &mpsc::Sender<std::io::Result<()>>) -> Option<ExitStatus> {
+    match spawn() {
+        Ok(mut child) => {
+            let _ = started.send(Ok(()));
+            child.wait().ok()
+        }
+        Err(e) => {
+            let _ = started.send(Err(e));
+            None
+        }
+    }
 }
 
 /// Waits up to `timeout` for the process `pid` to exit, and says whether it has. A process that
@@ -224,17 +281,22 @@ fn opener(target: &std::ffi::OsStr) -> Result<Command, String> {
 pub fn open_url_with_fallback(primary_url: &str, fallback_url: &str) {
     let primary = primary_url.to_string();
     let fallback = fallback_url.to_string();
-    std::thread::spawn(move || {
+    let opener = std::thread::Builder::new().name("open-url".into()).spawn(move || {
         if !open_and_wait_success(&primary, Duration::from_millis(1500)) {
-            log::warn!("couldn't open {primary}; falling back to {fallback}");
+            log::warn!("couldn't open a link; opening its fallback");
             let _ = open(std::ffi::OsStr::new(&fallback));
         }
     });
+    if let Err(e) = opener {
+        log::warn!("couldn't start a thread to open a link ({e})");
+    }
 }
 
 /// Starts the default system opener for `target` and returns `true` unless it fails to start
 /// or exits in error within `timeout`. An opener still running after `timeout` counts as
-/// success and is reaped in the background.
+/// success, and the call waits for it to exit before returning. An opener whose state can't be
+/// read is stopped and waited for, and counts as a failure. [`open_url_with_fallback`] calls this
+/// on its own `open-url` thread.
 ///
 /// `explorer` exits with an error status even when it opens the target, so on Windows only a
 /// failure to start counts; the scheme check covers the rest.
@@ -242,7 +304,7 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool {
     let mut cmd = match opener(std::ffi::OsStr::new(target)) {
         Ok(cmd) => cmd,
         Err(error) => {
-            log::warn!("couldn't resolve the system opener for {target}: {error}");
+            log::warn!("couldn't resolve the system opener: {error}");
             return false;
         }
     };
@@ -252,7 +314,7 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool {
     let mut child = match cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn() {
         Ok(c) => c,
         Err(e) => {
-            log::warn!("couldn't start the opener for {target}: {e}");
+            log::warn!("couldn't start the system opener: {e}");
             return false;
         }
     };
@@ -262,12 +324,14 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool {
             Ok(Some(status)) => return status.success(),
             Ok(None) if start.elapsed() < timeout => std::thread::sleep(Duration::from_millis(25)),
             Ok(None) => {
-                std::thread::spawn(move || {
-                    let _ = child.wait();
-                });
+                let _ = child.wait();
                 return true;
             }
-            Err(_) => return false,
+            Err(_) => {
+                let _ = child.kill();
+                let _ = child.wait();
+                return false;
+            }
         }
     }
 }
@@ -472,6 +536,54 @@ mod run_tests {
         assert!(start.elapsed() < Duration::from_secs(5));
     }
 
+    /// The thread that starts a detached process waits for it, so the process is reaped when it
+    /// exits: only a wait gives its exit status.
+    #[test]
+    fn a_detached_process_is_waited_for() {
+        let (program, args) = shell("exit 7");
+        let mut cmd = command(program);
+        cmd.args(args);
+        let (status_sent, status) = mpsc::channel();
+        let result = spawn_detached_with(
+            move || cmd.spawn(),
+            move |wait| {
+                std::thread::Builder::new()
+                    .spawn(move || {
+                        let _ = status_sent.send(wait());
+                    })
+                    .map(drop)
+            },
+        );
+        assert_eq!(result, Ok(()));
+        assert_eq!(status.recv().ok().flatten().and_then(|status| status.code()), Some(7), "the waiting thread gets the exit status");
+    }
+
+    /// When no thread can be started to wait for a process, the process is not started and the
+    /// call says why.
+    #[test]
+    fn no_process_starts_without_a_thread_to_wait_for_it() {
+        let (program, args) = shell("exit 0");
+        let mut cmd = command(program);
+        cmd.args(args);
+        let spawned = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
+        let spawn_called = std::sync::Arc::clone(&spawned);
+        let result = spawn_detached_with(
+            move || {
+                spawn_called.store(true, std::sync::atomic::Ordering::SeqCst);
+                cmd.spawn()
+            },
+            |_wait| Err(std::io::Error::other("no threads")),
+        );
+        assert_eq!(result, Err("couldn't start a thread to wait for it (no threads)".into()));
+        assert!(!spawned.load(std::sync::atomic::Ordering::SeqCst), "the process was started");
+    }
+
+    #[test]
+    fn a_detached_program_that_cannot_start_says_so() {
+        let result = spawn_detached(command("craft-update-test-no-such-program"));
+        assert!(matches!(&result, Err(e) if e.starts_with("couldn't start it (")), "{result:?}");
+    }
+
     #[test]
     fn a_silent_failure_and_a_success() {
         let (program, args) = shell("exit 4");

crates/update/src/updater_pin_tests.rs

@@ -30,7 +30,7 @@ fn a_pinned_app_is_not_prompted_and_the_network_is_not_asked() {
         assert_eq!(u.status(), &Status::Idle);
         u.check_now(&ctx);
         settle(&mut u, &ctx);
-        assert_eq!(u.status(), &Status::UpToDate, "{track:?}");
+        assert_eq!(u.status(), &Status::Managed, "{track:?}");
         assert_eq!(u.managed(), Some(&Choice::Managed(track.clone())));
         u.offer(newer());
         assert!(!u.prompt_open(), "an offer is held too: {track:?}");
@@ -109,7 +109,7 @@ fn a_check_after_the_pin_closes_a_prompt_opened_before_it() {
     u.check_now(&ctx);
     settle(&mut u, &ctx);
     assert!(!u.prompt_open());
-    assert_eq!(u.status(), &Status::UpToDate);
+    assert_eq!(u.status(), &Status::Managed);
 }
 
 #[test]
@@ -140,7 +140,7 @@ fn a_pin_made_while_the_check_runs_still_holds_its_answer() {
     u.check_now(&ctx);
     settle(&mut u, &ctx);
     assert!(!u.prompt_open());
-    assert_eq!(u.status(), &Status::UpToDate);
+    assert_eq!(u.status(), &Status::Managed);
 }
 
 #[test]
@@ -160,7 +160,7 @@ fn every_answer_after_a_pin_made_mid_check_reports_the_launchers_choice() {
         let mut u = Updater::new(&PHOTOCRAFT, "0.13.0").with_source(source).with_pins_path(&path);
         u.check_now(&ctx);
         settle(&mut u, &ctx);
-        assert_eq!(u.status(), &Status::UpToDate);
+        assert_eq!(u.status(), &Status::Managed);
         assert_eq!(u.managed(), Some(&Choice::Managed(crate::pins::VersionTrack::Nightly)));
     }
 }