diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md index 7aa91dfe3..8ea2580c6 100644 --- a/nori-rs/tui/docs.md +++ b/nori-rs/tui/docs.md @@ -413,6 +413,8 @@ The picker does not run during `/agent` switching, and unsupported ACP config ki `/model` acts as a convenience shortcut into the same config_options mechanism. `ChatWidget::open_model_popup()` in `chatwidget/pickers.rs` first fetches config_options via `AcpAgentHandle::get_session_config()`, finds a config option with `SessionConfigOptionCategory::Model`, and if present sends `AppEvent::OpenAcpSessionConfigValuePicker` to open the value picker directly (bypassing the top-level config picker). When no Model-category config option exists, it falls back to the unstable `SessionModelState` (behind `#[cfg(feature = "unstable")]`), and finally to a "not supported" empty model picker. This means real ACP agents that provide model selection through config_options (the stable mechanism) work correctly with `/model`, while agents that only provide the unstable `session/set_model` path still function via the fallback. Both paths persist the chosen model as the agent's default: the stable path through the Model-category persistence described above, and the unstable fallback in the `AppEvent::AcpModelSetResult` handler in `app/event_handling.rs`, whose info message reads `Model switched to: (saved as default)` when the write succeeds and omits the suffix when it fails. +**Pending-agent short-circuit:** ACP models are session-scoped -- an agent's models only arrive in the `session/new` response, so they are not knowable until a session starts. Because `/agent` only records a *pending* switch in `ChatWidget.pending_agent` (the live `acp_handle` and subprocess are not swapped until the next prompt submit rebuilds the `ChatWidget`; see "Agent-Provided Commands and Skill Mentions" and `set_pending_agent`), `open_model_popup()` checks `pending_agent` *before* touching the handle. When a switch is pending, it synchronously renders an explanatory picker built by `acp_model_picker_pending_agent_params(display_name)` in `nori/agent_picker.rs` telling the user to send a message to start the new session before `/model` can show that agent's models. This avoids querying the still-live OLD agent's handle, which would otherwise display the wrong agent's models. + **Selection Popup Row Layout (`bottom_pane/selection_popup_common.rs`):** `render_rows()` and `measure_rows_height()` are the shared rendering functions used by selection popups that render command-like rows (`ListSelectionView`, `CommandPopup`, `SkillPopup`, `FileSearchPopup`). Each popup item has an optional description that appears alongside the item name. The layout engine chooses between two modes per-row via `wrap_row()`: diff --git a/nori-rs/tui/src/chatwidget/pickers.rs b/nori-rs/tui/src/chatwidget/pickers.rs index 656b93556..c182c8cea 100644 --- a/nori-rs/tui/src/chatwidget/pickers.rs +++ b/nori-rs/tui/src/chatwidget/pickers.rs @@ -613,6 +613,16 @@ impl ChatWidget { /// unstable model state when no Model-category config option exists, /// and to a static "not supported" message when neither is available. pub(crate) fn open_model_popup(&mut self) { + // An agent switch is pending: the new session hasn't started, so the new + // agent's (session-scoped) models aren't available yet. Querying the live + // handle would show the OLD agent's models, so explain instead. + if let Some(pending) = self.pending_agent.as_ref() { + let params = crate::nori::agent_picker::acp_model_picker_pending_agent_params( + &pending.display_name, + ); + self.bottom_pane.show_selection_view(params); + return; + } if let Some(handle) = self.acp_handle.clone() { let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { diff --git a/nori-rs/tui/src/chatwidget/tests/part9.rs b/nori-rs/tui/src/chatwidget/tests/part9.rs index 12c3a1094..620198f02 100644 --- a/nori-rs/tui/src/chatwidget/tests/part9.rs +++ b/nori-rs/tui/src/chatwidget/tests/part9.rs @@ -127,3 +127,67 @@ fn model_popup_via_config_option_snapshot() { let popup = render_bottom_popup(&chat, 80); assert_snapshot!("model_popup_via_config_option", popup); } + +/// When an agent switch is pending (the user picked a new agent but hasn't +/// submitted a prompt yet, so no new session exists), /model must NOT query the +/// still-live OLD agent's handle — that would show stale models. Instead it +/// shows an explanatory message naming the pending agent. +#[tokio::test] +async fn model_popup_shows_pending_message_instead_of_stale_models() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // The OLD agent's handle would happily respond with a Model config option. + let (command_tx, mut command_rx) = + tokio::sync::mpsc::unbounded_channel::(); + tokio::spawn(async move { + while let Some(command) = command_rx.recv().await { + if let crate::chatwidget::agent::AcpAgentCommand::GetSessionConfig { response_tx } = + command + { + let _ = response_tx.send(vec![model_config_option()]); + } + } + }); + chat.acp_handle = Some(crate::chatwidget::agent::AcpAgentHandle::from_command_tx( + command_tx, + )); + + // But an agent switch is pending: no new session has started. + chat.set_pending_agent("newagent".to_string(), "New Agent".to_string()); + + chat.open_model_popup(); + + // Give any (incorrectly) spawned task time to query the OLD handle and + // route to its picker. The OLD agent's model picker must never open while a + // switch is pending — that is exactly the stale-models bug. + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + while let Ok(event) = rx.try_recv() { + assert!( + !matches!(event, AppEvent::OpenAcpSessionConfigValuePicker { .. }), + "must not route to the OLD agent's model picker while a switch is pending" + ); + } + + // The popup explains that a session must start first, naming the agent. + let popup = render_bottom_popup(&chat, 80); + assert!( + popup.contains("New Agent"), + "popup should name the pending agent:\n{popup}" + ); + assert!( + popup.contains("session"), + "popup should mention starting a session:\n{popup}" + ); +} + +/// Snapshot: the model picker shown while an agent switch is pending. +#[test] +fn model_popup_pending_agent_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + chat.set_pending_agent("newagent".to_string(), "New Agent".to_string()); + chat.open_model_popup(); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("model_popup_pending_agent", popup); +} diff --git a/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part9__model_popup_pending_agent.snap b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part9__model_popup_pending_agent.snap new file mode 100644 index 000000000..a4e80b9a9 --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part9__model_popup_pending_agent.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests/part9.rs +expression: popup +--- + Select Model + New Agent's models load once you start a session + +› 1. Switching to New Agent Send a message to start a session, then /model + will show its models. + + Press esc to dismiss. diff --git a/nori-rs/tui/src/nori/agent_picker.rs b/nori-rs/tui/src/nori/agent_picker.rs index 531237afa..24a02ba47 100644 --- a/nori-rs/tui/src/nori/agent_picker.rs +++ b/nori-rs/tui/src/nori/agent_picker.rs @@ -125,6 +125,36 @@ pub fn acp_model_picker_params() -> SelectionViewParams { } } +/// Create selection view parameters for the model picker when an agent switch +/// is pending but no new session has started yet. +/// +/// ACP models are session-scoped: a newly-switched agent's models only become +/// available after the next prompt creates a session. Until then the live agent +/// handle still belongs to the OLD agent, so querying it would show the OLD +/// agent's models. Show an explanatory message naming the pending agent instead. +pub fn acp_model_picker_pending_agent_params(display_name: &str) -> SelectionViewParams { + let items: Vec = vec![SelectionItem { + name: format!("Switching to {display_name}"), + description: Some( + "Send a message to start a session, then /model will show its models.".to_string(), + ), + is_current: false, + actions: vec![], + dismiss_on_select: true, + ..Default::default() + }]; + + SelectionViewParams { + title: Some("Select Model".to_string()), + subtitle: Some(format!( + "{display_name}'s models load once you start a session" + )), + footer_hint: Some(Line::from("Press esc to dismiss.")), + items, + ..Default::default() + } +} + /// Create selection view parameters for the ACP model picker with actual models. /// /// This function creates a picker showing models available from the ACP agent.