From a500c791f7faa2a29437a8a446f73541e0f056fb Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:51:35 +0800 Subject: [PATCH] fix(tool-cordis): normalize the JSON-Schema dialect at the defineTool boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field sessions showed models writing tool schemas in the JSON-Schema dialect by strong prior — type: 'integer', required: false, then the full { type:'object', properties, required: [...] } wrapper — and the rejection text itself pushed a nearly-correct DSL attempt BACK to raw JSON Schema: one stats tool cost three consecutive schema errors before mounting. The boundary now normalizes wherever the input has exactly one meaning (wrapper unwrapped with the required array becoming per-property flags at any nesting level, integer → number, required: false → optional, all rebuilt as fresh host-realm objects) and rejects only genuinely meaningless input, enumerating the valid vocabulary in the error. Re-running the failing session mounts first-try. The mount description documents both accepted forms. --- ...6-07-08-self-referential-cordis-toolset.md | 4 +- docs/tool-catalog.md | 2 +- packages/cordis/tool-cordis/src/guard.ts | 102 ++++++++++++------ packages/cordis/tool-cordis/src/index.ts | 5 +- .../cordis/tool-cordis/tests/mount.spec.ts | 68 ++++++++++-- 5 files changed, 132 insertions(+), 49 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-07-08-self-referential-cordis-toolset.md b/docs/rfc/implemented/feature/2026-07-08-self-referential-cordis-toolset.md index 95cb12b571..14e05b0afd 100644 --- a/docs/rfc/implemented/feature/2026-07-08-self-referential-cordis-toolset.md +++ b/docs/rfc/implemented/feature/2026-07-08-self-referential-cordis-toolset.md @@ -32,7 +32,7 @@ Sandbox globals are deliberately small: a tagged write-through `console` (`[cord Three boundary mechanisms make model-written code behave correctly across the realm seam. **Dual-realm `instanceof`**: most objects sandbox code touches are host-realm (tool `args`, event payloads, service returns), so a plain `x instanceof Array` in the vm would silently be false — a per-sandbox prelude gives the vm realm's own constructors a `Symbol.hasInstance` that checks both the vm constructor and its host counterpart, patching only vm-realm globals. **Realm normalization of tool results**: objects built inside the vm carry the vm realm's `Object.prototype`, which the session log's append-time plainness check (`isJsonValue` in `dsh-session`, a prototype-identity comparison) rejects, so the sandbox's `harness.defineTool` JSON round-trips every `execute` return into the host realm — which also projects it onto exactly what the log durably stores. **Guarded registration**: the `ctx` a mounted plugin receives is a proxy whose `tools.register` accepts only definitions returned by `harness.defineTool` (a marker symbol), so every dynamic tool passes SchemaSpec validation and realm normalization; everything else on `ctx` passes through with correct `this` binding, which is what keeps cross-mount `provide`/`inject` working. -Boundary errors are written around the mistakes models actually make (see [Consequences](#consequences) for how each was found): JSON Schema where the SchemaSpec DSL is expected gets a ✗/✓ example pair; an unbalanced `});` closing gets the vm's offending source line plus a "code is a function body" reminder; TypeScript syntax gets the remove-annotations fix (detected on the failing line only, so an ` as ` inside a description string does not misfire); a forgotten `return` gets the two valid plugin forms; a Node built-in call gets the redirect to its cordis service; a tool-name collision on re-mount gets the unmount-first-then-remount recipe. +Boundary errors are written around the mistakes models actually make (see [Consequences](#consequences) for how each was found), and the boundary normalizes rather than lectures wherever the input has exactly one meaning: schema `parameters` accept the JSON-Schema dialect models write by strong prior — the `{ type: 'object', properties, required: […] }` wrapper unwraps to the SchemaSpec DSL (the `required` array becoming per-property flags, at any nesting level), `type: 'integer'` maps to `number`, and `required: false` reads as optional — while genuinely meaningless input is rejected with the vocabulary enumerated (an unknown type lists the five valid ones; a non-boolean `required` names the rule). The remaining teaching errors: an unbalanced `});` closing gets the vm's offending source line plus a "code is a function body" reminder; TypeScript syntax gets the remove-annotations fix (detected on the failing line only, so an ` as ` inside a description string does not misfire); a forgotten `return` gets the two valid plugin forms; a Node built-in call gets the redirect to its cordis service; a tool-name collision on re-mount gets the unmount-first-then-remount recipe. ### The dynamic group and mount lifecycle @@ -79,6 +79,6 @@ The correctness investment therefore goes where it pays for every capability at The toolset is a deliberate opt-in with a fully-privileged `ctx`, so a deployment adopts it as consciously as a bash tool. Several facts follow that the tool descriptions warn the model about directly: a waterfall listener (e.g. `tools/pre-execute`) that returns without calling `next()` vetoes the chain, so a mounted listener can lobotomize the agent's own tool dispatch ([waterfall semantics](../../../cordis-primer.md#cordis-waterfall-semantics)); mount code runs inside a tool call of the current turn, so awaiting anything that resolves only after the turn deadlocks; `vmTimeoutMs` bounds synchronous evaluation only; and mounts do not survive session resume. -The instructive boundary errors were not guessed — they were written against a live self-design session in which a real model was asked to build itself coding tools. That session surfaced the failure modes now mitigated: the model closed a returned plugin object with `});` and got only a bare `Unexpected token ')'` it retried blind; it hit a false-positive "this is TypeScript" hint because a description string contained the word "as"; and, most costly, it guessed a bash run's `stdout` was a string and burned six steps building throwaway debug tools to discover it is `{ text, truncated }`. The fixes — source-line-plus-caret parse errors, line-scoped TypeScript detection, the type-shape closure in the API catalog, and the redirect traps — cut a second session from dozens of tool calls with repeated errors to a first-try success on every capability, including a model that hit a Node-`setTimeout` trap and self-corrected to `inject: ['timer']` in one step. +The instructive boundary errors were not guessed — they were written against live self-design sessions in which a real model was asked to build itself coding tools. Those sessions surfaced the failure modes now mitigated: the model closed a returned plugin object with `});` and got only a bare `Unexpected token ')'` it retried blind; it hit a false-positive "this is TypeScript" hint because a description string contained the word "as"; it guessed a bash run's `stdout` was a string and burned six steps building throwaway debug tools to discover it is `{ text, truncated }`; and it wrote tool schemas in the JSON-Schema dialect (`type: 'integer'`, `required: false`, then the full wrapper) three rejections in a row — the rejection text itself pushing it from a nearly-correct DSL attempt back to raw JSON Schema. The fixes — source-line-plus-caret parse errors, line-scoped TypeScript detection, the type-shape closure in the API catalog, the redirect traps, and schema-dialect normalization in place of rejection — cut later sessions from dozens of tool calls with repeated errors to a first-try success on every capability, including a model that hit a Node-`setTimeout` trap and self-corrected to `inject: ['timer']` in one step. Coverage is named per tier: package unit specs drive the three tools through a real `ToolRegistry` on a real fiber tree (the mount success/failure family, vm isolation, dual-realm `instanceof`, realm normalization against the real `isJsonValue`, the SchemaSpec and raw-registration rejections, the Node-API traps, the cross-mount provide/inject matrix, catalog-backed `api`/`events` rendering, config validation, presenters, quiescent unmount, and the HMR cascade), a `MockAdapter` loop test proves a tool mounted in one step is dispatchable in the next, and the example carries a keyless Loader smoke plus a with-key smoke that world-verifies a live model mounting a listener, building its own tool, and composing two mounts. No snapshot scenario is added: the toolset ships in no ACP-served app, so it changes no editor-facing transcript, and its presenters are unit-tested pure functions — adding it to the ACP example solely for a golden would rewrite the pinned request-header tool set of every recorded scenario. diff --git a/docs/tool-catalog.md b/docs/tool-catalog.md index f0a3fab35d..91c2104a13 100644 --- a/docs/tool-catalog.md +++ b/docs/tool-catalog.md @@ -136,7 +136,7 @@ Source: [`packages/cordis/tool-cordis/src/index.ts`](../packages/cordis/tool-cor ### `cordis_mount` -Mount a NEW cordis plugin into the live runtime that is running THIS agent (self-modification). `code` runs as the body of an async JavaScript function in an isolated sandbox and MUST `return` a plugin. Two forms: FUNCTION form `return (ctx) => { … }` — cannot declare inject, uses whatever services are on the parent context, and accessing a service without inject (e.g. ctx.bash) throws; use it only when you need no injected services. OBJECT form `return { name?, inject: ['bash', 'llm', …], apply(ctx) { … } }` — declares dependencies, and cordis activates the plugin only after the services exist; PREFER this form for any plugin that needs bash, llm, sessions, etc. BEFORE calling a service from your code, read cordis_inspect what:"api" — it lists method signatures AND the type shapes of their arguments/returns (do not guess a field's type; e.g. a bash run's stdout is an object, not a string). Inside `apply`, use the standard cordis API: `ctx.on(event, listener)` to observe events (see cordis_inspect what:"events"), or call `harness.registerTool(ctx, harness.defineTool({ name, description, parameters: { text: { type: 'string', required: true } }, async execute(args) { … } }))` to give yourself a new tool — it becomes callable on your NEXT step. A tool's `execute` MUST return an ARRAY of content blocks, e.g. `return [{ type: 'text', text: someString }]` — never a bare string. Mounts can COMPOSE: one plugin may `ctx.provide('name', value)` a service and another may declare `inject: ['name']` to consume it — the consumer stays pending until the provider exists and returns to pending when the provider is unmounted. Everything registered inside `apply` is cleaned up automatically on unmount. Sandbox globals: `console` (tagged `[cordis:]`, writes through to the harness terminal), `harness.defineTool`, `harness.registerTool`, `btoa`, `atob`, `TextEncoder`, `TextDecoder`. Node APIs are DISABLED — do filesystem/network/timer work through the cordis services, never Node built-ins: `require`, `setTimeout`/`setInterval`, and `fetch` throw redirect errors; `process` and `Buffer` are undefined. Instead use inject: ['fs'] + ctx.fs for files, inject: ['web'] + ctx.web for HTTP, inject: ['bash'] + ctx.bash for processes, and inject: ['timer'] + ctx.setTimeout/ctx.setInterval for timing (fiber effects, auto-cleaned on unmount) — cordis_inspect what:"api" shows what THIS runtime provides. Write PLAIN JavaScript, not TypeScript (no `as`, no type annotations). Cautions: (1) waterfall events (e.g. tools/pre-execute) hand the listener a trailing `next` callback which MUST be called — returning without `next()` VETOES the call; prefer plain notification events unless you intend to intercept. (2) Never await something that only resolves after the current turn (your code runs INSIDE a tool call of that turn — it would deadlock). (3) The sandbox prevents accidental global pollution, not malice: `ctx` is the real, fully privileged runtime handle. +Mount a NEW cordis plugin into the live runtime that is running THIS agent (self-modification). `code` runs as the body of an async JavaScript function in an isolated sandbox and MUST `return` a plugin. Two forms: FUNCTION form `return (ctx) => { … }` — cannot declare inject, uses whatever services are on the parent context, and accessing a service without inject (e.g. ctx.bash) throws; use it only when you need no injected services. OBJECT form `return { name?, inject: ['bash', 'llm', …], apply(ctx) { … } }` — declares dependencies, and cordis activates the plugin only after the services exist; PREFER this form for any plugin that needs bash, llm, sessions, etc. BEFORE calling a service from your code, read cordis_inspect what:"api" — it lists method signatures AND the type shapes of their arguments/returns (do not guess a field's type; e.g. a bash run's stdout is an object, not a string). Inside `apply`, use the standard cordis API: `ctx.on(event, listener)` to observe events (see cordis_inspect what:"events"), or call `harness.registerTool(ctx, harness.defineTool({ name, description, parameters: { text: { type: 'string', required: true } }, async execute(args) { … } }))` to give yourself a new tool — it becomes callable on your NEXT step. Tool parameters: each key IS a property — { type: 'string'|'number'|'boolean'|'object'|'array', required?: true, description?, enum?, items?, properties? }; a JSON-Schema-style { type: 'object', properties, required: […] } wrapper and type 'integer' are also accepted and normalized. A tool's `execute` MUST return an ARRAY of content blocks, e.g. `return [{ type: 'text', text: someString }]` — never a bare string. Mounts can COMPOSE: one plugin may `ctx.provide('name', value)` a service and another may declare `inject: ['name']` to consume it — the consumer stays pending until the provider exists and returns to pending when the provider is unmounted. Everything registered inside `apply` is cleaned up automatically on unmount. Sandbox globals: `console` (tagged `[cordis:]`, writes through to the harness terminal), `harness.defineTool`, `harness.registerTool`, `btoa`, `atob`, `TextEncoder`, `TextDecoder`. Node APIs are DISABLED — do filesystem/network/timer work through the cordis services, never Node built-ins: `require`, `setTimeout`/`setInterval`, and `fetch` throw redirect errors; `process` and `Buffer` are undefined. Instead use inject: ['fs'] + ctx.fs for files, inject: ['web'] + ctx.web for HTTP, inject: ['bash'] + ctx.bash for processes, and inject: ['timer'] + ctx.setTimeout/ctx.setInterval for timing (fiber effects, auto-cleaned on unmount) — cordis_inspect what:"api" shows what THIS runtime provides. Write PLAIN JavaScript, not TypeScript (no `as`, no type annotations). Cautions: (1) waterfall events (e.g. tools/pre-execute) hand the listener a trailing `next` callback which MUST be called — returning without `next()` VETOES the call; prefer plain notification events unless you intend to intercept. (2) Never await something that only resolves after the current turn (your code runs INSIDE a tool call of that turn — it would deadlock). (3) The sandbox prevents accidental global pollution, not malice: `ctx` is the real, fully privileged runtime handle. ```json { diff --git a/packages/cordis/tool-cordis/src/guard.ts b/packages/cordis/tool-cordis/src/guard.ts index 09d804771f..998997a8f9 100644 --- a/packages/cordis/tool-cordis/src/guard.ts +++ b/packages/cordis/tool-cordis/src/guard.ts @@ -1,19 +1,27 @@ /** * The registration boundary between sandboxed mount code and the real runtime: - * SchemaSpec validation with teaching errors, the marker-guarded - * `harness.defineTool` / `harness.registerTool` pair, the guarded `ctx` proxy a - * mounted plugin receives, and the plugin-shape helpers the mount lifecycle - * narrows sandbox return values with. + * SchemaSpec normalization + validation with teaching errors, the + * marker-guarded `harness.defineTool` / `harness.registerTool` pair, the + * guarded `ctx` proxy a mounted plugin receives, and the plugin-shape helpers + * the mount lifecycle narrows sandbox return values with. * * Two realm facts drive the design. Objects built inside the vm carry the vm * realm's `Object.prototype`, and the session log's append-time plainness check * (`dsh-session`'s `isJsonValue`, a prototype-identity comparison) rejects * foreign-realm data — so every dynamic tool's `execute` return is JSON - * round-tripped into the host realm before it reaches the registry. And a - * malformed tool schema must fail at REGISTRATION, not when a later request - * assembles it — so dynamic `ctx.tools.register` calls accept only definitions - * produced by the sandbox's `harness.defineTool`, which asserts the SchemaSpec - * DSL up front. + * round-tripped into the host realm before it reaches the registry, and the + * schema itself is rebuilt as fresh host-realm objects. And a malformed tool + * schema must fail at REGISTRATION, not when a later request assembles it — so + * dynamic `ctx.tools.register` calls accept only definitions produced by the + * sandbox's `harness.defineTool`, which normalizes `parameters` up front. + * + * Normalize, don't lecture, where the input has exactly one meaning: models + * write the JSON-Schema dialect by strong prior (the `{ type: 'object', + * properties, required: […] }` wrapper, `type: 'integer'`, `required: false`), + * and each rejection costs a model turn — so those convert to the SchemaSpec + * DSL silently, and only genuinely meaningless input (an unknown type, a + * non-boolean `required`) is rejected, with the error enumerating the valid + * vocabulary. * * @module @deepseek-ai/dsh-tool-cordis/guard */ @@ -24,6 +32,7 @@ import type { ToolDefinition, ToolExecuteReturn } from '@deepseek-ai/dsh-tools' const DYNAMIC_TOOL = Symbol('tool-cordis.dynamic-tool') const SCHEMA_TYPES = new Set(['string', 'number', 'boolean', 'object', 'array']) +const VALID_TYPES = '\'string\' | \'number\' | \'boolean\' | \'object\' | \'array\'' type DynamicToolDefinition = ToolDefinition & { [DYNAMIC_TOOL]: true } type DynamicToolMarker = { [DYNAMIC_TOOL]?: unknown } @@ -32,47 +41,70 @@ function isPlainRecord(value: unknown): value is Record { return Object.prototype.toString.call(value) === '[object Object]' } -/** Assert a sandbox-provided `parameters` value is a SchemaSpec object, with a teaching error for the common JSON-Schema mistake. */ -function assertSchemaSpec(value: unknown): void { +/** + * Normalize a sandbox-provided `parameters` value into a fresh host-realm + * SchemaSpec. Accepts the DSL directly, or the JSON-Schema-style + * `{ type: 'object', properties, required: […] }` wrapper models write by + * prior — the wrapper unwraps and its `required` array becomes per-property + * flags (see the module doc). + */ +function normalizeSchemaSpec(value: unknown, path = 'parameters'): Record { if (!isPlainRecord(value)) { - throw new Error('harness.defineTool parameters must be a SchemaSpec object') + throw new Error(`harness.defineTool ${path} must be a SchemaSpec object`) } + let entries = value + const requiredNames = new Set() if (value.type === 'object' && isPlainRecord(value.properties)) { - throw new Error( - 'harness.defineTool parameters use the SchemaSpec DSL (NOT JSON Schema).\n' - + ' ✗ { type: \'object\', properties: { name: { type: \'string\' } }, required: [\'name\'] }\n' - + ' ✓ { name: { type: \'string\', required: true } }\n' - + 'Remove the outer { type: \'object\', properties, required } wrapper; ' - + 'each key IS a property directly on the parameters object.', - ) + if (Array.isArray(value.required)) { + for (const name of value.required) requiredNames.add(name) + } + entries = value.properties } - for (const [key, prop] of Object.entries(value)) { - assertSchemaProp(prop, `parameters.${key}`) + const spec: Record = {} + for (const [key, prop] of Object.entries(entries)) { + spec[key] = normalizeSchemaProp(prop, `${path}.${key}`, requiredNames.has(key)) } + return spec } -function assertSchemaProp(value: unknown, path: string): void { +/** Normalize one property: `integer` → `number`, `required: false` → absent, nested wrappers unwrapped recursively. */ +function normalizeSchemaProp(value: unknown, path: string, forceRequired = false): Record { if (!isPlainRecord(value)) { throw new Error(`harness.defineTool ${path} must be a SchemaSpec property object`) } - if (!SCHEMA_TYPES.has(value.type)) { - throw new Error(`harness.defineTool ${path} must declare a valid type`) + const type = value.type === 'integer' ? 'number' : value.type + if (!SCHEMA_TYPES.has(type)) { + throw new Error(`harness.defineTool ${path} must declare a valid type: ${VALID_TYPES} (got ${JSON.stringify(value.type)})`) } - if (value.required !== undefined && value.required !== true) { - throw new Error(`harness.defineTool ${path}.required must be true when present`) + // On an object property a JSON-Schema-style `required` ARRAY names required + // children (handled by the nested unwrap below); everywhere else `required` + // must be a boolean, and `false` simply reads as optional. + const nestedRequiredArray = type === 'object' && Array.isArray(value.required) + if (value.required !== undefined && typeof value.required !== 'boolean' && !nestedRequiredArray) { + throw new Error(`harness.defineTool ${path}.required must be a boolean when present`) } + const prop: Record = { type } + if (forceRequired || value.required === true) prop.required = true + if (typeof value.description === 'string') prop.description = value.description + if (Array.isArray(value.enum)) prop.enum = [...value.enum as unknown[]] + if (value.default !== undefined) prop.default = value.default if (value.properties !== undefined) { - if (value.type !== 'object') { + if (type !== 'object') { throw new Error(`harness.defineTool ${path}.properties is only valid for type "object"`) } - assertSchemaSpec(value.properties) + // Re-wrap so the nested unwrap applies a nested `required` array too. + prop.properties = normalizeSchemaSpec( + { type: 'object', properties: value.properties, required: value.required }, + `${path}.properties`, + ) } if (value.items !== undefined) { - if (value.type !== 'array') { + if (type !== 'array') { throw new Error(`harness.defineTool ${path}.items is only valid for type "array"`) } - assertSchemaProp(value.items, `${path}.items`) + prop.items = normalizeSchemaProp(value.items, `${path}.items`) } + return prop } function markDynamicTool(tool: ToolDefinition): DynamicToolDefinition { @@ -87,17 +119,19 @@ function assertDynamicTool(tool: unknown): asserts tool is DynamicToolDefinition } /** - * The `harness.defineTool` handed into the sandbox: the real DSL, with the + * The `harness.defineTool` handed into the sandbox: the real DSL, with + * `parameters` normalized into a fresh host-realm SchemaSpec (JSON-Schema + * wrapper unwrapped, `integer` mapped, `required: false` dropped) and the * tool's `execute` return normalized into the host realm via a JSON round-trip * (see the module doc). The round-trip also projects the return onto exactly * what the log would durably store, so a non-JSON-serializable return surfaces * as that one call's error instead of poisoning the turn. - * @param options - the standard `defineTool` options, with `parameters` asserted against the SchemaSpec DSL before the DSL sees them. + * @param options - the standard `defineTool` options; `parameters` may be the SchemaSpec DSL or a JSON-Schema-style wrapper. * @returns the marker-tagged definition `harness.registerTool` (and the guarded `ctx.tools.register`) accepts. */ export function sandboxDefineTool(options: Parameters[0]): ToolDefinition { - assertSchemaSpec((options as { parameters?: unknown }).parameters) - const tool = defineTool(options) + const parameters = normalizeSchemaSpec((options as { parameters?: unknown }).parameters) + const tool = defineTool({ ...options, parameters } as Parameters[0]) const execute = tool.execute.bind(tool) return markDynamicTool({ ...tool, diff --git a/packages/cordis/tool-cordis/src/index.ts b/packages/cordis/tool-cordis/src/index.ts index de028b384a..7dff9b993d 100644 --- a/packages/cordis/tool-cordis/src/index.ts +++ b/packages/cordis/tool-cordis/src/index.ts @@ -143,7 +143,10 @@ export function apply(ctx: Context, config: Config): void { + 'events (see cordis_inspect what:"events"), or call ' + '`harness.registerTool(ctx, harness.defineTool({ name, description, parameters: ' + '{ text: { type: \'string\', required: true } }, async execute(args) { … } }))` ' - + 'to give yourself a new tool — it becomes callable on your NEXT step. A ' + + 'to give yourself a new tool — it becomes callable on your NEXT step. ' + + 'Tool parameters: each key IS a property — { type: \'string\'|\'number\'|\'boolean\'|\'object\'|\'array\', ' + + 'required?: true, description?, enum?, items?, properties? }; a JSON-Schema-style ' + + '{ type: \'object\', properties, required: […] } wrapper and type \'integer\' are also accepted and normalized. A ' + 'tool\'s `execute` MUST return an ARRAY of content blocks, e.g. `return ' + '[{ type: \'text\', text: someString }]` — never a bare string. ' + 'Mounts can COMPOSE: one plugin may `ctx.provide(\'name\', value)` a service and ' diff --git a/packages/cordis/tool-cordis/tests/mount.spec.ts b/packages/cordis/tool-cordis/tests/mount.spec.ts index 03038f818c..543b38307f 100644 --- a/packages/cordis/tool-cordis/tests/mount.spec.ts +++ b/packages/cordis/tool-cordis/tests/mount.spec.ts @@ -60,39 +60,85 @@ describe('cordis_mount', () => { expect(isJsonValue({ content: reversed.content, isError: reversed.isError })).toBe(true) }) - it('rejects JSON Schema passed to harness.defineTool with the SchemaSpec teaching error', async () => { + it('accepts a JSON-Schema-style parameters wrapper and normalizes it to the DSL', async () => { + // The dialect models write by strong prior: the { type:'object', + // properties, required: […] } wrapper, `type: 'integer'`, and + // `required: false`. All of it has exactly one meaning — normalize instead + // of burning a model turn on a lecture. const ctx = await setup() const result = await call(ctx, 'cordis_mount', { code: ` return { - name: 'bad-json-schema-tool', + name: 'json-schema-tool', inject: ['tools'], apply(ctx) { harness.registerTool(ctx, harness.defineTool({ - name: 'bad_json_schema_tool', - description: 'bad', + name: 'json_schema_tool', + description: 'written in the JSON-Schema dialect', parameters: { type: 'object', - properties: { text: { type: 'string' } }, + properties: { + text: { type: 'string', description: 'the text' }, + count: { type: 'integer', default: 1 }, + mode: { type: 'string', enum: ['fast', 'slow'] }, + extra: { type: 'string', required: false }, + }, required: ['text'], }, - async execute() { return [{ type: 'text', text: 'bad' }] }, + async execute(args) { return [{ type: 'text', text: args.text + ':' + (args.count ?? 0) }] }, })) }, } `, }) + expect(result.isError).toBe(false) - expect(result.isError).toBe(true) - expect(text(result)).toContain('harness.defineTool parameters use the SchemaSpec DSL') - expect(ctx.tools.get('bad_json_schema_tool')).toBeUndefined() + // The registered schema is canonical JSON Schema derived from the DSL: + // the required array survived, integer became number, extra is optional. + const schema = ctx.tools.schemas().find(s => s.name === 'json_schema_tool')! + const parameters = schema.parameters as { properties: Record; required?: string[] } + expect(parameters.required).toEqual(['text']) + expect(parameters.properties.count!.type).toBe('number') + expect(parameters.properties.mode!.enum).toEqual(['fast', 'slow']) + // Arg validation enforces the normalized spec: text required, extra not. + expect((await call(ctx, 'json_schema_tool', { count: 2 })).isError).toBe(true) + expect(text(await call(ctx, 'json_schema_tool', { text: 'ok', count: 2 }))).toBe('ok:2') + }) + + it('normalizes a nested object property carrying a JSON-Schema required array', async () => { + // On an object PROPERTY, a JSON-Schema-style `required` array names the + // required children — the nested unwrap converts it just like the top level. + const ctx = await setup() + const result = await call(ctx, 'cordis_mount', { + code: ` + return { + name: 'nested-json-schema', + inject: ['tools'], + apply(ctx) { + harness.registerTool(ctx, harness.defineTool({ + name: 'nested_json_schema_tool', + description: 'nested dialect', + parameters: { + cfg: { type: 'object', properties: { label: { type: 'string' } }, required: ['label'] }, + }, + async execute(args) { return [{ type: 'text', text: args.cfg.label }] }, + })) + }, + } + `, + }) + expect(result.isError).toBe(false) + const schema = ctx.tools.schemas().find(s => s.name === 'nested_json_schema_tool')! + const cfg = (schema.parameters as { properties: { cfg: { required?: string[] } } }).properties.cfg + expect(cfg.required).toEqual(['label']) + expect(text(await call(ctx, 'nested_json_schema_tool', { cfg: { label: 'hi' } }))).toBe('hi') }) it.each([ ['parameters: 42', 'must be a SchemaSpec object'], ['parameters: { text: 42 }', 'parameters.text must be a SchemaSpec property object'], - ['parameters: { text: { type: \'str\' } }', 'parameters.text must declare a valid type'], - ['parameters: { text: { type: \'string\', required: false } }', 'parameters.text.required must be true when present'], + ['parameters: { text: { type: \'str\' } }', 'parameters.text must declare a valid type: \'string\' | \'number\' | \'boolean\' | \'object\' | \'array\' (got "str")'], + ['parameters: { text: { type: \'string\', required: \'yes\' } }', 'parameters.text.required must be a boolean when present'], ['parameters: { text: { type: \'string\', properties: {} } }', 'parameters.text.properties is only valid for type "object"'], ['parameters: { text: { type: \'string\', items: { type: \'string\' } } }', 'parameters.text.items is only valid for type "array"'], ])('rejects a malformed SchemaSpec (%s) with a teaching error', async (parameters, message) => {