From bb0c63e73b07596890d60216ab47e411355d0ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 10 Mar 2026 22:13:28 +0000 Subject: [PATCH] fix: resolve model selector mismatch for ACP agents (#410) --- apps/backend/cmd/mock-agent/emitter.go | 2 +- apps/backend/cmd/mock-agent/main.go | 4 ++ apps/backend/go.mod | 2 +- apps/backend/go.sum | 4 +- .../server/adapter/transport/acp/adapter.go | 58 +++++++++++++++---- .../adapter/transport/acp/conversion_test.go | 2 +- .../adapter/transport/acp/meta_convert.go | 37 +++++++++++- .../orchestrator/event_handlers_streaming.go | 5 ++ apps/web/components/task/model-selector.tsx | 30 +++++++++- .../domains/session/use-session-model.ts | 13 ++++- apps/web/lib/ws/handlers/session-models.ts | 36 +++++++++++- 11 files changed, 170 insertions(+), 23 deletions(-) diff --git a/apps/backend/cmd/mock-agent/emitter.go b/apps/backend/cmd/mock-agent/emitter.go index f0464c0ddb..f65966d138 100644 --- a/apps/backend/cmd/mock-agent/emitter.go +++ b/apps/backend/cmd/mock-agent/emitter.go @@ -70,7 +70,7 @@ func (e *emitter) completeTool(id acp.ToolCallId, output any) { func (e *emitter) requestPermission(toolCallID acp.ToolCallId, title string, kind acp.ToolKind, input any) bool { resp, err := e.conn.RequestPermission(e.ctx, acp.RequestPermissionRequest{ SessionId: e.sid, - ToolCall: acp.RequestPermissionToolCall{ + ToolCall: acp.ToolCallUpdate{ ToolCallId: toolCallID, Title: acp.Ptr(title), Kind: acp.Ptr(kind), diff --git a/apps/backend/cmd/mock-agent/main.go b/apps/backend/cmd/mock-agent/main.go index 1d41bb1d75..f69d274864 100644 --- a/apps/backend/cmd/mock-agent/main.go +++ b/apps/backend/cmd/mock-agent/main.go @@ -116,6 +116,10 @@ func (a *mockAgent) SetSessionMode(_ context.Context, _ acp.SetSessionModeReques return acp.SetSessionModeResponse{}, nil } +func (a *mockAgent) SetSessionConfigOption(_ context.Context, _ acp.SetSessionConfigOptionRequest) (acp.SetSessionConfigOptionResponse, error) { + return acp.SetSessionConfigOptionResponse{}, nil +} + // extractPromptText concatenates text content blocks from the prompt. func extractPromptText(blocks []acp.ContentBlock) string { var parts []string diff --git a/apps/backend/go.mod b/apps/backend/go.mod index 455a8b1b13..3219938f2b 100644 --- a/apps/backend/go.mod +++ b/apps/backend/go.mod @@ -114,4 +114,4 @@ require ( // Fix for ACP notification ordering race condition // See: https://github.com/coder/acp-go-sdk/issues/20 -replace github.com/coder/acp-go-sdk => github.com/kdlbs/acp-go-sdk v0.6.3-fix.1 +replace github.com/coder/acp-go-sdk => github.com/agentcooper/acp-go-sdk v0.0.0-20260227120930-f21f3950f56d diff --git a/apps/backend/go.sum b/apps/backend/go.sum index f951826490..55a6135e2a 100644 --- a/apps/backend/go.sum +++ b/apps/backend/go.sum @@ -48,6 +48,8 @@ github.com/Microsoft/go-winio v0.4.21 h1:+6mVbXh4wPzUrl1COX9A+ZCvEpYsOBZ6/+kwDnv github.com/Microsoft/go-winio v0.4.21/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/UserExistsError/conpty v0.1.4 h1:+3FhJhiqhyEJa+K5qaK3/w6w+sN3Nh9O9VbJyBS02to= github.com/UserExistsError/conpty v0.1.4/go.mod h1:PDglKIkX3O/2xVk0MV9a6bCWxRmPVfxqZoTG/5sSd9I= +github.com/agentcooper/acp-go-sdk v0.0.0-20260227120930-f21f3950f56d h1:9VmeQddWE4m9UksAzVg0Me8V1KhXy8fOI1nLnWn94WM= +github.com/agentcooper/acp-go-sdk v0.0.0-20260227120930-f21f3950f56d/go.mod h1:yKzM/3R9uELp4+nBAwwtkS0aN1FOFjo11CNPy37yFko= github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk= github.com/bahlo/generic-list-go v0.2.0/go.mod h1:2KvAjgMlE5NNynlg/5iLrrCCZ2+5xWbdbCW3pNTGyYg= github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs= @@ -219,8 +221,6 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= -github.com/kdlbs/acp-go-sdk v0.6.3-fix.1 h1:zLSz0WHHtFvNTVa43LDvX8G3QTM6TnvCFU31HZMthKY= -github.com/kdlbs/acp-go-sdk v0.6.3-fix.1/go.mod h1:yKzM/3R9uELp4+nBAwwtkS0aN1FOFjo11CNPy37yFko= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.0 h1:Rnbp4K9EjcDuVuHtd0dgA4qNuv9yKDYKK1ulpJwgrqM= github.com/klauspost/compress v1.17.0/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go index 4cbd130f14..0443d0ed3f 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go @@ -102,7 +102,7 @@ type Adapter struct { // Available models from the most recent session creation/load. // Used by SetModel to validate the requested model exists. - availableModels []acp.ModelInfo + availableModels []acp.UnstableModelInfo // Synchronization mu sync.RWMutex @@ -266,7 +266,7 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer) // Emit session models if the agent returned model state if resp.Models != nil { - a.emitSessionModels(sessionID, resp.Models, resp.Meta) + a.emitSessionModels(sessionID, resp.Models, resp.Meta, resp.ConfigOptions) } // Emit session status event to normalize with other adapters. @@ -315,7 +315,7 @@ func toACPMcpServers(servers []types.McpServer) []acp.McpServer { switch server.Type { case "sse": out = append(out, acp.McpServer{ - Sse: &acp.McpServerSse{ + Sse: &acp.McpServerSseInline{ Name: server.Name, Url: server.URL, Type: "sse", @@ -394,7 +394,7 @@ func (a *Adapter) LoadSession(ctx context.Context, sessionID string) error { // Emit session models if the agent returned model state if resp.Models != nil { - a.emitSessionModels(sessionID, resp.Models, resp.Meta) + a.emitSessionModels(sessionID, resp.Models, resp.Meta, resp.ConfigOptions) } // Emit session status event to normalize with other adapters. @@ -838,8 +838,8 @@ func (a *Adapter) convertAvailableCommands(sessionID string, update *acp.Session Name: cmd.Name, Description: cmd.Description, } - if cmd.Input != nil && cmd.Input.UnstructuredCommandInput != nil { - ac.InputHint = cmd.Input.UnstructuredCommandInput.Hint + if cmd.Input != nil && cmd.Input.Unstructured != nil { + ac.InputHint = cmd.Input.Unstructured.Hint } commands[i] = ac } @@ -870,16 +870,50 @@ func (a *Adapter) emitInitialModeState(modes *acp.SessionModeState) { } // emitSessionModels emits a session_models event from the session response. -func (a *Adapter) emitSessionModels(sessionID string, models *acp.SessionModelState, meta any) { +func (a *Adapter) emitSessionModels(sessionID string, models *acp.UnstableSessionModelState, meta map[string]any, acpConfigOptions []acp.SessionConfigOption) { + currentModelID := string(models.CurrentModelId) + // Prefer typed config options from the response; fall back to _meta extraction for older agents + configOptions := convertACPConfigOptions(acpConfigOptions) + if len(configOptions) == 0 { + configOptions = extractConfigOptions(meta) + } + + // Fallback: if the SDK didn't parse currentModelId (some agents omit it), + // try to resolve it from configOptions or the first available model. + if currentModelID == "" { + currentModelID = resolveCurrentModelFromConfig(configOptions) + } + if currentModelID == "" && len(models.AvailableModels) > 0 { + currentModelID = string(models.AvailableModels[0].ModelId) + a.logger.Info("currentModelId empty, using first available model as fallback", + zap.String("fallback_model", currentModelID), + ) + } + + a.logger.Info("emitting session_models event", + zap.String("session_id", sessionID), + zap.String("current_model_id", currentModelID), + zap.Int("available_models", len(models.AvailableModels)), + ) a.sendUpdate(AgentEvent{ Type: streams.EventTypeSessionModels, SessionID: sessionID, - CurrentModelID: string(models.CurrentModelId), + CurrentModelID: currentModelID, SessionModels: convertSessionModels(models.AvailableModels), - ConfigOptions: extractConfigOptions(meta), + ConfigOptions: configOptions, }) } +// resolveCurrentModelFromConfig extracts current model ID from configOptions. +func resolveCurrentModelFromConfig(options []streams.ConfigOption) string { + for _, opt := range options { + if opt.ID == "model" || opt.Category == "model" { + return opt.CurrentValue + } + } + return "" +} + // SetMode changes the agent's session mode via ACP session/set_mode. func (a *Adapter) SetMode(ctx context.Context, modeID string) error { a.mu.RLock() @@ -937,9 +971,9 @@ func (a *Adapter) SetModel(ctx context.Context, modelID string) error { } } - _, err := conn.SetSessionModel(ctx, acp.SetSessionModelRequest{ + _, err := conn.UnstableSetSessionModel(ctx, acp.UnstableSetSessionModelRequest{ SessionId: acp.SessionId(sessionID), - ModelId: acp.ModelId(modelID), + ModelId: acp.UnstableModelId(modelID), }) if err != nil { return fmt.Errorf("set session model failed: %w", err) @@ -958,7 +992,7 @@ func (a *Adapter) Authenticate(ctx context.Context, methodID string) error { } _, err := conn.Authenticate(ctx, acp.AuthenticateRequest{ - MethodId: acp.AuthMethodId(methodID), + MethodId: methodID, }) if err != nil { return fmt.Errorf("authenticate failed: %w", err) diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/conversion_test.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/conversion_test.go index 14950786f7..d11ecae2ca 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/conversion_test.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/conversion_test.go @@ -646,7 +646,7 @@ func TestConvertAvailableCommands_WithInputHint(t *testing.T) { Name: "search", Description: "Search codebase", Input: &acp.AvailableCommandInput{ - UnstructuredCommandInput: &acp.AvailableCommandUnstructuredCommandInput{ + Unstructured: &acp.UnstructuredCommandInput{ Hint: "Enter search query", }, }, diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/meta_convert.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/meta_convert.go index 3ec5023c92..49ea9ef4c9 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/meta_convert.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/meta_convert.go @@ -64,7 +64,7 @@ func extractTerminalAuth(meta map[string]any) *streams.TerminalAuth { // convertSessionModels converts ACP model info to stream types, // normalizing known _meta patterns (copilotUsage) while preserving raw _meta. -func convertSessionModels(models []acp.ModelInfo) []streams.SessionModelInfo { +func convertSessionModels(models []acp.UnstableModelInfo) []streams.SessionModelInfo { if len(models) == 0 { return nil } @@ -106,6 +106,41 @@ func extractUsageMultiplier(meta map[string]any) string { } } +// convertACPConfigOptions converts typed ACP SessionConfigOption to stream ConfigOption. +// This is the preferred path when the SDK properly parses configOptions from the response. +func convertACPConfigOptions(opts []acp.SessionConfigOption) []streams.ConfigOption { + if len(opts) == 0 { + return nil + } + result := make([]streams.ConfigOption, 0, len(opts)) + for _, opt := range opts { + if opt.Select == nil { + continue + } + s := opt.Select + co := streams.ConfigOption{ + Type: s.Type, + ID: string(s.Id), + Name: s.Name, + CurrentValue: string(s.CurrentValue), + } + if s.Category != nil { + co.Category = string(*s.Category) + } + // Extract options from ungrouped list (most common case) + if s.Options.Ungrouped != nil { + for _, o := range *s.Options.Ungrouped { + co.Options = append(co.Options, streams.ConfigOptionValue{ + Value: string(o.Value), + Name: o.Name, + }) + } + } + result = append(result, co) + } + return result +} + // extractConfigOptions extracts config options from ACP session _meta. // Example _meta: {"configOptions": [{"type": "select", "id": "mode", "name": "Mode", ...}]} func extractConfigOptions(meta any) []streams.ConfigOption { diff --git a/apps/backend/internal/orchestrator/event_handlers_streaming.go b/apps/backend/internal/orchestrator/event_handlers_streaming.go index 75c0d345db..6f07d575f1 100644 --- a/apps/backend/internal/orchestrator/event_handlers_streaming.go +++ b/apps/backend/internal/orchestrator/event_handlers_streaming.go @@ -608,6 +608,11 @@ func (s *Service) handleSessionModelsEvent(ctx context.Context, payload *lifecyc ConfigOptions: payload.Data.ConfigOptions, Timestamp: time.Now().UTC().Format(time.RFC3339), } + s.logger.Info("publishing session_models event to WS", + zap.String("session_id", sessionID), + zap.String("current_model_id", payload.Data.CurrentModelID), + zap.Int("models_count", len(payload.Data.SessionModels)), + ) subject := events.BuildSessionModelsSubject(sessionID) _ = s.eventBus.Publish(ctx, subject, bus.NewEvent(events.SessionModelsUpdated, "orchestrator", eventPayload)) } diff --git a/apps/web/components/task/model-selector.tsx b/apps/web/components/task/model-selector.tsx index 44b16a4306..4088b98599 100644 --- a/apps/web/components/task/model-selector.tsx +++ b/apps/web/components/task/model-selector.tsx @@ -78,6 +78,24 @@ function buildModelOptions( return options; } +function resolveProfileModel(profileId: string | null | undefined, agents: Agent[]): string | null { + if (!profileId) return null; + for (const agent of agents) { + const profile = agent.profiles.find((p: AgentProfile) => p.id === profileId); + if (profile?.model) return profile.model; + } + return null; +} + +function resolveCurrentModel( + activeModel: string | null, + acpCurrentModel: string | null, + snapshotModel: string | null, + profileModel: string | null, +): string | null { + return activeModel || acpCurrentModel || snapshotModel || profileModel; +} + /** Resolves available models and current model from store state. */ function useModelSelectorState(sessionId: string | null) { useSettingsData(true); @@ -93,15 +111,23 @@ function useModelSelectorState(sessionId: string | null) { const session = sessionId ? (taskSessions[sessionId] ?? null) : null; const snapshotModel = resolveSnapshotModel(session?.agent_profile_snapshot); + const profileModel = useMemo( + () => resolveProfileModel(session?.agent_profile_id, settingsAgents as Agent[]), + [session?.agent_profile_id, settingsAgents], + ); - // Prefer dynamic ACP session models when available, fall back to static registry models const availableModels = sessionModelsData?.models?.length ? sessionModelsToOptions(sessionModelsData.models) : resolveStaticModels(settingsAgents as Agent[], session?.agent_profile_id, availableAgents); const activeModel = sessionId ? activeModels[sessionId] || null : null; const acpCurrentModel = sessionModelsData?.currentModelId || null; - const currentModel = activeModel || acpCurrentModel || snapshotModel; + const currentModel = resolveCurrentModel( + activeModel, + acpCurrentModel, + snapshotModel, + profileModel, + ); const modelOptions = buildModelOptions(availableModels, currentModel); const handleModelChange = useCallback( diff --git a/apps/web/hooks/domains/session/use-session-model.ts b/apps/web/hooks/domains/session/use-session-model.ts index 2662a7f1df..d9a529e33c 100644 --- a/apps/web/hooks/domains/session/use-session-model.ts +++ b/apps/web/hooks/domains/session/use-session-model.ts @@ -17,12 +17,23 @@ export function useSessionModel( return null; }, [agentProfileId, settingsAgents]); - const sessionModel = sessionProfile?.model ?? null; + const profileModel = sessionProfile?.model ?? null; + + // ACP agents report their actual current model via session_models events. + // Use that as the authoritative "session model" so comparisons in useMessageHandler + // use the same ID space (ACP IDs) as the model selector dropdown. + const acpCurrentModel = useAppStore((state) => + resolvedSessionId + ? state.sessionModels.bySessionId[resolvedSessionId]?.currentModelId || null + : null, + ); // Get active model state for this session (user's selected model) const activeModels = useAppStore((state) => state.activeModel.bySessionId); const activeModel = resolvedSessionId ? (activeModels[resolvedSessionId] ?? null) : null; + const sessionModel = acpCurrentModel || profileModel; + return { sessionModel, activeModel, diff --git a/apps/web/lib/ws/handlers/session-models.ts b/apps/web/lib/ws/handlers/session-models.ts index 30637dc3c7..f248824657 100644 --- a/apps/web/lib/ws/handlers/session-models.ts +++ b/apps/web/lib/ws/handlers/session-models.ts @@ -10,9 +10,27 @@ export function registerSessionModelsHandlers(store: StoreApi): WsHand if (!payload?.session_id) { return; } + const acpModels = payload.models ?? []; + // Resolve currentModelId: prefer the explicit field, fall back to the "model" + // config option's currentValue (some ACP agents send currentModelId as empty). + let currentModelId = payload.current_model_id || ""; + if (!currentModelId) { + const modelOpt = (payload.config_options ?? []).find( + (o) => o.id === "model" || o.category === "model", + ); + if (modelOpt?.current_value) { + currentModelId = modelOpt.current_value; + } + } + console.debug("[session-models] received session.models_updated", { + sessionId: payload.session_id, + rawCurrentModelId: payload.current_model_id, + resolvedCurrentModelId: currentModelId, + models: acpModels.map((m) => m.model_id), + }); store.getState().setSessionModels(payload.session_id, { - currentModelId: payload.current_model_id, - models: (payload.models ?? []).map((m) => ({ + currentModelId, + models: acpModels.map((m) => ({ modelId: m.model_id, name: m.name, description: m.description, @@ -28,6 +46,20 @@ export function registerSessionModelsHandlers(store: StoreApi): WsHand options: o.options, })), }); + + // Clear stale activeModel if it uses a profile ID that doesn't exist in ACP models. + // This happens when a user selected a static model before ACP models arrived. + if (acpModels.length > 0) { + const state = store.getState(); + const currentActive = state.activeModel.bySessionId[payload.session_id]; + if (currentActive && !acpModels.some((m) => m.model_id === currentActive)) { + console.debug("[session-models] clearing stale activeModel", { + sessionId: payload.session_id, + staleModel: currentActive, + }); + state.setActiveModel(payload.session_id, ""); + } + } }, }; }