From c7d76da630186854c1419f69647c1d5b516336e5 Mon Sep 17 00:00:00 2001 From: Mariotaku Date: Thu, 27 Aug 2026 13:02:00 +0900 Subject: [PATCH 1/3] ares-install: stop reporting a dead connection as a delete failure A run whose session drops mid-install printed the cleanup failure first and the reason second: Failed to delete /media/developer/temp/ares_install_fca561c25f.ipk: Ssh(Fatal("Inappropriate ioctl for device")) Failed to install: Fatal: Socket error: disconnected The delete is attempted on the way out, before the install result is propagated, so its failure always lands ahead of the error that explains the run. Worse, the ENOTTY is meaningless - it is a stale errno libssh reports through a session that is already gone, and it reads like a permission or path problem on the device. Check the session before cleaning up. When it is gone, say what was left behind - the upload had already succeeded, so the package really is still sitting in /media/developer/temp - and let the real error through on its own. "Deleting uploaded package..." moves inside the same branch, so it is no longer announced for a delete that cannot happen. Verified that libssh reports a vanished peer as `is_connected() == false` against a throwaway sshd: killing the child holding the session flips it, which is the condition this relies on. Errors here are printed with Display rather than Debug now, so the remaining two say what went wrong instead of dumping a variant. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Gde2tcn5SbUHKhesuTNB7a --- ares-install/src/install.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/ares-install/src/install.rs b/ares-install/src/install.rs index 93e04ad..51244aa 100644 --- a/ares-install/src/install.rs +++ b/ares-install/src/install.rs @@ -74,9 +74,8 @@ impl InstallApp for DeviceSession { IoError::new( ErrorKind::Other, format!( - "Failed to generate checksum for {}: {:?}", - package.as_ref().to_string_lossy(), - e + "Failed to generate checksum for {}: {e}", + package.as_ref().to_string_lossy() ), ) })?; @@ -114,7 +113,7 @@ impl InstallApp for DeviceSession { pb.set_message("Checking uploaded package"); let verified = verify_upload(self, &ipk_path, &checksum); if let Err(e) = &verified { - pb.suspend(|| eprintln!("Upload of {package_display_name} is broken: {e:?}")); + pb.suspend(|| eprintln!("Upload of {package_display_name} is broken: {e}")); } let result = verified.and_then(|_| { @@ -160,14 +159,25 @@ impl InstallApp for DeviceSession { if let Ok(package_id) = &result { pb.suspend(|| println!("Installed package {}!", package_id)); } - pb.suspend(|| println!("Deleting uploaded package...")); pb.set_prefix("Cleanup"); pb.set_message("Deleting uploaded package"); - if let Err(e) = self.rm(&ipk_path) { + // Cleaning up over a connection that is already gone only produces a + // second, confusing error - and it lands before the one that explains + // the run, because this happens on the way out. Say what was left + // behind instead, and let the real error through. + if self.is_connected() { + pb.suspend(|| println!("Deleting uploaded package...")); + if let Err(e) = self.rm(&ipk_path) { + pb.suspend(|| eprintln!("Failed to delete {ipk_path}: {e}")); + } + } else { pb.suspend(|| { - eprintln!("Failed to delete {}: {:?}", ipk_path, e); + eprintln!( + "Lost the connection to {}, so {ipk_path} is still on the device.", + self.device.name + ); }); } pb.finish_and_clear(); From 0af5c631bfc4c65847d8d53f8013044d7727f3a8 Mon Sep 17 00:00:00 2001 From: Mariotaku Date: Thu, 27 Aug 2026 18:50:33 +0900 Subject: [PATCH 2/3] ares-install: do not call a silent install stream a success `dev/install` is subscribed to, and the loop takes the first message that says "installed" or that names a failure. If the stream ends having said neither, `.next()` returns None - and that was mapped to `Ok(String::new())`, so ares-install printed "Installed package !" and exited 0 for an install whose outcome it never learned. A subscription can end that way whenever the channel closes early: the device's session going down, luna-send exiting, appinstalld stopping mid report. Claiming success there puts a package on the device that may not be on it. Report it instead. The device having stopped talking is not evidence either way, and the message says so. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Gde2tcn5SbUHKhesuTNB7a --- ares-install/src/install.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ares-install/src/install.rs b/ares-install/src/install.rs index 51244aa..6029333 100644 --- a/ares-install/src/install.rs +++ b/ares-install/src/install.rs @@ -19,6 +19,8 @@ pub(crate) trait InstallApp { #[derive(Debug)] pub enum InstallError { Response { error_code: i32, reason: String }, + /// The install stream ended without ever saying how it went. + NoVerdict, ChecksumMismatch { expected: String, actual: String }, Luna(LunaError), Transfer(TransferError), @@ -31,6 +33,10 @@ impl Display for InstallError { InstallError::Response { error_code, reason } => { write!(f, "{reason} (error {error_code})") } + InstallError::NoVerdict => write!( + f, + "the device stopped reporting before it said whether the package installed" + ), InstallError::ChecksumMismatch { expected, actual } => write!( f, "uploaded package is corrupted: expected sha256 {expected}, device has {actual}" @@ -151,7 +157,10 @@ impl InstallApp for DeviceSession { ) }) .next() - .unwrap_or_else(|| Ok(String::new())), + // Reaching the end of the stream having seen neither + // "installed" nor a failure is not a success. Reporting one + // claims a package is on the device that may well not be. + .unwrap_or(Err(InstallError::NoVerdict)), Err(e) => Err(e.into()), } }); From 2fecc139cd300bdfb152ae49a0e9669a2659210d Mon Sep 17 00:00:00 2001 From: Mariotaku Date: Thu, 27 Aug 2026 18:56:49 +0900 Subject: [PATCH 3/3] ares-install: stop reporting a successful install as a failure The install request goes out over an exec channel, and the device starts work on it before it answers. A connection that dies in that window leaves `request_exec` returning "Socket error: disconnected" - while the device carries on and installs the package. On a set that drops SSH connections around install time this is not a corner case. A peer session hit it four runs out of four: every one printed "Failed to install: Fatal: Socket error: disconnected", and every one succeeded on the device, confirmed against appinstalld's NL_APP_INSTALLED and the Installed-Time in opkg's status file. Anyone reading that output reinstalls, or goes hunting for a device fault that is not there. A dead connection is not evidence of a failed install. Say what is actually known - the connection went down mid-install, the package may well be on the device, and `ares-install --list` will settle it - and skip the "Failed to install" prefix, which contradicts the only thing the message has to say. The exit status stays non-zero: an unknown outcome is not a success either. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Gde2tcn5SbUHKhesuTNB7a --- ares-install/src/install.rs | 25 +++++++++++++++++++++++++ ares-install/src/main.rs | 9 ++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/ares-install/src/install.rs b/ares-install/src/install.rs index 6029333..42f837a 100644 --- a/ares-install/src/install.rs +++ b/ares-install/src/install.rs @@ -21,6 +21,9 @@ pub enum InstallError { Response { error_code: i32, reason: String }, /// The install stream ended without ever saying how it went. NoVerdict, + /// The connection went down with the install already under way, so what + /// the device did with the package is not known. + Interrupted { device: String }, ChecksumMismatch { expected: String, actual: String }, Luna(LunaError), Transfer(TransferError), @@ -37,6 +40,13 @@ impl Display for InstallError { f, "the device stopped reporting before it said whether the package installed" ), + InstallError::Interrupted { device } => write!( + f, + "lost the connection to {device} while the install was running. The device may \ + have installed the package anyway - `ares-install -d {device} --listfull` \ + prints the version that is on it. `--list` alone only names the app, which \ + says nothing about which build landed" + ), InstallError::ChecksumMismatch { expected, actual } => write!( f, "uploaded package is corrupted: expected sha256 {expected}, device has {actual}" @@ -165,6 +175,21 @@ impl InstallApp for DeviceSession { } }); + // The install request is sent before the device answers it, so a + // connection that dies here says nothing about what the device did - + // and on some sets the install goes through regardless. Calling that a + // failed install is a guess, and the wrong one often enough to matter. + let result = result.map_err(|e| match e { + InstallError::Luna(LunaError::Session(_)) | InstallError::Io(_) + if !self.is_connected() => + { + InstallError::Interrupted { + device: self.device.name.clone(), + } + } + other => other, + }); + if let Ok(package_id) = &result { pb.suspend(|| println!("Installed package {}!", package_id)); } diff --git a/ares-install/src/main.rs b/ares-install/src/main.rs index 28364e3..839b65f 100644 --- a/ares-install/src/main.rs +++ b/ares-install/src/main.rs @@ -5,7 +5,7 @@ use ares_connection_lib::session::NewSession; use ares_device_lib::DeviceManager; use ares_device_lib::cli::unwrap_or_exit; use clap::Parser; -use install::InstallApp; +use install::{InstallApp, InstallError}; use list::ListApps; use crate::remove::RemoveApp; @@ -80,6 +80,13 @@ fn main() { } else if let Some(package) = cli.package { match session.install_app(package) { Ok(_) => {} + // An interrupted install reports itself: "Failed to install" would + // contradict the one thing the message has to say, which is that + // nobody knows yet. + Err(e @ InstallError::Interrupted { .. }) => { + eprintln!("{e}"); + exit(1); + } Err(e) => { eprintln!("Failed to install: {e}"); exit(1);