Skip to content

Commit 7aba642

Browse files
committed
fix(agent-routing): resolve the picker against the runtime's effective route key
The runtime resolver normalizes routing keys (case-insensitive, hyphen/underscore-equivalent) and falls back to default, but the picker read and wrote agentRouting with the exact agentType key. So an existing general_purpose route showed general-purpose agents as inheriting, and selecting a model wrote a general-purpose sibling that first-wins lookup ignored while the menu claimed the change took effect. readAgentRoute now matches the normalized per-agent key the resolver would use, and surfaces a default-fallback route with viaDefault. computeSetRouteUpdate/computeClearRouteUpdate overwrite or clear that existing key spelling instead of writing a sibling. The clear option is hidden for default-inherited routes since there is no own key to remove.
1 parent 20f718f commit 7aba642

2 files changed

Lines changed: 153 additions & 26 deletions

File tree

‎src/services/api/agentRouteSettings.test.ts‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,37 @@ describe('readAgentRoute', () => {
7171
const s = { agentModels: { haiku: {} }, agentRouting: { verification: 'haiku' } } as unknown as SettingsJson
7272
expect(readAgentRoute(s, 'verification')).toEqual({ kind: 'model-only', routeKey: 'haiku', model: 'haiku' })
7373
})
74+
75+
test('matches a normalized routing key the runtime would resolve (hyphen vs underscore)', () => {
76+
// Runtime normalizes "general_purpose" and "general-purpose" to the same key,
77+
// so an exact-key read would wrongly report this agent as inheriting.
78+
const s = {
79+
agentModels: { mini: { model: 'gpt-5-mini' } },
80+
agentRouting: { general_purpose: 'mini' },
81+
} as unknown as SettingsJson
82+
expect(readAgentRoute(s, 'general-purpose')).toEqual({ kind: 'model-only', routeKey: 'mini', model: 'gpt-5-mini' })
83+
})
84+
85+
test('surfaces a default-fallback route with viaDefault', () => {
86+
const s = {
87+
agentModels: { mini: { model: 'gpt-5-mini' } },
88+
agentRouting: { default: 'mini' },
89+
} as unknown as SettingsJson
90+
expect(readAgentRoute(s, 'Explore')).toEqual({
91+
kind: 'model-only',
92+
routeKey: 'mini',
93+
model: 'gpt-5-mini',
94+
viaDefault: true,
95+
})
96+
})
97+
98+
test('an own route key wins over the default fallback', () => {
99+
const s = {
100+
agentModels: { mini: { model: 'gpt-5-mini' }, haiku: {} },
101+
agentRouting: { default: 'mini', Explore: 'haiku' },
102+
} as unknown as SettingsJson
103+
expect(readAgentRoute(s, 'Explore')).toEqual({ kind: 'model-only', routeKey: 'haiku', model: 'haiku' })
104+
})
74105
})
75106

76107
describe('computeSetRouteUpdate', () => {
@@ -92,16 +123,36 @@ describe('computeSetRouteUpdate', () => {
92123
const next = computeSetRouteUpdate(modelOnly, 'Explore', 'mini')
93124
expect(next.agentRouting).toEqual({ verification: 'mini', Explore: 'mini' })
94125
})
126+
127+
test('overwrites the existing normalized key in place instead of adding a sibling', () => {
128+
const s = {
129+
agentModels: { mini: { model: 'gpt-5-mini' }, haiku: {} },
130+
agentRouting: { general_purpose: 'mini' },
131+
} as unknown as SettingsJson
132+
const next = computeSetRouteUpdate(s, 'general-purpose', 'haiku')
133+
// The runtime first-wins lookup would ignore a "general-purpose" sibling, so
134+
// we must reuse the existing "general_purpose" spelling.
135+
expect(next.agentRouting).toEqual({ general_purpose: 'haiku' })
136+
})
95137
})
96138

