feat(acp): Add ACP wire proxy logging (#460)

CSRessel · web-flow · commit b765e540f957 · 2026-05-05T15:05:10.000-04:00
## Summary 🤖 Generated with [Nori](https://noriagentic.com/) - Adds config-gated ACP wire proxy logging via `[acp_proxy] enabled = true`, writing append-only JSONL files under `$NORI_HOME/acp-wire`. - Wraps each ACP child subprocess transport with raw JSON-RPC line logging in both directions, including prompt-summary subprocesses. - Documents the new diagnostic path and adds durable config plus real subprocess coverage. ## Test Plan - [x] `cargo test -p nori-acp` - [x] `cargo test -p nori-tui` - [x] `cargo build --bin nori && cargo test -p tui-pty-e2e` - [x] Manual TUI smoke with `elizacp`, temp `NORI_HOME`, `[acp_proxy] enabled = true`, and generated wire logs - [x] `just fmt` - [x] `just fix -p nori-acp` - [x] `just fix -p nori-tui` - [x] `git diff --check` Share Nori with your team: https://www.npmjs.com/package/nori-skillsets
diff --git a/nori-rs/acp/docs.md b/nori-rs/acp/docs.md
@@ -129,6 +129,19 @@ The config module provides the **canonical source of truth** for Nori home path
 - `CONFIG_FILE`: Config filename (`"config.toml"`)
 - `DEFAULT_AGENT`: Default agent (`"claude-code"`)
 
+**ACP Wire Proxy Configuration** (`config/types/mod.rs`, `connection/`):
+
+Nori can optionally wrap ACP subprocess transports with an append-only wire logger. The setting is top-level in `config.toml`:
+
+```toml
+[acp_proxy]
+enabled = true
+```
+
+When enabled, the resolved `AcpProxyConfig` stores logs under `$NORI_HOME/acp-wire`. The config layer intentionally owns this path resolution so every ACP entry point uses the same home directory semantics. The TUI passes the resolved proxy config into `AcpBackendConfig`; the backend passes it to each `SacpConnection::spawn()` call, including prompt-summary subprocesses, so every ACP child process gets its own log file.
+
+The connection layer uses `sacp::Lines` to observe raw newline-delimited JSON-RPC messages at the transport boundary before or after SACP parsing. Each child process gets a distinct JSONL file named from the launch timestamp, child PID, and sanitized agent slug. Records include the timestamp, direction (`client_to_agent` or `agent_to_client`), agent slug, child PID, and the parsed JSON message. If a line cannot be parsed as JSON, the logger preserves the raw line and parse error instead of disrupting the live session.
+
 **Agent Config Field Resolution:**
 
 | Field | Purpose | Persistence |
diff --git a/nori-rs/acp/src/backend/hooks.rs b/nori-rs/acp/src/backend/hooks.rs
@@ -10,12 +10,13 @@ pub(super) async fn run_prompt_summary(
     user_prompt: &str,
     auto_worktree: crate::config::AutoWorktree,
     auto_worktree_repo_root: Option<&std::path::Path>,
+    acp_proxy: crate::config::AcpProxyConfig,
 ) -> Result<()> {
     use tokio::time::Duration;
     use tokio::time::timeout;
 
     let agent_config = get_agent_config(agent_name)?;
-    let mut connection = SacpConnection::spawn(&agent_config, cwd).await?;
+    let mut connection = SacpConnection::spawn(&agent_config, cwd, acp_proxy).await?;
     let session_id = connection.create_session(cwd, vec![]).await?;
 
     let summarization_prompt = format!(
diff --git a/nori-rs/acp/src/backend/mod.rs b/nori-rs/acp/src/backend/mod.rs
@@ -185,6 +185,8 @@ pub struct AcpBackendConfig {
     pub nori_home: PathBuf,
     /// History persistence policy
     pub history_persistence: crate::config::HistoryPersistence,
+    /// ACP wire proxy logging settings
+    pub acp_proxy: crate::config::AcpProxyConfig,
     /// CLI version for transcript metadata
     pub cli_version: String,
     /// Auto-worktree mode (whether a worktree was created at startup)
@@ -276,6 +278,8 @@ pub struct AcpBackend {
     nori_home: PathBuf,
     /// History persistence policy
     history_persistence: crate::config::HistoryPersistence,
+    /// ACP wire proxy logging settings
+    acp_proxy: crate::config::AcpProxyConfig,
     /// Conversation ID for this session (used for history entries)
     conversation_id: ConversationId,
     /// Sender for broadcasting approval policy updates to the handler
diff --git a/nori-rs/acp/src/backend/session.rs b/nori-rs/acp/src/backend/session.rs
@@ -25,7 +25,7 @@ impl AcpBackend {
             acp_session_id, config.agent
         );
 
-        let mut connection = SacpConnection::spawn(&agent_config, &cwd)
+        let mut connection = SacpConnection::spawn(&agent_config, &cwd, config.acp_proxy.clone())
             .await
             .map_err(|e| {
                 let error_string = format!("{e:?}");
@@ -296,6 +296,7 @@ impl AcpBackend {
             idle_timer_abort: Arc::clone(&idle_timer_abort),
             nori_home: config.nori_home.clone(),
             history_persistence: config.history_persistence,
+            acp_proxy: config.acp_proxy.clone(),
             conversation_id,
             approval_policy_tx,
             pending_compact_summary: Arc::new(Mutex::new(pending_summary)),
diff --git a/nori-rs/acp/src/backend/spawn_and_relay.rs b/nori-rs/acp/src/backend/spawn_and_relay.rs
@@ -29,7 +29,8 @@ impl AcpBackend {
         debug!("Spawning ACP backend for agent: {}", config.agent);
 
         // Spawn the ACP connection with enhanced error handling
-        let connection_result = SacpConnection::spawn(&agent_config, &cwd).await;
+        let connection_result =
+            SacpConnection::spawn(&agent_config, &cwd, config.acp_proxy.clone()).await;
 
         let mut connection = match connection_result {
             Ok(conn) => conn,
@@ -158,6 +159,7 @@ impl AcpBackend {
             idle_timer_abort: Arc::clone(&idle_timer_abort),
             nori_home: config.nori_home.clone(),
             history_persistence: config.history_persistence,
+            acp_proxy: config.acp_proxy.clone(),
             conversation_id,
             approval_policy_tx,
             pending_compact_summary: Arc::new(Mutex::new(config.initial_context.clone())),
diff --git a/nori-rs/acp/src/backend/tests/mod.rs b/nori-rs/acp/src/backend/tests/mod.rs
@@ -242,6 +242,7 @@ fn build_test_config(temp_dir: &std::path::Path) -> AcpBackendConfig {
         os_notifications: crate::config::OsNotifications::Disabled,
         nori_home: temp_dir.to_path_buf(),
         history_persistence: crate::config::HistoryPersistence::SaveAll,
+        acp_proxy: crate::config::AcpProxyConfig::disabled(),
         cli_version: "test".to_string(),
         notify_after_idle: crate::config::NotifyAfterIdle::FiveSeconds,
         auto_worktree: crate::config::AutoWorktree::Off,
diff --git a/nori-rs/acp/src/backend/tests/part3.rs b/nori-rs/acp/src/backend/tests/part3.rs
@@ -375,6 +375,7 @@ async fn test_mock_agent_auth_failure_produces_actionable_error() {
         os_notifications: crate::config::OsNotifications::Disabled,
         nori_home: temp_dir.path().to_path_buf(),
         history_persistence: crate::config::HistoryPersistence::SaveAll,
+        acp_proxy: crate::config::AcpProxyConfig::disabled(),
         cli_version: "test".to_string(),
         notify_after_idle: crate::config::NotifyAfterIdle::FiveSeconds,
         auto_worktree: crate::config::AutoWorktree::Off,
@@ -1599,6 +1600,7 @@ async fn test_compact_sends_summarization_prompt_and_emits_events() {
         os_notifications: crate::config::OsNotifications::Disabled,
         nori_home: temp_dir.path().to_path_buf(),
         history_persistence: crate::config::HistoryPersistence::SaveAll,
+        acp_proxy: crate::config::AcpProxyConfig::disabled(),
         cli_version: "test".to_string(),
         notify_after_idle: crate::config::NotifyAfterIdle::FiveSeconds,
         auto_worktree: crate::config::AutoWorktree::Off,
diff --git a/nori-rs/acp/src/backend/tests/part4.rs b/nori-rs/acp/src/backend/tests/part4.rs
@@ -596,6 +596,7 @@ async fn test_compact_prepends_summary_to_next_prompt() {
         os_notifications: crate::config::OsNotifications::Disabled,
         nori_home: temp_dir.path().to_path_buf(),
         history_persistence: crate::config::HistoryPersistence::SaveAll,
+        acp_proxy: crate::config::AcpProxyConfig::disabled(),
         cli_version: "test".to_string(),
         notify_after_idle: crate::config::NotifyAfterIdle::FiveSeconds,
         auto_worktree: crate::config::AutoWorktree::Off,
@@ -758,6 +759,7 @@ async fn test_compact_not_in_unsupported_ops() {
         os_notifications: crate::config::OsNotifications::Disabled,
         nori_home: temp_dir.path().to_path_buf(),
         history_persistence: crate::config::HistoryPersistence::SaveAll,
+        acp_proxy: crate::config::AcpProxyConfig::disabled(),
         cli_version: "test".to_string(),
         notify_after_idle: crate::config::NotifyAfterIdle::FiveSeconds,
         auto_worktree: crate::config::AutoWorktree::Off,
diff --git a/nori-rs/acp/src/backend/user_input.rs b/nori-rs/acp/src/backend/user_input.rs
@@ -88,6 +88,7 @@ impl AcpBackend {
                     let prompt_for_summary = display_text.clone();
                     let auto_worktree = self.auto_worktree;
                     let auto_worktree_repo_root = self.auto_worktree_repo_root.clone();
+                    let acp_proxy = self.acp_proxy.clone();
                     tokio::spawn(async move {
                         if let Err(e) = run_prompt_summary(
                             &event_tx,
@@ -96,6 +97,7 @@ impl AcpBackend {
                             &prompt_for_summary,
                             auto_worktree,
                             auto_worktree_repo_root.as_deref(),
+                            acp_proxy,
                         )
                         .await
                         {
diff --git a/nori-rs/acp/src/config/loader.rs b/nori-rs/acp/src/config/loader.rs
@@ -123,6 +123,7 @@ impl NoriConfig {
 
         let skillset_per_session = toml.tui.skillset_per_session.unwrap_or(false);
         let auto_worktree = toml.tui.auto_worktree.unwrap_or_default();
+        let acp_proxy = super::types::AcpProxyConfig::from_toml(toml.acp_proxy, &nori_home);
 
         // Active agent is the runtime value: CLI override > config model > persisted agent > DEFAULT_AGENT
         // Using agent as fallback ensures the persisted preference is honored at startup
@@ -145,6 +146,7 @@ impl NoriConfig {
             history_persistence: toml
                 .history_persistence
                 .unwrap_or(super::types::HistoryPersistence::SaveAll),
+            acp_proxy,
             animations: toml.tui.animations.unwrap_or(true),
             terminal_notifications: toml
                 .tui
@@ -361,6 +363,37 @@ notify_after_idle = "30s"
         );
     }
 
+    #[test]
+    fn test_acp_proxy_defaults_disabled() {
+        let temp_dir = TempDir::new().unwrap();
+        let config_path = temp_dir.path().join(CONFIG_FILE);
+
+        std::fs::write(&config_path, "").unwrap();
+
+        let config = NoriConfig::load_from_path(&config_path).unwrap();
+        assert!(!config.acp_proxy.enabled);
+        assert_eq!(config.acp_proxy.log_dir, temp_dir.path().join("acp-wire"));
+    }
+
+    #[test]
+    fn test_acp_proxy_enabled_from_config() {
+        let temp_dir = TempDir::new().unwrap();
+        let config_path = temp_dir.path().join(CONFIG_FILE);
+
+        std::fs::write(
+            &config_path,
+            r#"
+[acp_proxy]
+enabled = true
+"#,
+        )
+        .unwrap();
+
+        let config = NoriConfig::load_from_path(&config_path).unwrap();
+        assert!(config.acp_proxy.enabled);
+        assert_eq!(config.acp_proxy.log_dir, temp_dir.path().join("acp-wire"));
+    }
+
     #[test]
     fn test_model_uses_persisted_agent_as_fallback() {
         let temp_dir = TempDir::new().unwrap();
diff --git a/nori-rs/acp/src/config/mod.rs b/nori-rs/acp/src/config/mod.rs
@@ -10,6 +10,8 @@ pub use loader::CONFIG_FILE;
 pub use loader::NORI_HOME_DIR;
 pub use loader::NORI_HOME_ENV;
 pub use loader::find_nori_home;
+pub use types::AcpProxyConfig;
+pub use types::AcpProxyConfigToml;
 pub use types::AgentConfigToml;
 pub use types::AgentDistributionToml;
 pub use types::ApprovalPolicy;
diff --git a/nori-rs/acp/src/config/types/mod.rs b/nori-rs/acp/src/config/types/mod.rs
@@ -199,6 +199,10 @@ pub struct NoriConfigToml {
     /// History persistence policy
     pub history_persistence: Option<HistoryPersistence>,
 
+    /// ACP wire proxy logging settings
+    #[serde(default)]
+    pub acp_proxy: AcpProxyConfigToml,
+
     /// TUI settings
     #[serde(default)]
     pub tui: TuiConfigToml,
@@ -220,6 +224,41 @@ pub struct NoriConfigToml {
     pub agents: Vec<AgentConfigToml>,
 }
 
+/// TOML settings for ACP wire proxy logging.
+#[derive(Debug, Clone, Default, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub struct AcpProxyConfigToml {
+    /// Whether to record raw ACP JSON-RPC messages to disk.
+    pub enabled: Option<bool>,
+}
+
+/// Resolved ACP wire proxy logging settings.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct AcpProxyConfig {
+    /// Whether wire logging is enabled.
+    pub enabled: bool,
+    /// Directory where per-child JSONL wire logs are written.
+    pub log_dir: PathBuf,
+}
+
+impl AcpProxyConfig {
+    /// Build resolved proxy settings from TOML and the Nori home directory.
+    pub fn from_toml(toml: AcpProxyConfigToml, nori_home: &std::path::Path) -> Self {
+        Self {
+            enabled: toml.enabled.unwrap_or(false),
+            log_dir: nori_home.join("acp-wire"),
+        }
+    }
+
+    /// Disabled proxy settings for tests and direct internal callers.
+    pub fn disabled() -> Self {
+        Self {
+            enabled: false,
+            log_dir: PathBuf::new(),
+        }
+    }
+}
+
 /// Whether terminal notifications (OSC 9) are enabled or disabled.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
@@ -1478,6 +1517,9 @@ pub struct NoriConfig {
     /// History persistence policy
     pub history_persistence: HistoryPersistence,
 
+    /// ACP wire proxy logging settings.
+    pub acp_proxy: AcpProxyConfig,
+
     /// Enable TUI animations
     pub animations: bool,
 
@@ -1594,6 +1636,10 @@ impl Default for NoriConfig {
             sandbox_mode: SandboxMode::WorkspaceWrite,
             approval_policy: ApprovalPolicy::OnRequest,
             history_persistence: HistoryPersistence::default(),
+            acp_proxy: AcpProxyConfig {
+                enabled: false,
+                log_dir: PathBuf::from(".nori/cli/acp-wire"),
+            },
             animations: true,
             terminal_notifications: TerminalNotifications::Enabled,
             os_notifications: OsNotifications::Enabled,
diff --git a/nori-rs/acp/src/connection/mod.rs b/nori-rs/acp/src/connection/mod.rs
@@ -11,6 +11,7 @@ use tokio::sync::oneshot;
 
 pub mod mcp;
 pub mod sacp_connection;
+mod wire_log;
 
 #[cfg(test)]
 mod sacp_connection_tests;
diff --git a/nori-rs/acp/src/connection/sacp_connection.rs b/nori-rs/acp/src/connection/sacp_connection.rs
@@ -13,11 +13,13 @@ use agent_client_protocol_schema as acp;
 use anyhow::Context;
 use anyhow::Result;
 use futures::AsyncBufReadExt;
+use futures::AsyncWriteExt;
+use futures::StreamExt;
 use futures::io::BufReader;
 use sacp::Agent;
-use sacp::ByteStreams;
 use sacp::Client;
 use sacp::ConnectionTo;
+use sacp::Lines;
 use tokio::process::Child;
 use tokio::process::Command;
 use tokio::sync::Mutex;
@@ -32,6 +34,9 @@ use super::AcpModelState;
 use super::ApprovalEventType;
 use super::ApprovalRequest;
 use super::ConnectionEvent;
+use super::wire_log::WireDirection;
+use super::wire_log::WireLogger;
+use crate::config::AcpProxyConfig;
 use crate::registry::AcpAgentConfig;
 use crate::translator;
 
@@ -87,7 +92,11 @@ pub struct SacpConnection {
 
 impl SacpConnection {
     /// Spawn a new ACP agent subprocess and establish a SACP v11 connection.
-    pub async fn spawn(config: &AcpAgentConfig, cwd: &Path) -> Result<Self> {
+    pub async fn spawn(
+        config: &AcpAgentConfig,
+        cwd: &Path,
+        proxy_config: AcpProxyConfig,
+    ) -> Result<Self> {
         debug!(
             "Spawning ACP agent (SACP v11): {} {:?} in {}",
             config.command,
@@ -141,6 +150,13 @@ impl SacpConnection {
 
         debug!("ACP agent spawned (pid: {:?})", child.id());
 
+        let wire_logger = if proxy_config.enabled {
+            let pid = child.id().unwrap_or(0);
+            Some(WireLogger::new(&proxy_config, config, pid)?)
+        } else {
+            None
+        };
+
         // Log stderr in background.
         let stderr_task = tokio::spawn(async move {
             let mut stderr = BufReader::new(stderr.compat());
@@ -157,9 +173,6 @@ impl SacpConnection {
         // --- Set up channels ---
         let (event_tx, event_rx) = mpsc::channel::<ConnectionEvent>(1024);
 
-        // --- Build SACP connection ---
-        let transport = ByteStreams::new(stdin.compat_write(), stdout.compat());
-
         let event_tx_for_notifications = event_tx.clone();
         let event_tx_for_write = event_tx.clone();
         let event_tx_for_read = event_tx.clone();
@@ -177,6 +190,36 @@ impl SacpConnection {
         let child = std::sync::Arc::new(Mutex::new(child));
 
         let connection_task = tokio::spawn(async move {
+            let outgoing_logger = wire_logger.clone();
+            let outgoing_sink = futures::sink::unfold(
+                Box::pin(stdin.compat_write()),
+                move |mut writer, line: String| {
+                    let logger = outgoing_logger.clone();
+                    async move {
+                        if let Some(logger) = &logger {
+                            logger.record(WireDirection::ClientToAgent, &line);
+                        }
+                        let mut bytes = line.into_bytes();
+                        bytes.push(b'\n');
+                        writer.write_all(&bytes).await?;
+                        Ok::<_, std::io::Error>(writer)
+                    }
+                },
+            );
+
+            let incoming_logger = wire_logger;
+            let incoming_lines = Box::pin(BufReader::new(stdout.compat()).lines().map(
+                move |line_result| {
+                    if let Ok(line) = &line_result
+                        && let Some(logger) = &incoming_logger
+                    {
+                        logger.record(WireDirection::AgentToClient, line);
+                    }
+                    line_result
+                },
+            ));
+            let transport = Lines::new(outgoing_sink, incoming_lines);
+
             let result = Client
                 .builder()
                 .on_receive_notification(
diff --git a/nori-rs/acp/src/connection/sacp_connection_tests.rs b/nori-rs/acp/src/connection/sacp_connection_tests.rs
diff --git a/nori-rs/acp/src/connection/wire_log.rs b/nori-rs/acp/src/connection/wire_log.rs
diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md
diff --git a/nori-rs/tui/src/chatwidget/agent.rs b/nori-rs/tui/src/chatwidget/agent.rs
diff --git a/nori-rs/tui/src/nori/config_picker.rs b/nori-rs/tui/src/nori/config_picker.rs
