Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions nori-rs/acp/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
3 changes: 2 additions & 1 deletion nori-rs/acp/src/backend/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
4 changes: 4 additions & 0 deletions nori-rs/acp/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion nori-rs/acp/src/backend/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}");
Expand Down Expand Up @@ -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)),
Expand Down
4 changes: 3 additions & 1 deletion nori-rs/acp/src/backend/spawn_and_relay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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())),
Expand Down
1 change: 1 addition & 0 deletions nori-rs/acp/src/backend/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions nori-rs/acp/src/backend/tests/part3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions nori-rs/acp/src/backend/tests/part4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions nori-rs/acp/src/backend/user_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -96,6 +97,7 @@ impl AcpBackend {
&prompt_for_summary,
auto_worktree,
auto_worktree_repo_root.as_deref(),
acp_proxy,
)
.await
{
Expand Down
33 changes: 33 additions & 0 deletions nori-rs/acp/src/config/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions nori-rs/acp/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
46 changes: 46 additions & 0 deletions nori-rs/acp/src/config/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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")]
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions nori-rs/acp/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use tokio::sync::oneshot;

pub mod mcp;
pub mod sacp_connection;
mod wire_log;

#[cfg(test)]
mod sacp_connection_tests;
Expand Down
53 changes: 48 additions & 5 deletions nori-rs/acp/src/connection/sacp_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand All @@ -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();
Expand All @@ -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(
Expand Down
Loading
Loading