97139
describe('computeClearRouteUpdate', () => {
98140
test('marks the routing key as undefined for deletion', () => {
99-
const next = computeClearRouteUpdate('verification') as unknown as {
141+
const next = computeClearRouteUpdate(modelOnly, 'verification') as unknown as {
100142
agentRouting: Record<string, string | undefined>
101143
}
102144
expect('verification' in next.agentRouting).toBe(true)
103145
expect(next.agentRouting.verification).toBeUndefined()
104146
})
147+
148+
test('clears the existing normalized key spelling', () => {
149+
const s = { agentRouting: { general_purpose: 'mini' } } as unknown as SettingsJson
150+
const next = computeClearRouteUpdate(s, 'general-purpose') as unknown as {
151+
agentRouting: Record<string, string | undefined>
152+
}
153+
expect('general_purpose' in next.agentRouting).toBe(true)
154+
expect(next.agentRouting.general_purpose).toBeUndefined()
155+
})
105156
})
106157

107158
describe('buildRouteOptions', () => {
@@ -123,6 +174,15 @@ describe('buildRouteOptions', () => {
123174
expect(ds?.label).toContain('cross-provider')
124175
})
125176

177+
test('omits the clear option for a default-inherited route (nothing own to clear)', () => {
178+
const s = {
179+
agentModels: { mini: { model: 'gpt-5-mini' } },
180+
agentRouting: { default: 'mini' },
181+
} as unknown as SettingsJson
182+
const opts = buildRouteOptions(s, { kind: 'model-only', routeKey: 'mini', model: 'gpt-5-mini', viaDefault: true })
183+
expect(opts.map(o => o.value)).not.toContain(CLEAR_ROUTE_VALUE)
184+
})
185+
126186
test('labels a partial cross-provider key as unconfigured, not cross-provider', () => {
127187
const half = {
128188
agentModels: { half: { base_url: 'https://api.example.com/v1' } },
@@ -150,4 +210,10 @@ describe('describeRouteLine', () => {
150210
expect(describeRouteLine({ kind: 'cross-provider', routeKey: 'ds', model: 'deepseek-chat', baseURL: 'x' })).toContain('cross-provider')
151211
expect(describeRouteLine({ kind: 'dangling', routeKey: 'ghost' })).toContain('ghost')
152212
})
213+
214+
test('marks a default-inherited route', () => {
215+
expect(
216+
describeRouteLine({ kind: 'model-only', routeKey: 'm', model: 'gpt-5-mini', viaDefault: true }),
217+
).toContain('via default')
218+
})
153219
})

‎src/services/api/agentRouteSettings.ts‎

Lines changed: 86 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,77 @@ export const CUSTOM_MODEL_VALUE = '__custom_model__'
1111
/** Sentinel Select value: clear the route so the agent inherits the parent model. */
1212
export const CLEAR_ROUTE_VALUE = '__clear_route__'
1313

14-
/** The route currently assigned to an agent type in user settings. */
14+
/**
15+
* The route currently assigned to an agent type in user settings. `viaDefault`
16+
* marks a route the agent only gets through the `default` fallback (it has no
17+
* own routing key), so the menu can show it without claiming it as the agent's
18+
* own assignment.
19+
*/
1520
export type CurrentAgentRoute =
1621
| { kind: 'none' }
17-
| { kind: 'model-only'; routeKey: string; model: string }
18-
| { kind: 'cross-provider'; routeKey: string; model: string; baseURL: string }
19-
| { kind: 'dangling'; routeKey: string }
22+
| { kind: 'model-only'; routeKey: string; model: string; viaDefault?: boolean }
23+
| { kind: 'cross-provider'; routeKey: string; model: string; baseURL: string; viaDefault?: boolean }
24+
| { kind: 'dangling'; routeKey: string; viaDefault?: boolean }
2025

21-
/** Read the route assigned to `agentType` from a settings value. Pure. */
22-
export function readAgentRoute(
23-
settings: SettingsJson | null,
26+
/** Normalize a routing key the same way the runtime resolver does. */
27+
function normalizeAgentKey(key: string): string {
28+
return key.toLowerCase().replace(/[-_]/g, '')
29+
}
30+
31+
/**
32+
* The existing `agentRouting` key (original spelling) that the runtime resolver
33+
* would match for `agentType`, or undefined if none. Mirrors
34+
* resolveAgentProvider's case-insensitive, hyphen/underscore-insensitive,
35+
* first-wins lookup. Pure.
36+
*/
37+
function findOwnRouteKey(
38+
routing: Record<string, string> | undefined,
2439
agentType: string,
40+
): string | undefined {
41+
if (!routing) return undefined
42+
const target = normalizeAgentKey(agentType)
43+
for (const key of Object.keys(routing)) {
44+
if (normalizeAgentKey(key) === target) return key
45+
}
46+
return undefined
47+
}
48+
49+
/** Build the route descriptor for a resolved model key. Pure. */
50+
function describeModelKey(
51+
settings: SettingsJson | null,
52+
modelKey: string,
53+
viaDefault: boolean,
2554
): CurrentAgentRoute {
26-
const routeKey = settings?.agentRouting?.[agentType]
27-
if (!routeKey) return { kind: 'none' }
28-
const entry = settings?.agentModels?.[routeKey]
29-
if (!entry) return { kind: 'dangling', routeKey }
30-
const model = entry.model?.trim() || routeKey
55+
const entry = settings?.agentModels?.[modelKey]
56+
if (!entry) return { kind: 'dangling', routeKey: modelKey, ...(viaDefault ? { viaDefault } : {}) }
57+
const model = entry.model?.trim() || modelKey
3158
// Mirror the runtime resolver (toAgentRoute): cross-provider needs BOTH
3259
// base_url and api_key. A partial entry is skipped at runtime and inherits,
3360
// so surface it as unconfigured rather than claiming a route that won't run.
3461
const baseURL = entry.base_url?.trim()
3562
const apiKey = entry.api_key?.trim()
36-
if (!baseURL && !apiKey) return { kind: 'model-only', routeKey, model }
63+
if (!baseURL && !apiKey) return { kind: 'model-only', routeKey: modelKey, model, ...(viaDefault ? { viaDefault } : {}) }
3764
if (baseURL && apiKey) {
38-
return { kind: 'cross-provider', routeKey, model, baseURL }
65+
return { kind: 'cross-provider', routeKey: modelKey, model, baseURL, ...(viaDefault ? { viaDefault } : {}) }
3966
}
40-
return { kind: 'dangling', routeKey }
67+
return { kind: 'dangling', routeKey: modelKey, ...(viaDefault ? { viaDefault } : {}) }
68+
}
69+
70+
/**
71+
* Read the route assigned to `agentType` from a settings value, mirroring the
72+
* runtime resolver: a normalized per-agent key wins, otherwise the `default`
73+
* fallback applies (surfaced with `viaDefault`). Pure.
74+
*/
75+
export function readAgentRoute(
76+
settings: SettingsJson | null,
77+
agentType: string,
78+
): CurrentAgentRoute {
79+
const routing = settings?.agentRouting
80+
const ownKey = findOwnRouteKey(routing, agentType)
81+
if (ownKey) return describeModelKey(settings, routing![ownKey], false)
82+
const defaultModelKey = routing?.default
83+
if (defaultModelKey) return describeModelKey(settings, defaultModelKey, true)
84+
return { kind: 'none' }
4185
}
4286

4387
/** The Select value representing the current route, if any. Pure. */
@@ -59,29 +103,39 @@ export function computeSetRouteUpdate(
59103
if (!agentModels[modelKey]) {
60104
agentModels[modelKey] = { model: modelKey }
61105
}
62-
const agentRouting = { ...(settings?.agentRouting ?? {}), [agentType]: modelKey }
106+
// Reuse the existing routing key the runtime would match so we overwrite it
107+
// in place instead of writing a normalized sibling the resolver's first-wins
108+
// lookup would ignore (e.g. "general-purpose" beside "general_purpose").
109+
const routingKey = findOwnRouteKey(settings?.agentRouting, agentType) ?? agentType
110+
const agentRouting = { ...(settings?.agentRouting ?? {}), [routingKey]: modelKey }
63111
return { agentModels, agentRouting } as unknown as SettingsJson
64112
}
65113

66114
/**
67-
* Next settings to clear `agentType`'s route. The explicit `undefined` is what
68-
* makes updateSettingsForSource delete the key on merge. Pure.
115+
* Next settings to clear `agentType`'s route. Clears the effective routing key
116+
* the runtime would match (not a normalized sibling). The explicit `undefined`
117+
* is what makes updateSettingsForSource delete the key on merge. Pure.
69118
*/
70-
export function computeClearRouteUpdate(agentType: string): SettingsJson {
71-
return { agentRouting: { [agentType]: undefined } } as unknown as SettingsJson
119+
export function computeClearRouteUpdate(
120+
settings: SettingsJson | null,
121+
agentType: string,
122+
): SettingsJson {
123+
const routingKey = findOwnRouteKey(settings?.agentRouting, agentType) ?? agentType
124+
return { agentRouting: { [routingKey]: undefined } } as unknown as SettingsJson
72125
}
73126

74127
/** Human-readable one-line route summary for the AgentDetail view. Pure. */
75128
export function describeRouteLine(current: CurrentAgentRoute): string {
129+
const viaDefault = current.kind !== 'none' && current.viaDefault ? ' (via default)' : ''
76130
switch (current.kind) {
77131
case 'none':
78132
return 'Route: inherits parent model'
79133
case 'model-only':
80-
return `Route: ${current.model} (current provider)`
134+
return `Route: ${current.model} (current provider)${viaDefault}`
81135
case 'cross-provider':
82-
return `Route: ${current.model} (cross-provider)`
136+
return `Route: ${current.model} (cross-provider)${viaDefault}`
83137
case 'dangling':
84-
return `Route: ${current.routeKey} (unconfigured, inherits)`
138+
return `Route: ${current.routeKey} (unconfigured, inherits)${viaDefault}`
85139
}
86140
}
87141

@@ -111,7 +165,11 @@ export function buildRouteOptions(
111165
}
112166
})
113167

114-
if (current.kind !== 'none') {
168+
// Only offer "clear" when the agent has its OWN routing key. A route inherited
169+
// via `default` has nothing agent-specific to remove, and clearing wouldn't
170+
// make it inherit the parent (default would still apply), so the option would
171+
// claim a change the runtime ignores.
172+
if (current.kind !== 'none' && !current.viaDefault) {
115173
modelOptions.push({
116174
value: CLEAR_ROUTE_VALUE,
117175
label: 'Clear route (inherit from parent)',
@@ -139,5 +197,8 @@ export function setAgentRoute(
139197

140198
/** Remove `agentType`'s route in user-global settings. */
141199
export function clearAgentRoute(agentType: string): { error: Error | null } {
142-
return updateSettingsForSource('userSettings', computeClearRouteUpdate(agentType))
200+
return updateSettingsForSource(
201+
'userSettings',
202+
computeClearRouteUpdate(getSettingsForSource('userSettings'), agentType),
203+
)
143204
}

0 commit comments

Comments
 (0)