Skip to content
Open
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
10 changes: 10 additions & 0 deletions nori-rs/acp/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,16 @@ The model is only applied if:

Failures to apply the default model (e.g., model unavailable, API error) produce warnings but do not block session startup. When users switch models via `/model` command, the TUI persists the selection by calling `ConfigEditsBuilder::set_default_model()` (see `@/nori-rs/core/docs.md`).

**Live Session Configuration** (`connection/sacp_connection.rs`, `backend/submit_and_ops.rs`):

ACP agents can expose runtime session configuration through `NewSessionResponse.config_options`, `LoadSessionResponse.config_options`, idle `SessionUpdate::ConfigOptionUpdate` notifications, and the `session/set_config_option` RPC. `SacpConnection` owns the latest live config snapshot in `AcpSessionConfigState`, updates it when a session is created/loaded or when config-option notifications arrive, and replaces it with the full response snapshot after `set_config_option()`.

This first implementation is deliberately live-session only:
- `AcpBackend::config_options()` returns the current in-memory ACP config snapshot for TUI pickers.
- `AcpBackend::set_config_option()` sends `session/set_config_option` for the current session and updates in-memory state from the response.
- No config form is shown during `/agent` switching yet.
- No ACP session config selections are persisted to `config.toml` yet.

**Hooks System** (`config/types/mod.rs`, `hooks.rs`, `backend/mod.rs`):

Hooks allow users to run custom scripts at lifecycle boundaries. There are two flavors: **synchronous** hooks (blocking, executed sequentially) and **async** hooks (fire-and-forget, spawned via `tokio::spawn`). Both are configured under `[hooks]` in `config.toml`, are **fail-open** (failures produce warnings but do not halt operations), and share the same execution engine (`execute_hooks_with_env()` in `hooks.rs`) and interpreter detection. Synchronous hooks support output routing and context injection; async hooks route all output exclusively to tracing.
Expand Down
19 changes: 18 additions & 1 deletion nori-rs/acp/src/backend/submit_and_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@ impl AcpBackend {
self.connection.model_state()
}

/// Get the current ACP session config snapshot from the connection.
pub fn config_options(&self) -> Vec<acp::SessionConfigOption> {
self.connection.config_options()
}

