Rewrite historical Rust comments as current rules

v2/crates/core/src/adb/batch.rs

@@ -1,9 +1,8 @@
 //! Batched device-side shell invocation.
 //!
-//! Several read paths used to fan out N separate `adb shell` calls where one
-//! compound shell would do. On the desktop driver each call is a process
-//! spawn; on the mobile driver every call is serialized behind a single TCP
-//! connection, so the fan-out costs N × Wi-Fi RTT for zero concurrency.
+//! On the desktop driver each `adb shell` call is a process spawn; on the
+//! mobile driver every call is serialized behind a single TCP connection, so
+//! N separate reads cost N × Wi-Fi RTT for zero concurrency.
 //! `batch_command` folds them into one invocation whose stdout `split_batch`
 //! cuts back apart on a sentinel.
 
@@ -76,8 +75,7 @@ pub fn parse_checked_batch(
 /// transport, which merges stdout and stderr into one stream.
 ///
 /// No double quotes are emitted, so the result survives being passed as a
-/// single `adb -s <serial> shell <cmd>` argument the same way the compound
-/// commands already in the tree (`devices.rs::harvest_properties`) do.
+/// single `adb -s <serial> shell <cmd>` argument.
 pub fn batch_command(cmds: &[&str]) -> String {
     let mut out = String::new();
     for (i, cmd) in cmds.iter().enumerate() {

v2/crates/core/src/adb/parse.rs

@@ -468,7 +468,7 @@ pub struct TemperatureReading {
 }
 
 fn current_celsius(value: f64) -> bool {
-    // Preserve the previous parser's sanity range; this is not a health grade.
+    // A plausibility range for a sensor reading; this is not a health grade.
     (10.0..120.0).contains(&value)
 }
 
@@ -1270,7 +1270,7 @@ mod tests {
     #[test]
     fn real_meminfo_package_map_never_trims_a_hal_into_a_package() {
         let map = parse_dumpsys_meminfo(SHIELD_MEMINFO);
-        // `foo@2.0-service` used to be cut at the `@` and filed as `foo`.
+        // `foo@2.0-service` must not be cut at the `@` and filed as `foo`.
         assert!(!map.contains_key("vendor.nvidia.hardware.graphics.composer"));
         assert!(!map.contains_key("android.hardware.audio"));
         assert!(!map.keys().any(|k| k.contains('@') || k.contains(':')));
@@ -1282,8 +1282,8 @@ mod tests {
 
     #[test]
     fn path_named_processes_are_kept_not_dropped() {
-        // Older builds name native daemons by path. The old pattern needed the
-        // name to start with a package character, so these rows vanished.
+        // Older builds name native daemons by path. Those rows must survive
+        // even though the name doesn't start with a package character.
         let input = "Total PSS by process:\n\
                          40,112K: /system/bin/surfaceflinger (pid 312)\n\
                           1,234K: /vendor/bin/hw/android.hardware.audio@2.0-service (pid 400)\n\

v2/crates/core/src/adb/remote_input.rs

@@ -1,11 +1,11 @@
 //! Persistent scrcpy control-socket session for low-latency remote input.
 //!
-//! Today each remote key press shells out `adb shell "input keyevent N"`, which
-//! cold-starts a JVM on the TV (~690 ms per press). This module replaces the
-//! transport with scrcpy's control-only server: push the jar once, run it
-//! resident via `app_process`, and stream fixed binary control messages over a
-//! persistent socket — dropping per-press cost to network RTT. Desktop reaches
-//! that socket through `adb forward`; Android opens the device service directly.
+//! `adb shell "input keyevent N"` cold-starts a JVM on the TV for every press
+//! (~690 ms). This module uses scrcpy's control-only server instead: push the
+//! jar once, run it resident via `app_process`, and stream fixed binary control
+//! messages over a persistent socket — dropping per-press cost to network RTT.
+//! Desktop reaches that socket through `adb forward`; Android opens the device
+//! service directly.
 //!
 //! Layering note: every adb invocation goes through the `AdbDriver` trait, per
 //! the one-wrapper rule. Desktop's raw `TcpStream` to its adb-forwarded local
@@ -714,7 +714,6 @@ mod tests {
     // Honest error-path coverage: with a driver that can't spawn a child
     // (MockAdb uses the trait's default `spawn`, which returns unsupported),
     // start() must surface the spawn failure rather than hang or fake success.
-    // The live socket path is left for the lead's on-device check.
     #[tokio::test]
     async fn start_fails_when_driver_cannot_spawn() {
         use crate::commands::test_support::MockAdb;
@@ -731,11 +730,12 @@ mod tests {
         }
     }
 
-    // Live end-to-end test against a real device — ignored in CI, run by hand:
-    //   SHIELD_TEST_SERIAL=192.168.x.x:5555 cargo test remote_input_live -- --ignored --nocapture
-    // Starts a real session, injects SLEEP (223) and verifies via `dumpsys
-    // power` that the device went to sleep, then WAKEUP (224) and verifies it
-    // woke, then tears down and checks no server process is left behind.
+    // Live end-to-end test against a real device. Compiled out with
+    // `cfg(any())`: it builds a `SubprocessAdb`, which lives in the desktop
+    // crate, not this one. Starts a real session, injects SLEEP (223) and
+    // verifies via `dumpsys power` that the device went to sleep, then WAKEUP
+    // (224) and verifies it woke, then tears down and checks no server process
+    // is left behind.
     #[cfg(any())]
     #[tokio::test]
     #[ignore]

v2/crates/core/src/adb/sim/tests.rs

@@ -193,7 +193,7 @@ async fn a_replayed_screenshot_is_a_png() {
     assert!(sim.world().replay.as_ref().unwrap().report().is_empty());
 }
 
-/// #122 on v2.3.0: stock disabled, Setup Wraith still enabled with a higher
+/// #122: stock disabled, Setup Wraith still enabled with a higher
 /// HOME filter priority, and Monet set as default. Monet holds the HOME role
 /// and the Home key opens it, but `resolve-activity` still names Setup
 /// Wraith. The current launcher is the role holder.
@@ -225,7 +225,7 @@ async fn setup_wraith_outranking_the_role_holder_is_not_the_current_launcher() {
         let mut w = sim.world();
         let d = w.devices.get_mut("1324619053514").unwrap();
         assert_eq!(d.home.role_holder.as_deref(), Some(MONET));
-        // The resolver alone, which 2.3.0 displayed, still names Setup Wraith.
+        // The resolver alone still names Setup Wraith.
         assert!(d.resolve_home(&stock).unwrap().starts_with(WRAITH));
     }
 
@@ -367,8 +367,7 @@ async fn google_tv_takeover_failure_re_enables_stock_and_wraith() {
     assert!(enabled(&sim, WRAITH_PKG), "Setup Wraith re-enabled");
 }
 
-/// A device without Setup Wraith behaves exactly as before: no command ever
-/// names the setup helper.
+/// A device without Setup Wraith: no command ever names the setup helper.
 #[tokio::test]
 async fn takeover_without_setup_wraith_never_touches_it() {
     let sim = google_tv_world(false);

v2/crates/core/src/commands/apps.rs

@@ -50,8 +50,9 @@ pub fn safety_info(state: State<'_, AppState>, package: String) -> Safety {
 /// Classify a package against the rule tables *and* the reviewed app lists.
 ///
 /// The catalog is a reviewed source and has to be consulted here: without it,
-/// every curated app that was not also in one of the two rule tables reported
-/// "role and effects unknown" while its own description sat in the next column.
+/// every curated app that is not also in one of the two rule tables would
+/// report "role and effects unknown" while its own description sits in the
+/// next column.
 pub fn safety_for(state: &AppState, package: &str) -> Safety {
     let catalog = state.app_lists.find(package).map(|entry| CatalogVerdict {
         risk: entry.risk,
@@ -439,12 +440,11 @@ pub(crate) async fn disable_package_impl(
             message: format!("Refusing to disable {package}: {reason}"),
         });
     }
-    // The launcher screen has always refused to disable the last enabled HOME
-    // handler, but this is the path the app list and the optimize wizard use,
-    // so the same action was unguarded depending on where it was started from.
-    // A third-party launcher is Unknown to the safety rules, so nothing else
-    // stops it. Best-effort: if the handler query fails we proceed as before
-    // rather than blocking every disable on a transient read.
+    // The launcher screen refuses to disable the last enabled HOME handler,
+    // and this is the path the app list and the optimize wizard use, so the
+    // same guard applies here. A third-party launcher is Unknown to the safety
+    // rules, so nothing else stops it. Best-effort: if the handler query fails
+    // the disable proceeds rather than blocking on a transient read.
     if let Some(refusal) = refuse_last_home_handler(state, serial, package).await {
         return Ok(refusal);
     }
@@ -930,10 +930,9 @@ async fn run(state: &AppState, serial: &str, cmd: &str) -> Result<ActionResult,
 mod tests {
     use super::*;
 
-    /// The bug was in the *wiring*, not the engine: `safety_info` never
-    /// consulted the app lists. An engine-level test passes happily while that
-    /// is broken, so this one goes through the real command path with the real
-    /// shipped catalog in state.
+    /// The catalog lookup is wiring, not engine: an engine-level test passes
+    /// even if `safety_info` ignores the app lists, so this one goes through
+    /// the real command path with the real shipped catalog in state.
     #[test]
     fn safety_info_consults_the_shipped_catalog() {
         let state = AppState::new(
@@ -1144,8 +1143,8 @@ mod tests {
     #[tokio::test]
     async fn play_installed_vendor_packages_read_as_third_party() {
         use crate::commands::test_support::{state_with, MockAdb};
-        // Amazon Music from the Play Store sits in `-3`; its prefix no longer
-        // overrides that. A preinstalled Google app outside `-3` stays system.
+        // Amazon Music from the Play Store sits in `-3`; its prefix doesn't
+        // override that. A preinstalled Google app outside `-3` stays system.
         let all = "package:com.amazon.music.tv\npackage:com.google.android.youtube.tv";
         let third = "package:com.amazon.music.tv";
         let state =

v2/crates/core/src/commands/devices.rs

@@ -304,7 +304,8 @@ pub struct ForgetResult {
 /// adb keeps one transport per key, and a Wireless debugging device is often
 /// held under two: the mDNS service name adb auto-connected, and an `ip:port`
 /// someone dialled. The device list shows one row for both, so disconnecting
-/// only the key that row displays left the other behind and the row stayed.
+/// only the key that row displays would leave the other behind and the row
+/// would stay.
 /// Aliases are matched on verified `ro.serialno` only; a transport whose id
 /// cannot be read is never assumed to be the same device.
 #[tauri::command]
@@ -788,8 +789,8 @@ fn normalize_pairing_address(address: &str) -> Result<String, String> {
 ///   input rather than something the app relies on.
 /// - `[IPv6]:port` — bracketed, as adb and every URL parser expect.
 /// - `adb-XXXX-YYYY._adb-tls-connect._tcp[:port]` — an mDNS service name.
-///   The daemon resolves these itself; rejecting them meant a user could not
-///   paste what discovery had just shown them (GitHub #88).
+///   The daemon resolves these itself; rejecting them would stop a user
+///   pasting what discovery has just shown them (GitHub #88).
 ///
 /// Rejects empty input and any port that is not a positive 16-bit number.
 pub fn normalize_connect_address(address: &str) -> Result<String, String> {
@@ -862,7 +863,7 @@ fn is_mdns_instance(host: &str) -> bool {
 /// `Get-Devices` and `Show-DeviceProfile`.
 ///
 /// Each read is its own sentinel-delimited section rather than one line of a
-/// combined stdout. Positional parsing looked equivalent but was not: the
+/// combined stdout. Positional parsing is not equivalent: the
 /// leading `settings get global device_name` can print nothing at all on some
 /// builds and a multi-line Exception on others, and either one shifts every
 /// later index — so a device would silently report its model as its Android
@@ -893,8 +894,9 @@ async fn harvest_properties(adb: &dyn AdbDriver, serial: &str) -> Result<DeviceP
 /// A batch reports only its LAST command's exit code, and the last read is
 /// `pm has-feature`, which exits 1 to say "no" — an answer, not a failure. The
 /// desktop driver turns any nonzero exit into an error before stdout is
-/// looked at, so a phone that answered every question was refused. The
-/// trailing `true` makes the batch's status say only that the shell ran.
+/// looked at, so without it a phone that answered every question would be
+/// refused. The trailing `true` makes the batch's status say only that the
+/// shell ran.
 fn property_batch_command() -> String {
     format!("{}; true", batch_command(&PROPERTY_READS))
 }
@@ -1135,11 +1137,11 @@ mod tests {
 
     #[tokio::test]
     async fn a_device_that_answers_no_to_leanback_is_still_profiled() {
-        // The live regression: a batch reports only its last command's exit
-        // code, and the last read is `pm has-feature`, which exits 1 to say
-        // "no". A Pixel that answered every question was refused with
-        // "adb process failed (exit code Some(1))". The exit code carries no
-        // information about the reads that came before it.
+        // A batch reports only its last command's exit code, and the last
+        // read is `pm has-feature`, which exits 1 to say "no". A device that
+        // answered every question must not be refused with "adb process
+        // failed (exit code Some(1))". The exit code carries no information
+        // about the reads that came before it.
         let mock = MockAdb::default().on_shell_exit(
             "settings get global device_name",
             &batched_props(&[
@@ -1181,8 +1183,8 @@ mod tests {
     #[tokio::test]
     async fn silent_device_name_read_does_not_shift_the_android_version() {
         // `settings get global device_name` prints nothing on some builds.
-        // Under the old positional parse every later value slid up one, so the
-        // TV reported its brand as its friendly name and its model as its
+        // A positional parse would slide every later value up one, so the TV
+        // would report its brand as its friendly name and its model as its
         // Android version. Sections keep each read in its own slot.
         let mock = MockAdb::default()
             .on_raw(
@@ -1219,8 +1221,8 @@ mod tests {
     #[tokio::test]
     async fn multiline_settings_exception_stays_inside_its_own_section() {
         // The other failure shape: `settings get` throws and prints a
-        // multi-line stack trace, which under positional parsing pushed every
-        // real property down by however many lines the trace happened to be.
+        // multi-line stack trace, which under positional parsing would push
+        // every real property down by however many lines the trace is.
         let mock = MockAdb::default()
             .on_raw(
                 "devices",
@@ -1286,8 +1288,8 @@ mod tests {
 
     #[test]
     fn one_device_reached_two_ways_collapses_to_the_usable_address() {
-        // The live regression: adb auto-connected the device under its mDNS
-        // service name, and the scan then dialled the same device by address.
+        // adb auto-connects the device under its mDNS service name, and the
+        // scan then dials the same device by address.
         let devices = vec![
             row(
                 "adb-58040DLCH005YV-jBeCEe._adb-tls-connect._tcp",
@@ -1531,9 +1533,9 @@ mod tests {
 
     #[test]
     fn normalize_accepts_a_scoped_ipv6_endpoint_from_discovery() {
-        // Codex on #128: mDNS can report a link-local host with its scope,
-        // and endpoint() brackets it as-is. Rejecting it here meant the
-        // auto-connect retried for 45 s and gave up on a device it had found.
+        // #128: mDNS can report a link-local host with its scope, and
+        // endpoint() brackets it as-is. Rejecting it here would make the
+        // auto-connect retry for 45 s and give up on a device it had found.
         assert_eq!(
             normalize_connect_address("[fe80::1%en0]:41541").unwrap(),
             "[fe80::1%en0]:41541"
@@ -1552,8 +1554,8 @@ mod tests {
 
     #[test]
     fn normalize_accepts_an_mdns_service_name_from_discovery() {
-        // Discovery shows these; refusing to accept one back meant a user
-        // could not paste what the app had just told them (GitHub #88).
+        // Discovery shows these; refusing to accept one back would stop a
+        // user pasting what the app has just told them (GitHub #88).
         let instance = "adb-58040DLCH005YV-jBeCEe._adb-tls-connect._tcp";
         assert_eq!(
             normalize_connect_address(&format!("{instance}:41541")).unwrap(),
@@ -1781,9 +1783,9 @@ mod tests {
 
     #[tokio::test]
     async fn forget_drops_every_transport_of_the_same_hardware() {
-        // What the owner's Pixel looked like: adb held it under its mDNS name
-        // *and* under ip:port. The row shows ip:port, so disconnecting only
-        // that left the mDNS transport behind and the row stayed.
+        // adb holds the device under its mDNS name *and* under ip:port. The
+        // row shows ip:port, so disconnecting only that would leave the mDNS
+        // transport behind and the row would stay.
         let mock = MockAdb::default()
             .on_raw(
                 "devices",
@@ -1914,8 +1916,8 @@ mod tests {
 
     #[test]
     fn probe_matches_the_connect_service_by_its_embedded_serial() {
-        // Codex on #128: the instances differ in suffix, so exact equality
-        // never auto-connected. The serial between them is the identity.
+        // #128: the instances differ in suffix, so exact equality would never
+        // auto-connect. The serial between them is the identity.
         let services = parse_mdns_services(&mdns_pixel(46545));
         assert_eq!(
             paired_connect_probe("192.168.42.211:45439", PIXEL_PAIR, &services, &[]),
@@ -2005,7 +2007,7 @@ mod tests {
 
     #[tokio::test]
     async fn a_stale_advertisement_pointing_at_another_attached_device_is_refused() {
-        // Codex on #128: the advertised endpoint names a transport adb holds,
+        // #128: the advertised endpoint names a transport adb holds,
         // but that transport is a different device. Its key must not be
         // enough to call it the one just paired.
         let mock = MockAdb::default()
@@ -2127,8 +2129,8 @@ mod tests {
 
     #[test]
     fn a_bracketed_ipv6_transport_still_counts_as_advertised() {
-        // Codex on #128: adb keys the transport `[fe80::1]:41541`, the
-        // advertisement is printed bare. Compared raw, Forget stayed silent.
+        // #128: adb keys the transport `[fe80::1]:41541`, the advertisement
+        // is printed bare. Compared raw, Forget would stay silent.
         let services = parse_mdns_services(
             "List of discovered mdns services\n\
              adb-V6SER-c2\t_adb-tls-connect._tcp\tfe80::1:41541\n",

v2/crates/core/src/commands/files.rs

@@ -131,8 +131,8 @@ pub struct FindResult {
     /// Directories the search could not be run against at all, because the ADB
     /// call itself failed. A directory that simply does not exist is *not*
     /// listed here — that is a real "no matches". Without this split, a
-    /// dropped connection rendered as "export from the app first", telling the
-    /// user to redo something that had worked (GitHub #86).
+    /// dropped connection would read as "export from the app first", telling
+    /// the user to redo something that had worked (GitHub #86).
     pub unsearched: Vec<String>,
 }