fix(web): keep the model selector visible for sessions with an agent config key (#2715)
Co-authored-by: Joao Manso <X363081@bcpcorp.net> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -2,8 +2,8 @@ package profileconfig
|
||||
|
||||
import "strings"
|
||||
|
||||
// SanitizeConfigOptions drops reserved model/mode keys and blank entries so
|
||||
// profile config options persist only auxiliary select values.
|
||||
// SanitizeConfigOptions drops the top-level model/mode keys and blank entries.
|
||||
// All other keys are provider-defined and must pass through unchanged.
|
||||
func SanitizeConfigOptions(in map[string]string) map[string]string {
|
||||
if len(in) == 0 {
|
||||
return nil
|
||||
|
||||
@@ -25,6 +25,11 @@ func TestSanitizeConfigOptions(t *testing.T) {
|
||||
in: map[string]string{"mode": "plan", "effort": "low"},
|
||||
want: map[string]string{"effort": "low"},
|
||||
},
|
||||
{
|
||||
name: "provider agent key preserved",
|
||||
in: map[string]string{"agent": "build", "effort": "high"},
|
||||
want: map[string]string{"agent": "build", "effort": "high"},
|
||||
},
|
||||
{
|
||||
name: "blank value dropped",
|
||||
in: map[string]string{"effort": "", "level": "high"},
|
||||
|
||||
@@ -313,7 +313,7 @@ func LoadEffectiveSessionRuntimeConfig(session *TaskSession) (SessionRuntimeConf
|
||||
if overrides, ok := LoadSessionRuntimeConfigOverrides(session.Metadata); ok {
|
||||
mergeSessionRuntimeConfig(&effective, overrides)
|
||||
}
|
||||
effective.ConfigOptions = cleanRuntimeConfigOptions(effective.ConfigOptions)
|
||||
effective.ConfigOptions = cleanRuntimeConfigOptions(effective.ConfigOptions, session)
|
||||
return effective, !effective.IsZero()
|
||||
}
|
||||
|
||||
@@ -350,19 +350,35 @@ func mergeSessionRuntimeConfig(target *SessionRuntimeConfig, source SessionRunti
|
||||
}
|
||||
}
|
||||
|
||||
func cleanRuntimeConfigOptions(options map[string]string) map[string]string {
|
||||
func cleanRuntimeConfigOptions(options map[string]string, session *TaskSession) map[string]string {
|
||||
if len(options) == 0 {
|
||||
return nil
|
||||
}
|
||||
cleaned := maps.Clone(options)
|
||||
// Model and mode are carried as top-level fields. Other keys are provider-defined.
|
||||
delete(cleaned, "model")
|
||||
delete(cleaned, "mode")
|
||||
if isLegacyAgentIdentity(session, cleaned["agent"]) {
|
||||
delete(cleaned, "agent")
|
||||
}
|
||||
if len(cleaned) == 0 {
|
||||
return nil
|
||||
}
|
||||
return cleaned
|
||||
}
|
||||
|
||||
func isLegacyAgentIdentity(session *TaskSession, value string) bool {
|
||||
if session == nil || value == "" || session.AgentProfileSnapshot == nil {
|
||||
return false
|
||||
}
|
||||
for _, key := range []string{"agent_id", "agent_name"} {
|
||||
if StringFromAny(session.AgentProfileSnapshot[key]) == value {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// SessionOriginalEffectiveConfiguration is the immutable configuration a task
|
||||
// session had after provider defaults and its profile settings settled.
|
||||
type SessionOriginalEffectiveConfiguration struct {
|
||||
|
||||
@@ -9,8 +9,9 @@ import (
|
||||
func TestLoadEffectiveSessionRuntimeConfigMergesInPrecedenceOrder(t *testing.T) {
|
||||
session := &TaskSession{
|
||||
AgentProfileSnapshot: map[string]interface{}{
|
||||
"model": "profile-model",
|
||||
"mode": "profile-mode",
|
||||
"agent_id": "provider-agent",
|
||||
"model": "profile-model",
|
||||
"mode": "profile-mode",
|
||||
"config_options": map[string]string{
|
||||
"profile_only": "profile",
|
||||
"provider_replaced": "profile-value",
|
||||
@@ -23,6 +24,7 @@ func TestLoadEffectiveSessionRuntimeConfigMergesInPrecedenceOrder(t *testing.T)
|
||||
ConfigOptions: map[string]string{
|
||||
"model": "provider-model",
|
||||
"mode": "provider-mode",
|
||||
"agent": "provider-agent",
|
||||
"provider_replaced": "provider-value",
|
||||
"provider_only": "provider",
|
||||
},
|
||||
@@ -52,6 +54,26 @@ func TestLoadEffectiveSessionRuntimeConfigMergesInPrecedenceOrder(t *testing.T)
|
||||
}, got.ConfigOptions)
|
||||
}
|
||||
|
||||
func TestLoadEffectiveSessionRuntimeConfigPreservesProviderAgentOption(t *testing.T) {
|
||||
session := &TaskSession{
|
||||
AgentProfileSnapshot: map[string]interface{}{
|
||||
"agent_id": "claude",
|
||||
"config_options": map[string]string{
|
||||
"agent": "build",
|
||||
},
|
||||
},
|
||||
Metadata: map[string]interface{}{
|
||||
SessionMetaKeyRuntimeConfig: SessionRuntimeConfig{
|
||||
ConfigOptions: map[string]string{"agent": "build"},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
got, ok := LoadEffectiveSessionRuntimeConfig(session)
|
||||
require.True(t, ok)
|
||||
require.Equal(t, map[string]string{"agent": "build"}, got.ConfigOptions)
|
||||
}
|
||||
|
||||
func TestLoadEffectiveSessionRuntimeConfigReturnsIndependentOptions(t *testing.T) {
|
||||
profileOptions := map[string]string{"profile": "value"}
|
||||
providerOptions := map[string]string{"provider": "value"}
|
||||
|
||||
@@ -281,6 +281,41 @@ describe("hasCompleteDynamicConfig", () => {
|
||||
expect(hydrated).toBe(false);
|
||||
});
|
||||
|
||||
it("treats a legacy agent identity key as satisfied without an ACP option", () => {
|
||||
const session = makeSession({
|
||||
metadata: {
|
||||
runtime_config: { config_options: { agent: "claude", model: flatModelId } },
|
||||
},
|
||||
agent_profile_snapshot: { agent_id: "claude" },
|
||||
});
|
||||
expect(requiredConfigKeys(session, noAgents)).toEqual(["agent", "model"]);
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: flatModelId, models: flatModelEntries(), configOptions: [] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps a provider-defined agent option required", () => {
|
||||
const session = makeSession({
|
||||
metadata: {
|
||||
runtime_config: { config_options: { agent: "build", model: flatModelId } },
|
||||
},
|
||||
agent_profile_snapshot: { agent_id: "claude" },
|
||||
});
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: flatModelId, models: flatModelEntries(), configOptions: [] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(false);
|
||||
});
|
||||
|
||||
it("is hydrated when there are no required config keys", () => {
|
||||
expect(hasCompleteDynamicConfig(makeSession(), undefined, noAgents)).toBe(true);
|
||||
});
|
||||
|
||||
@@ -46,7 +46,48 @@ function configValueKeys(value: unknown): string[] {
|
||||
.map(([key]) => key);
|
||||
}
|
||||
|
||||
function recordValue(value: unknown, key: string): unknown {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) return undefined;
|
||||
return (value as Record<string, unknown>)[key];
|
||||
}
|
||||
|
||||
function configValue(value: unknown, key: string): string | undefined {
|
||||
const candidate = recordValue(value, key);
|
||||
return typeof candidate === "string" && candidate ? candidate : undefined;
|
||||
}
|
||||
|
||||
const MODEL_CONFIG_KEY = "model";
|
||||
// Legacy snapshots used config_options.agent for agent identity. Provider-defined
|
||||
// agent options stay valid unless the value matches the session identity.
|
||||
const AGENT_CONFIG_KEY = "agent";
|
||||
|
||||
function isLegacyAgentIdentityConfig(session: TaskSession | null, agents: Agent[]): boolean {
|
||||
if (!session) return false;
|
||||
const snapshot = session.agent_profile_snapshot;
|
||||
const profile = agents
|
||||
.flatMap((agent) => agent.profiles)
|
||||
.find((item) => item.id === session.agent_profile_id);
|
||||
const identityValues = new Set(
|
||||
[
|
||||
configValue(snapshot, "agent_id"),
|
||||
configValue(snapshot, "agent_name"),
|
||||
profile?.agentId,
|
||||
profile?.agentDisplayName,
|
||||
].filter((value): value is string => !!value),
|
||||
);
|
||||
if (identityValues.size === 0) return false;
|
||||
const metadata = session.metadata;
|
||||
const optionSources = [
|
||||
snapshot?.config_options,
|
||||
profile?.configOptions,
|
||||
recordValue(recordValue(metadata, "runtime_config"), "config_options"),
|
||||
recordValue(recordValue(metadata, "runtime_config_overrides"), "config_options"),
|
||||
];
|
||||
return optionSources.some((source) => {
|
||||
const value = configValue(source, AGENT_CONFIG_KEY);
|
||||
return !!value && identityValues.has(value);
|
||||
});
|
||||
}
|
||||
|
||||
export function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): string[] {
|
||||
if (!session) return [];
|
||||
@@ -83,8 +124,12 @@ export function hasCompleteDynamicConfig(
|
||||
// configOptions. Treat that key as satisfied when the session has a flat model
|
||||
// list — the selector renders fine from it.
|
||||
const hasFlatModelList = !!sessionModelsData.models.length;
|
||||
const hasLegacyAgentIdentity = isLegacyAgentIdentityConfig(session, agents);
|
||||
return required.every(
|
||||
(key) => available.has(key) || (key === MODEL_CONFIG_KEY && hasFlatModelList),
|
||||
(key) =>
|
||||
available.has(key) ||
|
||||
(key === AGENT_CONFIG_KEY && hasLegacyAgentIdentity) ||
|
||||
(key === MODEL_CONFIG_KEY && hasFlatModelList),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user