fix(agentctl): preserve codex reasoning model ids (#1296)
This commit is contained in:
@@ -39,6 +39,16 @@ const (
|
||||
// configOptionIDModel is the well-known ConfigOption ID/Category value used
|
||||
// by ACP agents to surface the active model as a selectable option.
|
||||
configOptionIDModel = "model"
|
||||
|
||||
// Codex surfaces reasoning effort as a separate config option while its
|
||||
// availableModels list uses model/effort IDs such as gpt-5.5/medium.
|
||||
configOptionIDReasoningEffort = "reasoning_effort"
|
||||
configOptionCategoryThoughtLevel = "thought_level"
|
||||
|
||||
reasoningEffortLow = "low"
|
||||
reasoningEffortMedium = "medium"
|
||||
reasoningEffortHigh = "high"
|
||||
reasoningEffortXHigh = "xhigh"
|
||||
)
|
||||
|
||||
// wakeupPromptTimeout bounds how long a synthetic wakeup prompt can run.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter/transport/shared"
|
||||
@@ -378,7 +379,7 @@ func (a *Adapter) emitSessionModels(sessionID string, models *acp.SessionModelSt
|
||||
// CurrentModelId nor a configOption surface a value, emit empty and let
|
||||
// the frontend fall through to its profile/snapshot resolution.
|
||||
if currentModelID == "" {
|
||||
currentModelID = resolveCurrentModelFromConfig(configOptions)
|
||||
currentModelID = resolveCurrentModelFromConfig(configOptions, models.AvailableModels)
|
||||
}
|
||||
|
||||
// Cache config options so emitSetModelEvent can include them in the
|
||||
@@ -422,9 +423,19 @@ func (a *Adapter) emitSetModelEvent(sessionID, modelID string, cachedModels []ac
|
||||
// deep copy to avoid aliasing the caller's backing array.
|
||||
outConfig = make([]streams.ConfigOption, len(cachedConfig))
|
||||
copy(outConfig, cachedConfig)
|
||||
baseModelID, reasoningEffort, splitReasoningModel := splitReasoningModelID(modelID, outConfig)
|
||||
for i := range outConfig {
|
||||
if outConfig[i].ID == configOptionIDModel || outConfig[i].Category == configOptionIDModel {
|
||||
outConfig[i].CurrentValue = modelID
|
||||
if splitReasoningModel {
|
||||
outConfig[i].CurrentValue = baseModelID
|
||||
} else {
|
||||
outConfig[i].CurrentValue = modelID
|
||||
}
|
||||
}
|
||||
if splitReasoningModel &&
|
||||
(outConfig[i].ID == configOptionIDReasoningEffort ||
|
||||
outConfig[i].Category == configOptionCategoryThoughtLevel) {
|
||||
outConfig[i].CurrentValue = reasoningEffort
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -443,13 +454,82 @@ func (a *Adapter) emitSetModelEvent(sessionID, modelID string, cachedModels []ac
|
||||
}
|
||||
|
||||
// resolveCurrentModelFromConfig extracts current model ID from configOptions.
|
||||
func resolveCurrentModelFromConfig(options []streams.ConfigOption) string {
|
||||
func resolveCurrentModelFromConfig(options []streams.ConfigOption, available []acp.ModelInfo) string {
|
||||
modelID := ""
|
||||
reasoningEffort := ""
|
||||
for _, opt := range options {
|
||||
if opt.ID == configOptionIDModel || opt.Category == configOptionIDModel {
|
||||
return opt.CurrentValue
|
||||
modelID = opt.CurrentValue
|
||||
}
|
||||
if opt.ID == configOptionIDReasoningEffort || opt.Category == configOptionCategoryThoughtLevel {
|
||||
reasoningEffort = opt.CurrentValue
|
||||
}
|
||||
}
|
||||
return ""
|
||||
if modelID == "" {
|
||||
return ""
|
||||
}
|
||||
if reasoningEffort != "" {
|
||||
combined := modelID + "/" + reasoningEffort
|
||||
if modelIDExists(combined, available) {
|
||||
return combined
|
||||
}
|
||||
}
|
||||
if modelIDExists(modelID, available) {
|
||||
return modelID
|
||||
}
|
||||
// Keep the agent-reported config value as a best-effort fallback for
|
||||
// providers that expose the current model only through configOptions.
|
||||
return modelID
|
||||
}
|
||||
|
||||
func splitReasoningModelID(modelID string, options []streams.ConfigOption) (string, string, bool) {
|
||||
allowedReasoningEfforts := map[string]bool{}
|
||||
hasReasoningOption := false
|
||||
for _, opt := range options {
|
||||
if opt.ID == configOptionIDReasoningEffort || opt.Category == configOptionCategoryThoughtLevel {
|
||||
hasReasoningOption = true
|
||||
for _, optionValue := range opt.Options {
|
||||
if optionValue.Value != "" {
|
||||
allowedReasoningEfforts[optionValue.Value] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if hasReasoningOption && len(allowedReasoningEfforts) == 0 {
|
||||
for _, effort := range []string{
|
||||
reasoningEffortLow,
|
||||
reasoningEffortMedium,
|
||||
reasoningEffortHigh,
|
||||
reasoningEffortXHigh,
|
||||
} {
|
||||
allowedReasoningEfforts[effort] = true
|
||||
}
|
||||
}
|
||||
if len(allowedReasoningEfforts) == 0 {
|
||||
return "", "", false
|
||||
}
|
||||
slashIndex := strings.LastIndex(modelID, "/")
|
||||
if slashIndex < 1 || slashIndex == len(modelID)-1 {
|
||||
return "", "", false
|
||||
}
|
||||
baseModelID := modelID[:slashIndex]
|
||||
reasoningEffort := modelID[slashIndex+1:]
|
||||
if !allowedReasoningEfforts[reasoningEffort] {
|
||||
return "", "", false
|
||||
}
|
||||
return baseModelID, reasoningEffort, true
|
||||
}
|
||||
|
||||
func modelIDExists(modelID string, available []acp.ModelInfo) bool {
|
||||
if len(available) == 0 {
|
||||
return false
|
||||
}
|
||||
for _, model := range available {
|
||||
if string(model.ModelId) == modelID {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// SetMode changes the agent's session mode via ACP session/set_mode.
|
||||
|
||||
@@ -230,7 +230,7 @@ func (a *Adapter) convertNotification(n acp.SessionNotification) *AgentEvent {
|
||||
cachedModels := a.availableModels
|
||||
a.availableConfigOptions = configOptions
|
||||
a.mu.Unlock()
|
||||
currentModelID := resolveCurrentModelFromConfig(configOptions)
|
||||
currentModelID := resolveCurrentModelFromConfig(configOptions, cachedModels)
|
||||
return &AgentEvent{
|
||||
Type: streams.EventTypeSessionModels,
|
||||
SessionID: sessionID,
|
||||
|
||||
@@ -73,6 +73,123 @@ func TestEmitSessionModels_EmptyCurrentIDFromConfigOption(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestEmitSessionModels_EmptyCurrentIDComposesReasoningEffort pins Codex's
|
||||
// split config-option shape: configOptions reports model="gpt-5.5" and
|
||||
// reasoning_effort="medium", while availableModels carries the actual
|
||||
// selectable ID "gpt-5.5/medium".
|
||||
func TestEmitSessionModels_EmptyCurrentIDComposesReasoningEffort(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
models := &acp.SessionModelState{
|
||||
CurrentModelId: "",
|
||||
AvailableModels: []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
},
|
||||
}
|
||||
meta := map[string]any{
|
||||
"configOptions": []any{
|
||||
map[string]any{
|
||||
"type": "select",
|
||||
"id": "model",
|
||||
"name": "Model",
|
||||
"category": "model",
|
||||
"currentValue": "gpt-5.5",
|
||||
},
|
||||
map[string]any{
|
||||
"type": "select",
|
||||
"id": "reasoning_effort",
|
||||
"name": "Reasoning Effort",
|
||||
"category": "thought_level",
|
||||
"currentValue": "medium",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
a.emitSessionModels("sess-1", models, meta, nil)
|
||||
|
||||
ev := findSessionModelsEvent(t, drainEvents(a))
|
||||
if ev.CurrentModelID != "gpt-5.5/medium" {
|
||||
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "gpt-5.5/medium")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmitSessionModels_EmptyCurrentIDComposesReasoningEffortFromTypedOptions(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
modelCategory := acp.SessionConfigOptionCategoryModel
|
||||
thoughtCategory := acp.SessionConfigOptionCategoryThoughtLevel
|
||||
modelOptions := acp.SessionConfigSelectOptionsUngrouped{
|
||||
{Name: "GPT-5.5", Value: "gpt-5.5"},
|
||||
}
|
||||
effortOptions := acp.SessionConfigSelectOptionsUngrouped{
|
||||
{Name: "Low", Value: reasoningEffortLow},
|
||||
{Name: "Medium", Value: reasoningEffortMedium},
|
||||
}
|
||||
models := &acp.SessionModelState{
|
||||
CurrentModelId: "",
|
||||
AvailableModels: []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
},
|
||||
}
|
||||
configOptions := []acp.SessionConfigOption{
|
||||
{Select: &acp.SessionConfigOptionSelect{
|
||||
Type: "select",
|
||||
Id: "model",
|
||||
Name: "Model",
|
||||
Category: &modelCategory,
|
||||
CurrentValue: "gpt-5.5",
|
||||
Options: acp.SessionConfigSelectOptions{Ungrouped: &modelOptions},
|
||||
}},
|
||||
{Select: &acp.SessionConfigOptionSelect{
|
||||
Type: "select",
|
||||
Id: "reasoning_effort",
|
||||
Name: "Reasoning Effort",
|
||||
Category: &thoughtCategory,
|
||||
CurrentValue: reasoningEffortMedium,
|
||||
Options: acp.SessionConfigSelectOptions{Ungrouped: &effortOptions},
|
||||
}},
|
||||
}
|
||||
|
||||
a.emitSessionModels("sess-1", models, nil, configOptions)
|
||||
|
||||
ev := findSessionModelsEvent(t, drainEvents(a))
|
||||
if ev.CurrentModelID != "gpt-5.5/medium" {
|
||||
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "gpt-5.5/medium")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveCurrentModelFromConfig_ComposesReasoningEffort(t *testing.T) {
|
||||
options := []streams.ConfigOption{
|
||||
{Type: "select", ID: "model", Category: "model", CurrentValue: "gpt-5.5"},
|
||||
{Type: "select", ID: "reasoning_effort", Category: "thought_level", CurrentValue: reasoningEffortMedium},
|
||||
}
|
||||
available := []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5/low", Name: "GPT-5.5 (low)"},
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
}
|
||||
|
||||
got := resolveCurrentModelFromConfig(options, available)
|
||||
if got != "gpt-5.5/medium" {
|
||||
t.Errorf("resolveCurrentModelFromConfig() = %q, want %q", got, "gpt-5.5/medium")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveCurrentModelFromConfig_PrefersReasoningModelWhenBaseAlsoAvailable(t *testing.T) {
|
||||
options := []streams.ConfigOption{
|
||||
{Type: "select", ID: "model", Category: "model", CurrentValue: "gpt-5.5"},
|
||||
{Type: "select", ID: "reasoning_effort", Category: "thought_level", CurrentValue: reasoningEffortMedium},
|
||||
}
|
||||
available := []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5", Name: "GPT-5.5"},
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
}
|
||||
|
||||
got := resolveCurrentModelFromConfig(options, available)
|
||||
if got != "gpt-5.5/medium" {
|
||||
t.Errorf("resolveCurrentModelFromConfig() = %q, want %q", got, "gpt-5.5/medium")
|
||||
}
|
||||
}
|
||||
|
||||
// TestEmitSessionModels_NonEmptyCurrentIDPreserved checks the happy path:
|
||||
// when the agent populates CurrentModelId, we propagate it verbatim.
|
||||
func TestEmitSessionModels_NonEmptyCurrentIDPreserved(t *testing.T) {
|
||||
@@ -149,6 +266,150 @@ func TestEmitSetModelEvent_EmitsSessionModelsWithCachedState(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmitSetModelEvent_RewritesSplitReasoningOptions(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
|
||||
reasoningOptions := []streams.ConfigOptionValue{
|
||||
{Name: "Medium", Value: reasoningEffortMedium},
|
||||
{Name: "High", Value: reasoningEffortHigh},
|
||||
}
|
||||
cachedModels := []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
{ModelId: "gpt-5.5/high", Name: "GPT-5.5 (high)"},
|
||||
}
|
||||
cachedConfig := []streams.ConfigOption{
|
||||
{Type: "select", ID: "model", Category: "model", Name: "Model", CurrentValue: "gpt-5.5"},
|
||||
{
|
||||
Type: "select",
|
||||
ID: "reasoning_effort",
|
||||
Category: "thought_level",
|
||||
Name: "Reasoning Effort",
|
||||
CurrentValue: reasoningEffortMedium,
|
||||
Options: reasoningOptions,
|
||||
},
|
||||
}
|
||||
|
||||
a.emitSetModelEvent("sess-1", "gpt-5.5/high", cachedModels, cachedConfig)
|
||||
|
||||
ev := findSessionModelsEvent(t, drainEvents(a))
|
||||
if ev.CurrentModelID != "gpt-5.5/high" {
|
||||
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "gpt-5.5/high")
|
||||
}
|
||||
if len(ev.ConfigOptions) != 2 {
|
||||
t.Fatalf("ConfigOptions len = %d, want 2", len(ev.ConfigOptions))
|
||||
}
|
||||
for _, opt := range ev.ConfigOptions {
|
||||
switch opt.ID {
|
||||
case "model":
|
||||
if opt.CurrentValue != "gpt-5.5" {
|
||||
t.Errorf("model option CurrentValue = %q, want %q", opt.CurrentValue, "gpt-5.5")
|
||||
}
|
||||
case "reasoning_effort":
|
||||
if opt.CurrentValue != reasoningEffortHigh {
|
||||
t.Errorf("reasoning option CurrentValue = %q, want %q", opt.CurrentValue, reasoningEffortHigh)
|
||||
}
|
||||
default:
|
||||
t.Errorf("unexpected option ID %q in ConfigOptions", opt.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmitSetModelEvent_RewritesSplitReasoningOptionsWithSlashInBaseModel(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
|
||||
reasoningOptions := []streams.ConfigOptionValue{
|
||||
{Name: "Medium", Value: reasoningEffortMedium},
|
||||
{Name: "High", Value: reasoningEffortHigh},
|
||||
}
|
||||
cachedModels := []acp.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)"},
|
||||
}
|
||||
cachedConfig := []streams.ConfigOption{
|
||||
{Type: "select", ID: "model", Category: "model", Name: "Model", CurrentValue: "vendor/gpt-5.5"},
|
||||
{
|
||||
Type: "select",
|
||||
ID: "reasoning_effort",
|
||||
Category: "thought_level",
|
||||
Name: "Reasoning Effort",
|
||||
CurrentValue: reasoningEffortMedium,
|
||||
Options: reasoningOptions,
|
||||
},
|
||||
}
|
||||
|
||||
a.emitSetModelEvent("sess-1", "vendor/gpt-5.5/high", cachedModels, cachedConfig)
|
||||
|
||||
ev := findSessionModelsEvent(t, drainEvents(a))
|
||||
if ev.CurrentModelID != "vendor/gpt-5.5/high" {
|
||||
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "vendor/gpt-5.5/high")
|
||||
}
|
||||
if len(ev.ConfigOptions) != 2 {
|
||||
t.Fatalf("ConfigOptions len = %d, want 2", len(ev.ConfigOptions))
|
||||
}
|
||||
for _, opt := range ev.ConfigOptions {
|
||||
switch opt.ID {
|
||||
case "model":
|
||||
if opt.CurrentValue != "vendor/gpt-5.5" {
|
||||
t.Errorf("model option CurrentValue = %q, want %q", opt.CurrentValue, "vendor/gpt-5.5")
|
||||
}
|
||||
case "reasoning_effort":
|
||||
if opt.CurrentValue != reasoningEffortHigh {
|
||||
t.Errorf("reasoning option CurrentValue = %q, want %q", opt.CurrentValue, reasoningEffortHigh)
|
||||
}
|
||||
default:
|
||||
t.Errorf("unexpected option ID %q in ConfigOptions", opt.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmitSetModelEvent_DoesNotSplitSlashModelWithoutReasoningSuffix(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
|
||||
reasoningOptions := []streams.ConfigOptionValue{
|
||||
{Name: "Low", Value: reasoningEffortLow},
|
||||
{Name: "Medium", Value: reasoningEffortMedium},
|
||||
{Name: "High", Value: reasoningEffortHigh},
|
||||
}
|
||||
cachedModels := []acp.ModelInfo{
|
||||
{ModelId: "vendor/gpt-5.5", Name: "Vendor GPT-5.5"},
|
||||
}
|
||||
cachedConfig := []streams.ConfigOption{
|
||||
{Type: "select", ID: "model", Category: "model", Name: "Model", CurrentValue: "old-model"},
|
||||
{
|
||||
Type: "select",
|
||||
ID: "reasoning_effort",
|
||||
Category: "thought_level",
|
||||
Name: "Reasoning Effort",
|
||||
CurrentValue: reasoningEffortMedium,
|
||||
Options: reasoningOptions,
|
||||
},
|
||||
}
|
||||
|
||||
a.emitSetModelEvent("sess-1", "vendor/gpt-5.5", cachedModels, cachedConfig)
|
||||
|
||||
ev := findSessionModelsEvent(t, drainEvents(a))
|
||||
if ev.CurrentModelID != "vendor/gpt-5.5" {
|
||||
t.Errorf("CurrentModelID = %q, want %q", ev.CurrentModelID, "vendor/gpt-5.5")
|
||||
}
|
||||
if len(ev.ConfigOptions) != 2 {
|
||||
t.Fatalf("ConfigOptions len = %d, want 2", len(ev.ConfigOptions))
|
||||
}
|
||||
for _, opt := range ev.ConfigOptions {
|
||||
switch opt.ID {
|
||||
case "model":
|
||||
if opt.CurrentValue != "vendor/gpt-5.5" {
|
||||
t.Errorf("model option CurrentValue = %q, want %q", opt.CurrentValue, "vendor/gpt-5.5")
|
||||
}
|
||||
case "reasoning_effort":
|
||||
if opt.CurrentValue != reasoningEffortMedium {
|
||||
t.Errorf("reasoning option CurrentValue = %q, want %q", opt.CurrentValue, reasoningEffortMedium)
|
||||
}
|
||||
default:
|
||||
t.Errorf("unexpected option ID %q in ConfigOptions", opt.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestConfigOptionUpdate_RefreshesCachedConfig pins that an inbound
|
||||
// ConfigOptionUpdate notification refreshes the adapter's availableConfigOptions
|
||||
// cache, so a subsequent SetModel convergence event emits the latest options
|
||||
@@ -194,3 +455,55 @@ func TestConfigOptionUpdate_RefreshesCachedConfig(t *testing.T) {
|
||||
t.Errorf("availableConfigOptions = %+v, want one option with CurrentValue=fresh", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfigOptionUpdate_ComposesReasoningEffortCurrentModel(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
thoughtCategory := acp.SessionConfigOptionCategoryThoughtLevel
|
||||
modelOptions := acp.SessionConfigSelectOptionsUngrouped{
|
||||
{Name: "GPT-5.5", Value: "gpt-5.5"},
|
||||
}
|
||||
effortOptions := acp.SessionConfigSelectOptionsUngrouped{
|
||||
{Name: "Medium", Value: reasoningEffortMedium},
|
||||
{Name: "High", Value: reasoningEffortHigh},
|
||||
}
|
||||
|
||||
a.mu.Lock()
|
||||
a.availableModels = []acp.ModelInfo{
|
||||
{ModelId: "gpt-5.5/medium", Name: "GPT-5.5 (medium)"},
|
||||
{ModelId: "gpt-5.5/high", Name: "GPT-5.5 (high)"},
|
||||
}
|
||||
a.mu.Unlock()
|
||||
|
||||
notif := acp.SessionNotification{
|
||||
SessionId: "sess-1",
|
||||
Update: acp.SessionUpdate{
|
||||
ConfigOptionUpdate: &acp.SessionConfigOptionUpdate{
|
||||
ConfigOptions: []acp.SessionConfigOption{
|
||||
{Select: &acp.SessionConfigOptionSelect{
|
||||
Type: "select",
|
||||
Id: "model",
|
||||
Name: "Model",
|
||||
CurrentValue: "gpt-5.5",
|
||||
Options: acp.SessionConfigSelectOptions{Ungrouped: &modelOptions},
|
||||
}},
|
||||
{Select: &acp.SessionConfigOptionSelect{
|
||||
Type: "select",
|
||||
Id: "reasoning_effort",
|
||||
Name: "Reasoning Effort",
|
||||
Category: &thoughtCategory,
|
||||
CurrentValue: reasoningEffortHigh,
|
||||
Options: acp.SessionConfigSelectOptions{Ungrouped: &effortOptions},
|
||||
}},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
ev := a.convertNotification(notif)
|
||||
if ev == nil {
|
||||
t.Fatalf("expected a session_models event from ConfigOptionUpdate")
|
||||
}
|
||||
if ev.CurrentModelID != "gpt-5.5/high" {
|
||||
t.Errorf("event CurrentModelID = %q, want %q", ev.CurrentModelID, "gpt-5.5/high")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user