Fix agent session discovery: surface real errors, fix output capture

rust/src/agent_sessions.rs

@@ -381,22 +381,46 @@ impl LocalAgentSessionScanner {
             .await;
         let (processes, error) = match process_result {
             Ok(result) if result.timed_out => (
-                Vec::new(),
-                Some("Windows process discovery timed out; file-only sessions may still appear."),
-            ),
-            Ok(result) if result.exit_code == Some(0) => {
-                (WindowsProcessOutputParser::parse(&result.text), None)
-            }
-            Ok(_) => (
                 Vec::new(),
                 Some(
-                    "Windows process discovery failed; verify PowerShell and CIM access. File-only sessions may still appear.",
+                    "Windows process discovery timed out; file-only sessions may still appear."
+                        .to_string(),
                 ),
             ),
+            Ok(result) if result.exit_code == Some(0) => {
+                match WindowsProcessOutputParser::parse(&result.text) {
+                    Ok(processes) => (processes, None),
+                    Err(parse_error) => (
+                        Vec::new(),
+                        Some(format!(
+                            "Windows process discovery could not parse its output \
+                             ({parse_error}); file-only sessions may still appear."
+                        )),
+                    ),
+                }
+            }
+            Ok(result) => {
+                let stderr_tail = Self::error_tail(&result.stderr);
+                let hint = if stderr_tail.is_empty() {
+                    String::new()
+                } else {
+                    format!(": {stderr_tail}")
+                };
+                (
+                    Vec::new(),
+                    Some(format!(
+                        "Windows process discovery failed with exit code {}{}; \
+                         file-only sessions may still appear.",
+                        Self::exit_code_label(result.exit_code),
+                        hint
+                    )),
+                )
+            }
             Err(_) => (
                 Vec::new(),
                 Some(
-                    "Unable to launch PowerShell for process discovery; file-only sessions may still appear.",
+                    "Unable to launch PowerShell for process discovery; file-only sessions may still appear."
+                        .to_string(),
                 ),
             ),
         };
@@ -436,6 +460,31 @@ impl LocalAgentSessionScanner {
         }
     }
 
+    /// Redacted, length-bounded tail of captured stderr for error messages.
+    fn error_tail(stderr: &str) -> String {
+        let tail = crate::logging::safe_error_message(stderr);
+        tail.lines()
+            .map(|line| {
+                if line.chars().count() > CommandRunner::MAX_LINE_CHARS {
+                    let cut: String = line.chars().take(CommandRunner::MAX_LINE_CHARS).collect();
+                    format!("{cut}\u{2026}")
+                } else {
+                    line.to_string()
+                }
+            })
+            .rev()
+            .take(3)
+            .collect::<Vec<_>>()
+            .join("; ")
+    }
+
+    /// Human-readable exit-code label for user-facing messages.
+    fn exit_code_label(exit_code: Option<i32>) -> String {
+        exit_code
+            .map(|code| code.to_string())
+            .unwrap_or_else(|| "unknown".to_string())
+    }
+
     fn scan_files(
         &self,
         host: &str,

rust/src/agent_sessions/parsers.rs

@@ -113,18 +113,28 @@ struct WindowsProcessMetadata {
 }
 
 impl WindowsProcessOutputParser {
-    pub fn parse(output: &str) -> Vec<AgentProcessRecord> {
-        let Ok(value) = serde_json::from_str::<Value>(output.trim()) else {
-            return Vec::new();
+    /// Parse `ConvertTo-Json` output from the Windows process query.
+    ///
+    /// Distinguishes "legitimately no processes" (empty array) from an
+    /// unparseable response: only genuinely malformed JSON is an error, so
+    /// callers can surface parse failures instead of silently reporting an
+    /// empty discovery result.
+    pub fn parse(output: &str) -> Result<Vec<AgentProcessRecord>, String> {
+        let trimmed = output.trim();
+        if trimmed.is_empty() {
+            return Err("process query produced no output".to_string());
+        }
+        let Ok(value) = serde_json::from_str::<Value>(trimmed) else {
+            return Err("process query output was not valid JSON".to_string());
         };
         let values = match value {
             Value::Array(values) => values,
             Value::Object(_) => vec![value],
-            _ => return Vec::new(),
+            _ => return Err("process query output was not a JSON array".to_string()),
         };
         let mut seen = HashSet::new();
 
-        values
+        Ok(values
             .into_iter()
             .filter_map(|value| serde_json::from_value::<WindowsProcessMetadata>(value).ok())
             .filter(|process| process.process_id > 0 && seen.insert(process.process_id))
@@ -153,7 +163,7 @@ impl WindowsProcessOutputParser {
                     command: None,
                 }
             })
-            .collect()
+            .collect())
     }
 }
 

rust/src/agent_sessions/tests.rs

@@ -206,7 +206,7 @@ bad line
             }
         ]"#;
 
