dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(acp): bound long session resumes and restore legacy model surfaces (#1324)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-06-10 21:20:28 +01:00
committed by GitHub
parent bb15fcf474
commit dd182acfd2
23 changed files with 1164 additions and 257 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/cmd/mock-agent/main.go
+9 -18
View File
@@ -85,10 +85,11 @@ func (a *mockAgent) Initialize(_ context.Context, _ acp.InitializeRequest) (acp.
// NewSession creates a new conversation session.
// MCP servers from the ACP request are registered so callMCPTool can use them.
// The Models and Modes fields advertise available capabilities so the host
// utility capability probe can populate them in the cache — this is what makes
// the utility-agents settings page show model and mode options for mock-agent
// in E2E, and lets profile-mode tests select a non-default mode.
// The Modes field and the model-shaped entry in ConfigOptions advertise
// available capabilities so the host utility capability probe can populate
// them in the cache — this is what makes the utility-agents settings page
// show model and mode options for mock-agent in E2E, and lets profile-mode
// tests select a non-default mode.
func (a *mockAgent) NewSession(_ context.Context, req acp.NewSessionRequest) (acp.NewSessionResponse, error) {
sid := acp.SessionId(fmt.Sprintf("mock-session-%d", os.Getpid()))
a.mu.Lock()
@@ -106,25 +107,15 @@ func (a *mockAgent) NewSession(_ context.Context, req acp.NewSessionRequest) (ac
return acp.NewSessionResponse{
SessionId: sid,
Models: mockSessionModels(),
Modes: mockSessionModes(),
ConfigOptions: mockSessionConfigOptions(),
}, nil
}
// mockSessionModels returns the mock agent's advertised model list for ACP
// session responses. Two models are exposed so tests can verify both
// selection and default behavior (mock-fast is the default).
func mockSessionModels() *acp.SessionModelState {
fastDesc := "Fast mock model for testing"
smartDesc := "Smart mock model for testing"
return &acp.SessionModelState{
CurrentModelId: "mock-fast",
AvailableModels: []acp.ModelInfo{
{ModelId: "mock-fast", Name: "Mock Fast", Description: &fastDesc},
{ModelId: "mock-smart", Name: "Mock Smart", Description: &smartDesc},
},
}
// Logout terminates the current authenticated session. The mock agent has no
// persistent auth state so this is a no-op.
func (a *mockAgent) Logout(_ context.Context, _ acp.LogoutRequest) (acp.LogoutResponse, error) {
return acp.LogoutResponse{}, nil
}
// mockSessionModes returns the mock agent's advertised session-mode list for
apps/backend/go.mod
+3 -1
View File
@@ -5,7 +5,7 @@ go 1.26.0
require (
github.com/UserExistsError/conpty v0.1.4
github.com/bmatcuk/doublestar/v4 v4.10.0
github.com/coder/acp-go-sdk v0.13.0
github.com/coder/acp-go-sdk v0.13.5
github.com/creack/pty v1.1.21
github.com/docker/docker v28.5.2+incompatible
github.com/docker/go-connections v0.6.0
@@ -115,3 +115,5 @@ require (
gopkg.in/ini.v1 v1.67.0 // indirect
gotest.tools/v3 v3.5.2 // indirect
)
replace github.com/coder/acp-go-sdk => github.com/kdlbs/acp-go-sdk v0.13.6-0.20260609161428-91d5e43d6599
apps/backend/go.sum
+2 -2
View File
@@ -72,8 +72,6 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/coder/acp-go-sdk v0.13.0 h1:IAKBDIbe/iBfKAGikeIndzb8fowt4ioD+gCtSU4HwMA=
github.com/coder/acp-go-sdk v0.13.0/go.mod h1:yKzM/3R9uELp4+nBAwwtkS0aN1FOFjo11CNPy37yFko=
github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI=
github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M=
github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE=
@@ -219,6 +217,8 @@ 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.13.6-0.20260609161428-91d5e43d6599 h1:VeNo6BcOkY18Hj9HopG7/3t5PpD4ocmuxMIuGSobzLQ=
github.com/kdlbs/acp-go-sdk v0.13.6-0.20260609161428-91d5e43d6599/go.mod h1:yKzM/3R9uELp4+nBAwwtkS0aN1FOFjo11CNPy37yFko=
github.com/kevinburke/ssh_config v1.6.0 h1:J1FBfmuVosPHf5GRdltRLhPJtJpTlMdKTBjRgTaQBFY=
github.com/kevinburke/ssh_config v1.6.0/go.mod h1:q2RIzfka+BXARoNexmF9gkxEX7DmvbW9P4hIVx2Kg4M=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
apps/backend/internal/agent/runtime/lifecycle/session.go
+37
View File
@@ -133,6 +133,19 @@ func (sm *SessionManager) createOrLoadSession(
if err == nil {
return sessionID, nil
}
// If the underlying ACP connection is dead (peer disconnected, context
// cancelled), session/new on the same client will return the same
// transport error — falling back just emits a noisy duplicate failure
// and delays the FAILED transition. Short-circuit so the caller can
// rebuild the connection (next resume cycle gets a fresh agentctl
// instance and a fresh ACP connection).
if isTransportDeadErr(err) {
sm.logger.Warn("session/load failed at transport layer, not retrying with session/new",
zap.String("agent_type", agentConfig.ID()),
zap.String("existing_session_id", existingSessionID),
zap.String("reason", err.Error()))
return "", err
}
// session/load can fail for reasons that don't justify aborting the
// session: agent doesn't support the method (capability mismatch /
// method not found), the upstream agent CLI no longer recognises the
@@ -726,3 +739,27 @@ func isAgentStreamNotConnectedErr(err error) bool {
}
return strings.Contains(err.Error(), "agent stream not connected")
}
// isTransportDeadErr reports whether a session/load failure is caused by the
// underlying ACP connection being gone rather than an agent-side error. The
// coder/acp-go-sdk surfaces this as a JSON-RPC internal-error whose data map
// carries the canonical phrase "peer disconnected before response" (also
// emitted while waiting for pre-response notifications). The error reaches us
// as a string through the agentctl WS layer, so we match the phrase.
// "connection closed" is the SDK's own cause string emitted from
// shutdownReceive — pulling double duty as a fallback for paths where the
// peer-disconnected wrapping isn't applied. Canonical context cancellation
// errors short-circuit too: the caller's ctx going down means session/new
// retry will fail for the same reason, so treat it as transport-dead.
func isTransportDeadErr(err error) bool {
if err == nil {
return false
}
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
return true
}
msg := err.Error()
return strings.Contains(msg, "peer disconnected") ||
strings.Contains(msg, "connection closed") ||
strings.Contains(msg, "notification queue overflow")
}
apps/backend/internal/agent/runtime/lifecycle/session_test.go
+68
View File
@@ -622,6 +622,74 @@ func TestIsMethodNotFoundErr(t *testing.T) {
}
}
func TestIsTransportDeadErr(t *testing.T) {
tests := []struct {
name string
err error
expected bool
}{
{
name: "peer disconnected before response",
err: fmt.Errorf("session/load failed: %w", fmt.Errorf("peer disconnected before response")),
expected: true,
},
{
name: "peer disconnected while waiting for pre-response notifications",
err: fmt.Errorf("peer disconnected while waiting for pre-response notifications"),
expected: true,
},
{
name: "connection closed cause",
err: fmt.Errorf("load session failed: connection closed"),
expected: true,
},
{
name: "notification queue overflow",
err: fmt.Errorf("load session failed: notification queue overflow"),
expected: true,
},
{
name: "context canceled",
err: context.Canceled,
expected: true,
},
{
name: "context deadline exceeded wrapped",
err: fmt.Errorf("load session failed: %w", context.DeadlineExceeded),
expected: true,
},
{
name: "method not found is not transport-dead",
err: &acp.RequestError{Code: -32601, Message: "Method not found"},
expected: false,
},
{
name: "session unknown is not transport-dead",
err: &acp.RequestError{Code: -32002, Message: "Resource not found"},
expected: false,
},
{
name: "nil error",
err: nil,
expected: false,
},
{
name: "unrelated error",
err: fmt.Errorf("some other failure"),
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isTransportDeadErr(tt.err)
if got != tt.expected {
t.Errorf("isTransportDeadErr(%v) = %v, want %v", tt.err, got, tt.expected)
}
})
}
}
func TestInitializeSession_LoadsExistingSession(t *testing.T) {
mock := newMockAgentServer(t)
defer mock.Close()
apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go
+53 -6
View File
@@ -7,6 +7,8 @@ import (
"fmt"
"io"
"log/slog"
"os"
"strconv"
"sync"
"time"
@@ -60,11 +62,47 @@ const wakeupPromptTimeout = 30 * time.Minute
// notifQueueCapacity sizes the buffered channel that feeds the update
// worker. 4096 covers any realistic session/load replay burst (the failure
// case that motivated this was ~5-10k notifications spread over several
// seconds, well within the worker's sub-microsecond drain rate). Set big
// enough that the SDK's 1024-slot upstream queue stays near-empty under
// burst, small enough that worst-case memory is bounded.
// seconds, well within the worker's sub-microsecond drain rate). This is
// the internal hand-off between the SDK's update handler and our worker;
// it sits in front of the larger acpNotifQueueDefault SDK inbound queue and
// can be smaller because the worker drains it well under SDK fill rate.
const notifQueueCapacity = 4096
// acpNotifQueueDefault is the per-connection capacity passed to the SDK's
// inbound notification queue. Larger than the SDK's own default (1024) so
// long session/load replays (auggie 500+ exchanges, claude with heavy tool
// use) don't overflow and tear the connection down. The SDK still bounds
// memory to (capacity * avg notification size).
const acpNotifQueueDefault = 16384
// acpNotifQueueMin / acpNotifQueueMax clamp KANDEV_ACP_NOTIF_QUEUE so a
// misconfigured value can't either re-introduce the overflow (too low) or
// blow the heap (too high).
const (
acpNotifQueueMin = 1024
acpNotifQueueMax = 131072
)
// acpNotifQueueCapacity returns the per-connection inbound notification queue
// capacity, honoring KANDEV_ACP_NOTIF_QUEUE when set and parseable.
func acpNotifQueueCapacity() int {
raw := os.Getenv("KANDEV_ACP_NOTIF_QUEUE")
if raw == "" {
return acpNotifQueueDefault
}
n, err := strconv.Atoi(raw)
if err != nil || n <= 0 {
return acpNotifQueueDefault
}
if n < acpNotifQueueMin {
return acpNotifQueueMin
}
if n > acpNotifQueueMax {
return acpNotifQueueMax
}
return n
}
// AgentInfo contains information about the connected agent.
type AgentInfo struct {
Name string `json:"name"`
@@ -170,7 +208,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 []modelInfo
// usageDelta tracks the running cumulative `usage_update.used` and
// the most recent USD cost reported per session. codex-acp emits no
@@ -310,9 +348,18 @@ func (a *Adapter) Initialize(ctx context.Context) error {
acpclient.WithPermissionHandler(a.handlePermissionRequest),
)
// Create ACP SDK connection
a.acpConn = acp.NewClientSideConnection(a.acpClient, a.stdin, a.stdout)
// Create ACP SDK connection. Raise the inbound notification queue cap
// to acpNotifQueueDefault (well above the SDK's built-in default) so
// long session/load replays don't overflow and tear the connection
// down. The internal notifQueueCapacity channel sits in front of this
// queue and is drained by our update worker. Requires a coder/acp-go-sdk
// fork with WithMaxQueuedNotifications; see go.mod replace directive.
notifQueueCap := acpNotifQueueCapacity()
a.acpConn = acp.NewClientSideConnection(a.acpClient, a.stdin, a.stdout,
acp.WithMaxQueuedNotifications(notifQueueCap))
a.acpConn.SetLogger(slog.Default().With("component", "acp-conn"))
a.logger.Debug("ACP connection notification queue sized",
zap.Int("capacity", notifQueueCap))
// Perform ACP handshake - this exchanges capabilities with the agent
ctx, span := shared.TraceProtocolRequest(ctx, shared.ProtocolACP, a.agentID, "initialize")
apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_session.go
+59 -18
View File
@@ -55,7 +55,12 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
a.mu.Lock()
a.sessionID = string(resp.SessionId)
sessionID := a.sessionID
initialModels := initialSessionModelState(resp.Models, resp.Meta, resp.ConfigOptions)
// Reset session-scoped model caches before computing the new session's
// state so a session without a model surface can't reuse the previous
// session's models / configOptions for validation in SetModel.
a.availableModels = nil
a.availableConfigOptions = nil
initialModels := initialSessionModelState(resp.Meta, resp.ConfigOptions, resp.LegacyModels)
if initialModels != nil {
a.availableModels = initialModels.AvailableModels
}
@@ -70,8 +75,7 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
a.emitInitialModeState(resp.Modes)
}
// Emit session models if the agent returned model state, or if it exposes
// model selection only through configOptions.
// Emit session models when the session exposes a model-shaped config option.
if initialModels != nil {
a.emitSessionModels(sessionID, initialModels, resp.Meta, resp.ConfigOptions)
}
@@ -91,16 +95,32 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
return sessionID, nil
}
// initialSessionModelState resolves the initial model state for a session.
// Returns nil when no model-shaped surface exists, signalling that the agent
// doesn't advertise model selection on this session.
//
// Precedence (in order):
// 1. Typed ConfigOptions list with category="model" (v0.13.4+ agents).
// 2. Pre-v0.13.5 top-level `models` field (e.g. auggie 0.29.x), exposed by the
// kdlbs fork as acp.LegacyModels. Reached even when configOptions carries
// non-model entries (e.g. `category="mode"`) so an agent that mixes typed
// mode options with a legacy models block still surfaces its models.
// 3. _meta-only ConfigOption stub for legacy agents that surface options
// under `_meta.configOptions` (returns an empty state so emitSessionModels
// still fires; the event's ConfigOptions list is filled from _meta there).
func initialSessionModelState(
models *acp.SessionModelState,
meta map[string]any,
configOptions []acp.SessionConfigOption,
) *acp.SessionModelState {
if models != nil {
return models
legacy *acp.LegacyModels,
) *sessionModelState {
if state := modelsFromConfigOptions(configOptions); state != nil {
return state
}
if hasModelConfigOption(sessionConfigOptions(meta, configOptions)) {
return &acp.SessionModelState{}
if state := modelsFromLegacy(legacy); state != nil {
return state
}
if hasModelConfigOption(extractConfigOptions(meta)) {
return &sessionModelState{}
}
return nil
}
@@ -299,7 +319,11 @@ func (a *Adapter) LoadSession(ctx context.Context, sessionID string, mcpServers
a.mu.Lock()
a.sessionID = sessionID
initialModels := initialSessionModelState(resp.Models, resp.Meta, resp.ConfigOptions)
// Reset session-scoped model caches so a load that lands on a session
// without a model surface can't reuse the previous session's data.
a.availableModels = nil
a.availableConfigOptions = nil
initialModels := initialSessionModelState(resp.Meta, resp.ConfigOptions, resp.LegacyModels)
if initialModels != nil {
a.availableModels = initialModels.AvailableModels
}
@@ -399,8 +423,8 @@ 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 map[string]any, acpConfigOptions []acp.SessionConfigOption) {
currentModelID := string(models.CurrentModelId)
func (a *Adapter) emitSessionModels(sessionID string, models *sessionModelState, meta map[string]any, acpConfigOptions []acp.SessionConfigOption) {
currentModelID := models.CurrentModelId
// Prefer typed config options from the response; fall back to _meta
// extraction for older agents.
configOptions := sessionConfigOptions(meta, acpConfigOptions)
@@ -448,7 +472,7 @@ func (a *Adapter) emitSessionModels(sessionID string, models *acp.SessionModelSt
// — this prevents a downstream consumer that reads ConfigOptions[model]
// .CurrentValue (codex-style agents surface the current model there) from
// disagreeing with the CurrentModelID emitted on the same event.
func (a *Adapter) emitSetModelEvent(sessionID, modelID string, cachedModels []acp.ModelInfo, cachedConfig []streams.ConfigOption) {
func (a *Adapter) emitSetModelEvent(sessionID, modelID string, cachedModels []modelInfo, cachedConfig []streams.ConfigOption) {
outConfig := cachedConfig
if len(cachedConfig) > 0 {
// Shallow copy: only CurrentValue (a string) is rewritten below, so
@@ -488,7 +512,7 @@ func (a *Adapter) emitSetModelEvent(sessionID, modelID string, cachedModels []ac
}
// resolveCurrentModelFromConfig extracts current model ID from configOptions.
func resolveCurrentModelFromConfig(options []streams.ConfigOption, available []acp.ModelInfo) string {
func resolveCurrentModelFromConfig(options []streams.ConfigOption, available []modelInfo) string {
modelID := ""
reasoningEffort := ""
for _, opt := range options {
@@ -554,12 +578,12 @@ func splitReasoningModelID(modelID string, options []streams.ConfigOption) (stri
return baseModelID, reasoningEffort, true
}
func modelIDExists(modelID string, available []acp.ModelInfo) bool {
func modelIDExists(modelID string, available []modelInfo) bool {
if len(available) == 0 {
return false
}
for _, model := range available {
if string(model.ModelId) == modelID {
if model.ModelId == modelID {
return true
}
}
@@ -619,7 +643,7 @@ func (a *Adapter) SetModel(ctx context.Context, modelID string) error {
if len(available) > 0 {
found := false
for _, m := range available {
if string(m.ModelId) == modelID {
if m.ModelId == modelID {
found = true
break
}
@@ -636,9 +660,26 @@ func (a *Adapter) SetModel(ctx context.Context, modelID string) error {
if err != nil {
return fmt.Errorf("set session model failed via %s: %w", method, err)
}
a.finalizeSetModel(method, sessionID, modelID, available, cachedConfig)
return nil
}
// finalizeSetModel emits the post-apply convergence event when applySessionModel
// actually performed a switch. MethodNone means the agent supports neither the
// typed session/set_config_option nor the legacy session/set_model RPC, so no
// switch happened — skip the reset and the emit to avoid lying to the frontend.
func (a *Adapter) finalizeSetModel(
method sessionmodel.Method,
sessionID string,
modelID string,
available []modelInfo,
cachedConfig []streams.ConfigOption,
) {
if method == sessionmodel.MethodNone {
return
}
a.resetContextWindowMaxSize(sessionID)
a.emitSetModelEvent(sessionID, modelID, available, cachedConfig)
return nil
}
func applySessionModel(
apps/backend/internal/agentctl/server/adapter/transport/acp/load_burst_test.go
+4
View File
@@ -33,6 +33,10 @@ func (burstAgent) ListSessions(context.Context, acp.ListSessionsRequest) (acp.Li
return acp.ListSessionsResponse{}, nil
}
func (burstAgent) Logout(context.Context, acp.LogoutRequest) (acp.LogoutResponse, error) {
return acp.LogoutResponse{}, nil
}
func (burstAgent) NewSession(context.Context, acp.NewSessionRequest) (acp.NewSessionResponse, error) {
return acp.NewSessionResponse{}, nil
}
apps/backend/internal/agentctl/server/adapter/transport/acp/meta_convert.go
+3 -3
View File
@@ -68,16 +68,16 @@ func extractTerminalAuth(meta map[string]any) *streams.TerminalAuth {
return ta
}
// convertSessionModels converts ACP model info to stream types,
// convertSessionModels converts kandev-local model info to stream types,
// normalizing known _meta patterns (copilotUsage) while preserving raw _meta.
func convertSessionModels(models []acp.ModelInfo) []streams.SessionModelInfo {
func convertSessionModels(models []modelInfo) []streams.SessionModelInfo {
if len(models) == 0 {
return nil
}
result := make([]streams.SessionModelInfo, 0, len(models))
for _, m := range models {
info := streams.SessionModelInfo{
ModelID: string(m.ModelId),
ModelID: m.ModelId,
Name: m.Name,
Description: derefStr(m.Description),
Meta: toStringMap(m.Meta),
apps/backend/internal/agentctl/server/adapter/transport/acp/model_apply_test.go
+54 -5
View File
@@ -2,6 +2,7 @@ package acp
import (
"context"
"errors"
"reflect"
"testing"
@@ -11,7 +12,7 @@ import (
type fakeModelApplier struct {
configErr error
modelErr error
legacyErr error
calls []string
}
@@ -21,8 +22,8 @@ func (f *fakeModelApplier) SetSessionConfigOption(_ context.Context, req sdk.Set
}
func (f *fakeModelApplier) UnstableSetSessionModel(_ context.Context, req sdk.UnstableSetSessionModelRequest) (sdk.UnstableSetSessionModelResponse, error) {
f.calls = append(f.calls, "model:"+string(req.ModelId))
return sdk.UnstableSetSessionModelResponse{}, f.modelErr
f.calls = append(f.calls, "legacy:"+string(req.SessionId)+":"+req.ModelId)
return sdk.UnstableSetSessionModelResponse{}, f.legacyErr
}
func TestApplySessionModel_UsesConfigOptionForModelConfig(t *testing.T) {
@@ -46,7 +47,33 @@ func TestApplySessionModel_UsesConfigOptionForModelConfig(t *testing.T) {
}
}
func TestApplySessionModel_FallsBackToSetModelWhenSetConfigOptionMissing(t *testing.T) {
// TestApplySessionModel_PropagatesSetConfigOptionError pins that non
// MethodNotFound errors from the underlying SetSessionConfigOption RPC bubble
// up unchanged when the session advertises a model-shaped config option.
func TestApplySessionModel_PropagatesSetConfigOptionError(t *testing.T) {
t.Parallel()
boom := errors.New("boom")
conn := &fakeModelApplier{configErr: boom}
_, err := applySessionModel(context.Background(), conn, "sess-1", "claude-opus-4-8", []streams.ConfigOption{{
ID: "model",
Category: "model",
}})
if !errors.Is(err, boom) {
t.Fatalf("applySessionModel() error = %v, want boom", err)
}
wantCalls := []string{"config:model:claude-opus-4-8"}
if !reflect.DeepEqual(conn.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", conn.calls, wantCalls)
}
}
// TestApplySessionModel_SetConfigOptionMethodNotFoundFallsBack pins the
// partial-migration case: when the agent advertises the typed model-shaped
// config option but hasn't wired up the session/set_config_option handler,
// applySessionModel falls through to the legacy session/set_model RPC.
func TestApplySessionModel_SetConfigOptionMethodNotFoundFallsBack(t *testing.T) {
t.Parallel()
conn := &fakeModelApplier{configErr: sdk.NewMethodNotFound(sdk.AgentMethodSessionSetConfigOption)}
@@ -55,13 +82,35 @@ func TestApplySessionModel_FallsBackToSetModelWhenSetConfigOptionMissing(t *test
Category: "model",
}})
if err != nil {
t.Fatalf("applySessionModel() error = %v, want nil after legacy fallback", err)
}
if method != "session/set_model" {
t.Fatalf("method = %q, want session/set_model", method)
}
wantCalls := []string{"config:model:claude-opus-4-8", "legacy:sess-1:claude-opus-4-8"}
if !reflect.DeepEqual(conn.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", conn.calls, wantCalls)
}
}
// TestApplySessionModel_FallsBackToLegacySetModel pins the SDK-side fallback
// path: when the session has no model-shaped config option (e.g. auggie 0.29.x
// which surfaces models via the legacy top-level `models` field), the
// dispatcher uses the kdlbs-fork-restored session/set_model RPC instead.
func TestApplySessionModel_FallsBackToLegacySetModel(t *testing.T) {
t.Parallel()
conn := &fakeModelApplier{}
method, err := applySessionModel(context.Background(), conn, "sess-1", "claude-opus-4-7", nil)
if err != nil {
t.Fatalf("applySessionModel() error = %v", err)
}
if method != "session/set_model" {
t.Fatalf("method = %q, want session/set_model", method)
}
wantCalls := []string{"config:model:claude-opus-4-8", "model:claude-opus-4-8"}
wantCalls := []string{"legacy:sess-1:claude-opus-4-7"}
if !reflect.DeepEqual(conn.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", conn.calls, wantCalls)
}
apps/backend/internal/agentctl/server/adapter/transport/acp/model_types.go
+108
View File
@@ -0,0 +1,108 @@
package acp
import (
acp "github.com/coder/acp-go-sdk"
)
// modelInfo is the kandev-internal representation of a single model advertised
// by an ACP session. v0.13.0 of the SDK exposed an equivalent acp.ModelInfo
// type; v0.13.5 removed it in favor of routing model selection through
// SessionConfigOption with category="model". We keep the local shape so the
// rest of the adapter (caching, convergence events, fallback resolution)
// doesn't have to spread the configOption walk across every call site.
type modelInfo struct {
ModelId string
Name string
Description *string
Meta map[string]any
}
// sessionModelState mirrors the removed v0.13.0 acp.SessionModelState shape.
// Populated by initialSessionModelState from the session response's typed
// SessionConfigOption list (with _meta fallback for legacy agents).
type sessionModelState struct {
CurrentModelId string
AvailableModels []modelInfo
}
// modelsFromConfigOptions extracts the current model id and the available
// model list from a typed SessionConfigOption slice, looking for the option
// whose Select.Category is "model". Returns nil when no such option exists.
func modelsFromConfigOptions(opts []acp.SessionConfigOption) *sessionModelState {
for _, opt := range opts {
if opt.Select == nil {
continue
}
sel := opt.Select
isModel := string(sel.Id) == configOptionIDModel ||
(sel.Category != nil && string(*sel.Category) == configOptionIDModel)
if !isModel {
continue
}
state := &sessionModelState{
CurrentModelId: string(sel.CurrentValue),
AvailableModels: modelsFromSelectOptions(sel.Options),
}
return state
}
return nil
}
// modelsFromSelectOptions flattens the ungrouped/grouped variants of a
// SessionConfigSelectOptions payload into a flat []modelInfo. Grouped options
// are walked in declaration order so the resulting slice preserves the agent's
// presentation order.
func modelsFromSelectOptions(opts acp.SessionConfigSelectOptions) []modelInfo {
if opts.Ungrouped != nil {
return convertSelectOptionList(*opts.Ungrouped)
}
if opts.Grouped != nil {
var out []modelInfo
for _, group := range *opts.Grouped {
out = append(out, convertSelectOptionList(group.Options)...)
}
return out
}
return nil
}
// modelsFromLegacy extracts the session model state from the pre-v0.13.5
// top-level `models` field still emitted by agents (e.g. auggie 0.29.x) that
// haven't migrated to the SessionConfigOption(category="model") surface.
// Upstream removed this field from the SDK struct in v0.13.5; the kdlbs fork
// restores read-only parsing via acp.LegacyModels. Returns nil when the field
// is absent or carries no models.
func modelsFromLegacy(legacy *acp.LegacyModels) *sessionModelState {
if legacy == nil || len(legacy.AvailableModels) == 0 {
return nil
}
out := make([]modelInfo, 0, len(legacy.AvailableModels))
for _, m := range legacy.AvailableModels {
out = append(out, modelInfo{
ModelId: m.ModelId,
Name: m.Name,
Description: m.Description,
Meta: m.Meta,
})
}
return &sessionModelState{
CurrentModelId: legacy.CurrentModelId,
AvailableModels: out,
}
}
func convertSelectOptionList(in []acp.SessionConfigSelectOption) []modelInfo {
if len(in) == 0 {
return nil
}
out := make([]modelInfo, 0, len(in))
for _, o := range in {
out = append(out, modelInfo{
ModelId: string(o.Value),
Name: o.Name,
Description: o.Description,
Meta: o.Meta,
})
}
return out
}
apps/backend/internal/agentctl/server/adapter/transport/acp/session_models_test.go
+181 -22
View File
@@ -4,6 +4,7 @@ import (
"testing"
"github.com/coder/acp-go-sdk"
"github.com/kandev/kandev/internal/agentctl/sessionmodel"
"github.com/kandev/kandev/internal/agentctl/types/streams"
)
@@ -22,10 +23,10 @@ func findSessionModelsEvent(t *testing.T, events []AgentEvent) AgentEvent {
// auggieLikeModels mimics Auggie's response: empty CurrentModelId with an
// alphabetically-sorted list whose [0] is a pseudo-agent ("Build Analyzer").
func auggieLikeModels() *acp.SessionModelState {
return &acp.SessionModelState{
func auggieLikeModels() *sessionModelState {
return &sessionModelState{
CurrentModelId: "",
AvailableModels: []acp.ModelInfo{
AvailableModels: []modelInfo{
{ModelId: "build-fix-gpt5-2-responses-high-200k-v1-c4-p2-agent", Name: "Build Analyzer"},
{ModelId: "claude-opus-4-7", Name: "Opus 4.7"},
},
@@ -79,9 +80,9 @@ func TestEmitSessionModels_EmptyCurrentIDFromConfigOption(t *testing.T) {
// selectable ID "gpt-5.5/medium".
func TestEmitSessionModels_EmptyCurrentIDComposesReasoningEffort(t *testing.T) {
a := newTestAdapter()
models := &acp.SessionModelState{
models := &sessionModelState{
CurrentModelId: "",
AvailableModels: []acp.ModelInfo{
AvailableModels: []modelInfo{
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
},
@@ -124,9 +125,9 @@ func TestEmitSessionModels_EmptyCurrentIDComposesReasoningEffortFromTypedOptions
{Name: "Low", Value: reasoningEffortLow},
{Name: "Medium", Value: reasoningEffortMedium},
}
models := &acp.SessionModelState{
models := &sessionModelState{
CurrentModelId: "",
AvailableModels: []acp.ModelInfo{
AvailableModels: []modelInfo{
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
},
@@ -175,7 +176,7 @@ func TestInitialSessionModelState_UsesConfigOptionsWithoutModels(t *testing.T) {
}},
}
models := initialSessionModelState(nil, nil, configOptions)
models := initialSessionModelState(nil, configOptions, nil)
if models == nil {
t.Fatal("initialSessionModelState returned nil for configOptions-only response")
}
@@ -208,7 +209,7 @@ func TestInitialSessionModelState_UsesMetaConfigOptionsWithoutModels(t *testing.
},
}
models := initialSessionModelState(nil, meta, nil)
models := initialSessionModelState(meta, nil, nil)
if models == nil {
t.Fatal("initialSessionModelState returned nil for meta configOptions-only response")
}
@@ -245,12 +246,53 @@ func TestInitialSessionModelState_IgnoresNonModelConfigOptions(t *testing.T) {
}},
}
if models := initialSessionModelState(nil, nil, configOptions); models != nil {
if models := initialSessionModelState(nil, configOptions, nil); models != nil {
t.Fatalf("initialSessionModelState returned %+v for non-model configOptions", models)
}
}
func TestInitialSessionModelState_UsesTypedConfigOptionPrecedence(t *testing.T) {
// TestInitialSessionModelState_NonModelConfigOptionsFallThroughToLegacy
// pins that an agent emitting typed configOptions[category="mode"] alongside
// the legacy top-level `models` field still surfaces its models — the typed
// non-model option must not block the LegacyModels tier.
func TestInitialSessionModelState_NonModelConfigOptionsFallThroughToLegacy(t *testing.T) {
modeCategory := acp.SessionConfigOptionCategoryMode
modeOptions := acp.SessionConfigSelectOptionsUngrouped{
{Name: "Read Only", Value: "read-only"},
}
configOptions := []acp.SessionConfigOption{
{Select: &acp.SessionConfigOptionSelect{
Type: "select",
Id: "mode",
Name: "Approval Preset",
Category: &modeCategory,
CurrentValue: "read-only",
Options: acp.SessionConfigSelectOptions{Ungrouped: &modeOptions},
}},
}
legacy := &acp.LegacyModels{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.LegacyModelInfo{
{ModelId: "claude-opus-4-7", Name: "Opus 4.7"},
},
}
models := initialSessionModelState(nil, configOptions, legacy)
if models == nil {
t.Fatal("initialSessionModelState returned nil; expected legacy fallback")
}
if models.CurrentModelId != "claude-opus-4-7" {
t.Errorf("CurrentModelId = %q, want claude-opus-4-7", models.CurrentModelId)
}
if len(models.AvailableModels) != 1 {
t.Fatalf("AvailableModels = %d, want 1", len(models.AvailableModels))
}
}
// TestInitialSessionModelState_NonModelConfigOptionsAllowMetaFallback pins
// that the _meta tier-3 fallback still fires when typed configOptions has
// only non-model entries and no LegacyModels are present.
func TestInitialSessionModelState_NonModelConfigOptionsAllowMetaFallback(t *testing.T) {
modeCategory := acp.SessionConfigOptionCategoryMode
modeOptions := acp.SessionConfigSelectOptionsUngrouped{
{Name: "Read Only", Value: "read-only"},
@@ -277,8 +319,82 @@ func TestInitialSessionModelState_UsesTypedConfigOptionPrecedence(t *testing.T)
},
}
if models := initialSessionModelState(nil, meta, configOptions); models != nil {
t.Fatalf("initialSessionModelState returned %+v for non-model typed configOptions", models)
models := initialSessionModelState(meta, configOptions, nil)
if models == nil {
t.Fatal("initialSessionModelState returned nil; expected _meta fallback stub")
}
if len(models.AvailableModels) != 0 {
t.Errorf("AvailableModels = %d, want 0 (stub state)", len(models.AvailableModels))
}
}
func TestInitialSessionModelState_FallsBackToLegacyModels(t *testing.T) {
// auggie 0.29.x emits the pre-v0.13.5 top-level `models` field and does
// NOT surface a SessionConfigOption(category="model"). The kdlbs SDK fork
// restores read-only parsing as acp.LegacyModels; the third-tier fallback
// in initialSessionModelState must turn that into a fully populated state.
desc := "Anthropic Claude Opus 4.7"
legacy := &acp.LegacyModels{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.LegacyModelInfo{
{ModelId: "claude-opus-4-7", Name: "Opus 4.7", Description: &desc},
{ModelId: "claude-sonnet-4-5", Name: "Sonnet 4.5"},
},
}
models := initialSessionModelState(nil, nil, legacy)
if models == nil {
t.Fatal("initialSessionModelState returned nil for legacy-models-only response")
}
if models.CurrentModelId != "claude-opus-4-7" {
t.Errorf("CurrentModelId = %q, want claude-opus-4-7", models.CurrentModelId)
}
if len(models.AvailableModels) != 2 {
t.Fatalf("AvailableModels len = %d, want 2", len(models.AvailableModels))
}
if models.AvailableModels[0].Name != "Opus 4.7" {
t.Errorf("AvailableModels[0].Name = %q, want Opus 4.7", models.AvailableModels[0].Name)
}
a := newTestAdapter()
a.emitSessionModels("sess-1", models, nil, nil)
ev := findSessionModelsEvent(t, drainEvents(a))
if ev.CurrentModelID != "claude-opus-4-7" {
t.Errorf("event CurrentModelID = %q, want claude-opus-4-7", ev.CurrentModelID)
}
if len(ev.SessionModels) != 2 {
t.Fatalf("event SessionModels len = %d, want 2", len(ev.SessionModels))
}
}
func TestInitialSessionModelState_TypedConfigOptionsBeatLegacyModels(t *testing.T) {
// When both surfaces are present, the typed configOptions[category=model]
// list wins — matches the documented precedence.
modelCategory := acp.SessionConfigOptionCategoryModel
modelOptions := acp.SessionConfigSelectOptionsUngrouped{
{Name: "GPT-5.5", Value: "gpt-5.5"},
}
configOptions := []acp.SessionConfigOption{
{Select: &acp.SessionConfigOptionSelect{
Type: "select",
Id: "model",
Name: "Model",
Category: &modelCategory,
CurrentValue: "gpt-5.5",
Options: acp.SessionConfigSelectOptions{Ungrouped: &modelOptions},
}},
}
legacy := &acp.LegacyModels{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.LegacyModelInfo{{ModelId: "claude-opus-4-7", Name: "Opus 4.7"}},
}
models := initialSessionModelState(nil, configOptions, legacy)
if models == nil {
t.Fatal("initialSessionModelState returned nil")
}
if models.CurrentModelId != "gpt-5.5" {
t.Errorf("CurrentModelId = %q, want gpt-5.5 (typed must win over legacy)", models.CurrentModelId)
}
}
@@ -287,7 +403,7 @@ func TestResolveCurrentModelFromConfig_ComposesReasoningEffort(t *testing.T) {
{Type: "select", ID: "model", Category: "model", CurrentValue: "gpt-5.5"},
{Type: "select", ID: "reasoning_effort", Category: "thought_level", CurrentValue: reasoningEffortMedium},
}
available := []acp.ModelInfo{
available := []modelInfo{
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
}
@@ -303,7 +419,7 @@ func TestResolveCurrentModelFromConfig_PrefersReasoningModelWhenBaseAlsoAvailabl
{Type: "select", ID: "model", Category: "model", CurrentValue: "gpt-5.5"},
{Type: "select", ID: "reasoning_effort", Category: "thought_level", CurrentValue: reasoningEffortMedium},
}
available := []acp.ModelInfo{
available := []modelInfo{
{ModelId: "gpt-5.5", Name: "GPT-5.5"},
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
}
@@ -318,9 +434,9 @@ func TestResolveCurrentModelFromConfig_PrefersReasoningModelWhenBaseAlsoAvailabl
// when the agent populates CurrentModelId, we propagate it verbatim.
func TestEmitSessionModels_NonEmptyCurrentIDPreserved(t *testing.T) {
a := newTestAdapter()
models := &acp.SessionModelState{
models := &sessionModelState{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.ModelInfo{
AvailableModels: []modelInfo{
{ModelId: "claude-opus-4-7", Name: "Opus 4.7"},
},
}
@@ -340,7 +456,7 @@ func TestEmitSessionModels_NonEmptyCurrentIDPreserved(t *testing.T) {
func TestEmitSetModelEvent_EmitsSessionModelsWithCachedState(t *testing.T) {
a := newTestAdapter()
cachedModels := []acp.ModelInfo{
cachedModels := []modelInfo{
{ModelId: "claude-opus-4-7", Name: "Opus 4.7"},
{ModelId: "build-analyzer", Name: "Build Analyzer"},
}
@@ -397,7 +513,7 @@ func TestEmitSetModelEvent_RewritesSplitReasoningOptions(t *testing.T) {
{Name: "Medium", Value: reasoningEffortMedium},
{Name: "High", Value: reasoningEffortHigh},
}
cachedModels := []acp.ModelInfo{
cachedModels := []modelInfo{
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
{ModelId: "gpt-5.5/high", Name: "GPT-5.5 (high)"},
}
@@ -445,7 +561,7 @@ func TestEmitSetModelEvent_RewritesSplitReasoningOptionsWithSlashInBaseModel(t *
{Name: "Medium", Value: reasoningEffortMedium},
{Name: "High", Value: reasoningEffortHigh},
}
cachedModels := []acp.ModelInfo{
cachedModels := []modelInfo{
{ModelId: "vendor/gpt-5.5/medium", Name: "Vendor GPT-5.5 (medium)"},
{ModelId: "vendor/gpt-5.5/high", Name: "Vendor GPT-5.5 (high)"},
}
@@ -494,7 +610,7 @@ func TestEmitSetModelEvent_DoesNotSplitSlashModelWithoutReasoningSuffix(t *testi
{Name: "Medium", Value: reasoningEffortMedium},
{Name: "High", Value: reasoningEffortHigh},
}
cachedModels := []acp.ModelInfo{
cachedModels := []modelInfo{
{ModelId: "vendor/gpt-5.5", Name: "Vendor GPT-5.5"},
}
cachedConfig := []streams.ConfigOption{
@@ -592,7 +708,7 @@ func TestConfigOptionUpdate_ComposesReasoningEffortCurrentModel(t *testing.T) {
}
a.mu.Lock()
a.availableModels = []acp.ModelInfo{
a.availableModels = []modelInfo{
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
{ModelId: "gpt-5.5/high", Name: "GPT-5.5 (high)"},
}
@@ -631,3 +747,46 @@ func TestConfigOptionUpdate_ComposesReasoningEffortCurrentModel(t *testing.T) {
t.Errorf("event CurrentModelID = %q, want %q", ev.CurrentModelID, "gpt-5.5/high")
}
}
// TestFinalizeSetModel_MethodNoneEmitsNothing pins the contract that when
// applySessionModel returns MethodNone (agent supports neither
// session/set_config_option nor session/set_model), the adapter must NOT emit
// a session_models convergence event nor reset the cached context-window size,
// because no switch actually happened.
func TestFinalizeSetModel_MethodNoneEmitsNothing(t *testing.T) {
a := newTestAdapter()
cachedModels := []modelInfo{{ModelId: "claude-opus-4-7", Name: "Opus 4.7"}}
cachedConfig := []streams.ConfigOption{
{Type: "select", ID: "model", Name: "Model", CurrentValue: "old-model"},
}
a.finalizeSetModel(sessionmodel.MethodNone, "sess-1", "claude-opus-4-7", cachedModels, cachedConfig)
events := drainEvents(a)
for _, ev := range events {
if ev.Type == streams.EventTypeSessionModels {
t.Fatalf("expected no session_models event, got %+v", ev)
}
}
}
// TestFinalizeSetModel_RealMethodEmitsEvent pins the positive path: when
// applySessionModel reports a real RPC was used (MethodSetConfigOption or
// MethodSetModel), the adapter must emit a session_models event so the
// frontend converges on the new model.
func TestFinalizeSetModel_RealMethodEmitsEvent(t *testing.T) {
a := newTestAdapter()
cachedModels := []modelInfo{{ModelId: "claude-opus-4-7", Name: "Opus 4.7"}}
cachedConfig := []streams.ConfigOption{
{Type: "select", ID: "model", Name: "Model", CurrentValue: "old-model"},
}
a.finalizeSetModel(sessionmodel.MethodSetConfigOption, "sess-1", "claude-opus-4-7", cachedModels, cachedConfig)
ev := findSessionModelsEvent(t, drainEvents(a))
if ev.CurrentModelID != "claude-opus-4-7" {
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "claude-opus-4-7")
}
}
apps/backend/internal/agentctl/server/adapter/transport/acp/wakeup_concurrency_test.go
+3
View File
@@ -63,6 +63,9 @@ func (f *concurrencyFakeAgent) SetSessionConfigOption(_ context.Context, _ acp.S
func (f *concurrencyFakeAgent) SetSessionMode(_ context.Context, _ acp.SetSessionModeRequest) (acp.SetSessionModeResponse, error) {
return acp.SetSessionModeResponse{}, nil
}
func (f *concurrencyFakeAgent) Logout(_ context.Context, _ acp.LogoutRequest) (acp.LogoutResponse, error) {
return acp.LogoutResponse{}, nil
}
// setupConcurrencyFakeAgent wires an adapter to a blocking fake agent and
// registers cleanup immediately so early t.Fatal paths cannot leak pipes or
apps/backend/internal/agentctl/server/utility/acp_executor.go
+29 -23
View File
@@ -547,24 +547,15 @@ func buildInitProbeFields(initResp acp.InitializeResponse) *ProbeResponse {
// applySessionProbeFields populates models and modes from an ACP session/new response.
//
// The legacy `models`/`modes` fields were marked UNSTABLE in the ACP spec and
// newer agents (e.g. claude-agent-acp v0.42+) drop them in favour of the
// general `configOptions[]` carrier with `category: model | mode`. Read the
// legacy fields first, then fall back to configOptions when they are absent
// so both shapes work.
// As of acp-go-sdk v0.13.5 the legacy unstable `models` field on
// NewSessionResponse was removed upstream; model and mode selection are
// surfaced through the typed `configOptions[]` carrier with
// `category: model | mode`. The kdlbs fork restores read-only parsing of the
// top-level `models` field via acp.LegacyModels for agents that haven't
// migrated yet (auggie 0.29.x). The legacy `modes` field is still present on
// the SDK type and we keep populating from it for older agents.
func applySessionProbeFields(out *ProbeResponse, sessionResp acp.NewSessionResponse) {
out.ConfigOptions = probeConfigOptions(sessionResp.ConfigOptions)
if sessionResp.Models != nil {
out.CurrentModelID = string(sessionResp.Models.CurrentModelId)
for _, m := range sessionResp.Models.AvailableModels {
out.Models = append(out.Models, ProbeModel{
ID: string(m.ModelId),
Name: m.Name,
Description: derefString(m.Description),
Meta: m.Meta,
})
}
}
if sessionResp.Modes != nil {
out.CurrentModeID = string(sessionResp.Modes.CurrentModeId)
for _, m := range sessionResp.Modes.AvailableModes {
@@ -576,19 +567,34 @@ func applySessionProbeFields(out *ProbeResponse, sessionResp acp.NewSessionRespo
})
}
}
// Gate the fallback on the legacy field being absent (nil) rather than
// producing an empty slice. A non-nil `Models` with an empty
// `AvailableModels` is schema-valid and would otherwise mix sources:
// `CurrentModelID` set from the legacy field, then immediately
// overwritten by the configOptions fallback. Same for modes.
if sessionResp.Models == nil {
applyConfigOptionsAsModels(out, sessionResp.ConfigOptions)
applyConfigOptionsAsModels(out, sessionResp.ConfigOptions)
if len(out.Models) == 0 {
applyLegacyModelsFallback(out, sessionResp.LegacyModels)
}
if sessionResp.Modes == nil {
applyConfigOptionsAsModes(out, sessionResp.ConfigOptions)
}
}
// applyLegacyModelsFallback fills out.Models / out.CurrentModelID from the
// pre-v0.13.5 top-level `models` field still emitted by agents like
// auggie 0.29.x. Only invoked when typed configOptions[category=model] did
// not produce a model list, so the new surface always wins when present.
func applyLegacyModelsFallback(out *ProbeResponse, legacy *acp.LegacyModels) {
if legacy == nil || len(legacy.AvailableModels) == 0 {
return
}
out.CurrentModelID = legacy.CurrentModelId
for _, m := range legacy.AvailableModels {
out.Models = append(out.Models, ProbeModel{
ID: m.ModelId,
Name: m.Name,
Description: derefString(m.Description),
Meta: m.Meta,
})
}
}
func probeConfigOptions(opts []acp.SessionConfigOption) []ProbeConfigOption {
out := make([]ProbeConfigOption, 0, len(opts))
for _, opt := range opts {
apps/backend/internal/agentctl/server/utility/acp_executor_test.go
+55 -56
View File
@@ -251,81 +251,80 @@ func TestApplySessionProbeFields_FallsBackToConfigOptions(t *testing.T) {
}
}
// The legacy `models` field still wins when present so existing agents are
// unaffected; configOptions is only consulted as a fallback.
func TestApplySessionProbeFields_PrefersLegacyModelsField(t *testing.T) {
// auggie 0.29.x hasn't migrated to configOptions[category=model] and still
// emits the pre-v0.13.5 top-level `models` field. The kdlbs SDK fork keeps
// parsing it as acp.LegacyModels; the probe must fall through to that surface
// so the inference-agents endpoint still surfaces auggie's model list.
func TestApplySessionProbeFields_FallsBackToLegacyModels(t *testing.T) {
t.Parallel()
modelCat := acp.SessionConfigOptionCategoryModel
opusDesc := "Great for complex coding"
resp := acp.NewSessionResponse{
Models: &acp.SessionModelState{
CurrentModelId: "legacy",
AvailableModels: []acp.ModelInfo{
{ModelId: "legacy", Name: "Legacy"},
LegacyModels: &acp.LegacyModels{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.LegacyModelInfo{
{ModelId: "claude-opus-4-7", Name: "Opus 4.7", Description: &opusDesc},
{ModelId: "claude-sonnet-4-5", Name: "Sonnet 4.5"},
},
},
ConfigOptions: []acp.SessionConfigOption{
{Select: &acp.SessionConfigOptionSelect{
Category: &modelCat,
CurrentValue: "fallback",
Options: acp.SessionConfigSelectOptions{Ungrouped: &acp.SessionConfigSelectOptionsUngrouped{
{Value: "fallback", Name: "Fallback"},
}},
Type: "select",
}},
},
}
out := &ProbeResponse{}
applySessionProbeFields(out, resp)
if got, want := out.CurrentModelID, "legacy"; got != want {
if got, want := out.CurrentModelID, "claude-opus-4-7"; got != want {
t.Fatalf("CurrentModelID = %q, want %q", got, want)
}
if got, want := len(out.Models), 2; got != want {
t.Fatalf("len(Models) = %d, want %d", got, want)
}
if got, want := out.Models[0].ID, "claude-opus-4-7"; got != want {
t.Fatalf("Models[0].ID = %q, want %q", got, want)
}
if got, want := out.Models[0].Description, "Great for complex coding"; got != want {
t.Fatalf("Models[0].Description = %q, want %q", got, want)
}
}
// When both surfaces are present the typed configOptions list wins — the
// legacy field is only a fallback for unmigrated agents.
func TestApplySessionProbeFields_TypedConfigOptionsBeatLegacyModels(t *testing.T) {
t.Parallel()
modelCat := acp.SessionConfigOptionCategoryModel
resp := acp.NewSessionResponse{
ConfigOptions: []acp.SessionConfigOption{
{Select: &acp.SessionConfigOptionSelect{
Category: &modelCat,
CurrentValue: "opus",
Id: "model",
Name: "Model",
Options: acp.SessionConfigSelectOptions{Ungrouped: &acp.SessionConfigSelectOptionsUngrouped{
{Value: "opus", Name: "Opus"},
}},
Type: "select",
}},
},
LegacyModels: &acp.LegacyModels{
CurrentModelId: "claude-opus-4-7",
AvailableModels: []acp.LegacyModelInfo{{ModelId: "claude-opus-4-7", Name: "Opus 4.7"}},
},
}
out := &ProbeResponse{}
applySessionProbeFields(out, resp)
if got, want := out.CurrentModelID, "opus"; got != want {
t.Fatalf("CurrentModelID = %q, want %q (typed configOptions must win)", got, want)
}
if got, want := len(out.Models), 1; got != want {
t.Fatalf("len(Models) = %d, want %d", got, want)
}
if got, want := out.Models[0].ID, "legacy"; got != want {
if got, want := out.Models[0].ID, "opus"; got != want {
t.Fatalf("Models[0].ID = %q, want %q", got, want)
}
}
// A non-nil `Models` struct with an empty `AvailableModels` slice is
// schema-valid. The fallback must NOT fire in that case — otherwise
// `CurrentModelID` set from the legacy field gets clobbered by the
// configOptions value, mixing sources.
func TestApplySessionProbeFields_LegacyEmptyModelsBlocksFallback(t *testing.T) {
t.Parallel()
modelCat := acp.SessionConfigOptionCategoryModel
resp := acp.NewSessionResponse{
Models: &acp.SessionModelState{
CurrentModelId: "legacy-current",
AvailableModels: nil,
},
ConfigOptions: []acp.SessionConfigOption{
{Select: &acp.SessionConfigOptionSelect{
Category: &modelCat,
CurrentValue: "fallback-current",
Options: acp.SessionConfigSelectOptions{Ungrouped: &acp.SessionConfigSelectOptionsUngrouped{
{Value: "fallback-current", Name: "Fallback"},
}},
Type: "select",
}},
},
}
out := &ProbeResponse{}
applySessionProbeFields(out, resp)
if got, want := out.CurrentModelID, "legacy-current"; got != want {
t.Fatalf("CurrentModelID = %q, want %q (legacy must win, fallback must not fire)", got, want)
}
if len(out.Models) != 0 {
t.Fatalf("Models = %+v, want empty (fallback should be skipped when legacy field is non-nil)", out.Models)
}
}
// Grouped select-option payloads are flattened group-by-group so the
// fallback works regardless of whether the agent groups its options.
func TestApplySessionProbeFields_FlattensGroupedConfigOptions(t *testing.T) {
apps/backend/internal/agentctl/server/utility/model_apply_test.go
+52 -3
View File
@@ -18,7 +18,7 @@ func (f *fakeModelConn) SetSessionConfigOption(_ context.Context, req acp.SetSes
}
func (f *fakeModelConn) UnstableSetSessionModel(_ context.Context, req acp.UnstableSetSessionModelRequest) (acp.UnstableSetSessionModelResponse, error) {
f.calls = append(f.calls, "model:"+string(req.ModelId))
f.calls = append(f.calls, "legacy:"+string(req.SessionId)+":"+req.ModelId)
return acp.UnstableSetSessionModelResponse{}, nil
}
@@ -47,7 +47,13 @@ func TestApplySessionModel_UsesConfigOptionWhenSessionAdvertisesModelOption(t *t
}
}
func TestApplySessionModel_UsesSetModelWithoutModelConfigOption(t *testing.T) {
// TestApplySessionModel_FallsBackToLegacySetModel pins that when the session
// advertises no model-shaped config option, the dispatcher falls through to
// the pre-v0.13.5 session/set_model RPC restored by the kdlbs acp-go-sdk
// fork. This keeps model switching working for unmigrated agents like
// auggie 0.29.x, which surface their models via the legacy top-level
// `models` field on session/new instead of a typed config option.
func TestApplySessionModel_FallsBackToLegacySetModel(t *testing.T) {
t.Parallel()
conn := &fakeModelConn{}
@@ -59,8 +65,51 @@ func TestApplySessionModel_UsesSetModelWithoutModelConfigOption(t *testing.T) {
if method != "session/set_model" {
t.Fatalf("method = %q, want session/set_model", method)
}
wantCalls := []string{"model:legacy-model"}
wantCalls := []string{"legacy:sess-1:legacy-model"}
if !reflect.DeepEqual(conn.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", conn.calls, wantCalls)
}
}
// TestApplySessionModel_NoOpWhenAgentSupportsNeitherSurface pins that when
// the session has no model-shaped config option AND the legacy
// session/set_model RPC returns JSON-RPC -32601 (method not found), the
// dispatcher treats the request as a clean no-op (empty method, nil error).
// This is how agents that support no model-selection surface at all signal
// "I don't support model selection".
func TestApplySessionModel_NoOpWhenAgentSupportsNeitherSurface(t *testing.T) {
t.Parallel()
conn := &fakeModelConnLegacyErr{
err: acp.NewMethodNotFound(acp.LegacyAgentMethodSessionSetModel),
}
method, err := applySessionModel(context.Background(), conn, "sess-1", "legacy-model", nil)
if err != nil {
t.Fatalf("applySessionModel() error = %v, want nil", err)
}
if method != "" {
t.Fatalf("method = %q, want empty (MethodNone)", method)
}
wantCalls := []string{"legacy:sess-1:legacy-model"}
if !reflect.DeepEqual(conn.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", conn.calls, wantCalls)
}
}
// fakeModelConnLegacyErr returns a configurable error from the legacy
// session/set_model RPC. Used to exercise the -32601 no-op path.
type fakeModelConnLegacyErr struct {
err error
calls []string
}
func (f *fakeModelConnLegacyErr) SetSessionConfigOption(_ context.Context, req acp.SetSessionConfigOptionRequest) (acp.SetSessionConfigOptionResponse, error) {
f.calls = append(f.calls, "config:"+string(req.ValueId.ConfigId)+":"+string(req.ValueId.Value))
return acp.SetSessionConfigOptionResponse{}, nil
}
func (f *fakeModelConnLegacyErr) UnstableSetSessionModel(_ context.Context, req acp.UnstableSetSessionModelRequest) (acp.UnstableSetSessionModelResponse, error) {
f.calls = append(f.calls, "legacy:"+string(req.SessionId)+":"+req.ModelId)
return acp.UnstableSetSessionModelResponse{}, f.err
}
apps/backend/internal/agentctl/sessionmodel/sessionmodel.go
+39 -17
View File
@@ -17,8 +17,13 @@ type Method string
const (
MethodNone Method = ""
MethodSetModel Method = "session/set_model"
MethodSetConfigOption Method = "session/set_config_option"
// MethodSetModel is the pre-v0.13.5 unstable session/set_model RPC.
// Used as a fallback for agents (e.g. auggie 0.29.x) that surface their
// model list via the legacy top-level `models` field on session/new
// instead of SessionConfigOption(category="model"). The RPC is provided
// by the kdlbs acp-go-sdk fork (ClientSideConnection.UnstableSetSessionModel).
MethodSetModel Method = "session/set_model"
)
// ConfigOption is the subset of ACP session config options needed to decide
@@ -35,16 +40,23 @@ type Request struct {
ConfigOptions []ConfigOption
}
// Applier performs the actual ACP calls. Implementations wrap either the ACP
// Applier performs the actual ACP call. Implementations wrap either the ACP
// SDK connection or the agentctl websocket client.
type Applier interface {
SetConfigOption(ctx context.Context, sessionID, configID, value string) error
SetModel(ctx context.Context, sessionID, modelID string) error
// SetModelLegacy issues the pre-v0.13.5 unstable session/set_model RPC.
// Used only when the session advertises no model-shaped config option;
// implementations should return a JSON-RPC -32601 error (recognized by
// IsMethodNotFound) when the agent doesn't implement the legacy surface,
// so Apply can treat the model change as a clean no-op.
SetModelLegacy(ctx context.Context, sessionID, modelID string) error
}
// SDKConn is the subset of the ACP SDK connection used to apply model changes.
type SDKConn interface {
SetSessionConfigOption(context.Context, acp.SetSessionConfigOptionRequest) (acp.SetSessionConfigOptionResponse, error)
// UnstableSetSessionModel is the legacy session/set_model RPC restored by
// the kdlbs acp-go-sdk fork for compatibility with unmigrated agents.
UnstableSetSessionModel(context.Context, acp.UnstableSetSessionModelRequest) (acp.UnstableSetSessionModelResponse, error)
}
@@ -64,10 +76,10 @@ func (a SDKApplier) SetConfigOption(ctx context.Context, sessionID, configID, va
return err
}
func (a SDKApplier) SetModel(ctx context.Context, sessionID, modelID string) error {
func (a SDKApplier) SetModelLegacy(ctx context.Context, sessionID, modelID string) error {
_, err := a.Conn.UnstableSetSessionModel(ctx, acp.UnstableSetSessionModelRequest{
SessionId: acp.SessionId(sessionID),
ModelId: acp.UnstableModelId(modelID),
ModelId: modelID,
})
return err
}
@@ -92,27 +104,37 @@ func ApplySDKFromACP(
})
}
// Apply chooses the model-switching mechanism supported by the session. Agents
// that expose a model-shaped config option (Codex, Cursor, recent Claude) are
// configured through session/set_config_option. If that RPC is not implemented,
// we fall back to the older unstable session/set_model call.
// Apply selects the model by routing through the typed
// session/set_config_option RPC when the session advertises a model-shaped
// config option. When that option is absent — or when the modern RPC returns
// JSON-RPC -32601 because a partially-migrated agent advertises the option
// without implementing the handler — falls through to the legacy
// session/set_model RPC. A -32601 from the legacy call is treated as the
// agent declaring "I don't support model selection", and Apply returns
// MethodNone with no error so callers can no-op cleanly.
func Apply(ctx context.Context, applier Applier, req Request) (Method, error) {
if req.ModelID == "" {
return MethodNone, nil
}
if configID, ok := modelConfigID(req.ConfigOptions); ok {
if err := applier.SetConfigOption(ctx, req.SessionID, configID, req.ModelID); err != nil {
if !IsMethodNotFound(err) {
return MethodSetConfigOption, err
}
} else {
err := applier.SetConfigOption(ctx, req.SessionID, configID, req.ModelID)
if err == nil {
return MethodSetConfigOption, nil
}
if !IsMethodNotFound(err) {
return MethodSetConfigOption, err
}
// Agent advertises the typed option but its handler isn't wired up
// (partial migration); fall through to the legacy RPC below.
}
if err := applier.SetModel(ctx, req.SessionID, req.ModelID); err != nil {
return MethodSetModel, err
err := applier.SetModelLegacy(ctx, req.SessionID, req.ModelID)
if err == nil {
return MethodSetModel, nil
}
return MethodSetModel, nil
if IsMethodNotFound(err) {
return MethodNone, nil
}
return MethodSetModel, err
}
// FromACP converts typed ACP SDK options to the shared strategy shape.
apps/backend/internal/agentctl/sessionmodel/sessionmodel_test.go
+92 -34
View File
@@ -11,7 +11,7 @@ import (
type fakeApplier struct {
configErr error
modelErr error
legacyErr error
calls []string
}
@@ -20,12 +20,12 @@ func (f *fakeApplier) SetConfigOption(_ context.Context, _ string, configID, val
return f.configErr
}
func (f *fakeApplier) SetModel(_ context.Context, _, modelID string) error {
f.calls = append(f.calls, "model:"+modelID)
return f.modelErr
func (f *fakeApplier) SetModelLegacy(_ context.Context, sessionID, modelID string) error {
f.calls = append(f.calls, "legacy:"+sessionID+":"+modelID)
return f.legacyErr
}
func TestApply_PrefersModelConfigOption(t *testing.T) {
func TestApply_UsesModelConfigOption(t *testing.T) {
t.Parallel()
applier := &fakeApplier{}
@@ -50,38 +50,17 @@ func TestApply_PrefersModelConfigOption(t *testing.T) {
}
}
func TestApply_FallsBackToSetModelWhenConfigOptionMethodMissing(t *testing.T) {
t.Parallel()
applier := &fakeApplier{configErr: acp.NewMethodNotFound(acp.AgentMethodSessionSetConfigOption)}
method, err := Apply(context.Background(), applier, Request{
SessionID: "sess-1",
ModelID: "claude-opus-4-8",
ConfigOptions: []ConfigOption{{
ID: "model",
Category: "model",
}},
})
if err != nil {
t.Fatalf("Apply() error = %v", err)
}
if method != MethodSetModel {
t.Fatalf("method = %q, want %q", method, MethodSetModel)
}
wantCalls := []string{"config:model:claude-opus-4-8", "model:claude-opus-4-8"}
if !reflect.DeepEqual(applier.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", applier.calls, wantCalls)
}
}
func TestApply_UsesSetModelWhenNoModelConfigOption(t *testing.T) {
// TestApply_FallsBackToLegacySetModel verifies the legacy fallback path:
// when the session advertises no model-shaped config option, Apply issues
// the pre-v0.13.5 session/set_model RPC restored by the kdlbs fork. This
// keeps model switching working for unmigrated agents (e.g. auggie 0.29.x).
func TestApply_FallsBackToLegacySetModel(t *testing.T) {
t.Parallel()
applier := &fakeApplier{}
method, err := Apply(context.Background(), applier, Request{
SessionID: "sess-1",
ModelID: "legacy-model",
ModelID: "claude-opus-4-7",
ConfigOptions: []ConfigOption{{ID: "mode", Category: "mode"}},
})
@@ -91,13 +70,62 @@ func TestApply_UsesSetModelWhenNoModelConfigOption(t *testing.T) {
if method != MethodSetModel {
t.Fatalf("method = %q, want %q", method, MethodSetModel)
}
wantCalls := []string{"model:legacy-model"}
wantCalls := []string{"legacy:sess-1:claude-opus-4-7"}
if !reflect.DeepEqual(applier.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", applier.calls, wantCalls)
}
}
func TestApply_DoesNotFallbackForInvalidConfigOptionValue(t *testing.T) {
// TestApply_LegacyMethodNotFoundIsNoOp pins that when the legacy
// session/set_model RPC returns JSON-RPC -32601, Apply treats the request as
// a clean no-op (MethodNone, nil error). This is how agents that support
// neither surface (no config option AND no legacy RPC) signal "I don't
// support model selection".
func TestApply_LegacyMethodNotFoundIsNoOp(t *testing.T) {
t.Parallel()
notFound := acp.NewMethodNotFound(acp.LegacyAgentMethodSessionSetModel)
applier := &fakeApplier{legacyErr: notFound}
method, err := Apply(context.Background(), applier, Request{
SessionID: "sess-1",
ModelID: "some-model",
ConfigOptions: []ConfigOption{{ID: "mode", Category: "mode"}},
})
if err != nil {
t.Fatalf("Apply() error = %v, want nil", err)
}
if method != MethodNone {
t.Fatalf("method = %q, want %q", method, MethodNone)
}
wantCalls := []string{"legacy:sess-1:some-model"}
if !reflect.DeepEqual(applier.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", applier.calls, wantCalls)
}
}
// TestApply_PropagatesLegacySetModelError pins that non-32601 errors from
// the legacy RPC bubble up unchanged with MethodSetModel attribution.
func TestApply_PropagatesLegacySetModelError(t *testing.T) {
t.Parallel()
invalid := acp.NewInvalidParams(map[string]any{"message": "Unknown model"})
applier := &fakeApplier{legacyErr: invalid}
method, err := Apply(context.Background(), applier, Request{
SessionID: "sess-1",
ModelID: "bogus",
ConfigOptions: []ConfigOption{{ID: "mode", Category: "mode"}},
})
if !errors.Is(err, invalid) {
t.Fatalf("Apply() error = %v, want %v", err, invalid)
}
if method != MethodSetModel {
t.Fatalf("method = %q, want %q", method, MethodSetModel)
}
}
func TestApply_PropagatesSetConfigOptionError(t *testing.T) {
t.Parallel()
invalid := acp.NewInvalidParams(map[string]any{"message": "Invalid model value"})
@@ -122,3 +150,33 @@ func TestApply_DoesNotFallbackForInvalidConfigOptionValue(t *testing.T) {
t.Fatalf("calls = %#v, want %#v", applier.calls, wantCalls)
}
}
// TestApply_SetConfigOptionMethodNotFoundFallsBackToLegacy pins the
// partial-migration fallthrough: when the session advertises a model-shaped
// config option but the agent answers session/set_config_option with -32601,
// Apply falls through to the legacy session/set_model RPC instead of
// surfacing the error.
func TestApply_SetConfigOptionMethodNotFoundFallsBackToLegacy(t *testing.T) {
t.Parallel()
applier := &fakeApplier{configErr: acp.NewMethodNotFound(acp.AgentMethodSessionSetConfigOption)}
method, err := Apply(context.Background(), applier, Request{
SessionID: "sess-1",
ModelID: "claude-opus-4-7",
ConfigOptions: []ConfigOption{{
ID: "model",
Category: "model",
}},
})
if err != nil {
t.Fatalf("Apply() error = %v, want nil", err)
}
if method != MethodSetModel {
t.Fatalf("method = %q, want %q", method, MethodSetModel)
}
wantCalls := []string{"config:model:claude-opus-4-7", "legacy:sess-1:claude-opus-4-7"}
if !reflect.DeepEqual(applier.calls, wantCalls) {
t.Fatalf("calls = %#v, want %#v", applier.calls, wantCalls)
}
}
apps/backend/internal/orchestrator/event_handlers_agent.go
+17
View File
@@ -907,6 +907,23 @@ func (s *Service) handleAgentStopped(ctx context.Context, data watcher.AgentEven
// on that self-inflicted stop would abort the retry. The loop is freed on
// ready/completed (success), cancel, and exhaustion instead.
// Drop stopped events that belong to a previous (rotated) execution. The
// session might already be running a fresh resume cycle; flipping its state
// to CANCELLED based on the corpse of the prior execution poisons the
// recovery (the new cycle's session/load succeeds against a session that
// looks terminal to the rest of the system). Mirrors the rotation guard in
// handleAgentCompleted.
if s.agentManager != nil && data.AgentExecutionID != "" && data.SessionID != "" {
if liveExecID, _ := s.agentManager.GetExecutionIDForSession(ctx, data.SessionID); liveExecID != "" && liveExecID != data.AgentExecutionID {
s.logger.Info("ignoring agent.stopped for non-active (rotated) execution",
zap.String("task_id", data.TaskID),
zap.String("session_id", data.SessionID),
zap.String("event_execution_id", data.AgentExecutionID),
zap.String("live_execution_id", liveExecID))
return
}
}
// Complete the current turn if there is one
s.completeTurnForSession(ctx, data.SessionID)
apps/backend/internal/orchestrator/event_handlers_test.go
+38
View File
@@ -1650,6 +1650,44 @@ func TestHandleAgentStopped_PreservesRecoveryState(t *testing.T) {
models.TaskSessionStateIdle, updated.State)
}
})
// Rotated-execution regression: a previous cycle's agent.stopped event must
// not flip the session to CANCELLED when a fresh resume cycle has already
// taken over (executors_running.agent_execution_id rotated). Without this
// guard, an intermittent ACP notification-queue overflow on cycle 1
// produces a late stopped event whose state mutation poisons the in-
// progress cycle 2 — the user sees the task transition to FAILED even
// though cycle 2 loaded the session cleanly. See discussion in the cycle
// 1/2/3 analysis (task a263caf5).
t.Run("drops stopped events from rotated executions", func(t *testing.T) {
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
// Make sure session is in RUNNING — the state we'd clobber if the
// guard didn't fire.
session, _ := repo.GetTaskSession(ctx, "s1")
session.State = models.TaskSessionStateRunning
_ = repo.UpdateTaskSession(ctx, session)
// Live execution is exec-2 (cycle 2). The stale event below carries
// exec-1 (cycle 1).
seedExecutorRunning(t, repo, "s1", "t1", "exec-2")
agentMgr := &mockAgentManager{repoForExecutionLookup: repo}
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr)
svc.handleAgentStopped(ctx, watcher.AgentEventData{
TaskID: "t1",
SessionID: "s1",
AgentExecutionID: "exec-1",
})
updated, _ := repo.GetTaskSession(ctx, "s1")
if updated.State != models.TaskSessionStateRunning {
t.Errorf("expected state to remain %q (rotated event ignored), got %q",
models.TaskSessionStateRunning, updated.State)
}
})
}
// waitForStopCall polls until the mock agent manager has received at least one
apps/web/components/model-config-selector.tsx
+9 -8
View File
@@ -1,6 +1,6 @@
"use client";
import { memo, useCallback, useState } from "react";
import { memo, useState } from "react";
import { IconCheck, IconChevronDown } from "@tabler/icons-react";
import { cn } from "@/lib/utils";
@@ -207,13 +207,14 @@ export const ModelConfigSelector = memo(function ModelConfigSelector({
const currentModelValue = modelConfig?.currentValue || currentModel || "";
const label = resolveTriggerLabel(modelOptions, currentModel, modelConfig, configOptions);
const onModelSelect = useCallback(
(value: string) => {
if (!value) return;
onModelChange(value);
},
[onModelChange],
);
const hasExtraConfigOptions = extraConfigOptions.length > 0;
const onModelSelect = (value: string) => {
if (!value) return;
onModelChange(value);
if (!hasExtraConfigOptions) {
setOpen(false);
}
};
const triggerClassName =
variant === "compact"
apps/web/components/task/model-selector.tsx
+104 -41
View File
@@ -1,6 +1,6 @@
"use client";
import { memo, useCallback, useMemo } from "react";
import { memo, useCallback, useMemo, useRef } from "react";
import {
configOptionToModelOptions,
@@ -11,6 +11,7 @@ import {
usableConfigOptions,
} from "@/components/model-config-selector";
import { useAppStore } from "@/components/state-provider";
import { useToast } from "@/components/toast-provider";
import { useAvailableAgents } from "@/hooks/domains/settings/use-available-agents";
import { useSettingsData } from "@/hooks/domains/settings/use-settings-data";
import { setSessionConfigOption, setSessionModel } from "@/lib/api/domains/session-api";
@@ -20,6 +21,12 @@ import type {
SessionModelEntry,
} from "@/lib/state/slices/session-runtime/types";
type SessionModelsEntry = {
currentModelId: string;
models: SessionModelEntry[];
configOptions: ConfigOptionEntry[];
};
type ModelSelectorProps = {
sessionId: string | null;
};
@@ -127,6 +134,99 @@ function resolveAvailableModels({
return resolveStaticModels(settingsAgents, profileId, availableAgents);
}
function describeError(err: unknown): string {
return err instanceof Error ? err.message : "Unknown error";
}
/** Builds model/config change handlers with optimistic update + error toast + revert. */
function useModelChangeHandlers(
configOptions: SelectConfigOption[],
sessionModelsData: SessionModelsEntry | undefined,
) {
const activeModels = useAppStore((state) => state.activeModel.bySessionId);
const setActiveModel = useAppStore((state) => state.setActiveModel);
const setSessionModels = useAppStore((state) => state.setSessionModels);
const { toast } = useToast();
// Per-session monotonic request id so a stale failure doesn't clobber a
// newer successful selection (rapid A -> B -> C where B fails).
const latestReqId = useRef<Record<string, number>>({});
const updateLocalConfig = useCallback(
(sid: string, configId: string, value: string) => {
if (!sessionModelsData) return;
setSessionModels(sid, {
...sessionModelsData,
currentModelId: nextCurrentModelId(sessionModelsData, configId, value),
configOptions: updateConfigOptionValue(sessionModelsData.configOptions, configId, value),
});
},
[sessionModelsData, setSessionModels],
);
const onFail = useCallback(
(
sid: string,
reqId: number,
previousActive: string,
previousModels: SessionModelsEntry | undefined,
) =>
(err: unknown) => {
if (latestReqId.current[sid] !== reqId) return;
console.error("[ModelSelector] model change failed:", err);
setActiveModel(sid, previousActive);
if (previousModels) setSessionModels(sid, previousModels);
toast({
title: "Failed to change model",
description: describeError(err),
variant: "error",
});
},
[setActiveModel, setSessionModels, toast],
);
const nextReqId = useCallback((sid: string) => {
const id = (latestReqId.current[sid] ?? 0) + 1;
latestReqId.current[sid] = id;
return id;
}, []);
const handleModelChange = useCallback(
(sid: string, modelId: string) => {
const reqId = nextReqId(sid);
const fail = onFail(sid, reqId, activeModels[sid] ?? "", sessionModelsData);
setActiveModel(sid, modelId);
const modelConfig = configOptions.find(isModelConfigOption);
if (modelConfig) {
updateLocalConfig(sid, modelConfig.id, modelId);
setSessionConfigOption(sid, modelConfig.id, modelId).catch(fail);
return;
}
setSessionModel(sid, modelId).catch(fail);
},
[
activeModels,
configOptions,
nextReqId,
onFail,
sessionModelsData,
setActiveModel,
updateLocalConfig,
],
);
const handleConfigChange = useCallback(
(sid: string, configId: string, value: string) => {
const reqId = nextReqId(sid);
const fail = onFail(sid, reqId, activeModels[sid] ?? "", sessionModelsData);
updateLocalConfig(sid, configId, value);
setSessionConfigOption(sid, configId, value).catch(fail);
},
[activeModels, nextReqId, onFail, sessionModelsData, updateLocalConfig],
);
return { handleModelChange, handleConfigChange };
}
/** Resolves available models, config options and current model from store state. */
function useModelSelectorState(sessionId: string | null) {
useSettingsData(true);
@@ -134,8 +234,6 @@ function useModelSelectorState(sessionId: string | null) {
const settingsAgents = useAppStore((state) => state.settingsAgents.items);
const taskSessions = useAppStore((state) => state.taskSessions.items);
const activeModels = useAppStore((state) => state.activeModel.bySessionId);
const setActiveModel = useAppStore((state) => state.setActiveModel);
const setSessionModels = useAppStore((state) => state.setSessionModels);
const { items: availableAgents } = useAvailableAgents();
const sessionModelsData = useAppStore((state) =>
sessionId ? state.sessionModels.bySessionId[sessionId] : undefined,
@@ -170,44 +268,9 @@ function useModelSelectorState(sessionId: string | null) {
);
const modelOptions = buildModelOptions(availableModels, currentModel);
const updateLocalConfig = useCallback(
(sid: string, configId: string, value: string) => {
if (!sessionModelsData) return;
setSessionModels(sid, {
...sessionModelsData,
currentModelId: nextCurrentModelId(sessionModelsData, configId, value),
configOptions: updateConfigOptionValue(sessionModelsData.configOptions, configId, value),
});
},
[sessionModelsData, setSessionModels],
);
const handleModelChange = useCallback(
(sid: string, modelId: string) => {
setActiveModel(sid, modelId);
const modelConfig = configOptions.find(isModelConfigOption);
if (modelConfig) {
updateLocalConfig(sid, modelConfig.id, modelId);
setSessionConfigOption(sid, modelConfig.id, modelId).catch((err) => {
console.error("[ModelSelector] set-config-option API failed:", err);
});
return;
}
setSessionModel(sid, modelId).catch((err) => {
console.error("[ModelSelector] set-model API failed:", err);
});
},
[configOptions, setActiveModel, updateLocalConfig],
);
const handleConfigChange = useCallback(
(sid: string, configId: string, value: string) => {
updateLocalConfig(sid, configId, value);
setSessionConfigOption(sid, configId, value).catch((err) => {
console.error("[ModelSelector] set-config-option API failed:", err);
});
},
[updateLocalConfig],
const { handleModelChange, handleConfigChange } = useModelChangeHandlers(
configOptions,
sessionModelsData,
);
return { currentModel, modelOptions, configOptions, handleModelChange, handleConfigChange };
apps/web/e2e/tests/chat/model-selector-error.spec.ts
+145
View File
@@ -0,0 +1,145 @@
import { test, expect } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
/**
* Verifies the chat-input model selector's failure path:
* when the backend rejects a model change, the UI shows a toast carrying the
* backend error message and the trigger label reverts to the previous model.
*
* mock-agent exposes "model" as a config option, so changing the model goes
* through POST /set-config-option. Agents without a model config option use
* POST /set-model — both routes are stubbed for resilience.
*/
test.describe("Chat model selector — RPC failure", () => {
test("shows error toast and reverts trigger label when model change fails", async ({
testPage,
apiClient,
seedData,
}) => {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Model Selector Error Test",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
const trigger = testPage.getByRole("button", { name: "Session model settings" });
await expect(trigger).toBeVisible({ timeout: 15_000 });
// mock-agent ships with model="mock-fast" + effort="medium", so the
// composite trigger label starts as "Mock Fast / Medium".
await expect(trigger).toContainText("Mock Fast", { timeout: 15_000 });
const backendErrorMessage = "mock backend rejected model change";
const fail = (route: import("@playwright/test").Route) =>
route.fulfill({
status: 500,
contentType: "application/json",
body: JSON.stringify({ error: backendErrorMessage }),
});
await testPage.route("**/set-model", fail);
await testPage.route("**/set-config-option", fail);
await trigger.click();
const smartRow = testPage.getByRole("option", { name: /Mock Smart/ });
await expect(smartRow).toBeVisible({ timeout: 5_000 });
await smartRow.click();
const toast = testPage
.getByTestId("toast-message")
.filter({ hasText: "Failed to change model" });
await expect(toast).toBeVisible({ timeout: 5_000 });
await expect(toast).toContainText(backendErrorMessage);
// After revert the trigger label should match the original (model + extras).
await expect(trigger).toContainText("Mock Fast", { timeout: 5_000 });
await expect(trigger).not.toContainText("Mock Smart");
});
test("stale failure does not revert a newer successful change", async ({
testPage,
apiClient,
seedData,
}) => {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Model Selector Race Test",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
const trigger = testPage.getByRole("button", { name: "Session model settings" });
await expect(trigger).toContainText("Mock Fast", { timeout: 15_000 });
// Hold the first config-option request open so we can fire a second one
// before the first settles. Resolve the first as a 500 after the second
// has already been issued — the stale rejection must NOT clobber the
// newer optimistic state.
let releaseFirst: (() => void) | null = null;
const firstHeld = new Promise<void>((resolve) => {
releaseFirst = resolve;
});
// Resolves once the first (stale) request has fully completed its
// route.fulfill, so the test can deterministically await propagation
// instead of using a fixed sleep.
let firstSettled: (() => void) | null = null;
const firstSettledPromise = new Promise<void>((resolve) => {
firstSettled = resolve;
});
let callCount = 0;
await testPage.route("**/set-config-option", async (route) => {
callCount += 1;
if (callCount === 1) {
await firstHeld;
await route.fulfill({
status: 500,
contentType: "application/json",
body: JSON.stringify({ error: "stale failure" }),
});
firstSettled?.();
return;
}
return route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({ ok: true }),
});
});
await trigger.click();
await testPage.getByRole("option", { name: /Mock Smart/ }).click();
// Re-open and pick again — second request will succeed. mock-agent only
// ships two models (Mock Fast / Mock Smart), so we go back to Mock Fast.
await trigger.click();
await testPage.getByRole("option", { name: /Mock Fast/ }).click();
// Now release the first (stale) request — its 500 rejection should be
// swallowed (no toast).
releaseFirst?.();
await firstSettledPromise;
await expect(testPage.getByTestId("toast-message")).toHaveCount(0);
// Trigger should still reflect the newer (successful) selection.
await expect(trigger).toContainText("Mock Fast", { timeout: 5_000 });
});
});