Address PR review feedback

- Make --ssh and --ssh-wait mutually exclusive using conflicts_with
- Change error handling in wait_for_ssh_ready to return Ok(false) on error
  instead of propagating, to avoid noisy debug logging during polling
- Extract hardcoded 180 second timeout into SSH_WAIT_TIMEOUT_SECONDS constant
- Move debug logs outside suppress_output check so they're always logged

Assisted-by: Claude Code (Sonnet 4.5)

Author: cgwalters

crates/integration-tests/src/tests/libvirt_verb.rs

@@ -1440,5 +1440,3 @@ fn test_libvirt_run_bind_mounts() -> Result<()> {
     Ok(())
 }
 integration_test!(test_libvirt_run_bind_mounts);
-
-/// Test --ssh-wait flag waits for SSH without entering shell

crates/kit/src/libvirt/run.rs

@@ -19,6 +19,9 @@ use crate::libvirt::domain::VirtiofsFilesystem;
 use crate::utils::parse_memory_to_mb;
 use crate::xml_utils;
 
+/// SSH wait timeout in seconds
+const SSH_WAIT_TIMEOUT_SECONDS: u64 = 180;
+
 /// Create a virsh command with optional connection URI
 pub(super) fn virsh_command(connect_uri: Option<&str>) -> Result<std::process::Command> {
     let mut cmd = std::process::Command::new("virsh");
@@ -254,7 +257,7 @@ pub struct LibvirtRunOpts {
     pub ssh: bool,
 
     /// Wait for SSH to become available and verify connectivity (for testing)
-    #[clap(long)]
+    #[clap(long, conflicts_with = "ssh")]
     pub ssh_wait: bool,
 
     /// Mount host container storage (RO) at /run/host-container-storage
@@ -364,7 +367,10 @@ fn wait_for_ssh_ready(
             };
 
             // Try to connect
-            crate::libvirt::ssh::run_ssh_impl(&global_opts_clone, ssh_opts).map(|_| true)
+            match crate::libvirt::ssh::run_ssh_impl(&global_opts_clone, ssh_opts) {
+                Ok(_) => Ok(true),
+                Err(_) => Ok(false),
+            }
         },
         Duration::from_secs(timeout_secs),
         Duration::from_secs(2), // Poll every 2 seconds
@@ -502,12 +508,12 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
 
     if opts.ssh_wait {
         // Wait for SSH to be ready and verify connectivity
-        wait_for_ssh_ready(global_opts, &vm_name, 180)?;
+        wait_for_ssh_ready(global_opts, &vm_name, SSH_WAIT_TIMEOUT_SECONDS)?;
         println!("Ready; use bcvk libvirt ssh to connect");
         Ok(())
     } else if opts.ssh {
         // Wait for SSH then enter interactive shell
-        wait_for_ssh_ready(global_opts, &vm_name, 180)?;
+        wait_for_ssh_ready(global_opts, &vm_name, SSH_WAIT_TIMEOUT_SECONDS)?;
 
         // Use the libvirt SSH functionality directly
         let ssh_opts = crate::libvirt::ssh::LibvirtSshOpts {

crates/kit/src/libvirt/ssh.rs

@@ -319,17 +319,19 @@ impl LibvirtSshOpts {
                 .output()
                 .map_err(|e| eyre!("Failed to execute SSH command: {}", e))?;
 
-            if !self.suppress_output {
-                if !output.stdout.is_empty() {
+            if !output.stdout.is_empty() {
+                if !self.suppress_output {
                     // Forward stdout to parent process
                     print!("{}", String::from_utf8_lossy(&output.stdout));
-                    debug!("SSH stdout: {}", String::from_utf8_lossy(&output.stdout));
                 }
-                if !output.stderr.is_empty() {
+                debug!("SSH stdout: {}", String::from_utf8_lossy(&output.stdout));
+            }
+            if !output.stderr.is_empty() {
+                if !self.suppress_output {
                     // Forward stderr to parent process
                     eprint!("{}", String::from_utf8_lossy(&output.stderr));
-                    debug!("SSH stderr: {}", String::from_utf8_lossy(&output.stderr));
                 }
+                debug!("SSH stderr: {}", String::from_utf8_lossy(&output.stderr));
             }
 
             if !output.status.success() {