Kaynağa Gözat

fix(tool-cordis): normalize the JSON-Schema dialect at the defineTool boundary

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.
imccyu 2 ay önce
ebeveyn
işleme
a500c791f7

+ 2 - 2
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.

Dosya farkı çok büyük olduğundan ihmal edildi
+ 0 - 0
docs/tool-catalog.md


+ 68 - 34
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<unknown>(['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<string, unknown> {
   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<string, unknown> {
   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<unknown>()
   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<string, unknown> = {}
+  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<string, unknown> {
   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<string, unknown> = { 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<typeof defineTool>[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<typeof defineTool>[0])
   const execute = tool.execute.bind(tool)
   return markDynamicTool({
     ...tool,

+ 4 - 1
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 '

+ 57 - 11
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<string, { type: string; enum?: string[] }>; 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) => {

Bu fark içinde çok fazla dosya değişikliği olduğu için bazı dosyalar gösterilmiyor