Switch agent and model settings without rebuilding panes - #655
Conversation
Keep existing agent panes and helpers alive when built-in agent settings change, retire the previous ACP session before reconnecting, and let non-destructive rebinds bypass the active-tab focus gate. Consolidate Settings agent probing into one background refresh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
Hot-switch supported native ACP models in the current session, rebind launch-time and BYOK models through the existing helper/master, and retire model-scoped agent generations after their final helper disconnects. Resolve custom provider settings and credentials only at the master process boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
Treat master-resolved custom provider bindings as requiring a clean native cloud catalog, and allow native ACP CLIs more time to return models from session/new. This keeps the Settings model picker aggregated instead of falling back to BYOK-only rows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
There was a problem hiding this comment.
Pull request overview
This pull request enables in-place agent, model, and provider reconfiguration while preserving existing panes and helper processes.
Changes:
- Adds ACP transport shutdown and reconnect flows.
- Moves provider resolution and credential handling into
wta-master. - Adds settings propagation, agent probing, generation management, and tests.
Review finding: A critical race in tools/wta/src/master/mod.rs can intermittently cause helper initialization to fail during settings rebinding. Removal and binding must be made atomic, or stale agent acquisition must be retried.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Reviewed change |
|---|---|
tools/wta/src/shell/wt_channel/cli_channel.rs |
Adds settings retrieval routing. |
tools/wta/src/session_registry.rs |
Carries provider binding metadata. |
tools/wta/src/protocol/acp/spawn.rs |
Applies provider-specific environments. |
tools/wta/src/protocol/acp/conn.rs |
Adds transport shutdown signaling. |
tools/wta/src/protocol/acp/client.rs |
Implements in-place ACP reconnection. |
tools/wta/src/master/tests.rs |
Tests provider lifecycle behavior. |
tools/wta/src/master/mod.rs |
Manages providers, generations, and agent bindings. |
tools/wta/src/helper/runtime.rs |
Handles helper runtime bindings. |
tools/wta/src/custom_model_provider.rs |
Resolves provider configuration and credentials. |
tools/wta/src/commands.rs |
Updates WTA command handling. |
tools/wta/src/app.rs |
Coordinates runtime reconfiguration. |
tools/wta/src/app_tests.rs |
Tests runtime reconfiguration. |
tools/wta/src/app_events.rs |
Handles settings and reconnect events. |
tools/wta/src/app_contracts/event.rs |
Defines reconnect event contracts. |
tools/wta/src/agent_registry.rs |
Defines agent capabilities. |
src/tools/wtcli/main.cpp |
Adds the get-settings command. |
src/cascadia/TerminalSettingsEditor/ProfileViewModel.h |
Updates profile settings declarations. |
src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp |
Updates profile settings behavior. |
src/cascadia/TerminalSettingsEditor/MainPage.h |
Updates settings-page declarations. |
src/cascadia/TerminalSettingsEditor/MainPage.cpp |
Propagates settings changes and probes agents. |
src/cascadia/TerminalApp/TerminalPage.h |
Declares rebinding state and helpers. |
src/cascadia/TerminalApp/TerminalPage.cpp |
Reconciles agent settings and pane lifecycle. |
src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp |
Tests settings-change classification and focus behavior. |
src/cascadia/inc/AgentRegistry.h |
Defines shared agent metadata. |
Suppressed comments (4)
src/cascadia/TerminalApp/TerminalPage.cpp:3613
- A selected BYOK provider change reaches this path as
AgentRebind, somasterConfigurationChangedremains false. The!HasAgentOverride()guard therefore leaves overridden tabs untouched, even though_AutoCreateHiddenAgentPaneSharedapplies the global custom selection to any effective Copilot/OpenCode tab. An overridden BYOK pane can consequently keep its old ACP process/endpoint while its picker is updated to the new provider. Include overridden tabs whose effective agent consumes BYOK in the affected set, while retaining the existing scoping for native model changes.
affected = profileBackendChanged ||
((globalAgentChanged || globalModelBindingChanged) &&
followsGlobalAgent);
src/cascadia/TerminalSettingsEditor/MainPage.cpp:1105
- This adds a background
ProbeHostAgentIds()call, butAIAgentsViewModelstill calls the same probe synchronously when its page is created (src/cascadia/TerminalSettingsEditor/AIAgentsViewModel.cpp:170-176). Opening Settings therefore performs two independent probes, including a blocking one, so the new consolidated background path does not remove the UI stall or duplicate process work. Reuse the page-level result (or a shared async cache) in the view model instead of probing again.
safe_void_coroutine MainPage::_ProbeHostAgentAvailabilityAsync()
{
const auto dispatcher = Dispatcher();
auto weakThis = get_weak();
co_await winrt::resume_background();
auto availableHostAgents = ::Microsoft::Terminal::AgentAvailability::ProbeHostAgentIds();
tools/wta/src/app_events.rs:17
custom_model_selectionis semantically optional, but an omitted JSON field still fails to deserialize intoOption<String>unless serde defaults it. The existingagent_rebind_event_for_windowfixture omits this field, so these reconnect events are rejected before dispatch (and older senders without the field are incompatible). Add a serde default for this field so omission becomesNone.
custom_model_selection: Option<String>,
tools/wta/src/master/mod.rs:2706
- This initialization-error path forwards
error.root_cause().to_string()to the helper. When a Credential Manager entry is missing or invalid, that text includes the full credential target identifier, so the helper and its agent pane receive a credential reference even though provider metadata is otherwise credential-free. Return a generic/redacted ACP error to the helper and keep the detailed identifier only in master-side logs.
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Suppressed comments (6)
src/cascadia/TerminalSettingsEditor/MainPage.cpp:164
- This starts a background WTA availability probe, but
AIAgentsViewModelstill callsProbeHostAgentIds()synchronously when the AI Agents page is opened. The two independent probes duplicate WTA startup work, and the view-model call can block the Settings UI for its 2-second timeout despite this background result. Share this result/cache with the view-model (and refresh it asynchronously) rather than maintaining both paths.
_ProbeHostAgentAvailabilityAsync();
src/cascadia/inc/AgentRegistry.h:92
SupportsLiveModelSwitchis case-sensitive here, but the WTA master canonicalizes requested IDs withto_ascii_lowercase()and treats"Gemini"as Gemini. If a user has a mixed-case built-inacpAgentin settings, a model-only change is classified as a live hot update here even though Gemini requires a rebind because its model is launch-time-only; the update then targets a session that cannot apply it. Use the same case-insensitive comparison as the Rust path (and add a mixed-case regression test).
return agentId != L"gemini";
tools/wta/src/master/mod.rs:4614
custom_model_generationsis populated for every credential-free selection ID, but the disconnect cleanup only removes the correspondingmodel:entry fromstate.agents; it never removes the generation record. A long-lived master can therefore retain endpoint/credential-reference configuration and an ever-growing entry for every provider/model ever selected, even after that generation's final helper disconnects. Prune the generation record as part of model-agent retirement, while preserving it only when another live agent still uses that selection/generation.
let mut generations = state.custom_model_generations.lock().await;
let generation = update_custom_model_generation(
&mut generations,
requested_binding,
config.clone(),
)?;
tools/wta/src/master/mod.rs:2725
- The pool entry is looked up/spawned here, but
spawn_one_agentstarts a reaper before returning. If the CLI exits after initialization completes and beforebind_helper_to_agentruns, the reaper removes thisOnceCell; the subsequent bind check fails and the helper's initialization is rejected instead of retrying the same selection. Add a stale-agent retry/fencing step around spawn and bind so a process exit in this window does not strand the helper on a transient initialization error.
&agent_cmd,
agent_id.as_deref(),
&agent_source,
provider_binding,
supplied_cloud_models,
tools/wta/src/master/mod.rs:5405
- This retirement check is not atomic with
get_or_spawn_agent's acquisition andbind_helper_to_agent's later identity check. A new helper can obtain thisArcwhile its old helper is disconnecting; this block then seesbound_helpersempty, removes and shuts down the instance, and the subsequent bind returnsfalse, making initialization fail instead of retrying the newly created pool entry. Serialize acquisition/binding with retirement, or retry binding after detecting that the instance was retired.
let removed = {
let mut agents = state.agents.lock().await;
let matches_instance = agents
.get(&agent.cmd_key)
.and_then(|cell| cell.get())
.is_some_and(|current| Arc::ptr_eq(current, agent));
if !matches_instance || !agent.bound_helpers.lock().await.is_empty() {
false
} else {
agents.remove(&agent.cmd_key).is_some()
tools/wta/src/master/mod.rs:2727
- Passing
provider_bindingintoget_or_spawn_agentadds a second credential-sensitive failure path: the provider's Credential Manager lookup happens duringspawn_one_agent, and the existing handler below returnse.root_cause()to the helper. A missing credential can therefore expose its resource/ID even though resolution succeeded. Sanitize the spawn error before it crosses this ACP boundary, while retaining the detailed chain in the master log.
let agent = get_or_spawn_agent(
&self.state,
&agent_cmd,
agent_id.as_deref(),
&agent_source,
provider_binding,
supplied_cloud_models,
)
.await
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
tools/wta/src/app.rs:1975
- A model update can be lost if it arrives while the ACP client is still starting. This updates the deferred value, but
send_acp_model_update()only targets sessions already recorded intab_sessions; meanwhile the running client task has already captured the oldacp_model, andAgentConnecteddoes not reapply the newer value. Saving Settings during prewarm/handshake can therefore leave the eventual session on the old model. Preserve a pending model generation and apply it when the session attaches (or otherwise reconcile the startup result against the latest value).
self.acp_model = new_model.filter(|s| !s.trim().is_empty());
if let Some(params) = self.deferred_acp.as_mut() {
params.acp_model.clone_from(&self.acp_model);
}
self.send_acp_model_update();
src/cascadia/TerminalSettingsEditor/MainPage.cpp:164
- This does not fully consolidate Settings detection into one background probe:
AIAgentsViewModel.cpp:175still callsProbeHostAgentIds()synchronously whenever the AI Agents page is opened fromMainPage.cpp:661. That can launch a second probe concurrently with this one and still block the UI thread for the probe's two-second timeout. Pass this cached availability intoAIAgentsViewModeland remove its constructor-side probe.
_ProbeHostAgentAvailabilityAsync();
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/cascadia/TerminalApp/TerminalPage.cpp:3493
- The
ModelHotUpdatefast path advances the settings snapshot and returns before examining existing panes. A never-startedNotConnectedstashed pane has no helper subscribed to the precedingagent_config_changedevent, so it later starts with the old--acp-modelbaked into its command line and this diff is no longer retried. Reconcile these panes here as well: keep the hot update for started helpers, but recreate affected never-started panes from the current settings.
if (changeKind == AgentSettingsChangeKind::ModelHotUpdate)
{
_lastAgentSettings = current;
_agentPaneLog("_RebuildAgentStack: native model changed, preserving live ACP sessions");
return;
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tools/wta/src/master/mod.rs:2751
- A second
initializecan now leave this helper registered on an agent it does not use.acquire_and_bind_agentinserts the helper into the newly resolved agent beforeOnceCell::set; when the cell is already populated with a different agent, the failedsetis ignored and cleanup later removes the helper only from the first binding. This keeps a model-scoped generation alive indefinitely and can send it helper notifications incorrectly. On a failedset, remove the helper from the extra agent (and retire it if it becomes unbound), while retaining the binding when bothArcs refer to the same agent.
// `set` is idempotent-by-error; a helper that (incorrectly) sent
// initialize twice keeps its first binding, which is fine.
let _ = self.agent.set(Arc::clone(&agent));
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
Keep Pane, TermControl, ConPTY, and helper processes stable for supported /agent switches and shared-master /restart operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fee72426-83c1-4693-a18f-37ae0fcb0cec
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/cascadia/TerminalApp/TerminalPage.cpp:6914
- Queued restart requests are discarded here, but request IDs are now completion barriers for auth recovery. If a second helper requests recovery while another restart is in progress, it waits for its own ID, ignores the first
agent_master_restarted, and thisClear()ensures its request is never acknowledged; it eventually falls back to the sign-in screen. Preserve every coalesced request ID and either execute it or acknowledge it after the successful replacement.
strongThis->_pendingAgentStackRestart.Clear();
Respawn a crashed shared master for retained helpers, preserve queued restart correlation, stop stale ACP client loops, and split auth recovery deadlines by lifecycle phase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 551b8f07-a43d-4610-9b72-17ca2f2f99cb
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/cascadia/TerminalApp/TerminalPage.cpp:4240
- For a profile/default-profile backend change,
masterConfigurationChangedis false, so every affected pane is torn down here but the later non-master branch reopens only the active tab. Background tabs therefore permanently lose their pre-warmed helper and autofix until manually opened. Recreate each background pane stashed/unfocused, while preserving the active pane's prior visibility.
for (const auto& tabId : tabIdsThatHadAgentPane)
{
if (const auto tab = strongThis->_FindTabByStableId(tabId);
tab && tab->FindAgentPane())
{
tools/wta/src/master/mod.rs:4518
- These strings cross the ACP boundary and are displayed to users, but they bypass WTA's localization system. Add locale resource keys and construct both details with
t!(...)so non-English panes do not receive hard-coded English errors.
Summary
wta-helper, and sharedwta-masterwhen built-in Agent or Model settings changeValidation
cargo test --target x86_64-pc-windows-msvc --manifest-path tools\wta\Cargo.toml— 1666 passed, 0 failedcargo build --target x86_64-pc-windows-msvc --manifest-path tools\wta\Cargo.tomlcmd.exe /c "tools\razzle.cmd && bcz no_clean"— 0 errors