diff --git a/nori-rs/acp/docs.md b/nori-rs/acp/docs.md index 518190989..b48cd5f48 100644 --- 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 index 14c5ee6b1..f35bfd884 100644 --- 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 index 5e5feff22..00fdfcac8 100644 --- 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 index ab181f438..aad977840 100644 --- 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 index a3ccd0964..26b6d878e 100644 --- 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 index 13bc26a9d..0865ca3ae 100644 --- 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 index bb26f8921..9daf9545b 100644 --- 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 index a18f495fa..25dbd25d5 100644 --- 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 index 33737d7c4..30cc6edca 100644 --- 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 index 88e9f197f..a6f07ae9e 100644 --- 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 index 077e8a3b1..5173601db 100644 --- 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 index f69c940e8..63ad75028 100644 --- 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, + /// 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, } +/// 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, +} + +/// 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 index 6a75b3544..3c40e9b5b 100644 --- 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 index 9c75fb294..a5639a4f2 100644 --- 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 { + pub async fn spawn( + config: &AcpAgentConfig, + cwd: &Path, + proxy_config: AcpProxyConfig, + ) -> Result { 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::(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 index 716a41f24..048834b20 100644 --- a/nori-rs/acp/src/connection/sacp_connection_tests.rs +++ b/nori-rs/acp/src/connection/sacp_connection_tests.rs @@ -5,6 +5,7 @@ use super::ConnectionEvent; use super::sacp_connection::SacpConnection; use agent_client_protocol_schema as acp; use pretty_assertions::assert_eq; +use serde_json::Value; use serial_test::serial; use tempfile::tempdir; @@ -36,6 +37,35 @@ async fn recv_approval_request( } } +async fn drive_logged_prompt( + config: &crate::registry::AcpAgentConfig, + cwd: &std::path::Path, + proxy: crate::config::AcpProxyConfig, +) { + let conn = SacpConnection::spawn(config, cwd, proxy) + .await + .expect("spawn logged connection"); + let session_id = conn + .create_session(cwd, vec![]) + .await + .expect("create session"); + conn.prompt( + session_id, + vec![acp::ContentBlock::Text(acp::TextContent::new("Hello"))], + ) + .await + .expect("prompt"); + conn.shutdown().await; +} + +fn read_wire_log(path: &std::path::Path) -> Vec { + let content = std::fs::read_to_string(path).expect("read wire log"); + content + .lines() + .map(|line| serde_json::from_str::(line).expect("wire log line is json")) + .collect() +} + /// Test that SacpConnection can spawn a mock agent, perform the initialization /// handshake, and return a working connection. After spawn, the connection /// should be able to create a session (proving the transport is alive). @@ -47,9 +77,13 @@ async fn test_spawn_and_create_session() { }; let temp_dir = tempdir().expect("temp dir"); - let conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("Failed to spawn SacpConnection"); + let conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("Failed to spawn SacpConnection"); // Verify the connection is functional by creating a session. let session_id = conn @@ -64,6 +98,60 @@ async fn test_spawn_and_create_session() { ); } +#[tokio::test] +#[serial] +async fn test_proxy_logging_records_one_wire_log_per_child_subprocess() { + let Some(config) = mock_agent_config() else { + return; + }; + let temp_dir = tempdir().expect("temp dir"); + let proxy = crate::config::AcpProxyConfig { + enabled: true, + log_dir: temp_dir.path().join("acp-wire"), + }; + + drive_logged_prompt(&config, temp_dir.path(), proxy.clone()).await; + drive_logged_prompt(&config, temp_dir.path(), proxy.clone()).await; + + let mut log_paths = std::fs::read_dir(&proxy.log_dir) + .expect("wire log dir exists") + .map(|entry| entry.expect("wire log entry").path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "jsonl")) + .collect::>(); + log_paths.sort(); + + assert_eq!( + log_paths.len(), + 2, + "expected one JSONL wire log per spawned ACP child subprocess" + ); + + for path in log_paths { + let records = read_wire_log(&path); + assert!( + records.iter().any(|record| { + record["direction"] == "client_to_agent" + && record["message"]["method"] == "initialize" + }), + "expected client initialize request in {path:?}: {records:#?}" + ); + assert!( + records.iter().any(|record| { + record["direction"] == "client_to_agent" + && record["message"]["method"] == "session/prompt" + }), + "expected client prompt request in {path:?}: {records:#?}" + ); + assert!( + records.iter().any(|record| { + record["direction"] == "agent_to_client" + && record["message"]["method"] == "session/update" + }), + "expected agent session/update notification in {path:?}: {records:#?}" + ); + } +} + /// Test the full prompt lifecycle: spawn -> create session -> prompt -> receive /// text updates -> get stop reason. #[tokio::test] @@ -74,9 +162,13 @@ async fn test_prompt_receives_text_updates() { }; let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); @@ -120,9 +212,13 @@ async fn test_event_receiver_forwards_session_updates() { }; let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); let session_id = conn @@ -163,9 +259,13 @@ async fn test_tool_call_prompt_delivers_final_text_update() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); @@ -217,9 +317,13 @@ async fn test_model_state_after_session_creation() { }; let temp_dir = tempdir().expect("temp dir"); - let conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let _session_id = conn .create_session(temp_dir.path(), vec![]) @@ -261,9 +365,13 @@ async fn test_approval_receiver_forwards_requests() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); @@ -321,9 +429,13 @@ async fn test_event_receiver_preserves_update_then_approval_order() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); let session_id = conn @@ -415,9 +527,13 @@ async fn test_codex_home_not_inherited() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); @@ -465,9 +581,13 @@ async fn test_drop_kills_subprocess() { let temp_dir = tempdir().expect("temp dir"); - let conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let session_id = conn .create_session(temp_dir.path(), vec![]) @@ -514,9 +634,13 @@ async fn test_cancel_during_prompt() { let temp_dir = tempdir().expect("temp dir"); - let conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let session_id = conn .create_session(temp_dir.path(), vec![]) @@ -570,9 +694,13 @@ async fn test_sequential_prompt_after_cancel_receives_response() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); let session_id = conn @@ -684,9 +812,13 @@ async fn test_prompt_after_cancel_absorbs_empty_end_turn_tail() { let temp_dir = tempdir().expect("temp dir"); - let mut conn = SacpConnection::spawn(&config, temp_dir.path()) - .await - .expect("spawn"); + let mut conn = SacpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("spawn"); let mut event_rx = conn.take_event_receiver(); let session_id = conn diff --git a/nori-rs/acp/src/connection/wire_log.rs b/nori-rs/acp/src/connection/wire_log.rs new file mode 100644 index 000000000..6d93036bc --- /dev/null +++ b/nori-rs/acp/src/connection/wire_log.rs @@ -0,0 +1,130 @@ +use std::fs::File; +use std::fs::OpenOptions; +use std::io::Write; +use std::path::Path; +use std::sync::Arc; +use std::sync::Mutex; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; + +use anyhow::Context; +use anyhow::Result; +use serde_json::Value; +use serde_json::json; +use tracing::warn; + +use crate::config::AcpProxyConfig; +use crate::registry::AcpAgentConfig; + +#[derive(Clone)] +pub(super) struct WireLogger { + inner: Arc>, + agent: String, + child_pid: i64, +} + +impl WireLogger { + pub(super) fn new( + config: &AcpProxyConfig, + agent_config: &AcpAgentConfig, + pid: u32, + ) -> Result { + std::fs::create_dir_all(&config.log_dir).with_context(|| { + format!( + "Failed to create ACP wire log directory {}", + config.log_dir.display() + ) + })?; + + let child_pid = i64::from(pid); + let path = log_path(&config.log_dir, &agent_config.provider_slug, child_pid); + let file = OpenOptions::new() + .create_new(true) + .append(true) + .open(&path) + .with_context(|| format!("Failed to open ACP wire log {}", path.display()))?; + + Ok(Self { + inner: Arc::new(Mutex::new(file)), + agent: agent_config.provider_slug.clone(), + child_pid, + }) + } + + pub(super) fn record(&self, direction: WireDirection, line: &str) { + if let Err(error) = self.try_record(direction, line) { + warn!("Failed to write ACP wire log entry: {error}"); + } + } + + fn try_record(&self, direction: WireDirection, line: &str) -> Result<()> { + let record = match serde_json::from_str::(line) { + Ok(message) => json!({ + "ts_ms": unix_time_millis(), + "direction": direction.as_str(), + "agent": self.agent, + "child_pid": self.child_pid, + "message": message, + }), + Err(error) => json!({ + "ts_ms": unix_time_millis(), + "direction": direction.as_str(), + "agent": self.agent, + "child_pid": self.child_pid, + "raw_line": line, + "parse_error": error.to_string(), + }), + }; + + let mut file = self + .inner + .lock() + .map_err(|error| anyhow::anyhow!("ACP wire log lock poisoned: {error}"))?; + serde_json::to_writer(&mut *file, &record).context("Failed to serialize ACP wire log")?; + file.write_all(b"\n") + .context("Failed to append ACP wire log newline")?; + Ok(()) + } +} + +#[derive(Clone, Copy)] +pub(super) enum WireDirection { + ClientToAgent, + AgentToClient, +} + +impl WireDirection { + fn as_str(self) -> &'static str { + match self { + Self::ClientToAgent => "client_to_agent", + Self::AgentToClient => "agent_to_client", + } + } +} + +fn log_path(log_dir: &Path, agent_slug: &str, child_pid: i64) -> std::path::PathBuf { + let timestamp = unix_time_millis(); + let agent = sanitize_filename(agent_slug); + log_dir.join(format!("{timestamp}-{child_pid}-{agent}.jsonl")) +} + +fn sanitize_filename(value: &str) -> String { + value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.') { + ch + } else { + '_' + } + }) + .collect() +} + +fn unix_time_millis() -> i64 { + let millis = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_millis()) + .unwrap_or(0); + i64::try_from(millis).unwrap_or(i64::MAX) +} diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md index db9f5bcfe..c411e1010 100644 --- a/nori-rs/tui/docs.md +++ b/nori-rs/tui/docs.md @@ -31,6 +31,8 @@ Key dependencies: `ratatui` for rendering, `crossterm` for terminal events, `pul Entry point is `main.rs` which delegates to `run_app()` in `lib.rs`. The `run_main()` function loads `NoriConfig` once early and reuses it for both the auto-worktree setup and the `vertical_footer` setting (passed as a parameter to `run_ratatui_app()`). After loading config, `run_main()` initializes the agent registry via `nori_acp::initialize_registry()` with any custom `[[agents]]` defined in `config.toml` (see `@/nori-rs/acp/docs.md` for registry details). Initialization failure is non-fatal (logged as a warning). +`NoriConfig` is also the source of truth for ACP backend diagnostics that do not have direct TUI controls yet. The chat widget passes the resolved ACP proxy configuration into `AcpBackendConfig` when spawning or resuming sessions, so enabling `[acp_proxy]` in config wraps every backend ACP subprocess in the wire logger without needing UI state in the TUI layer. + The auto-worktree startup flow branches on the `AutoWorktree` enum (see `@/nori-rs/acp/docs.md`): | Variant | Timing | Behavior | diff --git a/nori-rs/tui/src/chatwidget/agent.rs b/nori-rs/tui/src/chatwidget/agent.rs index 9b0da36d2..02baae47e 100644 --- a/nori-rs/tui/src/chatwidget/agent.rs +++ b/nori-rs/tui/src/chatwidget/agent.rs @@ -232,6 +232,7 @@ fn spawn_acp_agent( notify_after_idle: nori_config.notify_after_idle, nori_home, history_persistence: HistoryPersistence::SaveAll, + acp_proxy: nori_config.acp_proxy.clone(), cli_version: env!("CARGO_PKG_VERSION").to_string(), auto_worktree, auto_worktree_repo_root, @@ -407,6 +408,7 @@ pub(crate) fn spawn_acp_agent_resume( notify_after_idle: nori_config.notify_after_idle, nori_home, history_persistence: HistoryPersistence::SaveAll, + acp_proxy: nori_config.acp_proxy.clone(), cli_version: env!("CARGO_PKG_VERSION").to_string(), auto_worktree, auto_worktree_repo_root, diff --git a/nori-rs/tui/src/nori/config_picker.rs b/nori-rs/tui/src/nori/config_picker.rs index aa0c7293f..7d584f576 100644 --- a/nori-rs/tui/src/nori/config_picker.rs +++ b/nori-rs/tui/src/nori/config_picker.rs @@ -628,6 +628,7 @@ mod tests { sandbox_mode: codex_protocol::config_types::SandboxMode::WorkspaceWrite, approval_policy: nori_acp::config::ApprovalPolicy::OnRequest, history_persistence: nori_acp::config::HistoryPersistence::SaveAll, + acp_proxy: nori_acp::config::AcpProxyConfig::disabled(), animations: true, terminal_notifications: TerminalNotifications::Enabled, os_notifications: OsNotifications::Enabled,