fix: capture ACP agent stderr and extract error.data from JSON-RPC errors

src/acp/connection.rs

@@ -121,6 +121,7 @@ pub struct AcpConnection {
     pub last_active: Instant,
     pub session_reset: bool,
     _reader_handle: JoinHandle<()>,
+    _stderr_handle: Option<JoinHandle<()>>,
 }
 
 /// Build the final set of env vars for the agent subprocess.
@@ -246,6 +247,7 @@ pub(crate) async fn run_reader_loop<R, W>(
             error: Some(crate::acp::protocol::JsonRpcError {
                 code: -1,
                 message: "connection closed".into(),
+                data: None,
             }),
             params: None,
         });
@@ -269,7 +271,7 @@ impl AcpConnection {
         cmd.args(args)
             .stdin(std::process::Stdio::piped())
             .stdout(std::process::Stdio::piped())
-            .stderr(std::process::Stdio::null())
+            .stderr(std::process::Stdio::piped())
             .current_dir(working_dir);
         // Create a new process group so we can kill the entire tree.
         // SAFETY: setpgid is async-signal-safe (POSIX.1-2008) and called
@@ -355,6 +357,36 @@ impl AcpConnection {
         let stdin = proc.stdin.take().ok_or_else(|| anyhow!("no stdin"))?;
         let stdin = Arc::new(Mutex::new(stdin));
 
+        // Capture agent stderr and log it (ACP spec: agents MAY write to stderr
+        // for logging; clients MAY capture or ignore it).
+        let stderr_handle = if let Some(stderr) = proc.stderr.take() {
+            let cmd_name = command.to_string();
+            Some(tokio::spawn(async move {
+                let mut reader = BufReader::new(stderr);
+                let mut line = String::new();
+                loop {
+                    line.clear();
+                    match reader.read_line(&mut line).await {
+                        Ok(0) => break,
+                        Ok(_) => {
+                            let trimmed = line.trim();
+                            if !trimmed.is_empty() {
+                                let sanitized: String = trimmed.chars()
+                                    .filter(|c| !c.is_control() || *c == '\t')
+                                    .collect();
+                                if !sanitized.is_empty() {
+                                    tracing::warn!(agent = %cmd_name, "{sanitized}");
+                                }
+                            }
+                        }
+                        Err(_) => break,
+                    }
+                }
+            }))
+        } else {
+            None
+        };
+
         let pending: Arc<Mutex<HashMap<u64, oneshot::Sender<JsonRpcMessage>>>> =
             Arc::new(Mutex::new(HashMap::new()));
         let notify_tx: Arc<Mutex<Option<mpsc::UnboundedSender<JsonRpcMessage>>>> =
@@ -380,6 +412,7 @@ impl AcpConnection {
             last_active: Instant::now(),
             session_reset: false,
             _reader_handle: reader_handle,
+            _stderr_handle: stderr_handle,
         })
     }
 
@@ -657,6 +690,9 @@ impl AcpConnection {
 
 impl Drop for AcpConnection {
     fn drop(&mut self) {
+        if let Some(handle) = self._stderr_handle.take() {
+            handle.abort();
+        }
         self.kill_process_group();
     }
 }

src/acp/protocol.rs

@@ -55,11 +55,32 @@ pub struct JsonRpcMessage {
 pub struct JsonRpcError {
     pub code: i64,
     pub message: String,
+    /// Optional structured data from the agent (JSON-RPC `error.data`).
+    /// Agents like codex-acp include `{"message": "...", "codex_error_info": "..."}`.
+    pub data: Option<Value>,
+}
+
+impl JsonRpcError {
+    /// Extract a human-readable detail from `error.data.message` if present.
+    ///
+    /// The `"message"` key is a convention used by codex-acp and aligns with
+    /// common JSON-RPC practice, but is NOT mandated by the ACP spec.
+    /// Other agents may use `"detail"`, `"reason"`, etc. — extend here if needed.
+    pub fn data_message(&self) -> Option<&str> {
+        self.data
+            .as_ref()
+            .and_then(|d| d.get("message"))
+            .and_then(|m| m.as_str())
+    }
 }
 
 impl std::fmt::Display for JsonRpcError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "JSON-RPC error {}: {}", self.code, self.message)
+        write!(f, "JSON-RPC error {}: {}", self.code, self.message)?;
+        if let Some(detail) = self.data_message() {
+            write!(f, " — {detail}")?;
+        }
+        Ok(())
     }
 }
 

