Merge branch 'codex/canonical-tool-output' into codex/code-mode-typed-results

# Conflicts: # docs/config-catalog.md # docs/cordis-catalog/services.md # examples/acp-agent/tests/snapshots/advanced-toolchain/system-prompt.expected.md # examples/acp-agent/tests/snapshots/both-mode-turn/system-prompt.expected.md # examples/acp-agent/tests/snapshots/code-mode-turn/system-prompt.expected.md # examples/acp-agent/tests/snapshots/code-mode-workspace-context/system-prompt.expected.md # packages/core/tools/README.md
2026-07-21 18:08:56 +08:00
parent 02da51ff81 e1633fbc3f
commit 9e01fb060e
54 changed files with 423 additions and 133 deletions
--- a/packages/core/agent-loop/src/tool-calls.ts
+++ b/packages/core/agent-loop/src/tool-calls.ts
@@ -248,7 +248,7 @@ function appendToolResult(
    callId: block.id,
    content: result.content,
    isError: result.isError,
-    ...result.error ? { error: result.error } : {},
+    ...result.error?.info ? { error: result.error.info } : {},
    // The tool's private presentation payload (e.g. a result-time diff),
    // persisted so a UI bridge reproduces the card on replay.
    ...result.meta !== undefined ? { meta: result.meta } : {},
--- a/packages/core/agent-loop/tests/cancel.spec.ts
+++ b/packages/core/agent-loop/tests/cancel.spec.ts
@@ -397,7 +397,7 @@ describe('Agent.cancel()', () => {
    expect(result?.type === 'tool/result' ? result.data : undefined).toMatchObject({
      callId: 'c1',
      isError: true,
-      error: { info: { name: 'AbortError', code: 'ABORTED' } },
+      error: { name: 'AbortError', code: 'ABORTED' },
    })

    send(agent, 'continue safely')
--- a/packages/core/agent-loop/tests/contract-regressions.spec.ts
+++ b/packages/core/agent-loop/tests/contract-regressions.spec.ts
@@ -259,7 +259,7 @@ describe('abort during tool execution ends the turn', () => {
        case 'assistant/message': order.push('assistant/message'); break
        case 'tool/call': order.push(`tool/call:${event.data.callId}`); break
        case 'tool/result': {
-          const outcome = event.data.error?.info?.code === 'ABORTED' ? 'synthetic-aborted' : 'real'
+          const outcome = event.data.error?.code === 'ABORTED' ? 'synthetic-aborted' : 'real'
          order.push(`tool/result:${event.data.callId}:${outcome}`)
          break
        }
@@ -308,7 +308,7 @@ describe('abort during tool execution ends the turn', () => {
    expect(results[1]!.data).toMatchObject({
      callId: CallId('c2'),
      isError: true,
-      error: { info: { name: 'AbortError', code: 'ABORTED' } },
+      error: { name: 'AbortError', code: 'ABORTED' },
    })
  })

--- a/packages/core/agent-loop/tests/tool-calls.spec.ts
+++ b/packages/core/agent-loop/tests/tool-calls.spec.ts
@@ -479,18 +479,12 @@ describe('tool-call scheduler: abort handling', () => {
      {
        callId: CallId('c1'),
        isError: true,
-        error: {
-          message: 'tool call skipped because the step was aborted before execution',
-          info: { name: 'AbortError', code: 'ABORTED' },
-        },
+        error: { name: 'AbortError', code: 'ABORTED' },
      },
      {
        callId: CallId('c2'),
        isError: true,
-        error: {
-          message: 'tool call skipped because the step was aborted before execution',
-          info: { name: 'AbortError', code: 'ABORTED' },
-        },
+        error: { name: 'AbortError', code: 'ABORTED' },
      },
    ])
  })
@@ -523,7 +517,7 @@ describe('tool-call scheduler: abort handling', () => {
    expect(events(agent).filter(e => e.type === 'tool/result').map(e => e.data.callId))
      .toEqual([CallId('c1'), CallId('c2')])
    expect(events(agent).filter(e => e.type === 'tool/result').at(-1)?.data)
-      .toMatchObject({ callId: CallId('c2'), isError: true, error: { info: { name: 'AbortError', code: 'ABORTED' } } })
+      .toMatchObject({ callId: CallId('c2'), isError: true, error: { name: 'AbortError', code: 'ABORTED' } })
  })

  it('stops replenishing after abort, commits started results, and drains accepted additional contexts', async () => {
@@ -555,7 +549,7 @@ describe('tool-call scheduler: abort handling', () => {
    expect(events(agent).filter(e => e.type === 'tool/result').slice(-2).map(e => ({
      callId: e.data.callId,
      isError: e.data.isError,
-      errorInfo: e.data.error?.info,
+      errorInfo: e.data.error,
    })))
      .toEqual([
        { callId: CallId('c3'), isError: true, errorInfo: { name: 'AbortError', code: 'ABORTED' } },
@@ -601,6 +595,6 @@ describe('tool-call scheduler: abort handling', () => {
    expect(events(agent).filter(e => e.type === 'tool/call').map(e => e.data.callId))
      .toEqual([CallId('c1'), CallId('c2'), CallId('c3')])
    expect(events(agent).filter(e => e.type === 'tool/result').at(-1)?.data)
-      .toMatchObject({ callId: CallId('c3'), isError: true, error: { info: { name: 'AbortError', code: 'ABORTED' } } })
+      .toMatchObject({ callId: CallId('c3'), isError: true, error: { name: 'AbortError', code: 'ABORTED' } })
  })
 })
--- a/packages/core/session/README.md
+++ b/packages/core/session/README.md
@@ -58,7 +58,7 @@ Durable values need one accepted representation, not a check followed by a secon

 `context/message` renders its `content` verbatim as a user-role message, and may attach JSON `meta` for replayable plugin state; metadata remains durable but is excluded from `deriveMessages()`.

-`tool/result` persists the model-facing content, canonical failure detail, and optional presentation metadata. A tool's successful canonical `value` is deliberately execution-local and never enters the session event, so replay reconstructs the Native/model presentation but cannot recover intermediate programmatic values. This does not change `SESSION_FORMAT_VERSION`: the persisted projection remains authoritative.
+`tool/result` persists the model-facing content, optional internal failure identity, and optional presentation metadata. A tool's successful canonical `value` and human-readable canonical failure message remain execution-local; rendered error content is the replay-authoritative message. This preserves the existing event shape and does not change `SESSION_FORMAT_VERSION`.

 ### Session event vocabulary (`types.ts`)

--- a/packages/core/session/src/json.ts
+++ b/packages/core/session/src/json.ts
@@ -3,11 +3,12 @@
 /**
 * A value that round-trips losslessly through JSON: `null`, a boolean, a finite
 * number other than negative zero, a string, an array of such values, or a
- * plain object whose values are such values. TypeScript cannot distinguish
- * `-0` from `number`, so {@link isJsonValue} and {@link snapshotJsonValue}
- * enforce that last numeric detail at runtime. Use this type for a payload that
- * must survive session-log persistence and replay byte-identically — e.g. a
- * tool's private presentation `meta`.
+ * plain object whose values are such values. Arrays may carry only their dense
+ * indexed elements; extra own properties would be discarded by JSON. TypeScript
+ * cannot distinguish `-0` from `number`, so {@link isJsonValue} and
+ * {@link snapshotJsonValue} enforce these details at runtime. Use this type for
+ * a payload that must survive session-log persistence and replay byte-identically
+ * — e.g. a tool's private presentation `meta`.
 */
 export type JsonValue = null | boolean | number | string | JsonValue[] | { [key: string]: JsonValue }

@@ -47,6 +48,10 @@ export function snapshotJsonValue<T>(value: T): T | undefined {
      if (Array.isArray(current)) {
        if (Object.getPrototypeOf(current) !== Array.prototype) return undefined
        const length = current.length
+        // Every ordinary array owns `length`; dense indexed elements account
+        // for the remaining keys. Anything else would be lost by JSON and by
+        // structured clone, including symbols and non-enumerable properties.
+        if (Reflect.ownKeys(current).length !== length + 1) return undefined
        const snapshot: JsonValue[] = []
        for (let index = 0; index < length; index++) {
          if (!Object.prototype.hasOwnProperty.call(current, index)) return undefined
@@ -111,6 +116,7 @@ export function isJsonValue(value: unknown, seen: Set<object> = new Set()): bool
  try {
    if (Array.isArray(value)) {
      if (Object.getPrototypeOf(value) !== Array.prototype) return false
+      if (Reflect.ownKeys(value).length !== value.length + 1) return false
      // Reject sparse arrays: a hole is skipped by `every`/`forEach` but
      // JSON.stringify writes it as `null`, so `[1, , 3]` would round-trip
      // lossily. Require every index 0..length-1 to be an OWN property.
--- a/packages/core/session/src/repair.ts
+++ b/packages/core/session/src/repair.ts
@@ -92,10 +92,7 @@ export function interruptedTurnClosers(events: readonly SessionEvent[]): Session
        callId,
        content: [{ type: 'text', text: 'Tool call interrupted by a crash; no result was recorded.' }],
        isError: true,
-        error: {
-          message: 'Tool call interrupted by a crash; no result was recorded.',
-          info: { name: 'InterruptedError', code: 'interrupted' },
-        },
+        error: { name: 'InterruptedError', code: 'interrupted' },
      },
      surfaceOp: 'append',
      ...callSeq !== undefined ? { sourceEventSeqs: [callSeq] } : {},
--- a/packages/core/session/src/types.ts
+++ b/packages/core/session/src/types.ts
@@ -243,12 +243,13 @@ export interface SessionEventMap {
   */
  'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string }
  /**
-   * A completed tool call's model-facing result, canonical failure detail, and
-   * optional tool-private `meta` presentation payload. `meta` is opaque to the
-   * core (the producing tool owns its shape and reads it back in `presentResult`)
-   * but MUST be JSON-serializable: `Session.append` runtime-validates all event
-   * data with `isJsonValue`, so a non-serializable `meta` is rejected at the
-   * source, and the durable log reproduces the identical card on replay. Absent
+   * A completed tool call's model-facing result, optional internal failure
+   * identity, and optional tool-private `meta` presentation payload. `meta` is
+   * opaque to the core (the producing tool owns its shape and reads it back in
+   * `presentResult`) but MUST be JSON-serializable: `Session.append`
+   * runtime-validates all event data with `isJsonValue`, so a non-serializable
+   * `meta` is rejected at the source, and the durable log reproduces the
+   * identical card on replay. Absent
   * unless the tool attaches one (e.g. `dsh-tool-fs` carries its result-time
   * contextual diff here).
   */
@@ -258,7 +259,7 @@ export interface SessionEventMap {
    callId: CallId
    content: ContentBlock[]
    isError: boolean
-    error?: { message: string; info?: { name: string; code: string } }
+    error?: { name: string; code: string }
    meta?: JsonValue
  }
  /** Steering content injected between steps of a running turn. */
--- a/packages/core/session/tests/json.spec.ts
+++ b/packages/core/session/tests/json.spec.ts
@@ -63,12 +63,16 @@ describe('snapshotJsonValue', () => {
    expect(arrayReads).toBe(1)
  })

-  it('rejects exotic containers, sparse arrays, cycles, and invalid children', () => {
+  it('rejects exotic containers, sparse or decorated arrays, cycles, and invalid children', () => {
    class ExoticObject {
      readonly value = 1
    }
    class ExoticArray extends Array<number> {}
    const sparse = new Array<number>(1)
+    const decorated = [1]
+    Object.defineProperty(decorated, 'extra', { value: true })
+    const symbolDecorated = [1]
+    Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
    const cyclic: Record<string, unknown> = {}
    cyclic.self = cyclic

@@ -76,6 +80,8 @@ describe('snapshotJsonValue', () => {
    expect(snapshotJsonValue(new Map([['value', 1]]))).toBeUndefined()
    expect(snapshotJsonValue(new ExoticArray(1))).toBeUndefined()
    expect(snapshotJsonValue(sparse)).toBeUndefined()
+    expect(snapshotJsonValue(decorated)).toBeUndefined()
+    expect(snapshotJsonValue(symbolDecorated)).toBeUndefined()
    expect(snapshotJsonValue(cyclic)).toBeUndefined()
    expect(snapshotJsonValue([undefined])).toBeUndefined()
    expect(snapshotJsonValue({ value: undefined })).toBeUndefined()
@@ -133,16 +139,21 @@ describe('isJsonValue', () => {
    expect(isJsonValue(nullPrototype)).toBe(true)
  })

-  it('rejects sparse arrays, invalid children, exotic objects, and cycles', () => {
+  it('rejects sparse or decorated arrays, invalid children, exotic objects, and cycles', () => {
    class Exotic {
      readonly value = 1
    }
    class ExoticArray extends Array<number> {}
    const sparse = new Array<number>(1)
+    const decorated = Object.assign([1], { extra: true })
+    const symbolDecorated = [1]
+    Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
    const cyclic: Record<string, unknown> = {}
    cyclic.self = cyclic

    expect(isJsonValue(sparse)).toBe(false)
+    expect(isJsonValue(decorated)).toBe(false)
+    expect(isJsonValue(symbolDecorated)).toBe(false)
    expect(isJsonValue(new ExoticArray(1))).toBe(false)
    expect(isJsonValue([undefined])).toBe(false)
    expect(isJsonValue({ value: undefined })).toBe(false)
--- a/packages/core/session/tests/repair.spec.ts
+++ b/packages/core/session/tests/repair.spec.ts
@@ -64,7 +64,7 @@ describe('interruptedTurnClosers', () => {
    expect(closers.map(e => e.seq)).toEqual([3, 4, 5])
    const result = closers[0]!
    expect(result.type === 'tool/result' && result.data).toMatchObject({
-      turn: 2, step: 1, callId: CallId('call-1'), isError: true, error: { info: { code: 'interrupted' } },
+      turn: 2, step: 1, callId: CallId('call-1'), isError: true, error: { code: 'interrupted' },
    })
  })

--- a/packages/core/tools/src/index.ts
+++ b/packages/core/tools/src/index.ts
@@ -350,6 +350,25 @@ export class ToolOutputError extends HarnessError {
  }
 }

+/** Convert one projector exception into the canonical invalid-output failure. */
+function projectionError(toolName: string, projector: 'render' | 'presentationMeta', error: unknown): ToolOutputError {
+  return new ToolOutputError(toolName, [`output.${projector} failed: ${errorMessage(error)}`])
+}
+
+/** Snapshot one projector result before later durable-result materialization. */
+function snapshotProjection<T>(toolName: string, projector: 'render' | 'presentationMeta', candidate: T): T {
+  try {
+    const detached = snapshotJsonValue(candidate)
+    if (detached === undefined) {
+      throw new ToolOutputError(toolName, [`output.${projector} returned non-lossless JSON`])
+    }
+    return detached
+  } catch (error: unknown) {
+    if (error instanceof ToolOutputError) throw error
+    throw projectionError(toolName, projector, error)
+  }
+}
+
 /** Successful canonical tool execution, including its Native/model projection. */
 export interface ToolExecutionSuccess {
  readonly isError: false
@@ -1167,10 +1186,23 @@ export class ToolRegistry extends Service {
    const violations = validateJsonSchemaValue(tool.output.schema, detached, 'value')
    if (violations.length > 0) throw new ToolOutputError(tool.name, violations)
    const value = deepFreeze(detached as JsonValue)
-    const content = tool.output.render(exec.arguments, value)
-    const meta = exec.parent === undefined && tool.output.presentationMeta !== undefined
-      ? tool.output.presentationMeta(exec.arguments, value)
-      : undefined
+    let rendered: ContentBlock[]
+    try {
+      rendered = tool.output.render(exec.arguments, value)
+    } catch (error: unknown) {
+      throw projectionError(tool.name, 'render', error)
+    }
+    const content = snapshotProjection(tool.name, 'render', rendered)
+    let meta: JsonValue | undefined
+    if (exec.parent === undefined && tool.output.presentationMeta !== undefined) {
+      let projected: JsonValue
+      try {
+        projected = tool.output.presentationMeta(exec.arguments, value)
+      } catch (error: unknown) {
+        throw projectionError(tool.name, 'presentationMeta', error)
+      }
+      meta = snapshotProjection(tool.name, 'presentationMeta', projected)
+    }
    return this.markCanonical(this.materializeFinalResult({
      isError: false,
      value,
--- a/packages/core/tools/src/json-schema.ts
+++ b/packages/core/tools/src/json-schema.ts
@@ -235,13 +235,21 @@ function checkSchemaNode(node: unknown, path: string, violations: string[], seen
      case 'boolean':
      case 'null': {
        const allowed = node.enum
+        const enumValid = Array.isArray(allowed)
+          && allowed.length > 0
+          && allowed.every(entry => scalarMatches(schemaType, entry))
        if (Object.hasOwn(node, 'enum')) {
-          if (!Array.isArray(allowed) || allowed.length === 0 || !allowed.every(entry => scalarMatches(schemaType, entry))) {
+          if (!enumValid) {
            violations.push(`${path}.enum must be a non-empty array of ${schemaType} values`)
          }
        }
-        if (Object.hasOwn(node, 'const') && !scalarMatches(schemaType, node.const)) {
-          violations.push(`${path}.const must be a ${schemaType} value`)
+        const constValid = scalarMatches(schemaType, node.const)
+        if (Object.hasOwn(node, 'const')) {
+          if (!constValid) {
+            violations.push(`${path}.const must be a ${schemaType} value`)
+          } else if (enumValid && !allowed.includes(node.const as JsonSchemaScalar)) {
+            violations.push(`${path}.const must be one of ${path}.enum when both are declared`)
+          }
        }
        break
      }
@@ -300,8 +308,20 @@ function propertyPath(path: string, key: string): string {
  return path === '' ? key : `${path}.${key}`
 }

-/** Collect value violations for one trusted schema node. */
+/** Contain hostile getters/proxies so validation remains total for arbitrary values. */
 function checkValue(node: JsonSchemaNode, value: unknown, path: string): string[] {
+  if (node.type !== undefined && !(SCHEMA_TYPES as readonly unknown[]).includes(node.type)) {
+    return checkValueUnchecked(node, value, path)
+  }
+  try {
+    return checkValueUnchecked(node, value, path)
+  } catch {
+    return [`"${diagnosticPath(path)}" must be a lossless JSON value`]
+  }
+}
+
+/** Collect value violations for one trusted schema node after the exception boundary. */
+function checkValueUnchecked(node: JsonSchemaNode, value: unknown, path: string): string[] {
  if (node.oneOf !== undefined) {
    const matches = node.oneOf.filter(branch => checkValue(branch, value, path).length === 0).length
    return matches === 1 ? [] : [`"${diagnosticPath(path)}" must match exactly one oneOf branch (matched ${matches})`]
--- a/packages/core/tools/src/schema.ts
+++ b/packages/core/tools/src/schema.ts
@@ -203,7 +203,12 @@ function compilePropertyMap(
      if (Object.hasOwn(property, 'required') && property.required !== true) {
        authorError(`${path}.${key}.required must be true when present`)
      }
-      properties[key] = compileValueSchema(property, `${path}.${key}`, seen, true)
+      Object.defineProperty(properties, key, {
+        value: compileValueSchema(property, `${path}.${key}`, seen, true),
+        enumerable: true,
+        configurable: true,
+        writable: true,
+      })
      if (property.required === true) required.push(key)
    }
    return required.length > 0 ? { properties, required } : { properties }
--- a/packages/core/tools/tests/json-schema.spec.ts
+++ b/packages/core/tools/tests/json-schema.spec.ts
@@ -160,6 +160,8 @@ describe('the enforced raw JSON Schema subset', () => {
      .toEqual(['schema.const must be a boolean value'])
    expect(violationsOf({ type: 'string', enum: undefined }))
      .toEqual(['schema.enum must be a non-empty array of string values'])
+    expect(violationsOf({ type: 'string', enum: ['a'], const: 'b' }))
+      .toEqual(['schema.const must be one of schema.enum when both are declared'])
  })

  it('validates annotation types and lossless JSON payloads', () => {
@@ -267,6 +269,21 @@ describe('validateJsonSchemaValue', () => {
      .toEqual(['"value" must be an object'])
  })

+  it('returns a violation instead of throwing for a container with a hostile getter', () => {
+    const value = Object.defineProperty({}, 'answer', {
+      enumerable: true,
+      get() { throw new Error('getter exploded') },
+    })
+    const schema = asserted({
+      type: 'object',
+      properties: { answer: { type: 'integer' } },
+      required: ['answer'],
+    })
+
+    expect(validateJsonSchemaValue(schema, value))
+      .toEqual(['"value" must be a lossless JSON value'])
+  })
+
  it('validates dense arrays per index and rejects lossy arrays', () => {
    const schema = asserted({ type: 'array', items: { type: 'integer' } })
    expect(validateJsonSchemaValue(schema, [1, 2])).toEqual([])
--- a/packages/core/tools/tests/schema.spec.ts
+++ b/packages/core/tools/tests/schema.spec.ts
@@ -62,6 +62,7 @@ describe('the unified author schema DSL', () => {
      { type: 'object' },
      { oneOf: [{ type: 'string' }] },
      { type: 'number', enum: ['1'] },
+      { type: 'string', enum: ['a'], const: 'b' },
      { type: 'integer', const: 1.5 },
      { type: 'json', default: undefined },
      { type: 'array', items: { type: 'string', required: true } },
@@ -91,6 +92,17 @@ describe('the unified author schema DSL', () => {
    expect(() => parameterSchemaSpecToJsonSchema(properties as ParameterSchemaSpec)).toThrow(/circular/)
  })

+  it('preserves a property literally named __proto__ as schema data', () => {
+    const properties = Object.create(null) as ParameterSchemaSpec
+    properties.__proto__ = { type: 'string', required: true }
+
+    const schema = parameterSchemaSpecToJsonSchema(properties)
+
+    expect(Object.hasOwn(schema.properties, '__proto__')).toBe(true)
+    expect(schema.properties.__proto__).toEqual({ type: 'string' })
+    expect(schema.required).toEqual(['__proto__'])
+  })
+
  it('infers scalar literals, arrays, objects, json, and exact-one unions', () => {
    expectTypeOf<InferValue<{ type: 'string'; enum: readonly ['a', 'b'] }>>().toEqualTypeOf<'a' | 'b'>()
    expectTypeOf<InferValue<{ type: 'number'; const: 1 }>>().toEqualTypeOf<1>()
--- a/packages/core/tools/tests/tools.spec.ts
+++ b/packages/core/tools/tests/tools.spec.ts
@@ -147,6 +147,7 @@ describe('ToolRegistry', () => {
    })
    expect(result.isError).toBe(true)
    expect(result.content[0]?.type === 'text' && result.content[0].text).toContain('Error:')
+    expect(result.error).toMatchObject({ info: { name: 'ToolOutputError', code: 'INVALID_TOOL_OUTPUT' } })
    expect(observedError).toBe(true)
  })

@@ -209,10 +210,10 @@ describe('ToolRegistry', () => {
    }))

    const result = await ctx.tools.execute({ callId: CallId(projector), name: `throwing-${projector}`, arguments: {} })
-    expect(result).toMatchObject({
-      isError: true,
-      error: { message: projector === 'render' ? 'renderer exploded' : 'metadata exploded' },
-    })
+    expect(result.isError).toBe(true)
+    expect(result.error?.message)
+      .toContain(projector === 'render' ? 'renderer exploded' : 'metadata exploded')
+    expect(result.error?.info).toEqual({ name: 'ToolOutputError', code: 'INVALID_TOOL_OUTPUT' })
    expect('value' in result).toBe(false)
  })