feat(tools): validate model-generated tool args at the boundary (RFC 005 pt 1)
defineTool now runs validateArgs against the SchemaSpec before execute, so a malformed model call returns a self-correctable isError result listing the violations instead of reaching the typed body untyped-in-practice. The validator mirrors schemaSpecToJsonSchema semantics exactly (required from required:true only, extra keys allowed, default not applied, object/array without properties/items only type-checks, enum membership). tool-bash's hand-rolled type/required checks (carrying the TODO(RFC 005) stopgap note) are slimmed to just the value constraints the DSL can't express (non-empty strings, positive timeout). Graduates RFC 005 pt 1 to ADR 0011.
This commit is contained in:
@@ -15,6 +15,8 @@ import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
export {
|
||||
defineTool,
|
||||
schemaSpecToJsonSchema,
|
||||
validateArgs,
|
||||
ToolArgsError,
|
||||
type SchemaSpec,
|
||||
type SchemaProp,
|
||||
type SchemaType,
|
||||
|
||||
@@ -167,6 +167,101 @@ export function schemaSpecToJsonSchema(spec: SchemaSpec): JsonSchemaObject {
|
||||
return result
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Runtime validation: model-generated args ↔ SchemaSpec
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Thrown by a {@link defineTool} tool when the model-generated arguments don't
|
||||
* match the declared {@link SchemaSpec}. The registry's execute waterfall
|
||||
* catches it and returns an `isError` result so the model can self-correct.
|
||||
*
|
||||
* Plain `Error` for now (carries a `code` field); a later change promotes the
|
||||
* harness error taxonomy and this extends a common base.
|
||||
*/
|
||||
export class ToolArgsError extends Error {
|
||||
/** Machine-routable code; stable across the message wording. */
|
||||
readonly code = 'INVALID_ARGS'
|
||||
/** The individual violation messages, in declaration order. */
|
||||
readonly violations: string[]
|
||||
|
||||
constructor(violations: string[]) {
|
||||
super(`invalid arguments: ${violations.join('; ')}`)
|
||||
this.name = 'ToolArgsError'
|
||||
this.violations = violations
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether a value is a non-null, non-array object (a JSON Schema `object`). */
|
||||
function isPlainObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value)
|
||||
}
|
||||
|
||||
/** Collect violations for one property value against its {@link SchemaProp}. */
|
||||
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 []
|
||||
}
|
||||
case 'number': {
|
||||
return typeof value === 'number' ? [] : [`"${path}" must be a number`]
|
||||
}
|
||||
case 'boolean': {
|
||||
return typeof value === 'boolean' ? [] : [`"${path}" must be a boolean`]
|
||||
}
|
||||
case 'object': {
|
||||
if (!isPlainObject(value)) return [`"${path}" must be an object`]
|
||||
// Mirror the converter: an object without `properties` only type-checks.
|
||||
return prop.properties ? checkSpec(prop.properties, value, path) : []
|
||||
}
|
||||
case 'array': {
|
||||
if (!Array.isArray(value)) return [`"${path}" must be an array`]
|
||||
// Mirror the converter: an array without `items` only type-checks.
|
||||
if (!prop.items) return []
|
||||
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.
|
||||
}
|
||||
}
|
||||
|
||||
/** Collect violations for an object value against a {@link SchemaSpec}. */
|
||||
function checkSpec(spec: SchemaSpec, value: unknown, path: string): string[] {
|
||||
if (!isPlainObject(value)) return [`"${path || 'arguments'}" must be an object`]
|
||||
const violations: string[] = []
|
||||
for (const [key, prop] of Object.entries(spec)) {
|
||||
const propPath = path ? `${path}.${key}` : key
|
||||
const v = value[key]
|
||||
if (v === undefined) {
|
||||
// A required key absent OR present-but-undefined is a violation; an
|
||||
// optional absent key is fine. `default` is NOT applied (validation only).
|
||||
if (prop.required === true) violations.push(`missing required property "${propPath}"`)
|
||||
continue
|
||||
}
|
||||
violations.push(...checkValue(prop, v, propPath))
|
||||
}
|
||||
return violations
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate model-generated `args` against a {@link SchemaSpec}, returning a
|
||||
* list of human-readable violation messages (empty = valid). Total — never
|
||||
* throws, regardless of how malformed `args` is.
|
||||
*
|
||||
* Semantics mirror {@link schemaSpecToJsonSchema} exactly: the top level must
|
||||
* be a non-array object; required keys come only from `required: true`; extra
|
||||
* keys are allowed (no `additionalProperties: false`); `default` is not
|
||||
* applied; an `object`/`array` prop without `properties`/`items` only
|
||||
* type-checks; `enum` is membership (strings only).
|
||||
*/
|
||||
export function validateArgs(spec: SchemaSpec, args: unknown): string[] {
|
||||
return checkSpec(spec, args, '')
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// defineTool — typed helper for first-party plugin authors
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -219,14 +314,22 @@ export interface DefineToolOptions<S extends SchemaSpec> {
|
||||
* first-party plugin authors.
|
||||
*/
|
||||
export function defineTool<S extends SchemaSpec>(options: DefineToolOptions<S>): ToolDefinition {
|
||||
// Object-literal execute methods don't use `this`; the reference is safe.
|
||||
// eslint-disable-next-line @typescript-eslint/unbound-method
|
||||
const userExecute = options.execute
|
||||
return {
|
||||
name: options.name,
|
||||
description: options.description,
|
||||
parameters: schemaSpecToJsonSchema(options.parameters) as unknown as Record<string, unknown>,
|
||||
...options.strict !== undefined ? { strict: options.strict } : {},
|
||||
// Object-literal execute methods don't use `this`; passing the reference
|
||||
// through is safe.
|
||||
// eslint-disable-next-line @typescript-eslint/unbound-method
|
||||
execute: options.execute,
|
||||
async execute(args: unknown, exec: ToolExecution): Promise<ContentBlock[]> {
|
||||
// Validate the model-generated args before the typed body runs. On
|
||||
// mismatch we throw ToolArgsError; the registry turns it into an
|
||||
// isError result so the model can self-correct. After this guard, the
|
||||
// cast to InferArgs<S> reflects the validated shape.
|
||||
const violations = validateArgs(options.parameters, args)
|
||||
if (violations.length > 0) throw new ToolArgsError(violations)
|
||||
return userExecute(args as InferArgs<S>, exec)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user