/// Get the current session ID.
///
/// Note: This clones the session ID since it may be replaced during /compact.
Expand All @@ -313,11 +318,23 @@ impl AcpBackend {

/// Get a reference to the underlying ACP connection.
///
/// This provides access to low-level ACP operations like model switching.
/// This provides access to low-level ACP operations like session controls.
pub fn connection(&self) -> &Arc<SacpConnection> {
&self.connection
}

/// Set an ACP session config option for the current session.
pub async fn set_config_option(
&self,
config_id: impl Into<acp::SessionConfigId>,
value: impl Into<acp::SessionConfigValueId>,
) -> Result<()> {
let session_id = self.session_id.read().await;
self.connection
.set_config_option(&session_id, config_id, value)
.await
}

/// Switch to a different model for the current session.
///
/// This sends a `session/set_model` request to the ACP agent and updates
Expand Down
15 changes: 15 additions & 0 deletions nori-rs/acp/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ pub struct AcpModelState {
pub available_models: Vec<acp::ModelInfo>,
}

/// Session config state captured from ACP session setup and updates.
///
/// This stores the complete current `configOptions` snapshot for the active
/// session. ACP responses and notifications replace the full list.
#[derive(Debug, Clone, Default)]
pub struct AcpSessionConfigState {
pub config_options: Vec<acp::SessionConfigOption>,
}

impl AcpSessionConfigState {
pub fn new() -> Self {
Self::default()
}
}

impl AcpModelState {
/// Create a new empty model state
pub fn new() -> Self {
Expand Down
66 changes: 66 additions & 0 deletions nori-rs/acp/src/connection/sacp_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use tracing::debug;
use tracing::warn;

use super::AcpModelState;
use super::AcpSessionConfigState;
use super::ApprovalEventType;
use super::ApprovalRequest;
use super::ConnectionEvent;
Expand Down Expand Up @@ -75,6 +76,9 @@ pub struct SacpConnection {
/// Thread-safe model state, updated on session creation and model switch.
model_state: std::sync::Arc<std::sync::RwLock<AcpModelState>>,

/// Thread-safe session config state, updated from complete ACP snapshots.
session_config_state: std::sync::Arc<std::sync::RwLock<AcpSessionConfigState>>,

/// Handle to the background task driving the SACP connection.
connection_task: tokio::task::JoinHandle<()>,

Expand Down Expand Up @@ -166,6 +170,9 @@ impl SacpConnection {
let prompt_state =
std::sync::Arc::new(Mutex::new(HashMap::<String, SessionPromptState>::new()));
let prompt_state_for_notifications = prompt_state.clone();
let session_config_state =
std::sync::Arc::new(std::sync::RwLock::new(AcpSessionConfigState::new()));
let session_config_state_for_notifications = session_config_state.clone();
let approval_cwd = cwd.to_path_buf();
let write_cwd = cwd.to_path_buf();
let read_cwd = cwd.to_path_buf();
Expand All @@ -183,6 +190,7 @@ impl SacpConnection {
{
let event_tx = event_tx_for_notifications;
let prompt_state = prompt_state_for_notifications;
let session_config_state = session_config_state_for_notifications;
async move |notification: acp::SessionNotification, _connection| {
let session_id = notification.session_id.to_string();
{
Expand All @@ -198,6 +206,12 @@ impl SacpConnection {
update_kind = super::session_update_kind(&notification.update),
"Transport received ACP session/update notification"
);
if let acp::SessionUpdate::ConfigOptionUpdate(update) =
&notification.update
&& let Ok(mut state) = session_config_state.write()
{
state.config_options = update.config_options.clone();
}
if event_tx
.send(ConnectionEvent::SessionUpdate(notification.update))
.await
Expand Down Expand Up @@ -484,6 +498,7 @@ impl SacpConnection {
event_rx,
prompt_state,
model_state: std::sync::Arc::new(std::sync::RwLock::new(AcpModelState::new())),
session_config_state,
connection_task,
child,
stderr_task,
Expand Down Expand Up @@ -518,6 +533,12 @@ impl SacpConnection {
);
}

if let Some(config_options) = response.config_options
&& let Ok(mut state) = self.session_config_state.write()
{
state.config_options = config_options;
}

Ok(response.session_id)
}

Expand All @@ -541,6 +562,12 @@ impl SacpConnection {
*state = AcpModelState::from_session_model_state(models);
}

if let Some(config_options) = response.config_options
&& let Ok(mut state) = self.session_config_state.write()
{
state.config_options = config_options;
}

// The session ID from the request is reused since the response
// doesn't contain one.
Ok(acp::SessionId::from(session_id.to_string()))
Expand Down Expand Up @@ -661,6 +688,45 @@ impl SacpConnection {
.clone()
}

/// Get the current ACP session config snapshot.
pub fn config_options(&self) -> Vec<acp::SessionConfigOption> {
#[expect(
clippy::expect_used,
reason = "RwLock poisoning indicates a bug elsewhere"
)]
self.session_config_state
.read()
.expect("Session config state lock poisoned")
.config_options
.clone()
}

/// Set an ACP session config option and replace state from the full response snapshot.
pub async fn set_config_option(
&self,
session_id: &acp::SessionId,
config_id: impl Into<acp::SessionConfigId>,
value: impl Into<acp::SessionConfigValueId>,
) -> Result<()> {
let value = acp::SessionConfigOptionValue::value_id(value.into());
let response = self
.cx
.send_request(acp::SetSessionConfigOptionRequest::new(
session_id.clone(),
config_id,
value,
))
.block_task()
.await
.context("Failed to set ACP session config option")?;

if let Ok(mut state) = self.session_config_state.write() {
state.config_options = response.config_options;
}

Ok(())
}

/// Explicitly tear down the ACP subprocess and background tasks.
///
/// Unlike `Drop`, this async path can wait for process termination so the
Expand Down
56 changes: 56 additions & 0 deletions nori-rs/acp/src/connection/sacp_connection_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,62 @@ async fn test_model_state_after_session_creation() {
);
}

#[tokio::test]
#[serial]
async fn test_session_config_options_after_session_creation() {
let Some(config) = mock_agent_config() else {
return;
};
let temp_dir = tempdir().expect("temp dir");

let conn = SacpConnection::spawn(&config, temp_dir.path())
.await
.expect("spawn");

let _session_id = conn
.create_session(temp_dir.path(), vec![])
.await
.expect("create session");

let config_options = conn.config_options();
let option_names = config_options
.iter()
.map(|option| option.name.as_str())
.collect::<Vec<_>>();

assert_eq!(option_names, vec!["Model", "Thought Level"]);
}

#[tokio::test]
#[serial]
async fn test_set_session_config_option_replaces_connection_state() {
let Some(config) = mock_agent_config() else {
return;
};
let temp_dir = tempdir().expect("temp dir");

let conn = SacpConnection::spawn(&config, temp_dir.path())
.await
.expect("spawn");

let session_id = conn
.create_session(temp_dir.path(), vec![])
.await
.expect("create session");

conn.set_config_option(&session_id, "model", "mock-model-fast")
.await
.expect("set config option");

let config_options = conn.config_options();
let option_names = config_options
.iter()
.map(|option| option.name.as_str())
.collect::<Vec<_>>();

assert_eq!(option_names, vec!["Model", "Speed"]);
}

/// Test that approval requests flow through the ordered event inbox and the
/// prompt completes after the approval response is sent back.
#[tokio::test]
Expand Down
11 changes: 11 additions & 0 deletions nori-rs/acp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub use backend::AcpBackend;
pub use backend::AcpBackendConfig;
pub use backend::BackendEvent;
pub use connection::AcpModelState;
pub use connection::AcpSessionConfigState;
pub use connection::ApprovalRequest;
pub use connection::sacp_connection::SacpConnection;
pub use registry::AcpAgentConfig;
Expand Down Expand Up @@ -86,8 +87,18 @@ pub use agent_client_protocol_schema::NewSessionRequest;
pub use agent_client_protocol_schema::NewSessionResponse;
pub use agent_client_protocol_schema::PromptRequest;
pub use agent_client_protocol_schema::PromptResponse;
pub use agent_client_protocol_schema::SessionConfigKind;
pub use agent_client_protocol_schema::SessionConfigOption;
pub use agent_client_protocol_schema::SessionConfigOptionCategory;
pub use agent_client_protocol_schema::SessionConfigOptionValue;
pub use agent_client_protocol_schema::SessionConfigSelectGroup;
pub use agent_client_protocol_schema::SessionConfigSelectOption;
pub use agent_client_protocol_schema::SessionConfigSelectOptions;
pub use agent_client_protocol_schema::SessionConfigValueId;
pub use agent_client_protocol_schema::SessionNotification;
pub use agent_client_protocol_schema::SessionUpdate;
pub use agent_client_protocol_schema::SetSessionConfigOptionRequest;
pub use agent_client_protocol_schema::SetSessionConfigOptionResponse;

// Re-export model-related types (unstable feature)
#[cfg(feature = "unstable")]
Expand Down
2 changes: 2 additions & 0 deletions nori-rs/mock-acp-agent/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Used by `@/nori-rs/tui-pty-e2e/` for end-to-end integration testing. The mock ag

**Prompt Echo**: The `MOCK_AGENT_ECHO_PROMPT` env var causes the mock agent's `prompt()` handler to echo back the full prompt text it received. Used by session context tests in `@/nori-rs/acp/src/backend/tests/part5.rs` to verify that `AcpBackendConfig.session_context` is correctly prepended to the first user prompt and consumed after that.

**Session Config Options**: The mock agent advertises live ACP session config options on `session/new` and `session/load`, and supports `session/set_config_option` for connection/TUI tests. The default config exposes `Model` plus `Thought Level`. Switching the model to `mock-model-fast` replaces `Thought Level` with a `Speed` selector, which lets tests verify that Nori replaces the full live config snapshot after a config mutation.

**Cancel Tail Ordering**: The `MOCK_AGENT_CANCEL_TAIL_EMPTY_END_TURNS` env var reproduces the Claude-style cancel tail that motivated the ACP cancellation-ordering fix. When a streaming prompt is cancelled, the mock agent queues N immediate empty `end_turn` responses for the next prompt attempts before finally allowing the real follow-up prompt to complete. `MOCK_AGENT_CANCEL_TAIL_FOLLOW_UP_RESPONSE` overrides the text returned by that eventual real follow-up turn. These knobs are used by `@/nori-rs/acp/src/connection/sacp_connection_tests.rs` and `@/nori-rs/tui-pty-e2e/tests/streaming.rs` to verify that Nori absorbs repeated stale terminal responses without admitting a new logical prompt turn too early.

**Stuck Tool Calls (No Completion)**: The `MOCK_AGENT_STUCK_TOOL_CALLS` env var triggers a scenario where 3 Read tool calls are sent with `Pending` status but never receive completion updates. After a short delay the agent sends its final text response and ends the turn. This reproduces the frozen-display bug where incomplete ExecCells fill the viewport and block `insert_history_lines()` from rendering the agent's text. The fix under test is `finalize_active_cell_as_failed()` in `@/nori-rs/tui/src/chatwidget.rs`.
Expand Down
Loading
Loading