agent: Allow all settings in the thread to apply immediately (#51762)

Ben Brandt created

Before we kind of had it mixed: you could change thinking but not model
etc. We brute forced a solution by disabling, but it seems better to
just allow all of them to be updated whenever we would build the
request.

Release Notes:

- N/A

Change summary

crates/agent/src/tests/mod.rs                        | 107 ++++++++++++++
crates/agent/src/thread.rs                           |  56 +++---
crates/agent_ui/src/conversation_view/thread_view.rs |  21 --
crates/agent_ui/src/model_selector_popover.rs        |  48 +-----
crates/agent_ui/src/profile_selector.rs              |  63 ++-----
5 files changed, 169 insertions(+), 126 deletions(-)

Detailed changes

crates/agent/src/tests/mod.rs 🔗

@@ -6478,3 +6478,110 @@ async fn test_streaming_tool_error_waits_for_prior_tools_to_complete(cx: &mut Te
         ]
     );
 }
+
+#[gpui::test]
+async fn test_mid_turn_model_and_settings_refresh(cx: &mut TestAppContext) {
+    let ThreadTest {
+        model, thread, fs, ..
+    } = setup(cx, TestModel::Fake).await;
+    let fake_model_a = model.as_fake();
+
+    thread.update(cx, |thread, _cx| {
+        thread.add_tool(EchoTool);
+        thread.add_tool(DelayTool);
+    });
+
+    // Set up two profiles: profile-a has both tools, profile-b has only DelayTool.
+    fs.insert_file(
+        paths::settings_file(),
+        json!({
+            "agent": {
+                "profiles": {
+                    "profile-a": {
+                        "name": "Profile A",
+                        "tools": {
+                            EchoTool::NAME: true,
+                            DelayTool::NAME: true,
+                        }
+                    },
+                    "profile-b": {
+                        "name": "Profile B",
+                        "tools": {
+                            DelayTool::NAME: true,
+                        }
+                    }
+                }
+            }
+        })
+        .to_string()
+        .into_bytes(),
+    )
+    .await;
+    cx.run_until_parked();
+
+    thread.update(cx, |thread, cx| {
+        thread.set_profile(AgentProfileId("profile-a".into()), cx);
+        thread.set_thinking_enabled(false, cx);
+    });
+
+    // Send a message — first iteration starts with model A, profile-a, thinking off.
+    thread
+        .update(cx, |thread, cx| {
+            thread.send(UserMessageId::new(), ["test mid-turn refresh"], cx)
+        })
+        .unwrap();
+    cx.run_until_parked();
+
+    // Verify first request has both tools and thinking disabled.
+    let completions = fake_model_a.pending_completions();
+    assert_eq!(completions.len(), 1);
+    let first_tools = tool_names_for_completion(&completions[0]);
+    assert_eq!(first_tools, vec![DelayTool::NAME, EchoTool::NAME]);
+    assert!(!completions[0].thinking_allowed);
+
+    // Model A responds with an echo tool call.
+    fake_model_a.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
+        LanguageModelToolUse {
+            id: "tool_1".into(),
+            name: "echo".into(),
+            raw_input: r#"{"text":"hello"}"#.into(),
+            input: json!({"text": "hello"}),
+            is_input_complete: true,
+            thought_signature: None,
+        },
+    ));
+    fake_model_a.end_last_completion_stream();
+
+    // Before the next iteration runs, switch to profile-b (only DelayTool),
+    // swap in a new model, and enable thinking.
+    let fake_model_b = Arc::new(FakeLanguageModel::with_id_and_thinking(
+        "test-provider",
+        "model-b",
+        "Model B",
+        true,
+    ));
+    thread.update(cx, |thread, cx| {
+        thread.set_profile(AgentProfileId("profile-b".into()), cx);
+        thread.set_model(fake_model_b.clone() as Arc<dyn LanguageModel>, cx);
+        thread.set_thinking_enabled(true, cx);
+    });
+
+    // Run until parked — processes the echo tool call, loops back, picks up
+    // the new model/profile/thinking, and makes a second request to model B.
+    cx.run_until_parked();
+
+    // The second request should have gone to model B.
+    let model_b_completions = fake_model_b.pending_completions();
+    assert_eq!(
+        model_b_completions.len(),
+        1,
+        "second request should go to model B"
+    );
+
+    // Profile-b only has DelayTool, so echo should be gone.
+    let second_tools = tool_names_for_completion(&model_b_completions[0]);
+    assert_eq!(second_tools, vec![DelayTool::NAME]);
+
+    // Thinking should now be enabled.
+    assert!(model_b_completions[0].thinking_allowed);
+}

