1
0
Эх сурвалжийг харах

fix(tool-cordis): validate a dynamic tool's execute return shape after the realm round-trip

The sandbox execute wrapper JSON round-tripped the return and blindly cast it
to ToolExecuteReturn. A JSON-valid but wrong-shape return — a bare string,
{ content: 'ok' }, blocks without a type tag — sailed through: the registry
spreads result.content, so { content: 'ok' } became ['o','k'], passed the
session log's isJsonValue gate, and the DeepSeek serializer then flattened it
to '(no output)' — silent corruption of the next model request and every
replay, instead of a contained tool error.

The round-tripped value is now shape-checked against the two ToolExecuteReturn
forms (array of content blocks, or { content: blocks, meta? }); block checks
are structural only (plain object + string type tag) because the ContentBlock
union is merge-extensible. A wrong shape — and the formerly cryptic
forgot-return/bare-string cases — fails that one call with a teaching error
echoing a truncated preview of what was returned and the two valid forms.
New specs pin the object-form pass-through (meta included), six rejection
shapes, and the preview truncation; per-file 100% coverage holds.
imccyu 2 сар өмнө
parent
commit
fe4da9244f

Файлын зөрүү хэтэрхий том тул дарагдсан байна
+ 0 - 0
docs/rfc/implemented/feature/2026-07-08-self-referential-cordis-toolset.md


+ 68 - 6
packages/cordis/tool-cordis/src/guard.ts

@@ -27,8 +27,12 @@
  * realm's `Object.prototype`, and the session log's append-time plainness check
  * realm's `Object.prototype`, and the session log's append-time plainness check
  * (`dsh-session`'s `isJsonValue`, a prototype-identity comparison) rejects
  * (`dsh-session`'s `isJsonValue`, a prototype-identity comparison) rejects
  * foreign-realm data — so every dynamic tool's `execute` return is JSON
  * foreign-realm data — so every dynamic tool's `execute` return is JSON
- * 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
+ * round-tripped into the host realm and shape-checked against the two
+ * `ToolExecuteReturn` forms before it reaches the registry (the registry
+ * trusts the shape blindly — it spreads `result.content`, so an unvalidated
+ * `{ content: 'ok' }` would enter the session log as `['o','k']` and silently
+ * corrupt the next model request), 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
  * schema must fail at REGISTRATION, not when a later request assembles it — so
  * dynamic tool registration accepts only definitions produced by the sandbox's
  * dynamic tool registration accepts only definitions produced by the sandbox's
  * `harness.defineTool`, which normalizes `parameters` up front.
  * `harness.defineTool`, which normalizes `parameters` up front.
@@ -137,14 +141,67 @@ function assertDynamicTool(tool: unknown): asserts tool is DynamicToolDefinition
   }
   }
 }
 }
 
 
