From a379a1ddc5d946a86fe0238dd1bda0fe976fbb6b Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:27:01 +0200 Subject: [PATCH 01/12] install.sh: close an old tray on upgrade and start the new one An upgrade left the old tray process running on its old lock (an earlier version kept it loose in the data folder), so the new window started a second tray: two icons until logout. The app gets `--close`, which tells the window and the tray to quit, in the new folder and at the old loose names, waits for them to let go of their locks and prints which ran. install.sh runs it before the copy and starts the new tray at the end, or the installed one from the EXIT trap when the install fails. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-desktop/src/main.rs | 9 ++ .../src/single_instance.rs | 99 +++++++++++++++++-- packaging/linux/install.sh | 25 ++++- xtask/src/package.rs | 13 ++- 4 files changed, 129 insertions(+), 17 deletions(-) diff --git a/apps/craft-cloud-desktop/src/main.rs b/apps/craft-cloud-desktop/src/main.rs index cf791e1..9f24411 100644 --- a/apps/craft-cloud-desktop/src/main.rs +++ b/apps/craft-cloud-desktop/src/main.rs @@ -5,6 +5,9 @@ //! - Linux: `--tray` is the tray process (no window: [`tray::process`]), which the start-at-login entry runs (an //! older entry's `--hidden` means the same). The window is a process of its own that ends when closed (Wayland can't //! hide a window), and starts the tray process if none runs. +//! +//! `--close` ends the window and the tray process that run (an installer's upgrade: also an older version's) and +//! prints which did (`window`, `tray`, a line each); the agent is left alone. #![cfg_attr(windows, windows_subsystem = "windows")] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -29,6 +32,12 @@ fn main() -> ExitCode { eprintln!("Craft Cloud can't find your user data folder"); return ExitCode::FAILURE; }; + if has("--close") { + for name in single_instance::close_running(&dir) { + println!("{}", if name == Name::Tray { "tray" } else { "window" }); + } + return ExitCode::SUCCESS; + } let (outside_tx, outside_rx) = std::sync::mpsc::channel::(); let window: Arc> = Arc::new(OnceLock::new()); match single_instance::claim(&dir, Name::Window) { diff --git a/apps/craft-cloud-desktop/src/single_instance.rs b/apps/craft-cloud-desktop/src/single_instance.rs index 15d0bc1..4f3f1e9 100644 --- a/apps/craft-cloud-desktop/src/single_instance.rs +++ b/apps/craft-cloud-desktop/src/single_instance.rs @@ -44,6 +44,27 @@ impl Name { pub fn port_file(self) -> String { format!("{}.port", self.stem()) } + + /// Its lock and port files in `dir`. + fn files(self, dir: &Path) -> Files { + Files { lock: dir.join(self.lock_file()), port: dir.join(self.port_file()) } + } + + /// Where versions before the folder of its own kept them: loose in the per-user data folder (`parent`). + fn old_files(self, parent: &Path) -> Files { + let stem = match self { + Name::Window => "craft-cloud-app", + Name::Tray => "craft-cloud-tray", + }; + Files { lock: parent.join(format!("{stem}.lock")), port: parent.join(format!("{stem}.port")) } + } +} + +/// A process kind's lock and port files. +#[derive(Debug, Clone)] +struct Files { + lock: PathBuf, + port: PathBuf, } /// What a later start asks the first. @@ -82,8 +103,7 @@ pub struct First { /// Another one's address. pub struct Running { - dir: PathBuf, - name: Name, + files: Files, } /// The folder of the locks and the channels, next to the agent's data folder (the uninstallers remove it). @@ -102,31 +122,67 @@ fn folder_beside(data: &Path) -> Option { /// Takes the lock of `name` in `dir`, or finds the process that holds it. pub fn claim(dir: &Path, name: Name) -> std::io::Result { std::fs::create_dir_all(dir)?; - let lock = OpenOptions::new().create(true).truncate(false).write(true).open(dir.join(name.lock_file()))?; + claim_files(name.files(dir)) +} + +fn claim_files(files: Files) -> std::io::Result { + let lock = OpenOptions::new().create(true).truncate(false).write(true).open(&files.lock)?; match lock.try_lock() { Ok(()) => {} - Err(std::fs::TryLockError::WouldBlock) => return Ok(Instance::Running(Running { dir: dir.to_owned(), name })), + Err(std::fs::TryLockError::WouldBlock) => return Ok(Instance::Running(Running { files })), Err(std::fs::TryLockError::Error(e)) => return Err(e), } let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0)))?; let token = token()?; let port = listener.local_addr()?.port(); // Written whole, then renamed into place, so a later start never reads half of it. - let temp = dir.join(format!("{}.tmp", name.port_file())); + let mut temp = files.port.clone().into_os_string(); + temp.push(".tmp"); std::fs::write(&temp, format!("{port} {token}\n"))?; - std::fs::rename(&temp, dir.join(name.port_file()))?; + std::fs::rename(&temp, &files.port)?; Ok(Instance::First(First { lock, listener, token })) } /// Whether a process of `name` runs (holds its lock), without taking it for long. pub fn is_running(dir: &Path, name: Name) -> bool { - let Ok(lock) = OpenOptions::new().create(true).truncate(false).write(true).open(dir.join(name.lock_file())) else { return false }; + holds(&name.files(dir).lock) +} + +/// Whether a process holds the lock `lock` (one that isn't there isn't made). +fn holds(lock: &Path) -> bool { + let Ok(lock) = OpenOptions::new().write(true).open(lock) else { return false }; matches!(lock.try_lock(), Err(std::fs::TryLockError::WouldBlock)) } /// Sends `command` to the running process of `name`, if one answers (within a few seconds). pub fn send(dir: &Path, name: Name, command: Command) -> std::io::Result<()> { - Running { dir: dir.to_owned(), name }.send(command) + Running { files: name.files(dir) }.send(command) +} + +/// Ends the window and the tray process that run for `dir`, also those of a version that kept its files loose in +/// the folder above (an upgrade replaces the programs under them), and waits (a few seconds) for each to let go of its +/// lock. The kinds that were running. The agent is left alone. +pub fn close_running(dir: &Path) -> Vec { + let parent = dir.parent().unwrap_or(dir); + let mut closed = Vec::new(); + for name in [Name::Window, Name::Tray] { + for files in [name.files(dir), name.old_files(parent)] { + if !holds(&files.lock) { + continue; + } + let lock = files.lock.clone(); + if (Running { files }).send(Command::Quit).is_ok() { + let deadline = Instant::now() + PATIENCE; + while holds(&lock) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(50)); + } + if !closed.contains(&name) { + closed.push(name); + } + } + } + } + closed } fn token() -> std::io::Result { @@ -180,14 +236,14 @@ impl Running { match self.try_send(command) { Ok(()) => return Ok(()), // The first may be starting: its port file comes a moment after its lock. - Err(_) if Instant::now() < deadline && is_running(&self.dir, self.name) => std::thread::sleep(Duration::from_millis(100)), + Err(_) if Instant::now() < deadline && holds(&self.files.lock) => std::thread::sleep(Duration::from_millis(100)), Err(e) => return Err(e), } } } fn try_send(&self, command: Command) -> std::io::Result<()> { - let text = std::fs::read_to_string(self.dir.join(self.name.port_file()))?; + let text = std::fs::read_to_string(&self.files.port)?; let (port, token) = text.trim().split_once(' ').ok_or_else(|| std::io::Error::other("the port file doesn't read"))?; let port: u16 = port.parse().map_err(|_| std::io::Error::other("the port file doesn't read"))?; let mut stream = TcpStream::connect_timeout(&SocketAddr::from((Ipv4Addr::LOCALHOST, port)), Duration::from_secs(1))?; @@ -253,6 +309,29 @@ mod tests { assert!(inside.join("window.lock").is_file() && inside.join("window.port").is_file()); } + /// An upgrade closes what runs: here a tray in the folder and a window of an older version, whose files are loose + /// in the folder above. Each is told to quit; nothing runs where nothing is told. + #[test] + fn close_running_tells_the_window_and_the_tray_new_and_old() { + let parent = tempfile::tempdir().unwrap(); + let dir = parent.path().join(FOLDER_NAME); + assert!(close_running(&dir).is_empty(), "nothing runs"); + std::fs::create_dir_all(&dir).unwrap(); + let (told, heard) = channel(); + let Instance::First(tray) = claim(&dir, Name::Tray).unwrap() else { panic!("not first") }; + let Instance::First(old_window) = claim_files(Name::Window.old_files(parent.path())).unwrap() else { panic!("not first") }; + for (name, first) in [(Name::Tray, tray), (Name::Window, old_window)] { + let told = told.clone(); + first.listen(move |command| { + let _ = told.send((name, command)); + }); + } + assert_eq!(close_running(&dir), [Name::Window, Name::Tray]); + let mut heard: Vec<_> = (0..2).map(|_| heard.recv_timeout(Duration::from_secs(10)).unwrap()).collect(); + heard.sort_by_key(|(name, _)| *name == Name::Tray); + assert_eq!(heard, [(Name::Window, Command::Quit), (Name::Tray, Command::Quit)]); + } + #[test] fn a_wrong_token_shows_nothing() { let dir = tempfile::tempdir().unwrap(); diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index 2aba19d..0f31af3 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -139,8 +139,25 @@ if [ "$was_running" = 1 ]; then stopped=1 fi -# From here on the agent is stopped: whatever ends the script (a failed copy too) starts it again. -trap 'if [ "$stopped" = 1 ] && [ "$restarted" = 0 ]; then start_again; fi' EXIT +# From here on the agent is stopped and the app may be closed: whatever ends the script (a failed copy too) starts +# them again, once. +closed="" +finished=0 +finish() { + [ "$finished" = 0 ] || return 0 + finished=1 + if [ "$stopped" = 1 ] && [ "$restarted" = 0 ]; then + start_again + fi + case "$closed" in + *tray*) [ -x "$lib/craft-cloud" ] && nohup "$lib/craft-cloud" --tray >/dev/null 2>&1 & ;; + esac +} +trap finish EXIT + +# The app's window and tray process, an older version's too, are closed before the copy: an old tray would keep its +# icon next to the new one's. A tray that ran is started again at the end. +closed=$("$here/craft-cloud" --close 2>/dev/null || true) mkdir -p "$lib" "$bin" "$apps" "$icons" for name in craft-cloud craft-cloud-agent; do @@ -177,9 +194,7 @@ if [ -f "$unit" ] || [ -f "$entry" ]; then "$lib/craft-cloud-agent" autostart on >/dev/null || echo "Start at login still runs the old agent: run 'craft-cloud-agent autostart on'." >&2 fi -if [ "$stopped" = 1 ]; then - start_again -fi +finish if [ "$menu" = 1 ]; then echo "Craft Cloud is installed in $lib. Open it from the app menu, or run craft-cloud." diff --git a/xtask/src/package.rs b/xtask/src/package.rs index 89055ad..6bfb3ba 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -306,7 +306,9 @@ mod tests { count = count.display() ); std::fs::write(src.join("craft-cloud-agent"), agent).unwrap(); - std::fs::write(src.join("craft-cloud"), "#!/bin/sh\nexit 0\n").unwrap(); + // The app: an (old) tray runs, which `--close` ends and says. + let app = format!("#!/bin/sh\necho \"$0 $*\" >> \"{}\"\n[ \"$1\" = --close ] && echo tray\nexit 0\n", calls.display()); + std::fs::write(src.join("craft-cloud"), app).unwrap(); std::fs::write(src.join("craft-cloud.png"), "png").unwrap(); std::fs::write(src.join(LICENSES), "licences").unwrap(); std::fs::copy(crate::root().join("packaging/linux/install.sh"), src.join("install.sh")).unwrap(); @@ -386,15 +388,22 @@ mod tests { let out = up.install("60"); assert!(out.status.success(), "{out:?}"); let quit = format!("{} quit [old agent running]", up.src_agent()); + let tray = format!("{} --tray", up.home.join(".local/lib/craft-cloud/craft-cloud").display()); let expected = [ quit.clone(), quit.clone(), quit, + format!("{} --close", up.src.join("craft-cloud").display()), format!("{} autostart on", up.lib_agent()), "systemctl --user stop craft-cloud-agent.service".to_owned(), "systemctl --user start craft-cloud-agent.service".to_owned(), + tray.clone(), ]; - assert_eq!(up.calls(), expected, "nothing after the quits until the old process had ended"); + assert_eq!( + up.ends_with(&tray), + expected, + "nothing after the quits until the old process had ended; the old tray closed first, the new one started last" + ); } /// The same without a systemd unit: the new copy is run once the old process has ended. From 1d8977d29be5619b361c813c401e3c76e6c3d6cf Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:28:03 +0200 Subject: [PATCH 02/12] install.sh: run the restart waiter under systemd, one at a time The waiter that starts the agent again after a timeout ran under nohup in the login session, so logind's KillUserProcesses ended it with the session, and every install that gave up queued another. Where the user has systemd it is now a unit of its own (systemd-run --user, a fixed name): it outlives the session, and a second install finds it waiting and adds none. Elsewhere it stays under nohup. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- packaging/linux/install.sh | 17 +++++++++-- xtask/src/package.rs | 59 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index 0f31af3..d0a4d79 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -47,9 +47,11 @@ has_unit() { # Starts the agent again once the process $1 (or, with no pid known, the one whose local-api.json is in the data # folder) has ended and so let go of the data folder: an agent started before that finds the folder taken and exits. -# Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. +# Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. Where the user +# has systemd, the waiter is a unit of its own (systemd-run --user): it outlives this login session (logind's +# KillUserProcesses), and its fixed name keeps it to one at a time; a second install that gives up finds it waiting. start_after_exit() { - nohup sh -c ' + waiter=' n=0 if [ -n "$1" ]; then while kill -0 "$1" 2>/dev/null && [ "$n" -lt 86400 ]; do @@ -68,7 +70,16 @@ start_after_exit() { else "$3" run --log fi - ' sh "$1" "$2" "$3" "$data" >/dev/null 2>&1 & + ' + if command -v systemd-run >/dev/null 2>&1; then + if systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart /bin/sh -c "$waiter" sh "$1" "$2" "$3" "$data" >/dev/null 2>&1; then + return 0 + fi + if command -v systemctl >/dev/null 2>&1 && systemctl --user --quiet is-active craft-cloud-agent-restart.service 2>/dev/null; then + return 0 + fi + fi + nohup sh -c "$waiter" sh "$1" "$2" "$3" "$data" >/dev/null 2>&1 & } # The agent that ran and was stopped is started again (once): this computer keeps syncing. diff --git a/xtask/src/package.rs b/xtask/src/package.rs index 6bfb3ba..b71dcc3 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -312,8 +312,24 @@ mod tests { std::fs::write(src.join("craft-cloud.png"), "png").unwrap(); std::fs::write(src.join(LICENSES), "licences").unwrap(); std::fs::copy(crate::root().join("packaging/linux/install.sh"), src.join("install.sh")).unwrap(); - std::fs::write(bin.join("systemctl"), format!("#!/bin/sh\necho \"systemctl $*{note}\" >> \"{}\"\n", calls.display())).unwrap(); - for file in [src.join("craft-cloud-agent"), src.join("craft-cloud"), src.join("install.sh"), bin.join("systemctl")] { + // systemd-run: one unit of a name at a time (`unit` marks it running); systemctl says so for is-active. + let unit = tmp.join("unit-running"); + let systemctl = format!( + "#!/bin/sh\nif [ \"$3\" = is-active ]; then [ -e \"{unit}\" ]; exit; fi\necho \"systemctl $*{note}\" >> \"{calls}\"\n", + unit = unit.display(), + calls = calls.display() + ); + std::fs::write(bin.join("systemctl"), systemctl).unwrap(); + let systemd_run = format!( + "#!/bin/sh\nopts=\"\"\nwhile [ \"${{1#--}}\" != \"$1\" ]; do opts=\"$opts $1\"; shift; done\n[ -e \"{unit}\" ] && exit 1\n\ + echo \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{unit}\"\n(\"$@\"; rm -f \"{unit}\") >/dev/null 2>&1 &\n", + unit = unit.display(), + calls = calls.display() + ); + std::fs::write(bin.join("systemd-run"), systemd_run).unwrap(); + for file in + [src.join("craft-cloud-agent"), src.join("craft-cloud"), src.join("install.sh"), bin.join("systemctl"), bin.join("systemd-run")] + { std::fs::set_permissions(file, std::fs::Permissions::from_mode(0o755)).unwrap(); } let unit = home.join(".config/systemd/user/craft-cloud-agent.service"); @@ -431,6 +447,45 @@ mod tests { assert!(!up.home.join(".local/lib/craft-cloud").exists(), "nothing installed"); std::thread::sleep(std::time::Duration::from_secs(2)); assert!(!up.calls().iter().any(|c| c.starts_with("systemctl")), "not while it runs: {:?}", up.calls()); + assert!(up.calls().iter().any(|c| c == "systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart"), "{:?}", up.calls()); + up.agent_ends(); + let start = "systemctl --user start craft-cloud-agent.service"; + assert_eq!(up.ends_with(start).last().map(String::as_str), Some(start)); + } + + /// Two installs that give up while the agent finishes a long pass: the second finds the first's waiter (a systemd + /// unit of a fixed name) and adds none, so the agent is started once. + #[cfg(target_os = "linux")] + #[test] + fn a_second_install_that_gives_up_adds_no_second_waiter() { + let up = Upgrade::new("timeout-twice", 1, 120); + for _ in 0..2 { + let _ = std::fs::remove_file(up.tmp.join("quits.txt")); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + } + up.agent_ends(); + let start = "systemctl --user start craft-cloud-agent.service"; + up.ends_with(start); + // Time for a second start, if there were a second waiter. + std::thread::sleep(std::time::Duration::from_secs(2)); + let calls = up.calls(); + assert_eq!(calls.iter().filter(|c| c.starts_with("systemd-run")).count(), 1, "{calls:?}"); + assert_eq!(calls.iter().filter(|c| *c == start).count(), 1, "{calls:?}"); + } + + /// Without systemd-run the waiter runs in the background of this shell (nohup). + #[cfg(target_os = "linux")] + #[test] + fn without_systemd_run_the_waiter_still_starts_the_agent() { + let up = Upgrade::new("timeout-nosystemdrun", 1, 120); + std::fs::remove_file(up.bin.join("systemd-run")).unwrap(); + // The test's PATH has /usr/bin too, where a real systemd-run may be: a stand-in that isn't there. + std::fs::write(up.bin.join("systemd-run"), "#!/bin/sh\nexit 1\n").unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(up.bin.join("systemd-run"), std::fs::Permissions::from_mode(0o755)).unwrap(); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); up.agent_ends(); let start = "systemctl --user start craft-cloud-agent.service"; assert_eq!(up.ends_with(start).last().map(String::as_str), Some(start)); From 7569546fe455316360e875b80c8e5ee69798399f Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:30:12 +0200 Subject: [PATCH 03/12] setup: make the cmd remover wait while tasklist can't tell The fallback remover (where PowerShell can't run) piped tasklist into find, so a tasklist that couldn't run read as "the setup has ended" and the folder was removed too early, then left. tasklist now writes to a file in the temporary folder first, and a failure means wait and ask again. find and ping are named by their full paths (cmd looks in its working folder first), and the removal is retried every second for half a minute, not once. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/tests.rs | 14 +++++++++++ apps/craft-cloud-setup/src/windows.rs | 34 +++++++++++++++++++++------ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 441539f..120513e 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -550,3 +550,17 @@ fn the_program_folder_goes_once_the_setup_has_ended() { assert!(outside.join("keep.txt").exists(), "the link wasn't followed (cmd: {with_cmd})"); } } + +/// The `cmd` remover with a `tasklist` that can't run never takes the setup for ended: it waits, and the folder stays. +#[cfg(windows)] +#[test] +fn the_cmd_remover_waits_while_tasklist_cant_tell() { + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("Craft Cloud"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join(SETUP_EXE), "setup").unwrap(); + // Three rounds of asking, then it gives up (an hour in the real one). + let mut remover = crate::windows::remove_after_with_cmd_without_tasklist(3, std::process::id(), "craft-cloud-setup.exe", &dir).spawn().unwrap(); + remover.wait().unwrap(); + assert!(dir.join(SETUP_EXE).exists(), "nothing was removed while it couldn't tell"); +} diff --git a/apps/craft-cloud-setup/src/windows.rs b/apps/craft-cloud-setup/src/windows.rs index 063b9cd..2804406 100644 --- a/apps/craft-cloud-setup/src/windows.rs +++ b/apps/craft-cloud-setup/src/windows.rs @@ -72,20 +72,40 @@ pub fn remove_after(pid: u32, dir: &Path) -> Command { } /// [`remove_after`] without PowerShell: `cmd` alone asks `tasklist` every second whether the process `pid` (whose -/// exe is `image`) still runs, for up to an hour, then removes the folder ([`RMDIR`], again a moment later if a file -/// was still held). The `if` is in parentheses of its own: cmd would take a following `& exit` as part of it. +/// exe is `image`) still runs, for up to an hour, then removes the folder ([`RMDIR`], tried again every second for +/// half a minute while a file is still held). pub fn remove_after_with_cmd(pid: u32, image: &str, dir: &Path) -> Command { - let tasklist = system_program("tasklist.exe"); + remove_after_with_cmd_using(&system_program("tasklist.exe"), 3600, pid, image, dir) +} + +/// [`remove_after_with_cmd`] with `tasklist` at `tasklist`, asked `rounds` times at most. +/// +/// `tasklist`'s answer goes to a file first (`CRAFT_CLOUD_TASKS`, in the temporary folder), so a `tasklist` that +/// fails (and writes nothing) is told apart from one that lists nothing: then cmd waits and asks again, rather than +/// take the setup for ended. `find` and `ping` are named by their full paths: cmd looks in its working folder first. +/// Each `if` is in parentheses of its own: cmd would take a following `& …` as part of it. +fn remove_after_with_cmd_using(tasklist: &Path, rounds: u32, pid: u32, image: &str, dir: &Path) -> Command { + let (find, ping) = (system_program("find.exe"), system_program("PING.EXE")); + let wait = format!("\"{}\" -n 2 127.0.0.1 >nul", ping.display()); + let remove = format!("for /l %j in (1,1,30) do @(if exist \"%CRAFT_CLOUD_DIR%\" ({RMDIR} & (if exist \"%CRAFT_CLOUD_DIR%\" ({wait}))))"); let line = format!( - "/d /c for /l %i in (1,1,3600) do @(\"{tasklist}\" /FI \"PID eq {pid}\" /NH | find /i \"{image}\" >nul && ping -n 2 127.0.0.1 >nul \ - || ({RMDIR} & (if exist \"%CRAFT_CLOUD_DIR%\" (ping -n 2 127.0.0.1 >nul & {RMDIR})) & exit))", - tasklist = tasklist.display() + "/d /c for /l %i in (1,1,{rounds}) do @((\"{tasklist}\" /FI \"PID eq {pid}\" /NH >\"%CRAFT_CLOUD_TASKS%\") \ + && (\"{find}\" /i \"{image}\" \"%CRAFT_CLOUD_TASKS%\" >nul && {wait} || ({remove} & del \"%CRAFT_CLOUD_TASKS%\" & exit)) || {wait})", + tasklist = tasklist.display(), + find = find.display() ); + let tasks = std::env::temp_dir().join(format!("craft-cloud-uninstall-{pid}.txt")); let mut command = quiet(&system_program("cmd.exe")); - command.raw_arg(line).env("CRAFT_CLOUD_DIR", dir); + command.raw_arg(line).env("CRAFT_CLOUD_DIR", dir).env("CRAFT_CLOUD_TASKS", tasks); detached_from(command, dir) } +/// For tests: [`remove_after_with_cmd_using`] with a `tasklist` that can't run, asked `rounds` times. +#[cfg(test)] +pub fn remove_after_with_cmd_without_tasklist(rounds: u32, pid: u32, image: &str, dir: &Path) -> Command { + remove_after_with_cmd_using(Path::new(r"C:\no-such-folder\tasklist.exe"), rounds, pid, image, dir) +} + /// In a process group of its own, and working in the folder above `dir` (a remover working inside it would hold it). fn detached_from(mut command: Command, dir: &Path) -> Command { command.creation_flags(CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP).current_dir(dir.parent().unwrap_or(dir)); From 0fe0dfe2c7c75499347b3e3121d92857ed6fa801 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:30:12 +0200 Subject: [PATCH 04/12] setup: put the agent back when an uninstall fails after stopping it An uninstall that failed after the stop (a shortcut or a file that couldn't go) left the agent stopped and its start at login removed. While the agent's program is still there, its Run values are now put back and, if it ran, it is started again, so the computer keeps syncing until the uninstall is run again; the error says so. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/lib.rs | 49 ++++++++++++++++++++++++++--- apps/craft-cloud-setup/src/tests.rs | 27 ++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/apps/craft-cloud-setup/src/lib.rs b/apps/craft-cloud-setup/src/lib.rs index 942eab7..a451ea8 100644 --- a/apps/craft-cloud-setup/src/lib.rs +++ b/apps/craft-cloud-setup/src/lib.rs @@ -330,16 +330,31 @@ fn restarted_after(places: &Places, registry: &dyn Registry, machine: &dyn Machi /// Removes the install at `places`; `running` is this setup's own exe (in the program folder when started from /// "Apps & features": then the folder goes once this process has ended). pub fn uninstall(places: &Places, registry: &dyn Registry, machine: &dyn Machine, running: &Path) -> Result<(), String> { - let dir = places.dir(); machine.close_app(); - match machine.stop_agent(&places.agent_data) { - AgentStop::NotRunning | AgentStop::Stopped => {} + let was_running = match machine.stop_agent(&places.agent_data) { + AgentStop::NotRunning => false, + AgentStop::Stopped => true, AgentStop::StillRunning(why) => return Err(still_running(&why)), AgentStop::Unknown(why) => return Err(restarted_after(places, registry, machine, &unknown_stop(&why))), - } + }; + let mut removed_values = Vec::new(); + uninstall_stopped(places, registry, machine, running, &mut removed_values) + .map_err(|e| put_back(places, registry, machine, was_running, &removed_values, &e)) +} + +/// The uninstall's changes once nothing runs; the `Run` values it removed (name, value) go into `removed_values`. +fn uninstall_stopped( + places: &Places, + registry: &dyn Registry, + machine: &dyn Machine, + running: &Path, + removed_values: &mut Vec<(&'static str, String)>, +) -> Result<(), String> { + let dir = places.dir(); for value in [AGENT_RUN_VALUE, APP_RUN_VALUE] { - if registry.get(&places.run_key, value).as_deref().is_some_and(|v| starts_from(v, &dir)) { + if let Some(v) = registry.get(&places.run_key, value).filter(|v| starts_from(v, &dir)) { registry.remove_value(&places.run_key, value)?; + removed_values.push((value, v)); } } let shortcut = places.shortcut(); @@ -366,6 +381,30 @@ pub fn uninstall(places: &Places, registry: &dyn Registry, machine: &dyn Machine Ok(()) } +/// The uninstall failed (`failure`) after the agent was stopped and maybe its start at login removed: while the +/// agent's program is still there, its `Run` values are put back and, if it ran, it is started again, so this +/// computer keeps syncing until the uninstall is run again. What is said. +fn put_back( + places: &Places, + registry: &dyn Registry, + machine: &dyn Machine, + was_running: bool, + removed_values: &[(&'static str, String)], + failure: &str, +) -> String { + if !places.dir().join(AGENT_BG_EXE).is_file() { + return format!("{failure}. Run the uninstall again to finish it."); + } + for (name, value) in removed_values { + let _ = registry.set_string(&places.run_key, name, value); + } + if was_running { + format!("{} Start at login was put back.", restarted_after(places, registry, machine, failure)) + } else { + format!("{failure}. Start at login was put back.") + } +} + /// The app's single-instance files in `app_files`, the folder itself once empty, and the loose ones older versions /// kept beside it. Only these names: anything else there stays. Last, and best effort: a file an app that couldn't be /// closed still holds stays, and is harmless. diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 120513e..6d63586 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -196,6 +196,33 @@ fn an_install_that_fails_after_the_stop_starts_the_agent_again() { assert_eq!(machine.asked(), ["close app", "stop agent"]); } +/// An uninstall that fails after the agent was stopped and its start at login removed (here the shortcut can't be +/// removed): the Run values are put back and the agent is started again; without a running agent, only put back. +#[test] +fn an_uninstall_that_fails_after_the_stop_puts_the_agent_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let value = format!("\"{}\" run --log", u.places.dir().join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + // A "shortcut" that is a folder with something in it: removing it fails. + std::fs::create_dir_all(u.places.shortcut().join("x")).unwrap(); + + let machine = Recorder::default(); + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &u.setup).unwrap_err(); + assert!(e.contains("The agent was started again.") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before, "the Run value is back, the entry stays"); + + let machine = Recorder::default(); + let e = uninstall(&u.places, ®istry, &machine, &u.setup).unwrap_err(); + assert!(e.ends_with("Start at login was put back.") && !e.contains("started again"), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent"]); + assert_eq!(*registry.values.lock().unwrap(), before); +} + /// The agent accepts the quit, then tasklist fails: the install and the uninstall stop with nothing else changed, /// and the agent (which may be stopping) is started again from the folder its start at login names. #[test] From 8c451250e26ad046036402260182249fcd9aa6c6 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:04:16 +0200 Subject: [PATCH 05/12] setup: move the program folder aside before deleting anything An uninstall that failed on a held file after deleting some files left a half-removed folder: the agent couldn't be put back, or was put back over missing files. The folder's files are now moved aside first, into "Craft Cloud.removing-" next to it: the whole folder in one move, or one file at a time when this setup runs from it, moved back if one can't move. A file held open fails the move as a whole, so then nothing is touched and the agent is put back and started again. Only after the move is anything deleted; what can't go yet (an exe still running) goes once the setup has ended. The put-back now needs all three exes in place, says when a Run value couldn't be restored, and says nothing about start at login when none was removed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/lib.rs | 79 ++++++++++++++++++++--------- apps/craft-cloud-setup/src/tests.rs | 51 +++++++++++++++---- 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/apps/craft-cloud-setup/src/lib.rs b/apps/craft-cloud-setup/src/lib.rs index a451ea8..597762a 100644 --- a/apps/craft-cloud-setup/src/lib.rs +++ b/apps/craft-cloud-setup/src/lib.rs @@ -146,6 +146,11 @@ pub trait Machine { fn shortcut(&self, lnk: &Path, target: &Path) -> Result<(), String>; /// Removes the folder `dir` once this process has ended (it holds this setup's own exe). fn remove_after_exit(&self, dir: &Path) -> Result<(), String>; + /// Renames (moves) `from` to `to` in one step, which fails as a whole: on Windows a file a program holds open (or + /// a folder with one in it) can't be moved, so a failure leaves everything where it was. (A running exe can be.) + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + std::fs::rename(from, to).map_err(|e| format!("{} can't be moved (is it still running?): {e}", from.display())) + } } /// How to install. @@ -361,29 +366,55 @@ fn uninstall_stopped( if shortcut.exists() { std::fs::remove_file(&shortcut).map_err(io(&shortcut))?; } - // The files before the "Apps & features" entry: if one can't go, the entry stays, to run it again. + // The files before the "Apps & features" entry: if they can't go, the entry stays, to run it again. if dir.exists() { - let running_inside = running.parent().is_some_and(|p| same_path(p, &dir)); - if running_inside { - for entry in std::fs::read_dir(&dir).map_err(io(&dir))? { - let path = entry.map_err(io(&dir))?.path(); - if !same_path(&path, running) { - remove_patiently(&path, places.patience)?; - } - } - machine.remove_after_exit(&dir)?; - } else { - remove_patiently(&dir, places.patience)?; - } + remove_program_folder(places, machine, running)?; } registry.remove_key(&places.uninstall_key)?; remove_app_files(&places.app_files); Ok(()) } +/// Removes the program folder. Its files are first moved aside, into `.removing-` next to it, each move +/// one step that fails as a whole if a file is held open: if any can't be moved, those moved go back, and the folder +/// is as it was (so the agent can be put back). Only then is anything deleted: the moved-aside folder (what can't be +/// deleted yet, such as an exe still running, once this setup has ended), and the program folder itself (once this +/// setup has ended, when this setup runs from it). +fn remove_program_folder(places: &Places, machine: &dyn Machine, running: &Path) -> Result<(), String> { + let dir = places.dir(); + let aside = dir.with_file_name(format!("{DIR_NAME}.removing-{}", std::process::id())); + let running_inside = running.parent().is_some_and(|p| same_path(p, &dir)); + if running_inside { + // This setup's own exe can't move: everything else does, one by one, back again if one can't. + std::fs::create_dir_all(&aside).map_err(io(&aside))?; + let mut moved: Vec<(PathBuf, PathBuf)> = Vec::new(); + for entry in std::fs::read_dir(&dir).map_err(io(&dir))? { + let path = entry.map_err(io(&dir))?.path(); + if same_path(&path, running) { + continue; + } + let to = aside.join(path.file_name().unwrap_or_default()); + if let Err(e) = machine.rename(&path, &to) { + for (from, to) in moved.iter().rev() { + let _ = machine.rename(to.as_path(), from.as_path()); + } + let _ = std::fs::remove_dir(&aside); + return Err(e); + } + moved.push((path, to)); + } + } else { + machine.rename(&dir, &aside)?; + } + if remove_patiently(&aside, places.patience).is_err() { + machine.remove_after_exit(&aside)?; + } + if running_inside { machine.remove_after_exit(&dir) } else { Ok(()) } +} + /// The uninstall failed (`failure`) after the agent was stopped and maybe its start at login removed: while the -/// agent's program is still there, its `Run` values are put back and, if it ran, it is started again, so this -/// computer keeps syncing until the uninstall is run again. What is said. +/// program folder is whole, its `Run` values are put back and, if it ran, it is started again, so this computer keeps +/// syncing until the uninstall is run again. What is said. fn put_back( places: &Places, registry: &dyn Registry, @@ -392,17 +423,17 @@ fn put_back( removed_values: &[(&'static str, String)], failure: &str, ) -> String { - if !places.dir().join(AGENT_BG_EXE).is_file() { + let dir = places.dir(); + if ![APP_EXE, AGENT_EXE, AGENT_BG_EXE].iter().all(|file| dir.join(file).is_file()) { return format!("{failure}. Run the uninstall again to finish it."); } - for (name, value) in removed_values { - let _ = registry.set_string(&places.run_key, name, value); - } - if was_running { - format!("{} Start at login was put back.", restarted_after(places, registry, machine, failure)) - } else { - format!("{failure}. Start at login was put back.") - } + let restored = removed_values.iter().all(|(name, value)| registry.set_string(&places.run_key, name, value).is_ok()); + let told = match (removed_values.is_empty(), restored) { + (true, _) => String::new(), + (false, true) => " Start at login was put back.".to_owned(), + (false, false) => " Start at login couldn't all be put back: run the uninstall again, or the setup.".to_owned(), + }; + if was_running { format!("{}{told}", restarted_after(places, registry, machine, failure)) } else { format!("{failure}.{told}") } } /// The app's single-instance files in `app_files`, the folder itself once empty, and the loose ones older versions diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 6d63586..112beea 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -13,6 +13,8 @@ struct Recorder { stuck: bool, /// The agent accepts the quit, then whether it has ended can't be told (tasklist fails). unknowable: bool, + /// Moving a file or folder fails (one is still in use). + rename_fails: bool, /// Where `autostart on` writes the agent's Run value, as the agent does (for the program it was run as)… registry: Option<(Arc, String)>, /// …unless an old agent still running answered it and registered itself. @@ -71,6 +73,12 @@ impl Machine for Recorder { self.note(format!("remove after exit {}", dir.display())); Ok(()) } + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + if self.rename_fails { + return Err(format!("{} can't be moved (is it still running?)", from.display())); + } + std::fs::rename(from, to).map_err(|e| e.to_string()) + } } /// A user's folders in a temporary place: programs, the Start menu, the agent's data folder and the synced folder. @@ -223,6 +231,32 @@ fn an_uninstall_that_fails_after_the_stop_puts_the_agent_back() { assert_eq!(*registry.values.lock().unwrap(), before); } +/// The commonest failure: a file in the program folder is still held, so it can't be moved aside. Nothing in the +/// folder has been touched: the Run values are put back and the agent started again. From "Apps & features" too. +#[test] +fn an_uninstall_with_a_file_still_held_leaves_the_folder_whole_and_puts_the_agent_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let value = format!("\"{}\" run --log", dir.join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + for running in [u.setup.clone(), dir.join(SETUP_EXE)] { + let machine = Recorder { rename_fails: true, ..Recorder::default() }; + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &running).unwrap_err(); + assert!(e.contains("can't be moved") && e.contains("The agent was started again.") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before); + for (file, bytes) in V2 { + assert_eq!(std::fs::read(dir.join(file)).unwrap(), *bytes, "{file} is in place"); + } + let siblings: Vec<_> = std::fs::read_dir(&u.places.programs).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(siblings, [DIR_NAME], "nothing left aside"); + } +} + /// The agent accepts the quit, then tasklist fails: the install and the uninstall stop with nothing else changed, /// and the agent (which may be stopping) is started again from the folder its start at login names. #[test] @@ -404,24 +438,23 @@ fn a_running_agents_file_is_read_and_it_is_asked_to_quit() { assert!(request.contains(&format!("Host: 127.0.0.1:{port}\r\n")) && request.contains("Authorization: Bearer a1b2c3\r\n"), "{request}"); } -/// Files that can't be removed (still held by a program that is ending): tried again for a while; if they still -/// can't, the uninstall fails with the "Apps & features" entry still there, so it can be run again. +/// Files moved aside that can't be deleted at once (here a read-only folder): the uninstall finishes, and the +/// moved-aside folder goes once the setup has ended. #[cfg(unix)] #[test] -fn files_that_cant_be_removed_leave_the_uninstall_entry() { +fn files_moved_aside_that_cant_be_deleted_yet_go_after_the_setup() { use std::os::unix::fs::PermissionsExt; let u = user("Test"); let registry = MemoryRegistry::default(); install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); let dir = u.places.dir(); std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o500)).unwrap(); - let removed = uninstall(&u.places, ®istry, &Recorder::default(), &u.setup); - std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).unwrap(); - assert!(removed.is_err()); - assert_eq!(registry.get(&u.places.uninstall_key, "DisplayName").as_deref(), Some("Craft Cloud"), "removed last"); - // Once they can be removed, running it again finishes. - uninstall(&u.places, ®istry, &Recorder::default(), &u.setup).unwrap(); + let machine = Recorder::default(); + uninstall(&u.places, ®istry, &machine, &u.setup).unwrap(); + let aside = dir.with_file_name(format!("{DIR_NAME}.removing-{}", std::process::id())); + std::fs::set_permissions(&aside, std::fs::Permissions::from_mode(0o700)).unwrap(); assert!(!dir.exists() && registry.get(&u.places.uninstall_key, "DisplayName").is_none()); + assert_eq!(machine.asked().last().cloned(), Some(format!("remove after exit {}", aside.display()))); } /// A file that is held for a moment (a program ending) is removed once it is free. From 1ee4a0ab349e5b9432ea7998b3ee5cabe85da405 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:04:16 +0200 Subject: [PATCH 06/12] setup: only find decides in the cmd remover, and its test checks the wait In the cmd fallback a ping that failed after find had matched (the setup still running) fell through to the removal. The wait now ends in ver, which always succeeds. The test with a tasklist that can't run now checks that each round waited (a broken line would end at once and pass) and cleans up its temporary file. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/tests.rs | 5 ++++- apps/craft-cloud-setup/src/windows.rs | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 112beea..f3aa822 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -619,8 +619,11 @@ fn the_cmd_remover_waits_while_tasklist_cant_tell() { let dir = root.path().join("Craft Cloud"); std::fs::create_dir_all(&dir).unwrap(); std::fs::write(dir.join(SETUP_EXE), "setup").unwrap(); - // Three rounds of asking, then it gives up (an hour in the real one). + // Three rounds of asking, a second's wait each, then it gives up (an hour in the real one). + let started = std::time::Instant::now(); let mut remover = crate::windows::remove_after_with_cmd_without_tasklist(3, std::process::id(), "craft-cloud-setup.exe", &dir).spawn().unwrap(); remover.wait().unwrap(); + assert!(started.elapsed() >= std::time::Duration::from_secs(2), "it waited each round (a broken line ends at once): {:?}", started.elapsed()); assert!(dir.join(SETUP_EXE).exists(), "nothing was removed while it couldn't tell"); + let _ = std::fs::remove_file(std::env::temp_dir().join(format!("craft-cloud-uninstall-{}.txt", std::process::id()))); } diff --git a/apps/craft-cloud-setup/src/windows.rs b/apps/craft-cloud-setup/src/windows.rs index 2804406..6a44cf5 100644 --- a/apps/craft-cloud-setup/src/windows.rs +++ b/apps/craft-cloud-setup/src/windows.rs @@ -82,7 +82,9 @@ pub fn remove_after_with_cmd(pid: u32, image: &str, dir: &Path) -> Command { /// /// `tasklist`'s answer goes to a file first (`CRAFT_CLOUD_TASKS`, in the temporary folder), so a `tasklist` that /// fails (and writes nothing) is told apart from one that lists nothing: then cmd waits and asks again, rather than -/// take the setup for ended. `find` and `ping` are named by their full paths: cmd looks in its working folder first. +/// take the setup for ended. Only `find` decides: the wait after it ends in `ver`, which always succeeds, so a `ping` +/// that fails isn't read as "ended" either. `find` and `ping` are named by their full paths: cmd looks in its working +/// folder first. /// Each `if` is in parentheses of its own: cmd would take a following `& …` as part of it. fn remove_after_with_cmd_using(tasklist: &Path, rounds: u32, pid: u32, image: &str, dir: &Path) -> Command { let (find, ping) = (system_program("find.exe"), system_program("PING.EXE")); @@ -90,7 +92,7 @@ fn remove_after_with_cmd_using(tasklist: &Path, rounds: u32, pid: u32, image: &s let remove = format!("for /l %j in (1,1,30) do @(if exist \"%CRAFT_CLOUD_DIR%\" ({RMDIR} & (if exist \"%CRAFT_CLOUD_DIR%\" ({wait}))))"); let line = format!( "/d /c for /l %i in (1,1,{rounds}) do @((\"{tasklist}\" /FI \"PID eq {pid}\" /NH >\"%CRAFT_CLOUD_TASKS%\") \ - && (\"{find}\" /i \"{image}\" \"%CRAFT_CLOUD_TASKS%\" >nul && {wait} || ({remove} & del \"%CRAFT_CLOUD_TASKS%\" & exit)) || {wait})", + && (\"{find}\" /i \"{image}\" \"%CRAFT_CLOUD_TASKS%\" >nul && ({wait} & ver >nul) || ({remove} & del \"%CRAFT_CLOUD_TASKS%\" & exit)) || {wait})", tasklist = tasklist.display(), find = find.display() ); From d5d7aa8805712395414826e42c665e3b5f63cf6e Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:06:42 +0200 Subject: [PATCH 07/12] install.sh: the restart waiter only waits, with the agent's folder variables - Without a systemd unit for the agent, the waiter ran the agent in the foreground, inside its own transient unit: the unit stayed active for the agent's whole life, so a later install that gave up took it for a waiter and nothing restarted the agent. The waiter now starts the agent outside itself (a unit of its own, else detached) and ends. - A transient unit doesn't get the installer's environment, so the restarted agent could use another data folder. CRAFT_CLOUD_AGENT_DIR and the XDG folder variables that are set are passed with --setenv, to the waiter and to the agent's unit. - The wait checks the process name as well as the pid, so a pid reused by another program doesn't hold the (now exclusive) waiter for a day. - The comment no longer claims the waiter outlives logout: only while the user's systemd runs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- packaging/linux/install.sh | 75 ++++++++++++++++------ xtask/src/package.rs | 126 +++++++++++++++++++++++++++++-------- 2 files changed, 157 insertions(+), 44 deletions(-) diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index d0a4d79..c7114c2 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -45,41 +45,76 @@ has_unit() { [ -f "$unit" ] && command -v systemctl >/dev/null 2>&1 } -# Starts the agent again once the process $1 (or, with no pid known, the one whose local-api.json is in the data -# folder) has ended and so let go of the data folder: an agent started before that finds the folder taken and exits. -# Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. Where the user -# has systemd, the waiter is a unit of its own (systemd-run --user): it outlives this login session (logind's -# KillUserProcesses), and its fixed name keeps it to one at a time; a second install that gives up finds it waiting. +# The variables that pick the agent's folders, those that are set, as `"--setenv=NAME=${NAME}"` words for the +# caller's `eval "set -- \"\$@\" $(env_flags)"`: a systemd unit doesn't get this shell's environment. +env_flags() { + for name in CRAFT_CLOUD_AGENT_DIR XDG_DATA_HOME XDG_CONFIG_HOME XDG_STATE_HOME; do + if eval "[ -n \"\${$name+x}\" ]"; then + printf ' "--setenv=%s=${%s}"' "$name" "$name" + fi + done +} + +# Whether the process $1 still runs and is still the one whose name (/proc//comm) was $2: a pid of an agent that +# has ended may be another program's by now. +alive() { + kill -0 "$1" 2>/dev/null && { [ -z "$2" ] || [ "$(cat "/proc/$1/comm" 2>/dev/null)" = "$2" ]; } +} + +# Starts the agent again once the process $1 (named $4; or, with no pid known, the one whose local-api.json is in the +# data folder) has ended and so let go of the data folder: an agent started before that finds the folder taken and +# exits. Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. +# +# The waiter only waits: the agent it starts runs outside it (its own systemd unit, or detached), so the waiter is +# gone once the agent is up. Where the user has systemd, the waiter is a unit of its own (systemd-run --user, with +# the folder variables passed along): it outlives this login session for as long as the user's systemd runs (another +# session, or lingering), and its fixed name keeps it to one at a time; a second install that gives up while it +# waits finds it and adds none. start_after_exit() { waiter=' + pid=$1 how=$2 agent=$3 data=$4 comm=$5 + alive() { + kill -0 "$1" 2>/dev/null && { [ -z "$2" ] || [ "$(cat "/proc/$1/comm" 2>/dev/null)" = "$2" ]; } + } n=0 - if [ -n "$1" ]; then - while kill -0 "$1" 2>/dev/null && [ "$n" -lt 86400 ]; do + if [ -n "$pid" ]; then + while alive "$pid" "$comm" && [ "$n" -lt 86400 ]; do n=$((n + 1)) sleep 1 done else - while [ -e "$4/local-api.json" ] && [ "$n" -lt 86400 ]; do + while [ -e "$data/local-api.json" ] && [ "$n" -lt 86400 ]; do n=$((n + 1)) sleep 1 done sleep 5 fi - if [ "$2" = systemd ]; then + if [ "$how" = systemd ]; then systemctl --user start craft-cloud-agent.service - else - "$3" run --log + exit + fi + set -- + for name in CRAFT_CLOUD_AGENT_DIR XDG_DATA_HOME XDG_CONFIG_HOME XDG_STATE_HOME; do + if eval "[ -n \"\${$name+x}\" ]"; then + eval "set -- \"\$@\" \"--setenv=$name=\${$name}\"" + fi + done + if ! { command -v systemd-run >/dev/null 2>&1 && systemd-run --user --quiet --collect --unit="craft-cloud-agent-run-$$" "$@" "$agent" run --log >/dev/null 2>&1; }; then + nohup "$agent" run --log >/dev/null 2>&1 & fi ' + w_pid=$1 w_how=$2 w_agent=$3 w_comm=$4 if command -v systemd-run >/dev/null 2>&1; then - if systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart /bin/sh -c "$waiter" sh "$1" "$2" "$3" "$data" >/dev/null 2>&1; then + set -- + eval "set -- \"\$@\" $(env_flags)" + if systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart "$@" /bin/sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" >/dev/null 2>&1; then return 0 fi if command -v systemctl >/dev/null 2>&1 && systemctl --user --quiet is-active craft-cloud-agent-restart.service 2>/dev/null; then return 0 fi fi - nohup sh -c "$waiter" sh "$1" "$2" "$3" "$data" >/dev/null 2>&1 & + nohup sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" >/dev/null 2>&1 & } # The agent that ran and was stopped is started again (once): this computer keeps syncing. @@ -101,11 +136,11 @@ start_again() { give_up() { again=", and it starts again once done" if has_unit; then - start_after_exit "$pid" systemd "" + start_after_exit "$pid" systemd "" "$comm" elif [ -x "$lib/craft-cloud-agent" ]; then - start_after_exit "$pid" run "$lib/craft-cloud-agent" + start_after_exit "$pid" run "$lib/craft-cloud-agent" "$comm" elif command -v craft-cloud-agent >/dev/null 2>&1; then - start_after_exit "$pid" run "$(command -v craft-cloud-agent)" + start_after_exit "$pid" run "$(command -v craft-cloud-agent)" "$comm" else again=" (it stops once done; start it again with 'craft-cloud-agent run')" fi @@ -122,9 +157,13 @@ stopped=0 tries=0 # Read before the quit: the agent removes the file as it stops. pid=$(agent_pid) +comm=$(cat "/proc/$pid/comm" 2>/dev/null || true) while "$here/craft-cloud-agent" quit >/dev/null 2>&1; do was_running=1 - [ -n "$pid" ] || pid=$(agent_pid) + if [ -z "$pid" ]; then + pid=$(agent_pid) + comm=$(cat "/proc/$pid/comm" 2>/dev/null || true) + fi tries=$((tries + 1)) if [ "$tries" = 3 ]; then echo "Waiting for the Craft Cloud agent to finish its sync…" @@ -136,7 +175,7 @@ while "$here/craft-cloud-agent" quit >/dev/null 2>&1; do done if [ "$was_running" = 1 ]; then if [ -n "$pid" ]; then - while kill -0 "$pid" 2>/dev/null; do + while alive "$pid" "$comm"; do tries=$((tries + 1)) if [ "$tries" -ge "$wait_for" ]; then give_up diff --git a/xtask/src/package.rs b/xtask/src/package.rs index b71dcc3..39f08c0 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -258,6 +258,17 @@ mod tests { let _ = std::fs::remove_dir_all(&tmp); } + /// A running agent for install.sh: a `sleep` of `lasts` seconds that isn't this test's child (so it never lingers + /// as a zombie), named in `local-api.json` in `data`. Its pid. + #[cfg(target_os = "linux")] + fn agent_process(data: &Path, lasts: u32) -> u32 { + let out = Command::new("sh").args(["-c", &format!("sleep {lasts} >/dev/null 2>&1 & echo $!")]).output().unwrap(); + let pid = String::from_utf8_lossy(&out.stdout).trim().parse::().unwrap(); + std::fs::write(data.join("local-api.json"), format!("{{\n \"schema\": 1,\n \"port\": 1,\n \"token\": \"t\",\n \"pid\": {pid}\n}}")) + .unwrap(); + pid + } + /// An unpacked tarball in a temporary home, with an agent stand-in that answers `quit` `quits` times (it runs, /// finishing a pass) and then no more, a `systemctl` stand-in, and the agent's systemd unit: lptp1's upgrade. The /// running agent's process is a `sleep` of `lasts` seconds (none for 0), named in `local-api.json`; every call @@ -269,7 +280,7 @@ mod tests { src: PathBuf, calls: PathBuf, bin: PathBuf, - pid: Option, + pid: std::cell::Cell>, } #[cfg(target_os = "linux")] @@ -284,17 +295,7 @@ mod tests { std::fs::create_dir_all(dir).unwrap(); } // The agent's process: a `sleep` that isn't this test's child (so it never lingers as a zombie). - let pid = (lasts > 0).then(|| { - let out = Command::new("sh").args(["-c", &format!("sleep {lasts} >/dev/null 2>&1 & echo $!")]).output().unwrap(); - String::from_utf8_lossy(&out.stdout).trim().parse::().unwrap() - }); - if let Some(pid) = pid { - std::fs::write( - data.join("local-api.json"), - format!("{{\n \"schema\": 1,\n \"port\": 1,\n \"token\": \"t\",\n \"pid\": {pid}\n}}"), - ) - .unwrap(); - } + let pid = (lasts > 0).then(|| agent_process(&data, lasts)); let pid_text = pid.map(|p| p.to_string()).unwrap_or_default(); let note = format!("$(if [ -n \"{pid_text}\" ] && kill -0 {pid_text} 2>/dev/null; then echo ' [old agent running]'; fi)"); let calls = tmp.join("calls.txt"); @@ -312,18 +313,21 @@ mod tests { std::fs::write(src.join("craft-cloud.png"), "png").unwrap(); std::fs::write(src.join(LICENSES), "licences").unwrap(); std::fs::copy(crate::root().join("packaging/linux/install.sh"), src.join("install.sh")).unwrap(); - // systemd-run: one unit of a name at a time (`unit` marks it running); systemctl says so for is-active. - let unit = tmp.join("unit-running"); + // systemd-run: one unit of a name at a time (a file in `units/` while it runs; it records its options); + // systemctl's is-active says whether a unit runs. + let units = tmp.join("units"); + std::fs::create_dir_all(&units).unwrap(); let systemctl = format!( - "#!/bin/sh\nif [ \"$3\" = is-active ]; then [ -e \"{unit}\" ]; exit; fi\necho \"systemctl $*{note}\" >> \"{calls}\"\n", - unit = unit.display(), + "#!/bin/sh\nif [ \"$3\" = is-active ]; then [ -e \"{units}/${{4%.service}}\" ]; exit; fi\necho \"systemctl $*{note}\" >> \"{calls}\"\n", + units = units.display(), calls = calls.display() ); std::fs::write(bin.join("systemctl"), systemctl).unwrap(); let systemd_run = format!( - "#!/bin/sh\nopts=\"\"\nwhile [ \"${{1#--}}\" != \"$1\" ]; do opts=\"$opts $1\"; shift; done\n[ -e \"{unit}\" ] && exit 1\n\ - echo \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{unit}\"\n(\"$@\"; rm -f \"{unit}\") >/dev/null 2>&1 &\n", - unit = unit.display(), + "#!/bin/sh\nopts=\"\"\nunit=\"\"\nwhile [ \"${{1#--}}\" != \"$1\" ]; do\n opts=\"$opts $1\"\n case \"$1\" in --unit=*) unit=${{1#--unit=}} ;; esac\n shift\ndone\n\ + [ -e \"{units}/$unit\" ] && exit 1\necho \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{units}/$unit\"\n\ + (\"$@\"; rm -f \"{units}/$unit\") >/dev/null 2>&1 &\n", + units = units.display(), calls = calls.display() ); std::fs::write(bin.join("systemd-run"), systemd_run).unwrap(); @@ -335,7 +339,7 @@ mod tests { let unit = home.join(".config/systemd/user/craft-cloud-agent.service"); std::fs::create_dir_all(unit.parent().unwrap()).unwrap(); std::fs::write(&unit, "[Service]\nExecStart=/home/fabio/.local/bin/craft-cloud-agent run\n").unwrap(); - Self { tmp, home, src, calls, bin, pid } + Self { tmp, home, src, calls, bin, pid: std::cell::Cell::new(pid) } } fn without_unit(self) -> Self { @@ -344,14 +348,29 @@ mod tests { } fn install(&self, wait: &str) -> std::process::Output { - Command::new("sh") - .arg(self.src.join("install.sh")) + self.install_with(wait, &[]) + } + + /// install.sh, in a desktop session (a session bus), with `vars` set too. + fn install_with(&self, wait: &str, vars: &[(&str, &str)]) -> std::process::Output { + let mut sh = Command::new("sh"); + sh.arg(self.src.join("install.sh")) .env_clear() .env("HOME", &self.home) .env("PATH", format!("{}:/usr/bin:/bin", self.bin.display())) - .env("CRAFT_CLOUD_INSTALL_WAIT", wait) - .output() - .unwrap() + .env("DBUS_SESSION_BUS_ADDRESS", "unix:path=/test-session-bus") + .env("CRAFT_CLOUD_INSTALL_WAIT", wait); + for (name, value) in vars { + sh.env(name, value); + } + sh.output().unwrap() + } + + /// A new agent runs (`lasts` seconds), busy with a pass: it answers one more `quit`. + fn agent_runs_again(&self, lasts: u32) { + let pid = agent_process(&self.home.join(".local/share/craft-cloud-agent"), lasts); + self.pid.set(Some(pid)); + let _ = std::fs::remove_file(self.tmp.join("quits.txt")); } fn calls(&self) -> Vec { @@ -360,7 +379,7 @@ mod tests { /// The old agent's process ends (its pass is done). fn agent_ends(&self) { - if let Some(pid) = self.pid { + if let Some(pid) = self.pid.get() { let _ = Command::new("kill").arg(pid.to_string()).status(); } } @@ -474,6 +493,61 @@ mod tests { assert_eq!(calls.iter().filter(|c| *c == start).count(), 1, "{calls:?}"); } + /// Without a unit, the waiter starts the agent in a unit of its own and ends: once the agent is up no waiter is + /// left, so a later install that gives up adds one again, and the agent is started again a second time. + #[cfg(target_os = "linux")] + #[test] + fn after_a_restart_without_a_unit_a_later_give_up_waits_again() { + let up = Upgrade::new("rerun", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let run = format!("{} run --log", up.lib_agent()); + let waiter = "systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart"; + for round in 1..=2 { + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "round {round}: {out:?}"); + up.agent_ends(); + up.ends_with(&run); + let calls = up.calls(); + assert_eq!(calls.iter().filter(|c| *c == waiter).count(), round, "a waiter each time: {calls:?}"); + assert_eq!(calls.iter().filter(|c| **c == run).count(), round, "{calls:?}"); + assert!(calls.iter().any(|c| c.starts_with("systemd-run --user --quiet --collect --unit=craft-cloud-agent-run-")), "{calls:?}"); + // The waiter's unit is gone (the agent runs in its own). + let unit = up.tmp.join("units/craft-cloud-agent-restart"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while unit.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!(!unit.exists(), "round {round}: the waiter has ended"); + up.agent_runs_again(120); + } + } + + /// A unit doesn't get the installer's environment: the variables that pick the agent's folders are passed to the + /// waiter's unit and the agent's, those that are set and no others. + #[cfg(target_os = "linux")] + #[test] + fn the_folder_variables_go_to_the_units() { + let up = Upgrade::new("setenv", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let data = up.home.join(".local/share/craft-cloud-agent").display().to_string(); + let config = up.home.join("my config").display().to_string(); + let out = up.install_with("2", &[("CRAFT_CLOUD_AGENT_DIR", &data), ("XDG_CONFIG_HOME", &config)]); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + up.agent_ends(); + let calls = up.ends_with(&format!("{} run --log", up.lib_agent())); + let flags = format!(" --setenv=CRAFT_CLOUD_AGENT_DIR={data} --setenv=XDG_CONFIG_HOME={config}"); + let units: Vec<&String> = calls.iter().filter(|c| c.starts_with("systemd-run")).collect(); + assert_eq!(units.len(), 2, "the waiter's and the agent's: {calls:?}"); + for unit in units { + assert!(unit.ends_with(&flags), "{unit}"); + assert!(!unit.contains("XDG_DATA_HOME") && !unit.contains("XDG_STATE_HOME"), "only those set: {unit}"); + } + } + /// Without systemd-run the waiter runs in the background of this shell (nohup). #[cfg(target_os = "linux")] #[test] From bedeb9d4411de70379da89bc66455df014ee0a03 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:06:42 +0200 Subject: [PATCH 08/12] install.sh: restart the tray only in a desktop session, and clean up on Ctrl-C Run over ssh or without a session bus, the tray closed for the upgrade couldn't register again and the icon was gone. It is now restarted only where the session bus is reachable; elsewhere the script says how to get it back. INT, TERM and HUP end the script through exit, so dash runs the EXIT trap and the agent and tray are started again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- packaging/linux/install.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index c7114c2..e210f8c 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -199,11 +199,20 @@ finish() { if [ "$stopped" = 1 ] && [ "$restarted" = 0 ]; then start_again fi + # The tray needs the desktop session's bus: from elsewhere (ssh), it is left to the next login, and said. case "$closed" in - *tray*) [ -x "$lib/craft-cloud" ] && nohup "$lib/craft-cloud" --tray >/dev/null 2>&1 & ;; + *tray*) + if [ -n "${DBUS_SESSION_BUS_ADDRESS:-}" ] || [ -S "${XDG_RUNTIME_DIR:-/nonexistent}/bus" ]; then + [ -x "$lib/craft-cloud" ] && nohup "$lib/craft-cloud" --tray >/dev/null 2>&1 & + else + echo "The Craft Cloud tray was closed for the upgrade; run 'craft-cloud --tray' in your desktop session (or log in again) to get it back." >&2 + fi + ;; esac } trap finish EXIT +# Ctrl-C and the like end the script through `exit`, so the EXIT trap runs (dash doesn't run it on a signal). +trap 'exit 130' INT TERM HUP # The app's window and tray process, an older version's too, are closed before the copy: an old tray would keep its # icon next to the new one's. A tray that ran is started again at the end. From 80d85df2c98952a50da5ded842197759548a3d64 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:31:54 +0200 Subject: [PATCH 09/12] setup: report a roll-back that falls short, and finish leftovers on the next run - Uninstalling from inside the folder moves the files aside one by one; when one can't move, those moved go back. A file that can't go back is now named in the message instead of being dropped silently. - A folder an earlier run left aside (.removing-, its setup no longer running) is finished by the next run: an install first moves back whatever the program folder lacks and then installs over it; an uninstall deletes it. - Once the files are moved aside, failing to delete them now and failing to schedule the delete for later no longer fails the uninstall: the next run removes the leftover. - Each cmd remover has a temporary file of its own: two may run at once. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/lib.rs | 47 ++++++++++- apps/craft-cloud-setup/src/tests.rs | 108 +++++++++++++++++++++++++- apps/craft-cloud-setup/src/windows.rs | 16 +++- 3 files changed, 165 insertions(+), 6 deletions(-) diff --git a/apps/craft-cloud-setup/src/lib.rs b/apps/craft-cloud-setup/src/lib.rs index 597762a..db8be11 100644 --- a/apps/craft-cloud-setup/src/lib.rs +++ b/apps/craft-cloud-setup/src/lib.rs @@ -146,6 +146,8 @@ pub trait Machine { fn shortcut(&self, lnk: &Path, target: &Path) -> Result<(), String>; /// Removes the folder `dir` once this process has ended (it holds this setup's own exe). fn remove_after_exit(&self, dir: &Path) -> Result<(), String>; + /// Whether the process `pid` runs (any program); `true` when that can't be told. + fn process_runs(&self, pid: u32) -> bool; /// Renames (moves) `from` to `to` in one step, which fails as a whole: on Windows a file a program holds open (or /// a folder with one in it) can't be moved, so a failure leaves everything where it was. (A running exe can be.) fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { @@ -260,6 +262,7 @@ fn install_stopped( let dir = places.dir(); let old = places.programs.join(OLD_DIR_NAME); let mut warnings = Vec::new(); + clear_leftovers(places, machine, true); std::fs::create_dir_all(&dir).map_err(io(&dir))?; for (name, bytes) in payload { write_replacing(&dir.join(name), bytes)?; @@ -356,6 +359,7 @@ fn uninstall_stopped( removed_values: &mut Vec<(&'static str, String)>, ) -> Result<(), String> { let dir = places.dir(); + clear_leftovers(places, machine, false); for value in [AGENT_RUN_VALUE, APP_RUN_VALUE] { if let Some(v) = registry.get(&places.run_key, value).filter(|v| starts_from(v, &dir)) { registry.remove_value(&places.run_key, value)?; @@ -395,23 +399,60 @@ fn remove_program_folder(places: &Places, machine: &dyn Machine, running: &Path) } let to = aside.join(path.file_name().unwrap_or_default()); if let Err(e) = machine.rename(&path, &to) { + let mut stranded = Vec::new(); for (from, to) in moved.iter().rev() { - let _ = machine.rename(to.as_path(), from.as_path()); + if machine.rename(to, from).is_err() { + stranded.push(from.file_name().unwrap_or_default().to_string_lossy().into_owned()); + } } let _ = std::fs::remove_dir(&aside); - return Err(e); + if stranded.is_empty() { + return Err(e); + } + return Err(format!( + "{e}; and {} couldn't be moved back from {} (the next run of the setup puts them back)", + stranded.join(", "), + aside.display() + )); } moved.push((path, to)); } } else { machine.rename(&dir, &aside)?; } + // Past the point of no return: what can't be deleted now or after the setup is a leftover the next run removes. if remove_patiently(&aside, places.patience).is_err() { - machine.remove_after_exit(&aside)?; + let _ = machine.remove_after_exit(&aside); } if running_inside { machine.remove_after_exit(&dir) } else { Ok(()) } } +/// Folders an earlier uninstall moved aside (`.removing-`) and left behind, its setup no longer running: +/// a roll-back that couldn't finish, or a delete that couldn't. On install (`restore`), every file the program folder +/// lacks is moved back into it first; then, and on uninstall, the leftover is deleted (best effort). +fn clear_leftovers(places: &Places, machine: &dyn Machine, restore: bool) { + let dir = places.dir(); + let prefix = format!("{DIR_NAME}.removing-"); + let Ok(entries) = std::fs::read_dir(&places.programs) else { return }; + for entry in entries.filter_map(Result::ok) { + let name = entry.file_name().to_string_lossy().into_owned(); + let Some(pid) = name.strip_prefix(&prefix).and_then(|pid| pid.parse::().ok()) else { continue }; + if pid == std::process::id() || machine.process_runs(pid) { + continue; + } + let leftover = entry.path(); + if restore && std::fs::create_dir_all(&dir).is_ok() { + for file in std::fs::read_dir(&leftover).into_iter().flatten().filter_map(Result::ok) { + let back = dir.join(file.file_name()); + if !back.exists() { + let _ = machine.rename(&file.path(), &back); + } + } + } + let _ = remove_patiently(&leftover, places.patience); + } +} + /// The uninstall failed (`failure`) after the agent was stopped and maybe its start at login removed: while the /// program folder is whole, its `Run` values are put back and, if it ran, it is started again, so this computer keeps /// syncing until the uninstall is run again. What is said. diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index f3aa822..203f41e 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -15,6 +15,9 @@ struct Recorder { unknowable: bool, /// Moving a file or folder fails (one is still in use). rename_fails: bool, + /// Only the move with this number (from 1) fails. + rename_fails_at: Option, + renames: Mutex, /// Where `autostart on` writes the agent's Run value, as the agent does (for the program it was run as)… registry: Option<(Arc, String)>, /// …unless an old agent still running answered it and registered itself. @@ -73,8 +76,16 @@ impl Machine for Recorder { self.note(format!("remove after exit {}", dir.display())); Ok(()) } + fn process_runs(&self, _pid: u32) -> bool { + false + } fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { - if self.rename_fails { + let number = { + let mut renames = self.renames.lock().unwrap(); + *renames += 1; + *renames + }; + if self.rename_fails || self.rename_fails_at == Some(number) { return Err(format!("{} can't be moved (is it still running?)", from.display())); } std::fs::rename(from, to).map_err(|e| e.to_string()) @@ -257,6 +268,94 @@ fn an_uninstall_with_a_file_still_held_leaves_the_folder_whole_and_puts_the_agen } } +/// From "Apps & features" (the setup runs from the folder) the files move one by one: the third can't, so the two +/// moved come back, the Run values are put back and the agent is started again. +#[test] +fn a_move_that_fails_partway_puts_the_moved_files_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let value = format!("\"{}\" run --log", dir.join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + let machine = Recorder { rename_fails_at: Some(3), ..Recorder::default() }; + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &dir.join(SETUP_EXE)).unwrap_err(); + assert!(e.contains("can't be moved") && !e.contains("couldn't be moved back") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(*machine.renames.lock().unwrap(), 5, "two moves, the failed third, two moves back"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before); + for (file, bytes) in V2 { + assert_eq!(std::fs::read(dir.join(file)).unwrap(), *bytes, "{file} is back"); + } + let siblings: Vec<_> = std::fs::read_dir(&u.places.programs).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(siblings, [DIR_NAME], "nothing left aside"); + + // Moves back that fail too: the two files left aside are named. + let e = uninstall(&u.places, ®istry, &FailingBack(Recorder::default()), &dir.join(SETUP_EXE)).unwrap_err(); + let (_, stranded) = e.split_once("; and ").unwrap(); + assert_eq!(V2.iter().filter(|(file, _)| stranded.contains(file)).count(), 2, "{e}"); + assert!(e.contains("the next run of the setup puts them back"), "{e}"); +} + +/// A machine whose third move fails, and every move after it. +struct FailingBack(Recorder); + +impl Machine for FailingBack { + fn stop_agent(&self, data: &Path) -> AgentStop { + self.0.stop_agent(data) + } + fn close_app(&self) {} + fn run_agent(&self, agent: &Path, args: &[&str]) -> Result<(), String> { + self.0.run_agent(agent, args) + } + fn start_agent(&self, agent_bg: &Path, data: &Path) -> Result<(), String> { + self.0.start_agent(agent_bg, data) + } + fn shortcut(&self, lnk: &Path, target: &Path) -> Result<(), String> { + self.0.shortcut(lnk, target) + } + fn remove_after_exit(&self, dir: &Path) -> Result<(), String> { + self.0.remove_after_exit(dir) + } + fn process_runs(&self, _pid: u32) -> bool { + false + } + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + let mut renames = self.0.renames.lock().unwrap(); + *renames += 1; + if *renames >= 3 { + return Err(format!("{} can't be moved", from.display())); + } + std::fs::rename(from, to).map_err(|e| e.to_string()) + } +} + +/// A folder an earlier uninstall left aside (its setup gone) holding the agent's exe the program folder lacks: the +/// next install moves it back first and installs over it; the leftover is gone. An uninstall just deletes one. +#[test] +fn a_leftover_aside_folder_is_restored_by_install_and_removed_by_uninstall() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let leftover = dir.with_file_name(format!("{DIR_NAME}.removing-4000000")); + std::fs::create_dir_all(&leftover).unwrap(); + std::fs::rename(dir.join(AGENT_BG_EXE), leftover.join(AGENT_BG_EXE)).unwrap(); + std::fs::write(leftover.join("notes.txt"), "kept by an older version").unwrap(); + // The install's payload lacks notes.txt: only a file moved back can be there afterwards. + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + assert!(!leftover.exists(), "the leftover is gone"); + assert_eq!(std::fs::read(dir.join(AGENT_BG_EXE)).unwrap(), b"agent-bg v2"); + assert!(dir.join("notes.txt").is_file(), "what the folder lacked was moved back"); + + std::fs::create_dir_all(&leftover).unwrap(); + std::fs::write(leftover.join(AGENT_EXE), "old").unwrap(); + uninstall(&u.places, ®istry, &Recorder::default(), &u.setup).unwrap(); + assert!(!leftover.exists() && !dir.exists()); +} + /// The agent accepts the quit, then tasklist fails: the install and the uninstall stop with nothing else changed, /// and the agent (which may be stopping) is started again from the folder its start at login names. #[test] @@ -625,5 +724,10 @@ fn the_cmd_remover_waits_while_tasklist_cant_tell() { remover.wait().unwrap(); assert!(started.elapsed() >= std::time::Duration::from_secs(2), "it waited each round (a broken line ends at once): {:?}", started.elapsed()); assert!(dir.join(SETUP_EXE).exists(), "nothing was removed while it couldn't tell"); - let _ = std::fs::remove_file(std::env::temp_dir().join(format!("craft-cloud-uninstall-{}.txt", std::process::id()))); + let mine = format!("craft-cloud-uninstall-{}-", std::process::id()); + for file in std::fs::read_dir(std::env::temp_dir()).unwrap().filter_map(Result::ok) { + if file.file_name().to_string_lossy().starts_with(&mine) { + let _ = std::fs::remove_file(file.path()); + } + } } diff --git a/apps/craft-cloud-setup/src/windows.rs b/apps/craft-cloud-setup/src/windows.rs index 6a44cf5..5360e68 100644 --- a/apps/craft-cloud-setup/src/windows.rs +++ b/apps/craft-cloud-setup/src/windows.rs @@ -96,7 +96,10 @@ fn remove_after_with_cmd_using(tasklist: &Path, rounds: u32, pid: u32, image: &s tasklist = tasklist.display(), find = find.display() ); - let tasks = std::env::temp_dir().join(format!("craft-cloud-uninstall-{pid}.txt")); + // One file per remover: two may run at once (the moved-aside folder's and the program folder's). + static REMOVERS: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let n = REMOVERS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let tasks = std::env::temp_dir().join(format!("craft-cloud-uninstall-{pid}-{n}.txt")); let mut command = quiet(&system_program("cmd.exe")); command.raw_arg(line).env("CRAFT_CLOUD_DIR", dir).env("CRAFT_CLOUD_TASKS", tasks); detached_from(command, dir) @@ -281,6 +284,17 @@ impl Machine for ThisMachine { if status.success() && lnk.exists() { Ok(()) } else { Err(format!("the shortcut wasn't written ({status})")) } } + fn process_runs(&self, pid: u32) -> bool { + let out = quiet(&system_program("tasklist.exe")).args(["/FI", &format!("PID eq {pid}"), "/NH", "/FO", "CSV"]).stdout(Stdio::piped()).output(); + match out { + Ok(out) if out.status.success() && !out.stdout.is_empty() => { + String::from_utf8_lossy(&out.stdout).lines().any(|line| line.split("\",\"").nth(1) == Some(pid.to_string().as_str())) + } + // Can't tell: taken as running, so nothing of it is touched. + _ => true, + } + } + fn remove_after_exit(&self, dir: &Path) -> Result<(), String> { // Waits for this setup to end, however long its last message stays open: through PowerShell, else `cmd`. let pid = std::process::id(); From d61f2e7655251fcb838911fd5e260242593e16f8 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:31:54 +0200 Subject: [PATCH 10/12] install.sh: in its own unit, the waiter reports an agent it can't start Inside the waiter's unit, a nohup fallback for a failed systemd-run would be killed with the unit as the waiter ends. There it now says in the journal that the agent couldn't be started again; nohup stays the fallback only outside systemd. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- packaging/linux/install.sh | 7 ++++++- xtask/src/package.rs | 28 ++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index e210f8c..2e82711 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -99,7 +99,12 @@ start_after_exit() { eval "set -- \"\$@\" \"--setenv=$name=\${$name}\"" fi done - if ! { command -v systemd-run >/dev/null 2>&1 && systemd-run --user --quiet --collect --unit="craft-cloud-agent-run-$$" "$@" "$agent" run --log >/dev/null 2>&1; }; then + if command -v systemd-run >/dev/null 2>&1 && systemd-run --user --quiet --collect --unit="craft-cloud-agent-run-$$" "$@" "$agent" run --log >/dev/null 2>&1; then + : + elif [ -n "${INVOCATION_ID:-}" ]; then + # In a unit of its own (this waiter), whatever it starts ends with it: said in the journal instead. + echo "Craft Cloud: the agent couldn'"'"'t be started again; run '"'"'craft-cloud-agent run --log'"'"' or log in again" >&2 + else nohup "$agent" run --log >/dev/null 2>&1 & fi ' diff --git a/xtask/src/package.rs b/xtask/src/package.rs index 39f08c0..96f28ce 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -325,8 +325,9 @@ mod tests { std::fs::write(bin.join("systemctl"), systemctl).unwrap(); let systemd_run = format!( "#!/bin/sh\nopts=\"\"\nunit=\"\"\nwhile [ \"${{1#--}}\" != \"$1\" ]; do\n opts=\"$opts $1\"\n case \"$1\" in --unit=*) unit=${{1#--unit=}} ;; esac\n shift\ndone\n\ - [ -e \"{units}/$unit\" ] && exit 1\necho \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{units}/$unit\"\n\ - (\"$@\"; rm -f \"{units}/$unit\") >/dev/null 2>&1 &\n", + [ -e \"{units}/$unit\" ] && exit 1\ncase \"$unit\" in craft-cloud-agent-run-*) [ -e \"{units}/refuse-agent\" ] && exit 1 ;; esac\n\ + echo \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{units}/$unit\"\n\ + (INVOCATION_ID=test \"$@\" 2>> \"{units}/journal\"; rm -f \"{units}/$unit\") >/dev/null &\n", units = units.display(), calls = calls.display() ); @@ -524,6 +525,29 @@ mod tests { } } + /// The agent's own unit can't be made: the waiter, in a unit of its own, starts nothing that would die with it, + /// and says so in the journal. + #[cfg(target_os = "linux")] + #[test] + fn a_waiter_that_cant_start_the_agents_unit_says_so() { + let up = Upgrade::new("refused", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let units = up.tmp.join("units"); + std::fs::write(units.join("refuse-agent"), "").unwrap(); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + up.agent_ends(); + let journal = units.join("journal"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + while !std::fs::read_to_string(&journal).unwrap_or_default().contains("couldn't be started again") { + assert!(std::time::Instant::now() < deadline, "nothing said: {:?}", up.calls()); + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!(!up.calls().iter().any(|c| c.ends_with("run --log")), "{:?}", up.calls()); + } + /// A unit doesn't get the installer's environment: the variables that pick the agent's folders are passed to the /// waiter's unit and the agent's, those that are set and no others. #[cfg(target_os = "linux")] From e362101076ff4487f1b4750eef5195feeceb93c4 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:06:53 +0200 Subject: [PATCH 11/12] install.sh: start nothing in the background that holds its streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run over ssh, install.sh finished but never returned: the tray it starts kept the session's output open. Under bash (Arch's sh), a background `[ -x … ] && nohup … &` list's subshell keeps the script's streams while the program runs; dash execs it instead, which hid the bug here. Every background start now goes through detach: stdin, stdout and stderr on /dev/null, in a session of its own where setsid is there. The waiter's own fallback does the same. A test runs install.sh under sh and bash with its output a pipe and checks the pipe ends while the tray runs on. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- packaging/linux/install.sh | 23 ++++++++++++--- xtask/src/package.rs | 58 ++++++++++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index 2e82711..26e83cf 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -45,6 +45,17 @@ has_unit() { [ -f "$unit" ] && command -v systemctl >/dev/null 2>&1 } +# Runs "$@" in the background holding none of this script's streams (stdin, stdout and stderr on /dev/null), in a +# session of its own where setsid is there: whatever waits for this script's output (ssh, a pipe) isn't kept waiting +# for the programs it starts. +detach() { + if command -v setsid >/dev/null 2>&1; then + setsid "$@" /dev/null 2>&1 & + else + nohup "$@" /dev/null 2>&1 & + fi +} + # The variables that pick the agent's folders, those that are set, as `"--setenv=NAME=${NAME}"` words for the # caller's `eval "set -- \"\$@\" $(env_flags)"`: a systemd unit doesn't get this shell's environment. env_flags() { @@ -104,8 +115,10 @@ start_after_exit() { elif [ -n "${INVOCATION_ID:-}" ]; then # In a unit of its own (this waiter), whatever it starts ends with it: said in the journal instead. echo "Craft Cloud: the agent couldn'"'"'t be started again; run '"'"'craft-cloud-agent run --log'"'"' or log in again" >&2 + elif command -v setsid >/dev/null 2>&1; then + setsid "$agent" run --log /dev/null 2>&1 & else - nohup "$agent" run --log >/dev/null 2>&1 & + nohup "$agent" run --log /dev/null 2>&1 & fi ' w_pid=$1 w_how=$2 w_agent=$3 w_comm=$4 @@ -119,7 +132,7 @@ start_after_exit() { return 0 fi fi - nohup sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" >/dev/null 2>&1 & + detach sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" } # The agent that ran and was stopped is started again (once): this computer keeps syncing. @@ -131,7 +144,7 @@ start_again() { systemctl --user stop craft-cloud-agent.service || true systemctl --user start craft-cloud-agent.service || true elif [ -x "$lib/craft-cloud-agent" ]; then - nohup "$lib/craft-cloud-agent" run --log >/dev/null 2>&1 & + detach "$lib/craft-cloud-agent" run --log fi } @@ -208,7 +221,9 @@ finish() { case "$closed" in *tray*) if [ -n "${DBUS_SESSION_BUS_ADDRESS:-}" ] || [ -S "${XDG_RUNTIME_DIR:-/nonexistent}/bus" ]; then - [ -x "$lib/craft-cloud" ] && nohup "$lib/craft-cloud" --tray >/dev/null 2>&1 & + if [ -x "$lib/craft-cloud" ]; then + detach "$lib/craft-cloud" --tray + fi else echo "The Craft Cloud tray was closed for the upgrade; run 'craft-cloud --tray' in your desktop session (or log in again) to get it back." >&2 fi diff --git a/xtask/src/package.rs b/xtask/src/package.rs index 96f28ce..7dea318 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -307,8 +307,13 @@ mod tests { count = count.display() ); std::fs::write(src.join("craft-cloud-agent"), agent).unwrap(); - // The app: an (old) tray runs, which `--close` ends and says. - let app = format!("#!/bin/sh\necho \"$0 $*\" >> \"{}\"\n[ \"$1\" = --close ] && echo tray\nexit 0\n", calls.display()); + // The app: an (old) tray runs, which `--close` ends and says; a tray started stays (its pid in tray.pid). + let app = format!( + "#!/bin/sh\necho \"$0 $*\" >> \"{calls}\"\n[ \"$1\" = --close ] && echo tray\n\ + if [ \"$1\" = --tray ]; then echo $$ > \"{tray}\"; exec sleep 60; fi\nexit 0\n", + calls = calls.display(), + tray = tmp.join("tray.pid").display() + ); std::fs::write(src.join("craft-cloud"), app).unwrap(); std::fs::write(src.join("craft-cloud.png"), "png").unwrap(); std::fs::write(src.join(LICENSES), "licences").unwrap(); @@ -354,7 +359,12 @@ mod tests { /// install.sh, in a desktop session (a session bus), with `vars` set too. fn install_with(&self, wait: &str, vars: &[(&str, &str)]) -> std::process::Output { - let mut sh = Command::new("sh"); + self.install_in("sh", wait, vars) + } + + /// [`Self::install_with`] run by `shell` (`sh` is dash here; Arch's is bash). + fn install_in(&self, shell: &str, wait: &str, vars: &[(&str, &str)]) -> std::process::Output { + let mut sh = Command::new(shell); sh.arg(self.src.join("install.sh")) .env_clear() .env("HOME", &self.home) @@ -393,6 +403,18 @@ mod tests { self.home.join(".local/lib/craft-cloud/craft-cloud-agent").display().to_string() } + /// Waits (up to 20 s) for the calls to include `call`. + fn has(&self, call: &str) -> Vec { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + loop { + let calls = self.calls(); + if calls.iter().any(|c| c == call) || std::time::Instant::now() > deadline { + return calls; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + } + /// Waits (up to 20 s) for the calls to end with `last`: the agent is started again in the background. fn ends_with(&self, last: &str) -> Vec { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); @@ -410,6 +432,9 @@ mod tests { impl Drop for Upgrade { fn drop(&mut self) { self.agent_ends(); + if let Ok(pid) = std::fs::read_to_string(self.tmp.join("tray.pid")) { + let _ = Command::new("kill").arg(pid.trim()).status(); + } let _ = std::fs::remove_dir_all(&self.tmp); } } @@ -450,8 +475,9 @@ mod tests { let out = up.install("60"); assert!(out.status.success(), "{out:?}"); let run = format!("{} run --log", up.lib_agent()); - let calls = up.ends_with(&run); - assert_eq!(calls.last(), Some(&run), "{calls:?}"); + // The agent and the tray both start in the background: either may come last. + let calls = up.has(&run); + assert!(calls.contains(&run), "{calls:?}"); assert!(calls.iter().filter(|c| c.ends_with("[old agent running]")).all(|c| c.ends_with("quit [old agent running]")), "{calls:?}"); } @@ -494,6 +520,28 @@ mod tests { assert_eq!(calls.iter().filter(|c| *c == start).count(), 1, "{calls:?}"); } + /// install.sh's output is a pipe (ssh, a script): it ends when the script does, though the tray it started still + /// runs. Nothing the script starts in the background holds its streams. Under bash too, which (unlike dash) keeps + /// a background `a && b &` list's subshell, and so its streams, for as long as `b` runs. + #[cfg(target_os = "linux")] + #[test] + fn the_scripts_output_ends_with_it_while_the_tray_runs_on() { + for shell in ["sh", "bash"] { + let up = Upgrade::new(&format!("pipe-{shell}"), 1, 1); + let started = std::time::Instant::now(); + let out = up.install_in(shell, "60", &[]); + assert!(out.status.success(), "{shell}: {out:?}"); + assert!(started.elapsed() < std::time::Duration::from_secs(20), "{shell}: the pipe stayed open: {:?}", started.elapsed()); + let tray = up.tmp.join("tray.pid"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while !tray.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + let pid = std::fs::read_to_string(&tray).unwrap(); + assert!(Command::new("kill").args(["-0", pid.trim()]).status().unwrap().success(), "{shell}: the tray still runs"); + } + } + /// Without a unit, the waiter starts the agent in a unit of its own and ends: once the agent is up no waiter is /// left, so a later install that gives up adds one again, and the agent is started again a second time. #[cfg(target_os = "linux")] From 49e1220426536ef1f0ab87207dc46259a4cee4a6 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:07:34 +0200 Subject: [PATCH 12/12] setup: point start at login at the installed copy, wherever it pointed The app writes its tray Run value for whichever copy last ran, so a development build run once (from a cargo target folder) started at every login after the install. The setup only moved the agent's value, and only from the older hand-made install. Now, on every install, an agent value that starts another copy is taken over by the new agent (read back as before), and a tray value is pointed at this install's app. A value that isn't there stays away: start at login was turned off. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RC1oqfy9kZznj94RjMojyz --- apps/craft-cloud-setup/src/lib.rs | 26 ++++++++++++++++++-------- apps/craft-cloud-setup/src/tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/apps/craft-cloud-setup/src/lib.rs b/apps/craft-cloud-setup/src/lib.rs index db8be11..dd842da 100644 --- a/apps/craft-cloud-setup/src/lib.rs +++ b/apps/craft-cloud-setup/src/lib.rs @@ -6,7 +6,8 @@ //! API (it finishes the pass under way), waited for up to a minute, then ended, and its process checked gone. An //! agent that can't be stopped stops the install before anything is changed. Then the files, the Uninstall key //! (`HKCU\…\Uninstall\CraftCloud`, its `UninstallString` calling this setup, copied into the program folder, with -//! `--uninstall`), and the shortcut. An agent that ran is started again (and waited for), also when the install +//! `--uninstall`), and the shortcut. Start at login (the agent's and the tray's `Run` values, where there are any) is +//! pointed at this install, wherever it pointed before. An agent that ran is started again (and waited for), also when the install //! fails after the stop. An older install in //! `%LOCALAPPDATA%\Programs\craft-cloud` whose agent the `Run` value starts is moved: the new agent's //! `autostart on` points the value at the new folder (read back to check); the old folder's files are left alone. @@ -273,20 +274,29 @@ fn install_stopped( write_replacing(&setup_copy, &bytes)?; } - // The older install's agent starts at login: the new one takes that over (no agent runs now, so the new agent - // writes its own entry). The old files stay. The value is read back: only a value that starts the new agent is a - // move. + // Start at login runs this install from now on. The agent's value, wherever it points (the older install, or a + // build run from elsewhere): the new agent takes it over (no agent runs now, so it writes its own entry), and the + // value is read back; only one that starts the new agent counts. The app's tray value, which the app writes for + // whichever copy last ran: pointed at this one. A value that isn't there stays away (start at login was turned + // off). The older install's files stay. let mut moved_from = None; - let run_value = registry.get(&places.run_key, AGENT_RUN_VALUE); - if old.join(AGENT_EXE).is_file() && run_value.as_deref().is_some_and(|v| starts_from(v, &old)) { + if let Some(value) = registry.get(&places.run_key, AGENT_RUN_VALUE).filter(|v| !starts_from(v, &dir)) { let moved = machine.run_agent(&dir.join(AGENT_EXE), &["autostart", "on"]).and_then(|()| match registry.get(&places.run_key, AGENT_RUN_VALUE) { Some(v) if starts_from(&v, &dir) => Ok(()), other => Err(format!("the Run value is still {}", other.unwrap_or_else(|| "missing".into()))), }); match moved { - Ok(()) => moved_from = Some(old.clone()), - Err(e) => warnings.push(format!("start at login still runs the old agent in {} ({e})", old.display())), + Ok(()) if starts_from(&value, &old) => moved_from = Some(old.clone()), + Ok(()) => {} + Err(e) if starts_from(&value, &old) => warnings.push(format!("start at login still runs the old agent in {} ({e})", old.display())), + Err(e) => warnings.push(format!("start at login still runs another agent ({value}): {e}")), + } + } + if registry.get(&places.run_key, APP_RUN_VALUE).is_some_and(|v| !starts_from(&v, &dir)) { + let tray = format!("\"{}\" --hidden", dir.join(APP_EXE).display()); + if let Err(e) = registry.set_string(&places.run_key, APP_RUN_VALUE, &tray) { + warnings.push(format!("the tray still starts at login from another copy ({e})")); } } diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 203f41e..2da7a85 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -383,6 +383,33 @@ fn an_agent_asked_to_quit_whose_end_cant_be_told_is_started_again() { assert_eq!(*registry.values.lock().unwrap(), before); } +/// Start-at-login values that run another copy (here a development build in a cargo target folder, and an agent +/// somewhere else) are pointed at the installed one; a value that isn't there stays away. +#[test] +fn start_at_login_is_pointed_at_the_installed_copy() { + let u = user("Test"); + let registry = Arc::new(MemoryRegistry::default()); + let dir = u.places.dir(); + let dev_tray = r#""C:\Users\u\craft-cloud-target\release\craft-cloud.exe" --hidden"#; + let dev_agent = r#""C:\Users\u\craft-cloud-target\release\craft-cloud-agent-bg.exe" run --log"#; + registry.set_string(&u.places.run_key, APP_RUN_VALUE, dev_tray).unwrap(); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, dev_agent).unwrap(); + let machine = Recorder { registry: Some((registry.clone(), u.places.run_key.clone())), ..Recorder::default() }; + let installed = install(&u.places, &*registry, &machine, V2, &u.setup, &options(false)).unwrap(); + assert!(installed.warnings.is_empty(), "{:?}", installed.warnings); + assert_eq!(installed.moved_from, None, "not the older install"); + assert_eq!(registry.get(&u.places.run_key, APP_RUN_VALUE), Some(format!("\"{}\" --hidden", dir.join(APP_EXE).display()))); + assert!(registry.get(&u.places.run_key, AGENT_RUN_VALUE).is_some_and(|v| starts_from(&v, &dir))); + assert!(machine.asked().contains(&"Craft Cloud/craft-cloud-agent.exe autostart on".to_owned())); + + // Already pointing here: left as it is, and the agent isn't asked. Not there: stays away. + let machine = Recorder { registry: Some((registry.clone(), u.places.run_key.clone())), ..Recorder::default() }; + registry.remove_value(&u.places.run_key, APP_RUN_VALUE).unwrap(); + install(&u.places, &*registry, &machine, V2, &u.setup, &options(false)).unwrap(); + assert_eq!(registry.get(&u.places.run_key, APP_RUN_VALUE), None); + assert!(!machine.asked().iter().any(|a| a.contains("autostart")), "{:?}", machine.asked()); +} + /// BEAST's install, made by hand in `Programs\craft-cloud`: its agent starts at login from there. The new agent takes /// start at login over; the old files stay. #[test]