crates/agent/src/thread.rs 🔗

@@ -12,8 +12,7 @@ use feature_flags::{FeatureFlagAppExt as _, StreamingEditFileToolFeatureFlag};
 
 use agent_client_protocol as acp;
 use agent_settings::{
-    AgentProfileId, AgentProfileSettings, AgentSettings, SUMMARIZE_THREAD_DETAILED_PROMPT,
-    SUMMARIZE_THREAD_PROMPT,
+    AgentProfileId, AgentSettings, SUMMARIZE_THREAD_DETAILED_PROMPT, SUMMARIZE_THREAD_PROMPT,
 };
 use anyhow::{Context as _, Result, anyhow};
 use chrono::{DateTime, Utc};
@@ -1771,11 +1770,6 @@ impl Thread {
         self.flush_pending_message(cx);
         self.cancel(cx).detach();
 
-        let model = self.model.clone().context("No language model configured")?;
-        let profile = AgentSettings::get_global(cx)
-            .profiles
-            .get(&self.profile_id)
-            .context("Profile not found")?;
         let (events_tx, events_rx) = mpsc::unbounded::<Result<ThreadEvent>>();
         let event_stream = ThreadEventStream(events_tx);
         let message_ix = self.messages.len().saturating_sub(1);
@@ -1783,20 +1777,15 @@ impl Thread {
         let (cancellation_tx, mut cancellation_rx) = watch::channel(false);
         self.running_turn = Some(RunningTurn {
             event_stream: event_stream.clone(),
-            tools: self.enabled_tools(profile, &model, cx),
+            tools: self.enabled_tools(cx),
             cancellation_tx,
             streaming_tool_inputs: HashMap::default(),
             _task: cx.spawn(async move |this, cx| {
                 log::debug!("Starting agent turn execution");
 
-                let turn_result = Self::run_turn_internal(
-                    &this,
-                    model,
-                    &event_stream,
-                    cancellation_rx.clone(),
-                    cx,
-                )
-                .await;
+                let turn_result =
+                    Self::run_turn_internal(&this, &event_stream, cancellation_rx.clone(), cx)
+                        .await;
 
                 // Check if we were cancelled - if so, cancel() already took running_turn
                 // and we shouldn't touch it (it might be a NEW turn now)
@@ -1838,7 +1827,6 @@ impl Thread {
 
     async fn run_turn_internal(
         this: &WeakEntity<Self>,
-        model: Arc<dyn LanguageModel>,
         event_stream: &ThreadEventStream,
         mut cancellation_rx: watch::Receiver<bool>,
         cx: &mut AsyncApp,
@@ -1846,8 +1834,15 @@ impl Thread {
         let mut attempt = 0;
         let mut intent = CompletionIntent::UserPrompt;
         loop {
-            let request =
-                this.update(cx, |this, cx| this.build_completion_request(intent, cx))??;
+            // Re-read the model and refresh tools on each iteration so that
+            // mid-turn changes (e.g. the user switches model, toggles tools,
+            // or changes profile) take effect between tool-call rounds.
+            let (model, request) = this.update(cx, |this, cx| {
+                let model = this.model.clone().context("No language model configured")?;
+                this.refresh_turn_tools(cx);
+                let request = this.build_completion_request(intent, cx)?;
+                anyhow::Ok((model, request))
+            })??;
 
             telemetry::event!(
                 "Agent Thread Completion",
@@ -2700,12 +2695,13 @@ impl Thread {
         Ok(request)
     }
 
-    fn enabled_tools(
-        &self,
-        profile: &AgentProfileSettings,
-        model: &Arc<dyn LanguageModel>,
-        cx: &App,
-    ) -> BTreeMap<SharedString, Arc<dyn AnyAgentTool>> {
+    fn enabled_tools(&self, cx: &App) -> BTreeMap<SharedString, Arc<dyn AnyAgentTool>> {
+        let Some(model) = self.model.as_ref() else {
+            return BTreeMap::new();
+        };
+        let Some(profile) = AgentSettings::get_global(cx).profiles.get(&self.profile_id) else {
+            return BTreeMap::new();
+        };
         fn truncate(tool_name: &SharedString) -> SharedString {
             if tool_name.len() > MAX_TOOL_NAME_LENGTH {
                 let mut truncated = tool_name.to_string();
@@ -2786,6 +2782,13 @@ impl Thread {
         tools
     }
 
+    fn refresh_turn_tools(&mut self, cx: &App) {
+        let tools = self.enabled_tools(cx);
+        if let Some(turn) = self.running_turn.as_mut() {
+            turn.tools = tools;
+        }
+    }
+
     fn tool(&self, name: &str) -> Option<Arc<dyn AnyAgentTool>> {
         self.running_turn.as_ref()?.tools.get(name).cloned()
     }
@@ -3029,7 +3032,8 @@ struct RunningTurn {
     /// The current event stream for the running turn. Used to report a final
     /// cancellation event if we cancel the turn.
     event_stream: ThreadEventStream,
-    /// The tools that were enabled for this turn.
+    /// The tools that are enabled for the current iteration of the turn.
+    /// Refreshed at the start of each iteration via `refresh_turn_tools`.
     tools: BTreeMap<SharedString, Arc<dyn AnyAgentTool>>,
     /// Sender to signal tool cancellation. When cancel is called, this is
     /// set to true so all tools can detect user-initiated cancellation.

crates/agent_ui/src/conversation_view/thread_view.rs 🔗

@@ -2707,14 +2707,6 @@ impl ThreadView {
             return div().into_any_element();
         }
 
-        let is_generating = self.thread.read(cx).status() != ThreadStatus::Idle;
-        if let Some(model_selector) = &self.model_selector {
-            model_selector.update(cx, |selector, _| selector.set_disabled(is_generating));
-        }
-        if let Some(profile_selector) = &self.profile_selector {
-            profile_selector.update(cx, |selector, _| selector.set_disabled(is_generating));
-        }
-
         let focus_handle = self.message_editor.focus_handle(cx);
         let editor_bg_color = cx.theme().colors().editor_background;
         let editor_expanded = self.editor_expanded;
@@ -3264,7 +3256,6 @@ impl ThreadView {
             return None;
         }
 
-        let is_generating = self.thread.read(cx).status() != ThreadStatus::Idle;
         let thinking = thread.thinking_enabled();
 
         let (tooltip_label, icon, color) = if thinking {
@@ -3286,13 +3277,8 @@ impl ThreadView {
         let thinking_toggle = IconButton::new("thinking-mode", icon)
             .icon_size(IconSize::Small)
             .icon_color(color)
-            .disabled(is_generating)
-            .tooltip(move |window, cx| {
-                if is_generating {
-                    Tooltip::text("Disabled until generation is done")(window, cx)
-                } else {
-                    Tooltip::for_action_in(tooltip_label, &ToggleThinkingMode, &focus_handle, cx)
-                }
+            .tooltip(move |_, cx| {
+                Tooltip::for_action_in(tooltip_label, &ToggleThinkingMode, &focus_handle, cx)
             })
             .on_click(cx.listener(move |this, _, _window, cx| {
                 if let Some(thread) = this.as_native_thread(cx) {
@@ -3324,7 +3310,6 @@ impl ThreadView {
         let right_btn = self.render_effort_selector(
             model.supported_effort_levels(),
             thread.thinking_effort().cloned(),
-            is_generating,
             cx,
         );
 
@@ -3339,7 +3324,6 @@ impl ThreadView {
         &self,
         supported_effort_levels: Vec<LanguageModelEffortLevel>,
         selected_effort: Option<String>,
-        disabled: bool,
         cx: &Context<Self>,
     ) -> impl IntoElement {
         let weak_self = cx.weak_entity();
@@ -3408,7 +3392,6 @@ impl ThreadView {
         PopoverMenu::new("effort-selector")
             .trigger_with_tooltip(
                 ButtonLike::new_rounded_right("effort-selector-trigger")
-                    .disabled(disabled)
                     .selected_style(ButtonStyle::Tinted(TintColor::Accent))
                     .child(Label::new(label).size(LabelSize::Small).color(label_color))
                     .child(Icon::new(icon).size(IconSize::XSmall).color(Color::Muted)),

crates/agent_ui/src/model_selector_popover.rs 🔗

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use acp_thread::{AgentModelIcon, AgentModelInfo, AgentModelSelector};
 use fs::Fs;
-use gpui::{AnyView, Entity, FocusHandle};
+use gpui::{Entity, FocusHandle};
 use picker::popover_menu::PickerPopoverMenu;
 use ui::{PopoverMenuHandle, Tooltip, prelude::*};
 
@@ -13,7 +13,6 @@ use crate::{ModelSelector, model_selector::acp_model_selector};
 pub struct ModelSelectorPopover {
     selector: Entity<ModelSelector>,
     menu_handle: PopoverMenuHandle<ModelSelector>,
-    disabled: bool,
 }
 
 impl ModelSelectorPopover {
@@ -31,18 +30,10 @@ impl ModelSelectorPopover {
                 acp_model_selector(selector, agent_server, fs, focus_handle.clone(), window, cx)
             }),
             menu_handle,
-            disabled: false,
         }
     }
 
-    pub fn set_disabled(&mut self, disabled: bool) {
-        self.disabled = disabled;
-    }
-
     pub fn toggle(&self, window: &mut Window, cx: &mut Context<Self>) {
-        if self.disabled {
-            return;
-        }
         self.menu_handle.toggle(window, cx);
     }
 
@@ -51,9 +42,6 @@ impl ModelSelectorPopover {
     }
 
     pub fn cycle_favorite_models(&self, window: &mut Window, cx: &mut Context<Self>) {
-        if self.disabled {
-            return;
-        }
         self.selector.update(cx, |selector, cx| {
             selector.delegate.cycle_favorite_models(window, cx);
         });
@@ -73,33 +61,25 @@ impl Render for ModelSelectorPopover {
 
         let (color, icon) = if self.menu_handle.is_deployed() {
             (Color::Accent, IconName::ChevronUp)
-        } else if self.disabled {
-            (Color::Disabled, IconName::ChevronDown)
         } else {
             (Color::Muted, IconName::ChevronDown)
         };
 
         let show_cycle_row = selector.delegate.favorites_count() > 1;
-        let disabled = self.disabled;
 
-        let tooltip: Box<dyn Fn(&mut Window, &mut App) -> AnyView> = if disabled {
-            Box::new(Tooltip::text("Disabled until generation is done"))
-        } else {
-            Box::new(Tooltip::element({
-                move |_, _cx| {
-                    ModelSelectorTooltip::new()
-                        .show_cycle_row(show_cycle_row)
-                        .into_any_element()
-                }
-            }))
-        };
+        let tooltip = Tooltip::element({
+            move |_, _cx| {
+                ModelSelectorTooltip::new()
+                    .show_cycle_row(show_cycle_row)
+                    .into_any_element()
+            }
+        });
 
         PickerPopoverMenu::new(
             self.selector.clone(),
             Button::new("active-model", model_name)
                 .label_size(LabelSize::Small)
                 .color(color)
-                .disabled(self.disabled)
                 .when_some(model_icon, |this, icon| {
                     this.start_icon(
                         match icon {
@@ -110,17 +90,7 @@ impl Render for ModelSelectorPopover {
                         .size(IconSize::XSmall),
                     )
                 })
-                .end_icon(
-                    Icon::new(icon)
-                        .map(|this| {
-                            if self.disabled {
-                                this.color(Color::Disabled)
-                            } else {
-                                this.color(Color::Muted)
-                            }
-                        })
-                        .size(IconSize::XSmall),
-                ),
+                .end_icon(Icon::new(icon).color(Color::Muted).size(IconSize::XSmall)),
             tooltip,
             gpui::Corner::BottomRight,
             cx,

crates/agent_ui/src/profile_selector.rs 🔗

@@ -34,7 +34,6 @@ pub trait ProfileProvider {
 pub struct ProfileSelector {
     profiles: AvailableProfiles,
     pending_refresh: bool,
-    disabled: bool,
     fs: Arc<dyn Fs>,
     provider: Arc<dyn ProfileProvider>,
     picker: Option<Entity<Picker<ProfilePickerDelegate>>>,
@@ -58,7 +57,6 @@ impl ProfileSelector {
         Self {
             profiles: AgentProfile::available_profiles(cx),
             pending_refresh: false,
-            disabled: false,
             fs,
             provider,
             picker: None,
@@ -72,19 +70,7 @@ impl ProfileSelector {
         self.picker_handle.clone()
     }
 
-    pub fn set_disabled(&mut self, disabled: bool) {
-        self.disabled = disabled;
-    }
-
-    pub fn is_disabled(&self) -> bool {
-        self.disabled
-    }
-
     pub fn cycle_profile(&mut self, cx: &mut Context<Self>) {
-        if self.disabled {
-            return;
-        }
-
         if !self.provider.profiles_supported(cx) {
             return;
         }
@@ -189,38 +175,31 @@ impl Render for ProfileSelector {
         };
 
         let trigger_button = Button::new("profile-selector", selected_profile)
-            .disabled(self.disabled)
             .label_size(LabelSize::Small)
             .color(Color::Muted)
             .end_icon(Icon::new(icon).size(IconSize::XSmall).color(Color::Muted));
 
-        let disabled = self.disabled;
-
-        let tooltip: Box<dyn Fn(&mut Window, &mut App) -> AnyView> = if disabled {
-            Box::new(Tooltip::text("Disabled until generation is done"))
-        } else {
-            Box::new(Tooltip::element({
-                move |_window, cx| {
-                    let container = || h_flex().gap_1().justify_between();
-                    v_flex()
-                        .gap_1()
-                        .child(
-                            container()
-                                .child(Label::new("Change Profile"))
-                                .child(KeyBinding::for_action(&ToggleProfileSelector, cx)),
-                        )
-                        .child(
-                            container()
-                                .pt_1()
-                                .border_t_1()
-                                .border_color(cx.theme().colors().border_variant)
-                                .child(Label::new("Cycle Through Profiles"))
-                                .child(KeyBinding::for_action(&CycleModeSelector, cx)),
-                        )
-                        .into_any()
-                }
-            }))
-        };
+        let tooltip: Box<dyn Fn(&mut Window, &mut App) -> AnyView> = Box::new(Tooltip::element({
+            move |_window, cx| {
+                let container = || h_flex().gap_1().justify_between();
+                v_flex()
+                    .gap_1()
+                    .child(
+                        container()
+                            .child(Label::new("Change Profile"))
+                            .child(KeyBinding::for_action(&ToggleProfileSelector, cx)),
+                    )
+                    .child(
+                        container()
+                            .pt_1()
+                            .border_t_1()
+                            .border_color(cx.theme().colors().border_variant)
+                            .child(Label::new("Cycle Through Profiles"))
+                            .child(KeyBinding::for_action(&CycleModeSelector, cx)),
+                    )
+                    .into_any()
+            }
+        }));
 
         PickerPopoverMenu::new(
             picker,