+/**
+ * Structurally a content block, checked AFTER the JSON round-trip: a plain
+ * object carrying a string `type` tag. Deliberately nothing deeper — the
+ * ContentBlock union is merge-extensible (an unknown tag must pass), and every
+ * downstream consumer dispatches on `type` and falls through unknowns.
+ */
+function isContentBlockShape(value: unknown): boolean {
+  return isPlainRecord(value) && typeof value.type === 'string'
+}
+
+/**
+ * How much of an invalid execute return the teaching error echoes back — a
+ * huge blob would burn the model turn the error is trying to save.
+ */
+const RETURN_PREVIEW_LIMIT = 120
+
+/**
+ * Compact JSON preview of an invalid execute return for the teaching error
+ * (`String(…)` for the un-stringifiable undefined case), truncated to
+ * {@link RETURN_PREVIEW_LIMIT}.
+ */
+function describeReturn(value: unknown): string {
+  // JSON.stringify is TYPED as always returning string, but it yields
+  // undefined for an undefined input (the routed forgot-return case) — the
+  // assertion widens the type back to the runtime truth.
+  const json = JSON.stringify(value) as string | undefined
+  if (json === undefined) return String(value)
+  return json.length > RETURN_PREVIEW_LIMIT ? `${json.slice(0, RETURN_PREVIEW_LIMIT)}…` : json
+}
+
+/**
+ * Validate a round-tripped `execute` return against the two shapes
+ * {@link ToolExecuteReturn} allows: an ARRAY of content blocks, or
+ * `{ content: blocks, meta? }`. The registry trusts the shape blindly — it
+ * spreads `result.content`, so an unvalidated `{ content: 'ok' }` would enter
+ * the session log as `['o','k']` and silently corrupt the next model request —
+ * so a wrong shape fails THIS call with a teaching error instead.
+ */
+function assertExecuteReturn(value: unknown): ToolExecuteReturn {
+  if (Array.isArray(value) && value.every(isContentBlockShape)) {
+    return value as ToolExecuteReturn
+  }
+  if (isPlainRecord(value) && Array.isArray(value.content) && value.content.every(isContentBlockShape)) {
+    return value as ToolExecuteReturn
+  }
+  throw new Error(
+    `execute returned ${describeReturn(value)} — a tool's execute must return an ARRAY of content blocks, never a bare string:\n`
+    + '  ✓ return [{ type: \'text\', text: someString }]\n'
+    + '  ✓ return { content: [{ type: \'text\', text: someString }], meta: anyJsonValue }',
+  )
+}
+
 /**
 /**
  * The `harness.defineTool` handed into the sandbox: the real DSL, with
  * The `harness.defineTool` handed into the sandbox: the real DSL, with
  * `parameters` normalized into a fresh host-realm SchemaSpec (JSON-Schema
  * `parameters` normalized into a fresh host-realm SchemaSpec (JSON-Schema
  * wrapper unwrapped, `integer` mapped, `required: false` dropped) and the
  * wrapper unwrapped, `integer` mapped, `required: false` dropped) and the
  * tool's `execute` return normalized into the host realm via a JSON round-trip
  * 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.
+ * (see the module doc). The round-trip projects the return onto exactly what
+ * the log would durably store, and {@link assertExecuteReturn} then vets that
+ * projection — so a non-JSON-serializable OR wrong-shape return surfaces as
+ * that one call's teaching error instead of poisoning the turn.
  * @param options - the standard `defineTool` options; `parameters` may be the SchemaSpec DSL or a JSON-Schema-style wrapper.
  * @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.
  * @returns the marker-tagged definition `harness.registerTool` (and the guarded `ctx.tools.register`) accepts.
  */
  */
@@ -155,7 +212,12 @@ export function sandboxDefineTool(options: Parameters<typeof defineTool>[0]): To
   return markDynamicTool({
   return markDynamicTool({
     ...tool,
     ...tool,
     async execute(args, exec) {
     async execute(args, exec) {
-      return JSON.parse(JSON.stringify(await execute(args, exec))) as ToolExecuteReturn
+      // JSON.stringify yields NO JSON for an undefined (or function/symbol)
+      // return despite its string-typed signature — route that into
+      // assertExecuteReturn's teaching error rather than letting JSON.parse
+      // throw its cryptic '"undefined" is not valid JSON'.
+      const json = JSON.stringify(await execute(args, exec)) as string | undefined
+      return assertExecuteReturn(json === undefined ? undefined : JSON.parse(json) as unknown)
     },
     },
   })
   })
 }
 }

+ 89 - 0
packages/cordis/tool-cordis/tests/mount.spec.ts

@@ -60,6 +60,95 @@ describe('cordis_mount', () => {
     expect(isJsonValue({ content: reversed.content, isError: reversed.isError })).toBe(true)
     expect(isJsonValue({ content: reversed.content, isError: reversed.isError })).toBe(true)
   })
   })
 
 
