fix(tools): address Codex review of arg validation (PR 1)
- enum membership now checked uniformly for all SchemaTypes, mirroring the converter which emits `enum` regardless of type (was string-only) - checkValue switch ends in assertNever per the closed-union convention - sync the adding-a-tool cookbook to the validate-for-you behavior - soften ADR 0011's property-test claim (RFC 001 not yet landed)
This commit is contained in:
@@ -20,6 +20,7 @@
|
||||
*/
|
||||
|
||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import { assertNever } from '@deepseek-ai/dsh-llm'
|
||||
import type { ToolDefinition, ToolExecution } from './index.ts'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -202,16 +203,15 @@ function checkValue(prop: SchemaProp, value: unknown, path: string): string[] {
|
||||
switch (prop.type) {
|
||||
case 'string': {
|
||||
if (typeof value !== 'string') return [`"${path}" must be a string`]
|
||||
if (prop.enum && !prop.enum.includes(value)) {
|
||||
return [`"${path}" must be one of ${JSON.stringify(prop.enum)}`]
|
||||
}
|
||||
return []
|
||||
break
|
||||
}
|
||||
case 'number': {
|
||||
return typeof value === 'number' ? [] : [`"${path}" must be a number`]
|
||||
if (typeof value !== 'number') return [`"${path}" must be a number`]
|
||||
break
|
||||
}
|
||||
case 'boolean': {
|
||||
return typeof value === 'boolean' ? [] : [`"${path}" must be a boolean`]
|
||||
if (typeof value !== 'boolean') return [`"${path}" must be a boolean`]
|
||||
break
|
||||
}
|
||||
case 'object': {
|
||||
if (!isPlainObject(value)) return [`"${path}" must be an object`]
|
||||
@@ -225,8 +225,16 @@ function checkValue(prop: SchemaProp, value: unknown, path: string): string[] {
|
||||
const items = prop.items
|
||||
return value.flatMap((el, i) => checkValue(items, el, `${path}[${i}]`))
|
||||
}
|
||||
// No default: SchemaType is a closed union; every case is handled above.
|
||||
default: return assertNever(prop.type, 'validateArgs')
|
||||
}
|
||||
// Enum membership, checked uniformly: the converter emits `enum` for any
|
||||
// type ([prop.enum]), so the validator must too. `enum` is `string[]`, so a
|
||||
// non-string value can never be a member — it falls out here, consistent
|
||||
// with the schema the model was given.
|
||||
if (prop.enum && !(prop.enum as unknown[]).includes(value)) {
|
||||
return [`"${path}" must be one of ${JSON.stringify(prop.enum)}`]
|
||||
}
|
||||
return []
|
||||
}
|
||||
|
||||
/** Collect violations for an object value against a {@link SchemaSpec}. */
|
||||
|
||||
@@ -623,6 +623,18 @@ describe('validateArgs (RFC 005 part 1)', () => {
|
||||
expect(validateArgs(spec, { color: 'blue' })).toEqual(['"color" must be one of ["red","green"]'])
|
||||
})
|
||||
|
||||
it('checks enum uniformly with the converter (enum on a non-string prop)', () => {
|
||||
// The converter emits `enum` regardless of type; the validator must agree.
|
||||
// `enum` is string[], so a number value can never be a member.
|
||||
const spec = { n: { type: 'number', enum: ['1', '2'] } } as unknown as SchemaSpec
|
||||
expect(validateArgs(spec, { n: 1 })).toEqual(['"n" must be one of ["1","2"]'])
|
||||
})
|
||||
|
||||
it('rejects an unknown SchemaType at runtime (assertNever guard)', () => {
|
||||
const spec = { x: { type: 'weird' } } as unknown as SchemaSpec
|
||||
expect(() => validateArgs(spec, { x: 1 })).toThrow(/unreachable variant.*validateArgs/)
|
||||
})
|
||||
|
||||
it('recurses into nested objects (and an object without properties only type-checks)', () => {
|
||||
const spec = {
|
||||
config: {
|
||||
|
||||
Reference in New Issue
Block a user