src/adapter.rs

@@ -564,7 +564,7 @@ impl AdapterRouter {
                                 continue;
                             }
                             if let Some(ref err) = notification.error {
-                                response_error = Some(format_coded_error(err.code, &err.message));
+                                response_error = Some(format_coded_error(err.code, &err.message, err.data_message()));
                             }
                             break;
                         }

src/error_display.rs

@@ -50,8 +50,9 @@ pub fn format_user_error(message: &str) -> String {
 
 /// Format coded error from ACP agent for display in Discord.
 /// Used for response errors that have a JSON-RPC or HTTP status code.
+/// `data_message` is the optional detail extracted from `error.data.message`.
 /// Public for reuse by other adapters (e.g. Slack).
-pub fn format_coded_error(code: i64, message: &str) -> String {
+pub fn format_coded_error(code: i64, message: &str, data_message: Option<&str>) -> String {
     let prefix = match code {
         400 => "**Bad Request**",
         401 => "**Unauthorized**",
@@ -70,11 +71,18 @@ pub fn format_coded_error(code: i64, message: &str) -> String {
         -32099..=-32000 => "**Server Error**",
         _ => "**Error**",
     };
-    if message.is_empty() {
+    let mut out = if message.is_empty() {
         format!("{} (code: {})", prefix, code)
     } else {
         format!("{} (code: {})\n{}", prefix, code, message)
+    };
+    if let Some(detail) = data_message {
+        if !detail.is_empty() && !message.contains(detail) {
+            out.push_str("\n> ");
+            out.push_str(detail);
+        }
     }
+    out
 }
 
 #[cfg(test)]
@@ -166,54 +174,68 @@ mod tests {
 
     #[test]
     fn test_format_coded_error_401() {
-        let result = format_coded_error(401, "invalid token");
+        let result = format_coded_error(401, "invalid token", None);
         assert!(result.contains("Unauthorized"));
         assert!(result.contains("401"));
         assert!(result.contains("invalid token"));
     }
 
     #[test]
     fn test_format_coded_error_429() {
-        let result = format_coded_error(429, "");
+        let result = format_coded_error(429, "", None);
         assert!(result.contains("Rate Limited"));
         assert!(result.contains("429"));
         assert!(!result.contains("\n")); // no message, no newline
     }
 
     #[test]
     fn test_format_coded_error_503() {
-        let result = format_coded_error(503, "service unavailable");
+        let result = format_coded_error(503, "service unavailable", None);
         assert!(result.contains("Service Unavailable"));
         assert!(result.contains("503"));
         assert!(result.contains("service unavailable"));
     }
 
     #[test]
     fn test_format_coded_error_json_rpc() {
-        let result = format_coded_error(-32602, "missing required parameter");
+        let result = format_coded_error(-32602, "missing required parameter", None);
         assert!(result.contains("Invalid Params"));
         assert!(result.contains("-32602"));
     }
 
     #[test]
     fn test_format_coded_error_server_error_range() {
-        let result = format_coded_error(-32050, "internal failure");
+        let result = format_coded_error(-32050, "internal failure", None);
         assert!(result.contains("Server Error"));
         assert!(result.contains("-32050"));
     }
 
     #[test]
     fn test_format_coded_error_connection_error() {
-        let result = format_coded_error(-32000, "connection refused");
+        let result = format_coded_error(-32000, "connection refused", None);
         assert!(result.contains("Server Error")); // -32000 falls in -32099..=-32000 range
         assert!(result.contains("-32000"));
     }
 
     #[test]
     fn test_format_coded_error_unknown_code() {
-        let result = format_coded_error(999, "something happened");
+        let result = format_coded_error(999, "something happened", None);
         assert!(result.contains("Error"));
         assert!(result.contains("999"));
         assert!(result.contains("something happened"));
     }
+
+    #[test]
+    fn test_format_coded_error_with_data_message() {
+        let result = format_coded_error(-32603, "Internal error", Some("model not supported"));
+        assert!(result.contains("Internal Error"));
+        assert!(result.contains("model not supported"));
+    }
+
+    #[test]
+    fn test_format_coded_error_data_message_not_duplicated() {
+        // If data_message is already in message, don't repeat it
+        let result = format_coded_error(-32603, "model not supported", Some("model not supported"));
+        assert_eq!(result.matches("model not supported").count(), 1);
+    }
 }