+  it('threads the { content, meta } object return form through to the registry result', async () => {
+    const ctx = await setup()
+    await call(ctx, 'cordis_mount', {
+      code: `
+        return {
+          name: 'meta-return',
+          inject: ['tools'],
+          apply(ctx) {
+            harness.registerTool(ctx, harness.defineTool({
+              name: 'meta_tool',
+              description: 'attaches a private presentation payload',
+              parameters: {},
+              async execute() {
+                return { content: [{ type: 'text', text: 'ok' }], meta: { kind: 'demo' } }
+              },
+            }))
+          },
+        }
+      `,
+    })
+    const result = await call(ctx, 'meta_tool', {})
+    expect(result.isError).toBe(false)
+    expect(text(result)).toBe('ok')
+    expect(result.meta).toEqual({ kind: 'demo' })
+  })
+
+  it.each([
+    ['a bare string', 'return \'ok\'', '"ok"'],
+    ['an object whose content is a string', 'return { content: \'ok\' }', '{"content":"ok"}'],
+    ['an array of non-objects', 'return [\'ok\']', '["ok"]'],
+    ['blocks missing the type tag', 'return [{ text: \'hi\' }]', '[{"text":"hi"}]'],
+    ['object-form blocks missing the type tag', 'return { content: [{ text: \'hi\' }] }', '{"content":[{"text":"hi"}]}'],
+    ['undefined — a forgotten return', 'return undefined', 'undefined'],
+  ])('rejects an execute return of %s as that one call\'s teaching error', async (_label, returnStatement, preview) => {
+    // The failure this prevents: the registry trusts the return shape
+    // (postExecute spreads result.content), so an unvalidated { content: 'ok' }
+    // would enter the session log as ['o','k'] and silently corrupt the next
+    // model request. The shape check turns it into THIS call's error instead —
+    // one well-formed text block the log and the model can digest.
+    const ctx = await setup()
+    await call(ctx, 'cordis_mount', {
+      code: `
+        return {
+          name: 'bad-return',
+          inject: ['tools'],
+          apply(ctx) {
+            harness.registerTool(ctx, harness.defineTool({
+              name: 'bad_return_tool',
+              description: 'returns a wrong shape',
+              parameters: {},
+              async execute() { ${returnStatement} },
+            }))
+          },
+        }
+      `,
+    })
+    const result = await call(ctx, 'bad_return_tool', {})
+    expect(result.isError).toBe(true)
+    expect(result.content).toHaveLength(1)
+    expect(result.content[0]!.type).toBe('text')
+    expect(text(result)).toContain(`execute returned ${preview}`)
+    expect(text(result)).toContain('must return an ARRAY of content blocks')
+    expect(text(result)).toContain('✓ return { content: [{ type: \'text\', text: someString }], meta: anyJsonValue }')
+  })
+
+  it('truncates a huge invalid execute return in the teaching error', async () => {
+    const ctx = await setup()
+    await call(ctx, 'cordis_mount', {
+      code: `
+        return {
+          name: 'huge-return',
+          inject: ['tools'],
+          apply(ctx) {
+            harness.registerTool(ctx, harness.defineTool({
+              name: 'huge_return_tool',
+              description: 'returns a huge wrong shape',
+              parameters: {},
+              async execute() { return 'x'.repeat(500) },
+            }))
+          },
+        }
+      `,
+    })
+    const result = await call(ctx, 'huge_return_tool', {})
+    expect(result.isError).toBe(true)
+    expect(text(result)).toContain('…')
+    expect(text(result)).not.toContain('x'.repeat(200))
+  })
+
   it('accepts a JSON-Schema-style parameters wrapper and normalizes it to the DSL', 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',
     // The dialect models write by strong prior: the { type:'object',
     // properties, required: […] } wrapper, `type: 'integer'`, and
     // properties, required: […] } wrapper, `type: 'integer'`, and

Энэ ялгаанд хэт олон файл өөрчлөгдсөн тул зарим файлыг харуулаагүй болно