perf: remove redundant update_provider ACP method

baxen · baxen · commit f993a3f593b8 · 2026-04-17T12:28:57.000-07:00
The custom _goose/session/provider/update method duplicated the SACP 11
set_config_option path for config_id="provider". Having both meant:

  1. Two separate wire entry points for the same operation.
  2. The set_config_option handler called build_config_update twice
     per request — once inside update_provider (which built and
     discarded the Vec&lt;SessionConfigOption&gt;) and once in the handler
     tail. On providers that refresh their model list over HTTP
     (e.g. Databricks) each call costs ~1s, for ~2s of wasted work
     per provider switch.

Nothing in this codebase calls the custom method; goose2 uses
set_config_option exclusively.

Changes:
  - Remove UpdateProviderRequest/UpdateProviderResponse from goose-sdk.
  - Remove on_update_provider from goose-acp server.
  - Change internal update_provider helper to return () and stop
    calling build_config_update at the end — the set_config_option
    handler's single trailing build_config_update is now the only one.
  - Rewrite test_provider_switching_updates_session_state to exercise
    the set_config_option path three times instead of the removed
    custom method.
  - Regenerate acp-schema.json, acp-meta.json, and ui/sdk/src/generated.

Verified:
  - cargo test -p goose-acp (35 passed)
  - cargo clippy --all-targets -- -D warnings
  - ui/goose2 pnpm typecheck
Signed-off-by: Bradley Axen &lt;baxen@squareup.com&gt;
diff --git a/crates/goose-acp/acp-meta.json b/crates/goose-acp/acp-meta.json
@@ -40,11 +40,6 @@
       "requestType": "GetSessionExtensionsRequest",
       "responseType": "GetSessionExtensionsResponse"
     },
-    {
-      "method": "_goose/session/provider/update",
-      "requestType": "UpdateProviderRequest",
-      "responseType": "UpdateProviderResponse"
-    },
     {
       "method": "_goose/providers/list",
       "requestType": "ListProvidersRequest",
diff --git a/crates/goose-acp/acp-schema.json b/crates/goose-acp/acp-schema.json
@@ -195,61 +195,6 @@
       "x-side": "agent",
       "x-method": "_goose/session/extensions"
     },
-    "UpdateProviderRequest": {
-      "type": "object",
-      "properties": {
-        "sessionId": {
-          "type": "string"
-        },
-        "provider": {
-          "type": "string"
-        },
-        "model": {
-          "type": [
-            "string",
-            "null"
-          ]
-        },
-        "contextLimit": {
-          "type": [
-            "integer",
-            "null"
-          ],
-          "format": "uint",
-          "minimum": 0
-        },
-        "requestParams": {
-          "type": [
-            "object",
-            "null"
-          ],
-          "additionalProperties": {}
-        }
-      },
-      "required": [
-        "sessionId",
-        "provider"
-      ],
-      "description": "Atomically update the provider for a live session.",
-      "x-side": "agent",
-      "x-method": "_goose/session/provider/update"
-    },
-    "UpdateProviderResponse": {
-      "type": "object",
-      "properties": {
-        "configOptions": {
-          "type": "array",
-          "items": {},
-          "description": "Refreshed session config options after the provider/model change."
-        }
-      },
-      "required": [
-        "configOptions"
-      ],
-      "description": "Provider update response.",
-      "x-side": "agent",
-      "x-method": "_goose/session/provider/update"
-    },
     "ListProvidersRequest": {
       "type": "object",
       "description": "List providers available through goose, including the config-default sentinel.",
@@ -746,15 +691,6 @@
                   "description": "Params for _goose/session/extensions",
                   "title": "GetSessionExtensionsRequest"
                 },
