Spaces:
Sleeping
Sleeping
nyk commited on
fix(ui): null-safe trim calls in agent model config (#329)
Browse files* fix(ui): null-safe trim calls in agent model config (#319)
Guard against undefined values in .trim() calls when editing agent
model configuration (primary model and fallback array entries).
Closes #319
* test: add unit tests for null-safe trim in agent model config
Tests verify that undefined/null values in model primary and fallback
arrays don't cause trim() TypeError crashes.
* test(e2e): add agent model config edge case tests
Tests verify PUT /api/agents/:id with valid config, empty fallbacks,
empty primary, and whitespace-only fallback entries.
src/components/panels/agent-detail-tabs.tsx
CHANGED
|
@@ -1443,7 +1443,7 @@ export function ConfigTab({
|
|
| 1443 |
const updateModelConfig = (updater: (current: { primary?: string; fallbacks?: string[] }) => { primary?: string; fallbacks?: string[] }) => {
|
| 1444 |
setConfig((prev: any) => {
|
| 1445 |
const nextModel = updater({ ...(prev?.model || {}) })
|
| 1446 |
-
const dedupedFallbacks = [...new Set((nextModel.fallbacks || []).map((value) => value.trim()).filter(Boolean))]
|
| 1447 |
return {
|
| 1448 |
...prev,
|
| 1449 |
model: {
|
|
@@ -2788,8 +2788,8 @@ export function ModelsTab({ agent }: { agent: Agent }) {
|
|
| 2788 |
body: JSON.stringify({
|
| 2789 |
gateway_config: {
|
| 2790 |
model: {
|
| 2791 |
-
primary: primary.trim(),
|
| 2792 |
-
fallbacks: fallbacks.filter(f => f.trim()),
|
| 2793 |
},
|
| 2794 |
},
|
| 2795 |
write_to_gateway: true,
|
|
|
|
| 1443 |
const updateModelConfig = (updater: (current: { primary?: string; fallbacks?: string[] }) => { primary?: string; fallbacks?: string[] }) => {
|
| 1444 |
setConfig((prev: any) => {
|
| 1445 |
const nextModel = updater({ ...(prev?.model || {}) })
|
| 1446 |
+
const dedupedFallbacks = [...new Set((nextModel.fallbacks || []).map((value) => (value || '').trim()).filter(Boolean))]
|
| 1447 |
return {
|
| 1448 |
...prev,
|
| 1449 |
model: {
|
|
|
|
| 2788 |
body: JSON.stringify({
|
| 2789 |
gateway_config: {
|
| 2790 |
model: {
|
| 2791 |
+
primary: (primary || '').trim(),
|
| 2792 |
+
fallbacks: fallbacks.filter(f => f && f.trim()),
|
| 2793 |
},
|
| 2794 |
},
|
| 2795 |
write_to_gateway: true,
|
src/lib/__tests__/agent-model-trim.test.ts
ADDED
|
@@ -0,0 +1,86 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
import { describe, expect, it } from 'vitest'
|
| 2 |
+
|
| 3 |
+
/**
|
| 4 |
+
* Tests for the null-safe trim patterns used in agent model config editing.
|
| 5 |
+
* These mirror the logic in agent-detail-tabs.tsx (updateModelConfig + handleSave).
|
| 6 |
+
*/
|
| 7 |
+
|
| 8 |
+
function dedupFallbacks(fallbacks: (string | undefined | null)[]): string[] {
|
| 9 |
+
return [...new Set((fallbacks || []).map((value) => (value || '').trim()).filter(Boolean))]
|
| 10 |
+
}
|
| 11 |
+
|
| 12 |
+
function safePrimary(primary: string | undefined | null): string {
|
| 13 |
+
return (primary || '').trim()
|
| 14 |
+
}
|
| 15 |
+
|
| 16 |
+
function filterFallbacks(fallbacks: (string | undefined | null)[]): string[] {
|
| 17 |
+
return fallbacks.filter(f => f && f.trim()) as string[]
|
| 18 |
+
}
|
| 19 |
+
|
| 20 |
+
describe('agent model config trim safety', () => {
|
| 21 |
+
describe('dedupFallbacks (updateModelConfig pattern)', () => {
|
| 22 |
+
it('handles normal string values', () => {
|
| 23 |
+
expect(dedupFallbacks(['gpt-4', 'claude-3'])).toEqual(['gpt-4', 'claude-3'])
|
| 24 |
+
})
|
| 25 |
+
|
| 26 |
+
it('handles undefined values without throwing', () => {
|
| 27 |
+
expect(dedupFallbacks([undefined, 'gpt-4', undefined])).toEqual(['gpt-4'])
|
| 28 |
+
})
|
| 29 |
+
|
| 30 |
+
it('handles null values without throwing', () => {
|
| 31 |
+
expect(dedupFallbacks([null, 'gpt-4'])).toEqual(['gpt-4'])
|
| 32 |
+
})
|
| 33 |
+
|
| 34 |
+
it('filters out empty strings', () => {
|
| 35 |
+
expect(dedupFallbacks(['', ' ', 'gpt-4'])).toEqual(['gpt-4'])
|
| 36 |
+
})
|
| 37 |
+
|
| 38 |
+
it('deduplicates models', () => {
|
| 39 |
+
expect(dedupFallbacks(['gpt-4', 'gpt-4', 'claude-3'])).toEqual(['gpt-4', 'claude-3'])
|
| 40 |
+
})
|
| 41 |
+
|
| 42 |
+
it('handles empty array', () => {
|
| 43 |
+
expect(dedupFallbacks([])).toEqual([])
|
| 44 |
+
})
|
| 45 |
+
})
|
| 46 |
+
|
| 47 |
+
describe('safePrimary (handleSave pattern)', () => {
|
| 48 |
+
it('trims normal string', () => {
|
| 49 |
+
expect(safePrimary(' gpt-4 ')).toBe('gpt-4')
|
| 50 |
+
})
|
| 51 |
+
|
| 52 |
+
it('handles undefined without throwing', () => {
|
| 53 |
+
expect(safePrimary(undefined)).toBe('')
|
| 54 |
+
})
|
| 55 |
+
|
| 56 |
+
it('handles null without throwing', () => {
|
| 57 |
+
expect(safePrimary(null)).toBe('')
|
| 58 |
+
})
|
| 59 |
+
|
| 60 |
+
it('handles empty string', () => {
|
| 61 |
+
expect(safePrimary('')).toBe('')
|
| 62 |
+
})
|
| 63 |
+
})
|
| 64 |
+
|
| 65 |
+
describe('filterFallbacks (handleSave pattern)', () => {
|
| 66 |
+
it('filters valid values', () => {
|
| 67 |
+
expect(filterFallbacks(['gpt-4', 'claude-3'])).toEqual(['gpt-4', 'claude-3'])
|
| 68 |
+
})
|
| 69 |
+
|
| 70 |
+
it('filters out undefined without throwing', () => {
|
| 71 |
+
expect(filterFallbacks([undefined, 'gpt-4'])).toEqual(['gpt-4'])
|
| 72 |
+
})
|
| 73 |
+
|
| 74 |
+
it('filters out null without throwing', () => {
|
| 75 |
+
expect(filterFallbacks([null, 'gpt-4'])).toEqual(['gpt-4'])
|
| 76 |
+
})
|
| 77 |
+
|
| 78 |
+
it('filters out empty strings', () => {
|
| 79 |
+
expect(filterFallbacks(['', 'gpt-4'])).toEqual(['gpt-4'])
|
| 80 |
+
})
|
| 81 |
+
|
| 82 |
+
it('filters out whitespace-only strings', () => {
|
| 83 |
+
expect(filterFallbacks([' ', 'gpt-4'])).toEqual(['gpt-4'])
|
| 84 |
+
})
|
| 85 |
+
})
|
| 86 |
+
})
|
tests/agent-model-config.spec.ts
ADDED
|
@@ -0,0 +1,95 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
import { test, expect } from '@playwright/test'
|
| 2 |
+
import { API_KEY_HEADER, createTestAgent, deleteTestAgent } from './helpers'
|
| 3 |
+
|
| 4 |
+
/**
|
| 5 |
+
* E2E tests for agent model configuration updates.
|
| 6 |
+
* Verifies the API handles edge cases like empty/null model values.
|
| 7 |
+
*/
|
| 8 |
+
|
| 9 |
+
test.describe('Agent Model Config', () => {
|
| 10 |
+
const cleanup: number[] = []
|
| 11 |
+
|
| 12 |
+
test.afterEach(async ({ request }) => {
|
| 13 |
+
for (const id of cleanup) {
|
| 14 |
+
await deleteTestAgent(request, id).catch(() => {})
|
| 15 |
+
}
|
| 16 |
+
cleanup.length = 0
|
| 17 |
+
})
|
| 18 |
+
|
| 19 |
+
test('PUT with valid model config succeeds', async ({ request }) => {
|
| 20 |
+
const { id } = await createTestAgent(request)
|
| 21 |
+
cleanup.push(id)
|
| 22 |
+
|
| 23 |
+
const res = await request.put(`/api/agents/${id}`, {
|
| 24 |
+
headers: API_KEY_HEADER,
|
| 25 |
+
data: {
|
| 26 |
+
gateway_config: {
|
| 27 |
+
model: {
|
| 28 |
+
primary: 'claude-3-5-sonnet-20241022',
|
| 29 |
+
fallbacks: ['gpt-4o'],
|
| 30 |
+
},
|
| 31 |
+
},
|
| 32 |
+
},
|
| 33 |
+
})
|
| 34 |
+
|
| 35 |
+
expect(res.status()).toBe(200)
|
| 36 |
+
})
|
| 37 |
+
|
| 38 |
+
test('PUT with empty fallbacks array succeeds', async ({ request }) => {
|
| 39 |
+
const { id } = await createTestAgent(request)
|
| 40 |
+
cleanup.push(id)
|
| 41 |
+
|
| 42 |
+
const res = await request.put(`/api/agents/${id}`, {
|
| 43 |
+
headers: API_KEY_HEADER,
|
| 44 |
+
data: {
|
| 45 |
+
gateway_config: {
|
| 46 |
+
model: {
|
| 47 |
+
primary: 'claude-3-5-sonnet-20241022',
|
| 48 |
+
fallbacks: [],
|
| 49 |
+
},
|
| 50 |
+
},
|
| 51 |
+
},
|
| 52 |
+
})
|
| 53 |
+
|
| 54 |
+
expect(res.status()).toBe(200)
|
| 55 |
+
})
|
| 56 |
+
|
| 57 |
+
test('PUT with empty string primary returns appropriate response', async ({ request }) => {
|
| 58 |
+
const { id } = await createTestAgent(request)
|
| 59 |
+
cleanup.push(id)
|
| 60 |
+
|
| 61 |
+
const res = await request.put(`/api/agents/${id}`, {
|
| 62 |
+
headers: API_KEY_HEADER,
|
| 63 |
+
data: {
|
| 64 |
+
gateway_config: {
|
| 65 |
+
model: {
|
| 66 |
+
primary: '',
|
| 67 |
+
fallbacks: [],
|
| 68 |
+
},
|
| 69 |
+
},
|
| 70 |
+
},
|
| 71 |
+
})
|
| 72 |
+
|
| 73 |
+
// Server should accept the update (model config is optional gateway config)
|
| 74 |
+
expect([200, 400]).toContain(res.status())
|
| 75 |
+
})
|
| 76 |
+
|
| 77 |
+
test('PUT with whitespace-only fallbacks filters them', async ({ request }) => {
|
| 78 |
+
const { id } = await createTestAgent(request)
|
| 79 |
+
cleanup.push(id)
|
| 80 |
+
|
| 81 |
+
const res = await request.put(`/api/agents/${id}`, {
|
| 82 |
+
headers: API_KEY_HEADER,
|
| 83 |
+
data: {
|
| 84 |
+
gateway_config: {
|
| 85 |
+
model: {
|
| 86 |
+
primary: 'claude-3-5-sonnet-20241022',
|
| 87 |
+
fallbacks: [' ', '', 'gpt-4o'],
|
| 88 |
+
},
|
| 89 |
+
},
|
| 90 |
+
},
|
| 91 |
+
})
|
| 92 |
+
|
| 93 |
+
expect(res.status()).toBe(200)
|
| 94 |
+
})
|
| 95 |
+
})
|