diff --git a/crates/fbuild-cli/src/cli/deploy.rs b/crates/fbuild-cli/src/cli/deploy.rs index b93d25b5..63b82f46 100644 --- a/crates/fbuild-cli/src/cli/deploy.rs +++ b/crates/fbuild-cli/src/cli/deploy.rs @@ -231,6 +231,11 @@ pub async fn run_deploy( if !message_is_streamed { output::result(&resp.message); } + // FastLED/fbuild#1152: the daemon may attach a typed exact-device USB + // recovery request. Apply the --admin/--no-admin policy here; with + // explicit --admin this launches the one-shot elevated helper at most + // once and then retries/rescans on freshly enumerated transports only. + let resp = maybe_recover_and_retry(resp, &client, &req, usb_recovery_policy).await?; if !resp.success { // process::exit skips normal destructor-based stdio flushing. Preserve // the daemon's final stdout/stderr when fbuild is piped by automation. @@ -250,6 +255,190 @@ pub async fn run_deploy( Ok(()) } +/// FastLED/fbuild#1152: consume a typed exact-device recovery request from +/// the daemon's deploy response. +/// +/// Policy-gated: only an explicit interactive `--admin` launches the #1148 +/// one-shot elevated helper, exactly once. After a successful helper run all +/// prior port/volume facts are discarded: a failed transfer is retried once +/// through a fresh deployment (never rebuilding), while an already-confirmed +/// flash only rescans for the recovered runtime endpoint. Default, +/// `--no-admin`, CI, and non-interactive sessions never elevate. +async fn maybe_recover_and_retry( + resp: OperationResponse, + client: &DaemonClient, + req: &DeployRequest, + policy: fbuild_core::usb::UsbRecoveryPolicy, +) -> fbuild_core::Result { + use super::usb_recovery::{self, RecoveryRunOutcome}; + + let Some(request) = resp.usb_recovery.clone() else { + return Ok(resp); + }; + output::warn(format!( + "deploy target {} is a known-unhealthy Windows devnode{}", + request.instance_id, + request + .problem_code + .map(|code| format!(" (problem code {code})")) + .unwrap_or_default() + )); + let context = usb_recovery::RecoveryLaunchContext { + is_windows: cfg!(windows), + is_ci: std::env::var_os("CI").is_some(), + is_interactive: std::io::IsTerminal::is_terminal(&std::io::stdin()), + }; + #[cfg(windows)] + let outcome = { + let request = request.clone(); + tokio::task::spawn_blocking(move || { + usb_recovery::run_recovery_for_typed_request( + policy, + &request, + context, + &mut usb_recovery::WindowsUacLauncher, + ) + }) + .await + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("recovery helper task failed: {error}")) + })?? + }; + #[cfg(not(windows))] + let outcome = match usb_recovery::decide_recovery_launch(policy, true, context) { + usb_recovery::RecoveryLaunchDecision::ManualGuidance => RecoveryRunOutcome::ManualGuidance, + _ => RecoveryRunOutcome::RefuseNonInteractive, + }; + match outcome { + RecoveryRunOutcome::ManualGuidance => { + if policy == fbuild_core::usb::UsbRecoveryPolicy::Default { + output::warn( + "rerun with --admin to attempt a scoped one-shot Windows PnP recovery (UAC), or physically re-enter BOOTSEL (hold BOOT, tap RESET) and retry", + ); + } else { + output::warn("--no-admin: privileged recovery skipped by request"); + } + Ok(resp) + } + RecoveryRunOutcome::RefuseNonInteractive => { + output::warn("scoped PnP recovery needs an interactive Windows session; not elevating"); + Ok(resp) + } + RecoveryRunOutcome::Cancelled => { + output::warn("UAC prompt was cancelled; no recovery was attempted"); + Ok(resp) + } + RecoveryRunOutcome::Completed(result) => { + output::result(format!( + "one-shot PnP recovery {}: {:?} on {} ({:?} -> {:?})", + if result.success { + "succeeded" + } else { + "failed" + }, + result.operation, + result + .validated_instance_id + .as_deref() + .unwrap_or("(unvalidated)"), + result.before, + result.after, + )); + if !result.success { + output::warn(format!( + "recovery helper reported {}; physical BOOTSEL replug remains the fallback", + result + .error_code + .as_deref() + .unwrap_or("an unspecified failure") + )); + return Ok(resp); + } + if !request.flash_completed { + // The transfer never succeeded: one fresh deployment attempt + // through freshly enumerated transports, never rebuilding and + // never re-entering recovery (the retry runs with DenyAdmin). + output::result("retrying the deployment once on freshly enumerated transports"); + let mut retry_req = req.clone(); + retry_req.usb_recovery_policy = fbuild_core::usb::UsbRecoveryPolicy::DenyAdmin; + retry_req.skip_build = true; + retry_req.clean_build = false; + retry_req.clean_all = false; + let retry = client.deploy(&retry_req).await?; + let retry_streamed = operation_streams_include_message(&retry); + print_operation_streams(&retry); + if !retry_streamed { + output::result(&retry.message); + } + return Ok(retry); + } + // Flash already confirmed: never reflash for recovery. Rescan for + // the freshly enumerated, health-eligible, openable endpoint. + match reacquire_recovered_port(&request).await { + Some(port) => { + output::result(format!( + "recovered runtime CDC endpoint {port} after PnP recovery" + )); + } + None => { + output::warn( + "no healthy runtime CDC endpoint appeared after recovery; physical BOOTSEL replug remains the fallback", + ); + } + } + Ok(resp) + } + } +} + +/// Bounded post-recovery rescan (FastLED/fbuild#1152): fresh enumerations +/// only, health-eligible records only, and a bounded open probe before any +/// name is reported. Stale COM names and prior scan facts are never reused. +async fn reacquire_recovered_port( + request: &fbuild_core::usb::UsbRecoveryRequest, +) -> Option { + const REACQUIRE_WINDOW: Duration = Duration::from_secs(10); + const REACQUIRE_POLL: Duration = Duration::from_millis(500); + let expected_serial = request.expected_serial.clone(); + let expected_vid = request.expected_vid; + let expected_pid = request.expected_pid; + let started = Instant::now(); + while started.elapsed() < REACQUIRE_WINDOW { + let expected_serial = expected_serial.clone(); + let found = tokio::task::spawn_blocking(move || { + let ports = fbuild_serial::ports::available_ports().ok()?; + ports + .into_iter() + .filter(|port| !port.health.is_known_unhealthy()) + .find_map(|port| { + let serialport::SerialPortType::UsbPort(usb) = &port.info.port_type else { + return None; + }; + let identity_matches = match expected_serial.as_deref() { + Some(serial) => usb.serial_number.as_deref() == Some(serial), + None => (usb.vid, usb.pid) == (expected_vid, expected_pid), + }; + if !identity_matches { + return None; + } + serialport::new(&port.info.port_name, 115_200) + .timeout(Duration::from_millis(250)) + .open() + .ok() + .map(|_| port.info.port_name) + }) + }) + .await + .ok() + .flatten(); + if found.is_some() { + return found; + } + tokio::time::sleep(REACQUIRE_POLL).await; + } + None +} + #[allow(clippy::too_many_arguments)] pub async fn run_test_emu( project_dir: String, @@ -541,6 +730,7 @@ mod tests { fn response(message: &str, stdout: Option<&str>, stderr: Option<&str>) -> OperationResponse { OperationResponse { + usb_recovery: None, success: false, request_id: "request-1".to_string(), message: message.to_string(), diff --git a/crates/fbuild-cli/src/cli/port_scan.rs b/crates/fbuild-cli/src/cli/port_scan.rs index 2b7e4ed8..dd09ee69 100644 --- a/crates/fbuild-cli/src/cli/port_scan.rs +++ b/crates/fbuild-cli/src/cli/port_scan.rs @@ -662,6 +662,8 @@ mod tests { ), location: Some("Port_#0001.Hub_#0011".to_string()), behind_external_hub: Some(true), + parent_instance_id: None, + device_class: None, }]; let warning = format_usb_problem_warning(&devices); assert!(warning.contains("problem code 43")); @@ -679,6 +681,8 @@ mod tests { friendly_name: None, location: Some("Port_#0014.Hub_#0001".to_string()), behind_external_hub: Some(false), + parent_instance_id: None, + device_class: None, }]; let warning = format_usb_problem_warning(&devices); assert!(warning.contains("Unknown USB device")); diff --git a/crates/fbuild-cli/src/cli/usb_recovery.rs b/crates/fbuild-cli/src/cli/usb_recovery.rs index 4a19e50a..23effbe2 100644 --- a/crates/fbuild-cli/src/cli/usb_recovery.rs +++ b/crates/fbuild-cli/src/cli/usb_recovery.rs @@ -157,6 +157,69 @@ pub fn launch_once_for_typed_request( Ok(decision) } +/// Terminal result of one policy-gated recovery attempt +/// (FastLED/fbuild#1152). +#[derive(Debug)] +pub enum RecoveryRunOutcome { + ManualGuidance, + RefuseNonInteractive, + Cancelled, + Completed(UsbRecoveryResult), +} + +/// Like [`launch_once_for_typed_request`], but consume the helper's result +/// file before the rendezvous is dropped: the result must carry the exact +/// rendezvous nonce and the request's operation ID, or the run fails closed. +pub fn run_recovery_for_typed_request( + policy: UsbRecoveryPolicy, + request: &UsbRecoveryRequest, + context: RecoveryLaunchContext, + launcher: &mut L, +) -> fbuild_core::Result { + match decide_recovery_launch(policy, true, context) { + RecoveryLaunchDecision::ManualGuidance => return Ok(RecoveryRunOutcome::ManualGuidance), + RecoveryLaunchDecision::RefuseNonInteractive => { + return Ok(RecoveryRunOutcome::RefuseNonInteractive); + } + RecoveryLaunchDecision::LaunchOnce => {} + } + let rendezvous = create_rendezvous(request.clone())?; + match launcher.launch(&rendezvous.request_path, &rendezvous.result_path)? { + HelperLaunchOutcome::Cancelled => Ok(RecoveryRunOutcome::Cancelled), + HelperLaunchOutcome::Completed { .. } => { + let result = read_recovery_result(&rendezvous.result_path)?; + if result.nonce != rendezvous.nonce { + return Err(fbuild_core::FbuildError::Other( + "recovery result nonce does not match this run's rendezvous".to_string(), + )); + } + if result.operation_id != request.operation_id { + return Err(fbuild_core::FbuildError::Other( + "recovery result does not correlate to this operation".to_string(), + )); + } + Ok(RecoveryRunOutcome::Completed(result)) + } + } +} + +fn read_recovery_result(result_path: &Path) -> fbuild_core::Result { + let file = File::open(result_path).map_err(|error| { + fbuild_core::FbuildError::Other(format!( + "elevated helper completed without a readable result: {error}" + )) + })?; + let mut contents = String::new(); + file.take(MAX_RENDEZVOUS_BYTES) + .read_to_string(&mut contents) + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot read recovery result: {error}")) + })?; + serde_json::from_str(&contents).map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot decode recovery result: {error}")) + }) +} + /// Windows implementation that launches only the current fbuild executable /// with the fixed hidden helper shape. It is deliberately not a general /// command runner and never launches a daemon. @@ -508,4 +571,129 @@ mod tests { assert!(!launcher.request_path.expect("request path").exists()); assert!(!launcher.result_path.expect("result path").exists()); } + + /// Emulates the elevated helper: read the envelope like the real hidden + /// mode would, then write a result bound to the given nonce. + struct CompletingLauncher { + calls: usize, + nonce_override: Option, + } + impl RecoveryHelperLauncher for CompletingLauncher { + fn launch( + &mut self, + request_path: &Path, + result_path: &Path, + ) -> fbuild_core::Result { + self.calls += 1; + let envelope: RecoveryHelperEnvelope = + serde_json::from_reader(File::open(request_path).expect("request file")) + .expect("request envelope"); + let result = UsbRecoveryResult { + operation_id: envelope.request.operation_id.clone(), + nonce: self + .nonce_override + .clone() + .unwrap_or_else(|| envelope.nonce.clone()), + validated_instance_id: Some(envelope.request.instance_id.clone()), + operation: Some(fbuild_core::usb::UsbRecoveryOperation::RestartVerifiedParent), + before: UsbRecoveryHealth::PresentProblem { problem_code: 28 }, + after: UsbRecoveryHealth::HealthyPresent, + success: true, + error_code: None, + }; + std::fs::write(result_path, serde_json::to_vec(&result).unwrap()) + .expect("result write"); + Ok(HelperLaunchOutcome::Completed { exit_code: 0 }) + } + } + + fn interactive_windows_context() -> RecoveryLaunchContext { + RecoveryLaunchContext { + is_windows: true, + is_ci: false, + is_interactive: true, + } + } + + #[test] + fn completed_helper_result_is_validated_and_returned() { + let mut launcher = CompletingLauncher { + calls: 0, + nonce_override: None, + }; + let outcome = run_recovery_for_typed_request( + UsbRecoveryPolicy::AllowAdmin, + &envelope().request, + interactive_windows_context(), + &mut launcher, + ) + .expect("run once"); + assert_eq!(launcher.calls, 1); + let RecoveryRunOutcome::Completed(result) = outcome else { + panic!("expected a completed result, got {outcome:?}"); + }; + assert!(result.success); + assert_eq!(result.operation_id, "deploy-1"); + assert_eq!( + result.operation, + Some(fbuild_core::usb::UsbRecoveryOperation::RestartVerifiedParent) + ); + } + + #[test] + fn mismatched_result_nonce_fails_closed() { + let mut launcher = CompletingLauncher { + calls: 0, + nonce_override: Some("f".repeat(64)), + }; + let error = run_recovery_for_typed_request( + UsbRecoveryPolicy::AllowAdmin, + &envelope().request, + interactive_windows_context(), + &mut launcher, + ) + .expect_err("a foreign result must be rejected"); + assert!(error.to_string().contains("nonce"), "{error}"); + } + + #[test] + fn cancelled_uac_is_expected_control_flow_not_an_error() { + struct CancellingLauncher; + impl RecoveryHelperLauncher for CancellingLauncher { + fn launch( + &mut self, + _request_path: &Path, + _result_path: &Path, + ) -> fbuild_core::Result { + Ok(HelperLaunchOutcome::Cancelled) + } + } + let outcome = run_recovery_for_typed_request( + UsbRecoveryPolicy::AllowAdmin, + &envelope().request, + interactive_windows_context(), + &mut CancellingLauncher, + ) + .expect("cancellation is not an error"); + assert!(matches!(outcome, RecoveryRunOutcome::Cancelled)); + } + + #[test] + fn default_and_no_admin_policies_never_reach_the_launcher() { + for policy in [UsbRecoveryPolicy::Default, UsbRecoveryPolicy::DenyAdmin] { + let mut launcher = CompletingLauncher { + calls: 0, + nonce_override: None, + }; + let outcome = run_recovery_for_typed_request( + policy, + &envelope().request, + interactive_windows_context(), + &mut launcher, + ) + .expect("policy decision"); + assert!(matches!(outcome, RecoveryRunOutcome::ManualGuidance)); + assert_eq!(launcher.calls, 0); + } + } } diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index 4578a0d4..f5de0f03 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -363,6 +363,7 @@ impl DaemonClient { } "result" => { final_response = Some(OperationResponse { + usb_recovery: None, success: event.success.unwrap_or(false), request_id: event.request_id.unwrap_or_default(), message: event.message.unwrap_or_default(), diff --git a/crates/fbuild-cli/src/daemon_client/types.rs b/crates/fbuild-cli/src/daemon_client/types.rs index f931ac75..c3917265 100644 --- a/crates/fbuild-cli/src/daemon_client/types.rs +++ b/crates/fbuild-cli/src/daemon_client/types.rs @@ -57,7 +57,7 @@ pub struct BuildRequest { pub bloat_analysis: bool, } -#[derive(Debug, Serialize)] +#[derive(Clone, Debug, Serialize)] pub struct DeployRequest { pub project_dir: String, #[serde(skip_serializing_if = "Option::is_none")] @@ -170,6 +170,12 @@ pub struct OperationResponse { pub stdout: Option, #[serde(default)] pub stderr: Option, + /// Typed exact-device USB recovery request from the daemon + /// (FastLED/fbuild#1152). `serde(default)` keeps older daemons + /// compatible; the CLI's `--admin`/`--no-admin` policy decides whether + /// the one-shot elevated helper is ever launched. + #[serde(default)] + pub usb_recovery: Option, } /// NDJSON event from a streaming build response. diff --git a/crates/fbuild-core/src/usb/mod.rs b/crates/fbuild-core/src/usb/mod.rs index 6ba85089..5ce6062f 100644 --- a/crates/fbuild-core/src/usb/mod.rs +++ b/crates/fbuild-core/src/usb/mod.rs @@ -32,8 +32,8 @@ pub use data::{ #[cfg(test)] pub use embedded::vendor_name as embedded_vendor_name; pub use recovery::{ - UsbRecoveryHealth, UsbRecoveryOperation, UsbRecoveryPolicy, UsbRecoveryRequest, - UsbRecoveryResult, + UNCLASSED_DEVICE_CLASS, UsbRecoveryHealth, UsbRecoveryOperation, UsbRecoveryPolicy, + UsbRecoveryRequest, UsbRecoveryResult, }; #[cfg(test)] pub use resolver::resolve_bundled; diff --git a/crates/fbuild-core/src/usb/recovery.rs b/crates/fbuild-core/src/usb/recovery.rs index 7b4b436e..d71322d3 100644 --- a/crates/fbuild-core/src/usb/recovery.rs +++ b/crates/fbuild-core/src/usb/recovery.rs @@ -24,18 +24,30 @@ pub enum UsbRecoveryPolicy { DenyAdmin, } -/// The sole two Windows PnP operations a recovery helper can represent. +/// The sole Windows PnP operations a recovery helper can represent. /// /// There is deliberately no generic command, executable, device-class, or /// "reset all USB" variant. A phantom endpoint may only re-enumerate its -/// verified parent; a present problematic endpoint may only restart itself. +/// verified parent; a present problematic endpoint may only restart itself; +/// and a present problematic USB *interface* devnode (a `&MI_xx` child of a +/// composite device, e.g. the RP2040 BOOTSEL PICOBOOT function) may only +/// restart its verified healthy parent composite, because restarting the +/// interface alone cannot re-initialize the sibling interfaces or remount +/// the synthetic BOOTSEL volume (FastLED/fbuild#1152). #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(rename_all = "kebab-case")] pub enum UsbRecoveryOperation { ReenumerateParent, RestartTarget, + RestartVerifiedParent, } +/// Sentinel for a devnode with no Windows device class (typical for a +/// driverless interface such as a BOOTSEL PICOBOOT function reporting +/// `CM_PROB_FAILED_INSTALL`). Used on both sides of the identity +/// revalidation so an absent class is an exact-match fact, not a wildcard. +pub const UNCLASSED_DEVICE_CLASS: &str = "(none)"; + /// Host health observed before or after a recovery operation. /// /// This is intentionally independent of `fbuild_serial::PortHealth` so the @@ -146,8 +158,9 @@ mod tests { let operations = [ UsbRecoveryOperation::ReenumerateParent, UsbRecoveryOperation::RestartTarget, + UsbRecoveryOperation::RestartVerifiedParent, ]; - assert_eq!(operations.len(), 2); + assert_eq!(operations.len(), 3); } #[test] diff --git a/crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs b/crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs index 4d0e4abb..d6feacee 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs @@ -264,6 +264,7 @@ pub async fn deploy_avr8js( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: format!("avr8js run succeeded: {}", message), exit_code: 0, @@ -278,6 +279,7 @@ pub async fn deploy_avr8js( StatusCode::OK, Json(OperationResponse { success: false, + usb_recovery: None, request_id, message: format!("avr8js run failed: {}", message), exit_code: 1, @@ -294,6 +296,7 @@ pub async fn deploy_avr8js( ( StatusCode::OK, Json(OperationResponse { + usb_recovery: None, success, request_id, message: if success { @@ -316,6 +319,7 @@ pub async fn deploy_avr8js( StatusCode::OK, Json(OperationResponse { success: false, + usb_recovery: None, request_id, message: format!( "internal: avr8js emitted ESP RecoverDownloadMode ({})", @@ -340,6 +344,7 @@ pub async fn deploy_avr8js( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: "deploy complete".to_string(), exit_code: 0, diff --git a/crates/fbuild-daemon/src/handlers/emulator/qemu_deploy.rs b/crates/fbuild-daemon/src/handlers/emulator/qemu_deploy.rs index 72c7284c..17369d27 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/qemu_deploy.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/qemu_deploy.rs @@ -296,6 +296,7 @@ pub async fn deploy_qemu( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: format!("QEMU run succeeded: {}", message), exit_code: real_exit_code.unwrap_or(0), @@ -310,6 +311,7 @@ pub async fn deploy_qemu( StatusCode::OK, Json(OperationResponse { success: false, + usb_recovery: None, request_id, message: format!("QEMU run failed: {}", message), exit_code: real_exit_code.unwrap_or(1), @@ -326,6 +328,7 @@ pub async fn deploy_qemu( ( StatusCode::OK, Json(OperationResponse { + usb_recovery: None, success, request_id, message: if success { @@ -348,6 +351,7 @@ pub async fn deploy_qemu( StatusCode::OK, Json(OperationResponse { success: false, + usb_recovery: None, request_id, message: format!( "internal: QEMU emitted ESP RecoverDownloadMode ({})", diff --git a/crates/fbuild-daemon/src/handlers/emulator/select.rs b/crates/fbuild-daemon/src/handlers/emulator/select.rs index d48d0be7..4c9ddb61 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/select.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/select.rs @@ -415,6 +415,7 @@ pub async fn test_emu( ( StatusCode::OK, Json(OperationResponse { + usb_recovery: None, success, request_id, message, diff --git a/crates/fbuild-daemon/src/handlers/operations/build.rs b/crates/fbuild-daemon/src/handlers/operations/build.rs index 03bed37b..05a45f25 100644 --- a/crates/fbuild-daemon/src/handlers/operations/build.rs +++ b/crates/fbuild-daemon/src/handlers/operations/build.rs @@ -834,6 +834,7 @@ pub async fn build( StatusCode::OK, Json(OperationResponse { success: build_result.success, + usb_recovery: None, request_id, message: msg, exit_code: code, diff --git a/crates/fbuild-daemon/src/handlers/operations/deploy.rs b/crates/fbuild-daemon/src/handlers/operations/deploy.rs index d11d3fd4..74a5ec60 100644 --- a/crates/fbuild-daemon/src/handlers/operations/deploy.rs +++ b/crates/fbuild-daemon/src/handlers/operations/deploy.rs @@ -946,6 +946,48 @@ pub async fn deploy( } } + // FastLED/fbuild#1152: when an RP2040 deploy failed outright, or flashed + // but recovered no runtime CDC endpoint, compose a typed exact-device + // recovery request from FRESH scan facts (never from pre-deploy state). + // The daemon only reports it in the response; the normal CLI applies the + // `--admin`/`--no-admin` policy, and the one-shot elevated helper + // re-proves the identity live before its single allowlisted operation. + let deploy_flashed = matches!(&deploy_result, Ok(r) if r.success); + let cdc_unrecovered = + port_discovery_owned && matches!(&deploy_result, Ok(r) if r.success && r.port.is_none()); + let usb_recovery_request = if cfg!(windows) + && platform == fbuild_core::Platform::RaspberryPi + && (!deploy_flashed || cdc_unrecovered) + { + ctx.refresh_devices_and_broadcast_serial_moves().await; + let devices: Vec<_> = ctx.device_manager.get_all_devices().into_values().collect(); + let problem_devices = + tokio::task::spawn_blocking(fbuild_serial::ports::present_usb_problem_devices) + .await + .unwrap_or_default(); + let generation = super::deploy_port::rp_generation_for(board.as_ref()); + super::recovery_request::compose_rp2040_recovery_request( + &devices, + &problem_devices, + &request_id, + deploy_flashed, + |vid, pid| { + super::deploy_port::rp_bootloader_profiles_match_generation( + &fbuild_core::usb::profiles::profiles_for(vid, pid), + generation, + ) + }, + |vid, pid| { + super::deploy_port::rp_profiles_match_generation( + &fbuild_core::usb::profiles::profiles_for(vid, pid), + generation, + ) + }, + ) + } else { + None + }; + let ( deploy_success, deploy_stdout, @@ -967,6 +1009,7 @@ pub async fn deploy( StatusCode::INTERNAL_SERVER_ERROR, Json(OperationResponse { success: false, + usb_recovery: usb_recovery_request, request_id, message: r.message, exit_code: 1, @@ -979,10 +1022,9 @@ pub async fn deploy( ); } Err(e) => { - return ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(deploy_error_response(request_id, &e)), - ); + let mut response = deploy_error_response(request_id, &e); + response.usb_recovery = usb_recovery_request; + return (StatusCode::INTERNAL_SERVER_ERROR, Json(response)); } }; append_warning_to_stderr(&mut deploy_stderr, deploy_port_warning); @@ -1023,6 +1065,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: usb_recovery_request, request_id, message: format!( "{} but monitor was skipped: no healthy runtime CDC endpoint was recovered after the flash", @@ -1083,6 +1126,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: format!("{} but monitor failed to open port: {}", deploy_prefix, e), exit_code: 0, @@ -1106,6 +1150,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: format!("{} but monitor could not attach reader", deploy_prefix), exit_code: 0, @@ -1153,6 +1198,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: None, request_id, message: format!("{}; monitor: {}", deploy_prefix, msg), exit_code: 0, @@ -1167,6 +1213,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: false, + usb_recovery: None, request_id, message: format!("{}; monitor error: {}", deploy_prefix, msg), exit_code: 1, @@ -1192,6 +1239,7 @@ pub async fn deploy( ( StatusCode::OK, Json(OperationResponse { + usb_recovery: None, success, request_id, message: format!( @@ -1219,6 +1267,7 @@ pub async fn deploy( StatusCode::OK, Json(OperationResponse { success: true, + usb_recovery: usb_recovery_request, request_id, message: deploy_prefix, exit_code: 0, @@ -1257,6 +1306,7 @@ fn deploy_error_response( let message = format!("deploy error: {error}"); OperationResponse { success: false, + usb_recovery: None, request_id, message: message.clone(), exit_code: 1, diff --git a/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs b/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs index b8f5de3d..e9608a2c 100644 --- a/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs +++ b/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs @@ -35,10 +35,7 @@ pub(super) fn choose_deploy_port( // reset. Never select by a built-in VID or fall back to an unrelated COM // port: FastLED/boards data is the sole identity source. if platform == Platform::RaspberryPi { - let expected_generation = board - .map(|board| board.mcu.to_ascii_lowercase()) - .filter(|mcu| mcu.starts_with("rp2350")) - .map_or(RpGeneration::Rp2040, |_| RpGeneration::Rp2350); + let expected_generation = rp_generation_for(board); let (matches, unhealthy) = partition_rp_candidates(devices, |vid, pid| { rp_profiles_match_generation( &fbuild_core::usb::profiles::profiles_for(vid, pid), @@ -190,25 +187,53 @@ fn rp_deploy_choice(matches: Vec, unhealthy: Vec) -> Depl } #[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum RpGeneration { +pub(super) enum RpGeneration { Rp2040, Rp2350, } -fn rp_profiles_match_generation( +impl RpGeneration { + fn family(self) -> &'static str { + match self { + RpGeneration::Rp2040 => "rp2040", + RpGeneration::Rp2350 => "rp2350", + } + } +} + +pub(super) fn rp_generation_for(board: Option<&BoardConfig>) -> RpGeneration { + board + .map(|board| board.mcu.to_ascii_lowercase()) + .filter(|mcu| mcu.starts_with("rp2350")) + .map_or(RpGeneration::Rp2040, |_| RpGeneration::Rp2350) +} + +pub(super) fn rp_profiles_match_generation( profiles: &[fbuild_core::usb::profiles::UsbTransportProfile], expected: RpGeneration, ) -> bool { use fbuild_core::usb::profiles::{UsbDeviceRole, UsbPurpose}; - let family = match expected { - RpGeneration::Rp2040 => "rp2040", - RpGeneration::Rp2350 => "rp2350", - }; profiles.iter().any(|profile| { profile.purpose == UsbPurpose::Runtime && profile.role == UsbDeviceRole::RuntimeCdc - && profile.family.as_deref() == Some(family) + && profile.family.as_deref() == Some(expected.family()) + }) +} + +/// Match a BOOTSEL UF2 bootloader profile of the expected RP generation +/// (FastLED/fbuild#1152: identifies the stock-ROM composite whose problem +/// interface may carry a typed recovery request). +pub(super) fn rp_bootloader_profiles_match_generation( + profiles: &[fbuild_core::usb::profiles::UsbTransportProfile], + expected: RpGeneration, +) -> bool { + use fbuild_core::usb::profiles::{UsbDeviceRole, UsbPurpose}; + + profiles.iter().any(|profile| { + profile.purpose == UsbPurpose::Bootloader + && profile.role == UsbDeviceRole::BootloaderUf2 + && profile.family.as_deref() == Some(expected.family()) }) } diff --git a/crates/fbuild-daemon/src/handlers/operations/mod.rs b/crates/fbuild-daemon/src/handlers/operations/mod.rs index dc2086ec..347cc610 100644 --- a/crates/fbuild-daemon/src/handlers/operations/mod.rs +++ b/crates/fbuild-daemon/src/handlers/operations/mod.rs @@ -11,6 +11,7 @@ mod deploy; mod deploy_port; mod install_deps; mod monitor; +mod recovery_request; mod reset; #[cfg(test)] diff --git a/crates/fbuild-daemon/src/handlers/operations/recovery_request.rs b/crates/fbuild-daemon/src/handlers/operations/recovery_request.rs new file mode 100644 index 00000000..f1af4718 --- /dev/null +++ b/crates/fbuild-daemon/src/handlers/operations/recovery_request.rs @@ -0,0 +1,275 @@ +//! Compose a typed exact-device USB recovery request (FastLED/fbuild#1152). +//! +//! The daemon never elevates and never performs PnP writes. When an RP2040 +//! deploy either failed outright or flashed without recovering a runtime CDC +//! endpoint, it composes a [`UsbRecoveryRequest`] from **fresh** scan facts +//! and returns it inside the deploy response. The normal CLI applies the +//! `--admin`/`--no-admin` policy and, at most once, launches the #1148 +//! one-shot elevated helper, which re-proves the identity live before the +//! single allowlisted PnP operation. + +use crate::device_manager::DeviceState; +use fbuild_core::usb::{UNCLASSED_DEVICE_CLASS, UsbRecoveryRequest}; +use fbuild_serial::ports::UsbProblemDevice; + +/// Pick the exact-device recovery target from fresh scan facts. +/// +/// Preference order: +/// 1. A present BOOTSEL problem **interface** devnode (`...&MI_xx\...`) of a +/// matching UF2-bootloader identity — it owns the transport the deploy +/// needs next (synthetic volume / PICOBOOT), and its verified parent +/// composite is the restart target that can actually remount the volume. +/// 2. The known-unhealthy (phantom / present-problem) runtime CDC record of +/// a matching runtime identity. +/// +/// Records without a canonical instance ID or a Config-Manager-proved parent +/// are skipped: the helper would fail closed on them anyway, and a typed +/// request must never be composed from guesses. +pub(super) fn compose_rp2040_recovery_request( + devices: &[DeviceState], + problem_devices: &[UsbProblemDevice], + operation_id: &str, + flash_completed: bool, + bootloader_match: impl Fn(u16, u16) -> bool, + runtime_match: impl Fn(u16, u16) -> bool, +) -> Option { + for device in problem_devices { + let Some((vid, pid)) = parse_usb_vid_pid(&device.instance_id) else { + continue; + }; + if !bootloader_match(vid, pid) || !is_composite_interface(&device.instance_id) { + continue; + } + let Some(parent) = device.parent_instance_id.clone() else { + continue; + }; + return Some(UsbRecoveryRequest { + operation_id: operation_id.to_string(), + instance_id: device.instance_id.clone(), + expected_class: device + .device_class + .clone() + .unwrap_or_else(|| UNCLASSED_DEVICE_CLASS.to_string()), + expected_serial: serial_from_matching_parent(&device.instance_id, &parent), + parent_instance_id: Some(parent), + expected_vid: vid, + expected_pid: pid, + problem_code: Some(device.problem_code), + flash_completed, + }); + } + for device in devices { + if !device.port_health.is_known_unhealthy() { + continue; + } + let (Some(vid), Some(pid)) = (device.vid, device.pid) else { + continue; + }; + if !runtime_match(vid, pid) { + continue; + } + let Some(instance_id) = device.instance_id.clone() else { + continue; + }; + let Some(parent) = device.parent_instance_id.clone() else { + continue; + }; + return Some(UsbRecoveryRequest { + operation_id: operation_id.to_string(), + instance_id, + // Runtime CDC devnodes are `usbser`-class serial ports; the + // helper revalidates this against the live (or phantom) record. + expected_class: "Ports".to_string(), + parent_instance_id: Some(parent), + expected_vid: vid, + expected_pid: pid, + expected_serial: device.serial_number.clone(), + problem_code: device.port_health.problem_code(), + flash_completed, + }); + } + None +} + +fn is_composite_interface(instance_id: &str) -> bool { + instance_id.to_ascii_uppercase().contains("&MI_") +} + +/// Parse `USB\VID_xxxx&PID_xxxx...` identity from a PnP instance ID. +fn parse_usb_vid_pid(instance_id: &str) -> Option<(u16, u16)> { + let upper = instance_id.to_ascii_uppercase(); + let rest = upper.strip_prefix("USB\\")?; + let vid_start = rest.find("VID_")? + 4; + let pid_start = rest.find("PID_")? + 4; + let vid = u16::from_str_radix(rest.get(vid_start..vid_start + 4)?, 16).ok()?; + let pid = u16::from_str_radix(rest.get(pid_start..pid_start + 4)?, 16).ok()?; + Some((vid, pid)) +} + +/// The composite parent's third instance segment is the device serial, but +/// only when the parent shares the child's VID/PID (interface devnodes get +/// synthetic instance suffixes; the serial lives on the parent). +fn serial_from_matching_parent(child_instance: &str, parent_instance: &str) -> Option { + let child = parse_usb_vid_pid(child_instance)?; + let parent = parse_usb_vid_pid(parent_instance)?; + if child != parent { + return None; + } + parent_instance + .split('\\') + .nth(2) + .filter(|segment| !segment.is_empty()) + .map(str::to_string) +} + +#[cfg(test)] +mod tests { + use super::*; + use fbuild_serial::ports::PortHealth; + use std::collections::HashMap; + + const BOOTSEL_INTERFACE: &str = "USB\\VID_2E8A&PID_0003&MI_01\\8&22CF742D&0&0001"; + const BOOTSEL_COMPOSITE: &str = "USB\\VID_2E8A&PID_0003\\E0C9125B0D9B"; + const PHANTOM_CDC: &str = "USB\\VID_2E8A&PID_000A\\5303284720C4641C"; + + fn phantom_cdc_device() -> DeviceState { + DeviceState { + device_id: "2e8a:000a".to_string(), + port: "COM12".to_string(), + description: "USB Serial Device".to_string(), + vid: Some(0x2E8A), + pid: Some(0x000A), + vendor_name: None, + product_name: None, + is_cdc: Some(true), + serial_number: Some("5303284720C4641C".to_string()), + port_health: PortHealth::Phantom { + problem_code: Some(45), + status: None, + }, + instance_id: Some(PHANTOM_CDC.to_string()), + parent_instance_id: Some("USB\\ROOT_HUB30\\5&23f8e3f5&0&0".to_string()), + previous_port: None, + exclusive_lease: None, + monitor_leases: HashMap::new(), + last_seen_at: 0.0, + is_connected: true, + trusted_firmware: None, + last_disconnect_at: None, + } + } + + fn bootsel_problem_interface() -> UsbProblemDevice { + UsbProblemDevice { + instance_id: BOOTSEL_INTERFACE.to_string(), + problem_code: 28, + friendly_name: Some("RP2 Boot".to_string()), + location: None, + behind_external_hub: Some(false), + parent_instance_id: Some(BOOTSEL_COMPOSITE.to_string()), + device_class: None, + } + } + + #[test] + fn bootsel_problem_interface_is_preferred_over_phantom_cdc() { + let request = compose_rp2040_recovery_request( + &[phantom_cdc_device()], + &[bootsel_problem_interface()], + "deploy-1", + false, + |_, _| true, + |_, _| true, + ) + .expect("a typed request must be composed"); + assert_eq!(request.instance_id, BOOTSEL_INTERFACE); + assert_eq!( + request.parent_instance_id.as_deref(), + Some(BOOTSEL_COMPOSITE) + ); + assert_eq!(request.expected_class, UNCLASSED_DEVICE_CLASS); + assert_eq!(request.expected_serial.as_deref(), Some("E0C9125B0D9B")); + assert_eq!( + (request.expected_vid, request.expected_pid), + (0x2E8A, 0x0003) + ); + assert_eq!(request.problem_code, Some(28)); + assert!(!request.flash_completed); + assert!(request.has_canonical_identity()); + } + + #[test] + fn phantom_cdc_yields_a_typed_exact_device_request() { + let request = compose_rp2040_recovery_request( + &[phantom_cdc_device()], + &[], + "deploy-2", + true, + |_, _| true, + |_, _| true, + ) + .expect("the phantom runtime CDC must compose a request"); + assert_eq!(request.instance_id, PHANTOM_CDC); + assert_eq!(request.expected_class, "Ports"); + assert_eq!(request.expected_serial.as_deref(), Some("5303284720C4641C")); + assert_eq!( + (request.expected_vid, request.expected_pid), + (0x2E8A, 0x000A) + ); + assert_eq!(request.problem_code, Some(45)); + assert!(request.flash_completed); + assert!(request.has_canonical_identity()); + } + + #[test] + fn healthy_devices_and_unrelated_problems_compose_nothing() { + let mut healthy = phantom_cdc_device(); + healthy.port_health = PortHealth::HealthyPresent; + let mut unrelated = bootsel_problem_interface(); + unrelated.instance_id = "USB\\VID_25A7&PID_2510\\receiver".to_string(); + assert_eq!( + compose_rp2040_recovery_request( + &[healthy], + &[unrelated], + "deploy-3", + false, + |vid, _| vid == 0x2E8A, + |vid, _| vid == 0x2E8A, + ), + None + ); + } + + #[test] + fn records_without_canonical_identity_or_parent_are_skipped() { + let mut no_instance = phantom_cdc_device(); + no_instance.instance_id = None; + let mut no_parent = phantom_cdc_device(); + no_parent.parent_instance_id = None; + let mut interface_without_parent = bootsel_problem_interface(); + interface_without_parent.parent_instance_id = None; + assert_eq!( + compose_rp2040_recovery_request( + &[no_instance, no_parent], + &[interface_without_parent], + "deploy-4", + false, + |_, _| true, + |_, _| true, + ), + None + ); + } + + #[test] + fn parent_serial_requires_matching_vid_pid() { + assert_eq!( + serial_from_matching_parent(BOOTSEL_INTERFACE, BOOTSEL_COMPOSITE).as_deref(), + Some("E0C9125B0D9B") + ); + assert_eq!( + serial_from_matching_parent(BOOTSEL_INTERFACE, "USB\\ROOT_HUB30\\5&23f8e3f5&0&0"), + None + ); + } +} diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index 7e07c1d6..2a8ae092 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -199,6 +199,14 @@ pub struct OperationResponse { /// Captured stderr from the deploy/build tool. #[serde(skip_serializing_if = "Option::is_none")] pub stderr: Option, + /// Typed exact-device USB recovery request (FastLED/fbuild#1152). + /// + /// Present only when a deploy detected that its exact target is a + /// known-unhealthy Windows devnode that the scoped one-shot elevated + /// helper could recover. Diagnostic data for the CLI's `--admin` policy + /// decision — never an instruction the daemon acts on itself. + #[serde(skip_serializing_if = "Option::is_none")] + pub usb_recovery: Option, } impl OperationResponse { @@ -213,6 +221,7 @@ impl OperationResponse { launch_url: None, stdout: None, stderr: None, + usb_recovery: None, } } @@ -227,6 +236,7 @@ impl OperationResponse { launch_url: None, stdout: None, stderr: None, + usb_recovery: None, } } } diff --git a/crates/fbuild-serial/src/ports.rs b/crates/fbuild-serial/src/ports.rs index 11b52e2f..e3a0e02d 100644 --- a/crates/fbuild-serial/src/ports.rs +++ b/crates/fbuild-serial/src/ports.rs @@ -175,6 +175,13 @@ pub struct UsbProblemDevice { /// `Some(false)` means the node reaches a root hub directly; `None` means /// the host could not provide enough ancestry to classify it. pub behind_external_hub: Option, + /// Immediate parent instance ID, when Config Manager can prove one. + /// Needed to compose an exact-device `UsbRecoveryRequest` for a problem + /// interface devnode (FastLED/fbuild#1152). + pub parent_instance_id: Option, + /// Windows device class (e.g. `Ports`, `USB`); `None` for driverless + /// devnodes that never got a class assigned. + pub device_class: Option, } /// Best-effort enumeration of present USB devnodes with a non-zero Windows @@ -268,10 +275,10 @@ mod imp { use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{ CM_Get_DevNode_Status, CM_Get_Device_IDW, CM_Get_Parent, CR_NO_SUCH_DEVINST, CR_SUCCESS, DICS_FLAG_GLOBAL, DIGCF_PRESENT, DIREG_DEV, GUID_DEVCLASS_USB, HDEVINFO, MAX_DEVICE_ID_LEN, - SP_DEVINFO_DATA, SPDRP_FRIENDLYNAME, SPDRP_HARDWAREID, SPDRP_LOCATION_INFORMATION, - SPDRP_MFG, SetupDiClassGuidsFromNameW, SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInfo, - SetupDiGetClassDevsW, SetupDiGetDeviceInstanceIdW, SetupDiGetDeviceRegistryPropertyW, - SetupDiOpenDevRegKey, + SP_DEVINFO_DATA, SPDRP_CLASS, SPDRP_FRIENDLYNAME, SPDRP_HARDWAREID, + SPDRP_LOCATION_INFORMATION, SPDRP_MFG, SetupDiClassGuidsFromNameW, + SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInfo, SetupDiGetClassDevsW, + SetupDiGetDeviceInstanceIdW, SetupDiGetDeviceRegistryPropertyW, SetupDiOpenDevRegKey, }; use windows_sys::Win32::Foundation::{FALSE, FILETIME, INVALID_HANDLE_VALUE, MAX_PATH}; use windows_sys::Win32::System::Registry::{ @@ -710,6 +717,8 @@ mod imp { friendly_name: property_from_info(hdi, &info, SPDRP_FRIENDLYNAME), location: property_from_info(hdi, &info, SPDRP_LOCATION_INFORMATION), behind_external_hub: classify_usb_ancestry(info.DevInst), + parent_instance_id: ancestor_ids(info.DevInst).into_iter().next(), + device_class: property_from_info(hdi, &info, SPDRP_CLASS), }); } unsafe { diff --git a/crates/fbuild-serial/src/usb_recovery.rs b/crates/fbuild-serial/src/usb_recovery.rs index d0746ca5..a0d1cc06 100644 --- a/crates/fbuild-serial/src/usb_recovery.rs +++ b/crates/fbuild-serial/src/usb_recovery.rs @@ -45,6 +45,10 @@ pub trait UsbPnpBackend { /// Restart only the exact, verified present target child. fn restart_target(&mut self, instance_id: &str) -> Result<(), Self::Error>; + /// Restart only the exact, verified healthy parent composite of a + /// present problematic USB interface devnode (FastLED/fbuild#1152). + fn restart_verified_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error>; + /// Number of bounded post-operation observations. Fakes stay instant; /// Windows waits briefly between observations for re-enumeration to settle. fn post_operation_poll_attempts(&self) -> usize { @@ -107,6 +111,32 @@ pub fn execute_recovery( backend.reenumerate_parent(parent_instance_id), ) } + // A composite-interface devnode (`...&MI_xx\...`) cannot recover + // alone: restarting it leaves the sibling interfaces and any mounted + // synthetic volume (e.g. the RP2040 BOOTSEL FAT) in their wedged + // state. When the request names a parent, restart the live-verified + // healthy parent composite instead (FastLED/fbuild#1152). A plain + // device target keeps the original exact-child restart. + UsbRecoveryHealth::PresentProblem { .. } if is_composite_interface(&target.instance_id) => { + let Some(parent_instance_id) = request.parent_instance_id.as_deref() else { + return failed(before, "missing-verified-parent"); + }; + let parent = match backend.inspect(parent_instance_id, false) { + Ok(device) => device, + Err(_) => return failed(before, "parent-not-live"), + }; + if !matches!(parent.health, UsbRecoveryHealth::HealthyPresent) + || !same_id(&parent.instance_id, parent_instance_id) + || parent.vid != target.vid + || parent.pid != target.pid + { + return failed(before, "parent-not-live"); + } + ( + UsbRecoveryOperation::RestartVerifiedParent, + backend.restart_verified_parent(parent_instance_id), + ) + } UsbRecoveryHealth::PresentProblem { .. } => ( UsbRecoveryOperation::RestartTarget, backend.restart_target(&target.instance_id), @@ -191,6 +221,12 @@ fn same_id(left: &str, right: &str) -> bool { left.eq_ignore_ascii_case(right) } +/// Whether the instance is a USB composite-interface devnode (`usbccgp` +/// child), recognizable by the `&MI_xx` hardware-ID component. +fn is_composite_interface(instance_id: &str) -> bool { + instance_id.to_ascii_uppercase().contains("&MI_") +} + /// Perform real host recovery when the one-shot helper is running on Windows. /// /// The non-Windows result deliberately fails closed. The CLI must render the @@ -227,8 +263,8 @@ mod windows { use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{ CM_Disable_DevNode, CM_Enable_DevNode, CM_Get_DevNode_PropertyW, CM_Get_DevNode_Status, CM_Get_Device_IDW, CM_Get_Parent, CM_LOCATE_DEVNODE_NORMAL, CM_LOCATE_DEVNODE_PHANTOM, - CM_Locate_DevNodeW, CM_Reenumerate_DevNode, CR_NO_SUCH_DEVINST, CR_SUCCESS, - MAX_DEVICE_ID_LEN, + CM_Locate_DevNodeW, CM_Reenumerate_DevNode, CR_NO_SUCH_DEVINST, CR_NO_SUCH_VALUE, + CR_SUCCESS, MAX_DEVICE_ID_LEN, }; use windows_sys::Win32::Devices::Properties::{DEVPKEY_Device_Class, DEVPROP_TYPE_STRING}; @@ -255,6 +291,14 @@ mod windows { restart_target(instance_id) } + fn restart_verified_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error> { + // Same bounded disable/enable as `restart_target`, applied to the + // parent composite that `execute_recovery` already re-proved live + // and identity-matched. Never reachable for a hub or controller: + // the ladder only passes a `USB\VID_...` composite here. + restart_target(parent_instance_id) + } + fn post_operation_poll_attempts(&self) -> usize { 8 } @@ -387,6 +431,13 @@ mod windows { 0, ) }; + if result == CR_NO_SUCH_VALUE { + // Driverless devnodes (e.g. a BOOTSEL PICOBOOT interface stuck at + // CM_PROB_FAILED_INSTALL) have no Device_Class property at all. + // Report the shared sentinel so identity revalidation treats the + // absence as an exact-match fact (FastLED/fbuild#1152). + return Ok(fbuild_core::usb::UNCLASSED_DEVICE_CLASS.to_string()); + } if result != CR_SUCCESS || property_type != DEVPROP_TYPE_STRING { return Err(format!( "CM_Get_DevNode_PropertyW(Device_Class) failed ({result})" @@ -500,6 +551,12 @@ mod tests { self.calls.push(format!("restart:{instance_id}")); Ok(()) } + + fn restart_verified_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error> { + self.calls + .push(format!("restart-parent:{parent_instance_id}")); + Ok(()) + } } fn request() -> UsbRecoveryRequest { @@ -647,4 +704,140 @@ mod tests { )); assert!(result.validated_instance_id.is_some()); } + + const BOOTSEL_INTERFACE: &str = "USB\\VID_2E8A&PID_0003&MI_01\\8&22CF742D&0&0001"; + const BOOTSEL_COMPOSITE: &str = "USB\\VID_2E8A&PID_0003\\E0C9125B0D9B"; + + fn interface_request() -> UsbRecoveryRequest { + UsbRecoveryRequest { + operation_id: "deploy-2".to_string(), + instance_id: BOOTSEL_INTERFACE.to_string(), + expected_class: fbuild_core::usb::UNCLASSED_DEVICE_CLASS.to_string(), + parent_instance_id: Some(BOOTSEL_COMPOSITE.to_string()), + expected_vid: 0x2e8a, + expected_pid: 0x0003, + expected_serial: Some("E0C9125B0D9B".to_string()), + problem_code: Some(28), + flash_completed: false, + } + } + + fn interface_target(health: UsbRecoveryHealth) -> UsbPnpDevice { + UsbPnpDevice { + instance_id: BOOTSEL_INTERFACE.to_string(), + parent_instance_id: Some(BOOTSEL_COMPOSITE.to_string()), + device_class: fbuild_core::usb::UNCLASSED_DEVICE_CLASS.to_string(), + vid: 0x2e8a, + pid: 0x0003, + serial: Some("E0C9125B0D9B".to_string()), + health, + } + } + + fn composite_parent(health: UsbRecoveryHealth) -> UsbPnpDevice { + UsbPnpDevice { + instance_id: BOOTSEL_COMPOSITE.to_string(), + parent_instance_id: Some("USB\\ROOT_HUB30\\5&23f8e3f5&0&0".to_string()), + device_class: "USB".to_string(), + vid: 0x2e8a, + pid: 0x0003, + serial: Some("E0C9125B0D9B".to_string()), + health, + } + } + + #[test] + fn problem_interface_restarts_only_its_verified_parent_composite() { + let mut backend = FakePnp::with_observations(vec![ + interface_target(UsbRecoveryHealth::PresentProblem { problem_code: 28 }), + composite_parent(UsbRecoveryHealth::HealthyPresent), + interface_target(UsbRecoveryHealth::PresentProblem { problem_code: 28 }), + ]); + + let result = execute_recovery(&interface_request(), "nonce".to_string(), &mut backend); + + assert!(result.success, "{:?}", result.error_code); + assert_eq!( + result.operation, + Some(UsbRecoveryOperation::RestartVerifiedParent) + ); + assert!( + backend + .calls + .iter() + .any(|call| call == &format!("restart-parent:{BOOTSEL_COMPOSITE}")) + ); + assert!( + !backend + .calls + .iter() + .any(|call| call.starts_with("restart:USB")) + ); + } + + #[test] + fn problem_interface_with_unhealthy_parent_fails_closed() { + let mut backend = FakePnp::with_observations(vec![ + interface_target(UsbRecoveryHealth::PresentProblem { problem_code: 28 }), + composite_parent(UsbRecoveryHealth::PresentProblem { problem_code: 31 }), + ]); + + let result = execute_recovery(&interface_request(), "nonce".to_string(), &mut backend); + + assert!(!result.success); + assert_eq!(result.error_code.as_deref(), Some("parent-not-live")); + assert!( + !backend + .calls + .iter() + .any(|call| call.starts_with("restart-parent:")) + ); + } + + #[test] + fn problem_interface_with_mismatched_parent_identity_fails_closed() { + let mut wrong_identity = composite_parent(UsbRecoveryHealth::HealthyPresent); + wrong_identity.pid = 0x000a; + let mut backend = FakePnp::with_observations(vec![ + interface_target(UsbRecoveryHealth::PresentProblem { problem_code: 28 }), + wrong_identity, + ]); + + let result = execute_recovery(&interface_request(), "nonce".to_string(), &mut backend); + + assert!(!result.success); + assert_eq!(result.error_code.as_deref(), Some("parent-not-live")); + } + + #[test] + fn problem_interface_without_parent_fact_fails_closed() { + let mut request = interface_request(); + request.parent_instance_id = None; + let mut target = interface_target(UsbRecoveryHealth::PresentProblem { problem_code: 28 }); + target.parent_instance_id = None; + let mut backend = FakePnp::with_observations(vec![target]); + + let result = execute_recovery(&request, "nonce".to_string(), &mut backend); + + assert!(!result.success); + assert_eq!( + result.error_code.as_deref(), + Some("missing-verified-parent") + ); + } + + #[test] + fn unclassed_sentinel_is_an_exact_class_match_not_a_wildcard() { + let mut request = interface_request(); + request.expected_class = "Ports".to_string(); + let mut backend = + FakePnp::with_observations(vec![interface_target(UsbRecoveryHealth::PresentProblem { + problem_code: 28, + })]); + + let result = execute_recovery(&request, "nonce".to_string(), &mut backend); + + assert!(!result.success); + assert_eq!(result.error_code.as_deref(), Some("device-class-mismatch")); + } }