-                {
-                  "allOf": [
-                    {
-                      "$ref": "#/$defs/UpdateProviderRequest"
-                    }
-                  ],
-                  "description": "Params for _goose/session/provider/update",
-                  "title": "UpdateProviderRequest"
-                },
                 {
                   "allOf": [
                     {
@@ -942,14 +878,6 @@
                       ],
                       "title": "GetSessionExtensionsResponse"
                     },
-                    {
-                      "allOf": [
-                        {
-                          "$ref": "#/$defs/UpdateProviderResponse"
-                        }
-                      ],
-                      "title": "UpdateProviderResponse"
-                    },
                     {
                       "allOf": [
                         {
diff --git a/crates/goose-acp/src/server.rs b/crates/goose-acp/src/server.rs
@@ -2084,7 +2084,7 @@ impl GooseAcpAgent {
         model_name: Option<&str>,
         context_limit: Option<usize>,
         request_params: Option<std::collections::HashMap<String, serde_json::Value>>,
-    ) -> Result<Vec<SessionConfigOption>, sacp::Error> {
+    ) -> Result<(), sacp::Error> {
         let sid = sid_short(thread_id);
         let t_total = std::time::Instant::now();
         debug!(target: "perf", sid = %sid, provider = %provider_name, "perf: update_provider start");
@@ -2236,12 +2236,6 @@ impl GooseAcpAgent {
             "perf: update_provider persist_session"
         );
 
-        let t_step = std::time::Instant::now();
-        let (_, config_options) = self
-            .build_config_update(&SessionId::new(thread_id.to_string()))
-            .await?;
-        debug!(target: "perf", sid = %sid, ms = t_step.elapsed().as_millis() as u64, "perf: update_provider build_config_update");
-
         debug!(
             target: "perf",
             sid = %sid,
@@ -2251,7 +2245,7 @@ impl GooseAcpAgent {
             changing = is_changing_provider,
             "perf: update_provider done"
         );
-        Ok(config_options)
+        Ok(())
     }
 
     async fn on_list_sessions(&self) -> Result<ListSessionsResponse, sacp::Error> {
@@ -2530,28 +2524,6 @@ impl GooseAcpAgent {
         })
     }
 
-    #[custom_method(UpdateProviderRequest)]
-    async fn on_update_provider(
-        &self,
-        req: UpdateProviderRequest,
-    ) -> Result<UpdateProviderResponse, sacp::Error> {
-        let config_options = self
-            .update_provider(
-                &req.session_id,
-                &req.provider,
-                req.model.as_deref(),
-                req.context_limit,
-                req.request_params,
-            )
-            .await?;
-        let config_options = config_options
-            .into_iter()
-            .map(|option| serde_json::to_value(&option))
-            .collect::<Result<Vec<_>, _>>()
-            .map_err(|e| sacp::Error::internal_error().data(e.to_string()))?;
-        Ok(UpdateProviderResponse { config_options })
-    }
-
     #[custom_method(ListProvidersRequest)]
     async fn on_list_providers(
         &self,
diff --git a/crates/goose-acp/tests/custom_requests_test.rs b/crates/goose-acp/tests/custom_requests_test.rs
@@ -208,48 +208,15 @@ fn test_provider_switching_updates_session_state() {
 
         conn.set_config_option(&session_id, "provider", "anthropic")
             .await
-            .expect("provider config option should succeed");
+            .expect("provider switch to anthropic should succeed");
 
-        let response = send_custom(
-            conn.cx(),
-            "_goose/session/provider/update",
-            serde_json::json!({
-                "sessionId": session_id,
-                "provider": "openai",
-                "model": "o4-mini",
-            }),
-        )
-        .await
-        .expect("provider update should succeed");
-        let config_options = response
-            .get("configOptions")
-            .and_then(|value| value.as_array())
-            .expect("missing config options");
-        assert!(
-            !config_options.is_empty(),
-            "expected refreshed config options"
-        );
+        conn.set_config_option(&session_id, "provider", "openai")
+            .await
+            .expect("provider switch to openai should succeed");
 
-        let response = send_custom(
-            conn.cx(),
-            "_goose/session/provider/update",
-            serde_json::json!({
-                "sessionId": session_id,
-                "provider": "goose",
-            }),
-        )
-        .await
-        .expect("provider reset to goose should succeed");
-        let config_options = response
-            .get("configOptions")
-            .and_then(|value| value.as_array())
-            .expect("missing config options after reset");
-        assert!(
-            config_options
-                .iter()
-                .any(|option| option.get("id") == Some(&serde_json::json!("provider"))),
-            "missing provider config option after reset"
-        );
+        conn.set_config_option(&session_id, "provider", "goose")
+            .await
+            .expect("provider reset to goose should succeed");
     });
 }
 
diff --git a/crates/goose-sdk/src/custom_requests.rs b/crates/goose-sdk/src/custom_requests.rs
@@ -1,7 +1,6 @@
 use sacp::{JsonRpcRequest, JsonRpcResponse};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;
 
 /// Schema descriptor for a single custom method, produced by the
 /// `#[custom_methods]` macro's generated `custom_method_schemas()` function.
@@ -116,26 +115,6 @@ pub struct GetSessionExtensionsResponse {
     pub extensions: Vec<serde_json::Value>,
 }
 
-/// Atomically update the provider for a live session.
-#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)]
-#[request(method = "_goose/session/provider/update", response = UpdateProviderResponse)]
-#[serde(rename_all = "camelCase")]
-pub struct UpdateProviderRequest {
-    pub session_id: String,
-    pub provider: String,
-    pub model: Option<String>,
-    pub context_limit: Option<usize>,
-    pub request_params: Option<HashMap<String, serde_json::Value>>,
-}
-
-/// Provider update response.
-#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)]
-#[serde(rename_all = "camelCase")]
-pub struct UpdateProviderResponse {
-    /// Refreshed session config options after the provider/model change.
-    pub config_options: Vec<serde_json::Value>,
-}
-
 /// Read a single non-secret config value.
 #[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)]
 #[request(method = "_goose/config/read", response = ReadConfigResponse)]
diff --git a/ui/sdk/src/generated/client.gen.ts b/ui/sdk/src/generated/client.gen.ts
@@ -37,8 +37,6 @@ import type {
   RemoveExtensionRequest,
   RemoveSecretRequest,
   UnarchiveSessionRequest,
-  UpdateProviderRequest,
-  UpdateProviderResponse,
   UpdateWorkingDirRequest,
   UpsertConfigRequest,
   UpsertSecretRequest,
@@ -55,7 +53,6 @@ import {
   zListProvidersResponse,
   zReadConfigResponse,
   zReadResourceResponse,
-  zUpdateProviderResponse,
 } from './zod.gen.js';
 
 export class GooseExtClient {
@@ -105,16 +102,6 @@ export class GooseExtClient {
     ) as GetSessionExtensionsResponse;
   }
 
-  async GooseSessionProviderUpdate(
-    params: UpdateProviderRequest,
-  ): Promise<UpdateProviderResponse> {
-    const raw = await this.conn.extMethod(
-      "_goose/session/provider/update",
-      params,
-    );
-    return zUpdateProviderResponse.parse(raw) as UpdateProviderResponse;
-  }
-
   async GooseProvidersList(
     params: ListProvidersRequest,
   ): Promise<ListProvidersResponse> {
diff --git a/ui/sdk/src/generated/index.ts b/ui/sdk/src/generated/index.ts
@@ -1,6 +1,6 @@
 // This file is auto-generated by @hey-api/openapi-ts
 
-export type { AddExtensionRequest, ArchiveSessionRequest, CheckSecretRequest, CheckSecretResponse, DeleteSessionRequest, EmptyResponse, ExportSessionRequest, ExportSessionResponse, ExtRequest, ExtResponse, GetExtensionsRequest, GetExtensionsResponse, GetProviderDetailsRequest, GetProviderDetailsResponse, GetProviderModelsRequest, GetProviderModelsResponse, GetSessionExtensionsRequest, GetSessionExtensionsResponse, GetToolsRequest, GetToolsResponse, ImportSessionRequest, ImportSessionResponse, ListProvidersRequest, ListProvidersResponse, ModelEntry, ProviderConfigKey, ProviderDetailEntry, ProviderListEntry, ReadConfigRequest, ReadConfigResponse, ReadResourceRequest, ReadResourceResponse, RemoveConfigRequest, RemoveExtensionRequest, RemoveSecretRequest, UnarchiveSessionRequest, UpdateProviderRequest, UpdateProviderResponse, UpdateWorkingDirRequest, UpsertConfigRequest, UpsertSecretRequest } from './types.gen.js';
+export type { AddExtensionRequest, ArchiveSessionRequest, CheckSecretRequest, CheckSecretResponse, DeleteSessionRequest, EmptyResponse, ExportSessionRequest, ExportSessionResponse, ExtRequest, ExtResponse, GetExtensionsRequest, GetExtensionsResponse, GetProviderDetailsRequest, GetProviderDetailsResponse, GetProviderModelsRequest, GetProviderModelsResponse, GetSessionExtensionsRequest, GetSessionExtensionsResponse, GetToolsRequest, GetToolsResponse, ImportSessionRequest, ImportSessionResponse, ListProvidersRequest, ListProvidersResponse, ModelEntry, ProviderConfigKey, ProviderDetailEntry, ProviderListEntry, ReadConfigRequest, ReadConfigResponse, ReadResourceRequest, ReadResourceResponse, RemoveConfigRequest, RemoveExtensionRequest, RemoveSecretRequest, UnarchiveSessionRequest, UpdateWorkingDirRequest, UpsertConfigRequest, UpsertSecretRequest } from './types.gen.js';
 
 export const GOOSE_EXT_METHODS = [
   {
@@ -43,11 +43,6 @@ export const GOOSE_EXT_METHODS = [
     requestType: "GetSessionExtensionsRequest",
     responseType: "GetSessionExtensionsResponse",
   },
-  {
-    method: "_goose/session/provider/update",
-    requestType: "UpdateProviderRequest",
-    responseType: "UpdateProviderResponse",
-  },
   {
     method: "_goose/providers/list",
     requestType: "ListProvidersRequest",
diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts
@@ -104,29 +104,6 @@ export type GetSessionExtensionsResponse = {
     extensions: Array<unknown>;
 };
 
-/**
- * Atomically update the provider for a live session.
- */
-export type UpdateProviderRequest = {
-    sessionId: string;
-    provider: string;
-    model?: string | null;
-    contextLimit?: number | null;
-    requestParams?: {
-        [key: string]: unknown;
-    } | null;
-};
-
-/**
- * Provider update response.
- */
-export type UpdateProviderResponse = {
-    /**
-     * Refreshed session config options after the provider/model change.
-     */
-    configOptions: Array<unknown>;
-};
-
 /**
  * List providers available through goose, including the config-default sentinel.
  */
@@ -307,14 +284,14 @@ export type UnarchiveSessionRequest = {
 export type ExtRequest = {
     id: string;
     method: string;
-    params?: AddExtensionRequest | RemoveExtensionRequest | GetToolsRequest | ReadResourceRequest | UpdateWorkingDirRequest | DeleteSessionRequest | GetExtensionsRequest | GetSessionExtensionsRequest | UpdateProviderRequest | ListProvidersRequest | GetProviderDetailsRequest | GetProviderModelsRequest | ReadConfigRequest | UpsertConfigRequest | RemoveConfigRequest | CheckSecretRequest | UpsertSecretRequest | RemoveSecretRequest | ExportSessionRequest | ImportSessionRequest | ArchiveSessionRequest | UnarchiveSessionRequest | {
+    params?: AddExtensionRequest | RemoveExtensionRequest | GetToolsRequest | ReadResourceRequest | UpdateWorkingDirRequest | DeleteSessionRequest | GetExtensionsRequest | GetSessionExtensionsRequest | ListProvidersRequest | GetProviderDetailsRequest | GetProviderModelsRequest | ReadConfigRequest | UpsertConfigRequest | RemoveConfigRequest | CheckSecretRequest | UpsertSecretRequest | RemoveSecretRequest | ExportSessionRequest | ImportSessionRequest | ArchiveSessionRequest | UnarchiveSessionRequest | {
         [key: string]: unknown;
     } | null;
 };
 
 export type ExtResponse = {
     id: string;
-    result?: EmptyResponse | GetToolsResponse | ReadResourceResponse | GetExtensionsResponse | GetSessionExtensionsResponse | UpdateProviderResponse | ListProvidersResponse | GetProviderDetailsResponse | GetProviderModelsResponse | ReadConfigResponse | CheckSecretResponse | ExportSessionResponse | ImportSessionResponse | unknown;
+    result?: EmptyResponse | GetToolsResponse | ReadResourceResponse | GetExtensionsResponse | GetSessionExtensionsResponse | ListProvidersResponse | GetProviderDetailsResponse | GetProviderModelsResponse | ReadConfigResponse | CheckSecretResponse | ExportSessionResponse | ImportSessionResponse | unknown;
 } | {
     error: {
         code: number;
diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts
