fix: resolve model selector mismatch for ACP agents (#410)
This commit is contained in:
@@ -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),
|
||||
|
||||
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+2
-2
@@ -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=
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
},
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -10,9 +10,27 @@ export function registerSessionModelsHandlers(store: StoreApi<AppState>): 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<AppState>): 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, "");
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user