-        let records = WindowsProcessOutputParser::parse(output);
+        let records = WindowsProcessOutputParser::parse(output).unwrap();
 
         assert_eq!(records.len(), 2);
         assert_eq!(records[0].provider, Some(AgentSessionProvider::Claude));
@@ -216,6 +216,56 @@ bad line
         assert!(!records[0].executable.contains("super-secret"));
     }
 
+    #[test]
+    fn windows_process_parser_treats_empty_array_as_empty_result() {
+        let records = WindowsProcessOutputParser::parse("[]").unwrap();
+        assert!(records.is_empty());
+    }
+
+    #[test]
+    fn windows_process_parser_reports_unparseable_output() {
+        let error = WindowsProcessOutputParser::parse("<html>gateway timeout</html>")
+            .expect_err("malformed JSON must be an error");
+        assert!(error.contains("JSON"), "{error}");
+
+        let error = WindowsProcessOutputParser::parse("   ")
+            .expect_err("empty output must be an error");
+        assert!(error.contains("no output"), "{error}");
+    }
+
+    #[test]
+    fn error_tail_caps_line_length_and_redacts() {
+        let stderr = "Authorization: Bearer sk-secret-token\nshort line\n";
+        let tail = LocalAgentSessionScanner::error_tail(stderr);
+
+        assert!(!tail.contains("sk-secret-token"), "{tail}");
+        assert!(tail.contains("[REDACTED]"), "{tail}");
+        for line in tail.split("; ") {
+            assert!(line.chars().count() <= CommandRunner::MAX_LINE_CHARS + 1);
+        }
+    }
+
+    #[test]
+    fn error_tail_truncates_long_lines() {
+        let long_line = "x".repeat(CommandRunner::MAX_LINE_CHARS + 50);
+        let tail = LocalAgentSessionScanner::error_tail(&long_line);
+
+        assert!(tail.ends_with('\u{2026}'), "{tail}");
+        assert!(
+            tail.chars().count() <= CommandRunner::MAX_LINE_CHARS + 1,
+            "{tail}"
+        );
+    }
+
+    #[test]
+    fn exit_code_label_is_human_readable() {
+        assert_eq!(LocalAgentSessionScanner::exit_code_label(Some(1)), "1");
+        assert_eq!(
+            LocalAgentSessionScanner::exit_code_label(None),
+            "unknown"
+        );
+    }
+
     #[test]
     fn windows_process_query_never_requests_raw_command_lines() {
         let options = LocalAgentSessionScanner::process_options(Duration::from_secs(2));

rust/src/host/command_runner.rs

@@ -69,8 +69,10 @@ impl Default for CommandOptions {
 /// Result of running a command
 #[derive(Debug, Clone)]
 pub struct CommandResult {
-    /// Captured output text
+    /// Captured output text (stdout)
     pub text: String,
+    /// Captured stderr text
+    pub stderr: String,
     /// Whether the command timed out
     pub timed_out: bool,
     /// Exit code if available
@@ -112,7 +114,10 @@ pub struct CommandRunner {
 }
 
 impl CommandRunner {
+    /// Upper bound for a single captured line fed into user-facing tails.
+    pub const MAX_LINE_CHARS: usize = 200;
     const MAX_CAPTURE_BYTES: usize = 1024 * 1024;
+
     pub fn new() -> Self {
         Self {
             env_additions: HashMap::new(),
@@ -151,12 +156,13 @@ impl CommandRunner {
         Self::send_initial_input(&mut child, input, options.initial_delay, deadline);
 
         // Capture output
-        let (output, timed_out) = self.capture_output(&mut child, options, deadline)?;
+        let (output, stderr, timed_out) = self.capture_output(&mut child, options, deadline)?;
 
         let exit_code = Self::finish_child(&mut child);
 
         Ok(CommandResult {
             text: output,
+            stderr,
             timed_out,
             exit_code,
         })
@@ -251,28 +257,55 @@ impl CommandRunner {
         let _flushed_stdin = stdin.flush();
     }
 
+    /// Grace period between stdout/stderr EOF and process-object exit.
+    ///
+    /// Windows is the motivating case: PowerShell tears down its console
+    /// handles slightly before the process object transitions to signaled, so
+    /// a single `try_wait` right after capture sees `Ok(None)` even though the
+    /// process exited successfully milliseconds later. Waiting (instead of
+    /// immediately killing) preserves the real exit code.
+    const EXIT_GRACE_PERIOD: Duration = Duration::from_millis(250);
+
     fn finish_child(child: &mut Child) -> Option<i32> {
-        match child.try_wait() {
-            Ok(Some(status)) => status.code(),
-            Ok(None) => {
-                // Teardown after timeout/capture: kill/wait results cannot
-                // change the outcome, already reported separately.
-                let _killed = child.kill();
-                let _reaped = child.wait();
-                None
+        let mut remaining = Self::EXIT_GRACE_PERIOD;
+        loop {
+            match child.try_wait() {
+                Ok(Some(status)) => return status.code(),
+                Ok(None) => {}
+                Err(_) => return None,
             }
-            Err(_) => None,
+            if remaining.is_zero() {
+                break;
+            }
+            let step = remaining.min(Duration::from_millis(10));
+            std::thread::sleep(step);
+            remaining -= step;
+        }
+        // Teardown after timeout/capture. If the process exited between the
+        // grace loop and kill (kill fails on an already-dead process), `wait`
+        // still yields the true exit status; when the kill lands, the code is
+        // ours and already reported separately.
+        let killed = child.kill().is_ok();
+        let reaped = child.wait().ok();
+        if killed {
+            return None;
         }
+        reaped.and_then(|status| status.code())
     }
 
     /// Capture output from a running process
+    ///
+    /// Returns `(stdout, stderr, timed_out)`. Stderr lines are kept in a
+    /// separate buffer (bounded like stdout) so callers can surface real
+    /// diagnostics; they are never folded into the stdout `text` field.
     fn capture_output(
         &self,
         child: &mut Child,
         options: &CommandOptions,
         deadline: Instant,
-    ) -> Result<(String, bool), CommandError> {
+    ) -> Result<(String, String, bool), CommandError> {
         let mut output = String::new();
+        let mut stderr_output = String::new();
         let mut last_output_time = Instant::now();
         let (sender, receiver) = mpsc::channel();
         Self::read_stream(Self::stdout_reader(child)?, sender.clone(), true);
@@ -281,7 +314,7 @@ impl CommandRunner {
 
         loop {
             if Self::past_deadline(deadline) {
-                return Ok((output, true));
+                return Ok((output, stderr_output, true));
             }
             if Self::idle_timed_out(options.idle_timeout, last_output_time) {
                 break;
@@ -292,6 +325,7 @@ impl CommandRunner {
                 Ok(StreamEvent::Line { text, capture }) => {
                     last_output_time = Instant::now();
                     if !capture {
+                        Self::append_output_line(&mut stderr_output, &text);
                         continue;
                     }
                     Self::append_output_line(&mut output, &text);
@@ -315,7 +349,7 @@ impl CommandRunner {
             }
         }
 
-        Ok((output, false))
+        Ok((output, stderr_output, false))
     }
 
     fn stdout_reader(child: &mut Child) -> Result<BufReader<ChildStdout>, CommandError> {
@@ -557,6 +591,71 @@ mod tests {
         );
     }
 
+    #[cfg(windows)]
+    #[test]
+    fn large_capture_is_not_truncated() {
+        let runner = CommandRunner::new();
+        let options = CommandOptions {
+            initial_delay: Duration::ZERO,
+            extra_args: vec![
+                "-NoProfile".to_string(),
+                "-NonInteractive".to_string(),
+                "-Command".to_string(),
+                "$big = 'A' * 90000; Write-Output $big".to_string(),
+            ],
+            ..CommandOptions::default()
+        };
+
+        let result = runner.run("powershell.exe", None, &options).unwrap();
+
+        assert!(!result.timed_out);
+        assert_eq!(result.exit_code, Some(0));
+        assert!(
+            result.text.len() >= 90_000,
+            "expected >= 90000 bytes, got {}",
+            result.text.len()
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn stderr_is_captured_separately_from_stdout() {
+        let runner = CommandRunner::new();
+        let options = CommandOptions {
+            initial_delay: Duration::ZERO,
+            extra_args: vec![
+                "-NoProfile".to_string(),
+                "-NonInteractive".to_string(),
+                "-Command".to_string(),
+                concat!(
+                    "$ErrorActionPreference='Stop';",
+                    "Write-Output 'stdout line';",
+                    "Write-Error 'boom diagnostics'"
+                )
+                .to_string(),
+            ],
+            ..CommandOptions::default()
+        };
+
+        let result = runner.run("powershell.exe", None, &options).unwrap();
+
+        assert_eq!(result.exit_code, Some(1));
+        assert!(result.text.contains("stdout line"), "stdout: {}", result.text);
+        assert!(
+            result.stderr.contains("boom diagnostics"),
+            "stderr: {}",
+            result.stderr
+        );
+        // PowerShell's error banner echoes the whole script line into
+        // stderr, so asserting stderr lacks "stdout line" would be wrong;
+        // instead assert stderr diagnostics never leaked into stdout.
+        assert!(
+            !result.text.contains("boom diagnostics"),
+            "stderr leaked into stdout: {}",
+            result.text
+        );
+    }
+
     #[test]
     fn test_error_display() {
         let err = CommandError::BinaryNotFound("codex".to_string());