Explorar el Código

Merge branch 'structured-output-subagent-seam' into worktree-dynamic-workflows

Tianyi Cui hace 3 meses
padre
commit
0ee5583953

+ 8 - 3
packages/subagent/subagent-inprocess/src/index.ts

@@ -119,9 +119,14 @@ export function startInProcessRun(
   if (request.maxDepth !== undefined && childDepth > request.maxDepth) {
     throw new SubagentDepthError(childDepth, request.maxDepth)
   }
-  // Assert the schema subset BEFORE any child exists (the service has already
-  // capability-gated; this rejects a schema outside the enforced subset loud).
-  const schema = request.outputSchema
+  // Snapshot, then assert, the schema subset BEFORE any child exists (the
+  // service has already capability-gated; this rejects a schema outside the
+  // enforced subset loud). The snapshot is load-bearing: the caller keeps its
+  // reference, so validating and attaching the ORIGINAL would let a
+  // post-start() mutation drift the enforced schema away from the asserted
+  // one — the clone pins assertion, the model-visible parameters, and
+  // validateStructuredValue to the same isolation-immutable value.
+  const schema = request.outputSchema === undefined ? undefined : structuredClone(request.outputSchema)
   if (schema !== undefined) assertSupportedOutputSchema(schema)
 
   const childId = AgentId(randomUUID())

+ 29 - 2
packages/subagent/subagent-inprocess/src/structured.ts

@@ -25,7 +25,11 @@
  * output is captured — without it, the loop's default "had tool calls ⇒
  * continue" buys a wasted extra model step per structured child. It is also
  * `prepend: true`: the veto must run before any earlier-registered listener
- * that could short-circuit the chain into a forced continue.
+ * that could short-circuit the chain into a forced continue. A third listener
+ * closes the within-step window the continuation veto cannot: a
+ * `tools/pre-execute` deny for any call arriving after the agent's capture, so
+ * a response that lists `structured_output` before further tool calls cannot
+ * run side effects after the final answer was accepted.
  *
  * Lifetime is refcounted with two kinds of holder: each backend acquires for
  * its plugin lifetime (so the tool exists before any run), and each structured
@@ -42,7 +46,7 @@ import type { Agent } from '@deepseek-ai/dsh-agent'
 import type { ContentBlock, ToolSchema } from '@deepseek-ai/dsh-llm'
 import type { ContinuationDecision } from '@deepseek-ai/dsh-agent'
 import type { AssembleContext, PromptAssembly } from '@deepseek-ai/dsh-system-prompt'
-import type { ToolExecution } from '@deepseek-ai/dsh-tools'
+import type { PreToolDecision, ToolExecution } from '@deepseek-ai/dsh-tools'
 import { ToolArgsError, validateStructuredValue, type StructuredOutputSchema } from '@deepseek-ai/dsh-tools'
 
 /** The model-facing tool name a structured child must call to finish. */
@@ -236,4 +240,27 @@ function registerRuntime(root: Context, runtime: StructuredRuntime): void {
     if (runtime.states.get(agent)?.captured) return Promise.resolve({ action: 'stop' })
     return next()
   }, { prepend: true }))
+
+  // Terminal means terminal WITHIN the step, not only at its end: the
+  // turn-continuation veto above runs after every call in the current model
+  // response has executed, so a response that puts `structured_output` before
+  // further tool calls would still perform those side effects after the final
+  // answer was accepted. Deny every later call for a captured agent at the
+  // allow/deny gate — dispatch is skipped and the model sees an `isError`
+  // result naming the contract. Calls that PRECEDE the capture in the same
+  // response ran before `captured` was set and are untouched; a second
+  // `structured_output` is denied like any other call. `prepend: true` for the
+  // same reason as the continuation veto: no earlier-registered allow may
+  // short-circuit past the terminal contract.
+  runtime.disposers.push(root.on('tools/pre-execute', function (
+    this: unknown, exec: ToolExecution, next: () => Promise<PreToolDecision>,
+  ): Promise<PreToolDecision> {
+    if (exec.agent && runtime.states.get(exec.agent)?.captured) {
+      return Promise.resolve({
+        kind: 'deny',
+        reason: `structured output already recorded: the run is complete, so \`${exec.name}\` is not executed`,
+      })
+    }
+    return next()
+  }, { prepend: true }))
 }

+ 85 - 1
packages/subagent/subagent-inprocess/tests/structured.spec.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest'
 import { Context } from 'cordis'
-import LlmService, { type GenerateOptions } from '@deepseek-ai/dsh-llm'
+import LlmService, { CallId, type ContentBlock, type GenerateOptions } from '@deepseek-ai/dsh-llm'
 import SessionStore from '@deepseek-ai/dsh-session'
 import SystemPrompt from '@deepseek-ai/dsh-system-prompt'
 import ToolRegistry from '@deepseek-ai/dsh-tools'
@@ -86,6 +86,90 @@ describe('in-process structured output', () => {
     await run.dispose()
   })
 
+  it('denies tool calls that FOLLOW the capture in the same response — terminal means terminal', async () => {
+    // One model response carrying structured_output FIRST and a side-effecting
+    // call after it: the continuation veto only fires at step end, so without
+    // the pre-execute deny the trailing call would still run after the final
+    // answer was accepted.
+    const response = [
+      ...toolCallResponse('c1', STRUCTURED_OUTPUT_TOOL, { answer: 5 }).slice(0, -2),
+      { type: 'block-start', index: 1, blockType: 'tool-call' },
+      { type: 'block-end', index: 1, block: { type: 'tool-call', id: CallId('c2'), name: 'side_effect', arguments: '{}' } },
+      { type: 'usage', usage: { inputTokens: 10, outputTokens: 5 } },
+      { type: 'finish', reason: { kind: 'tool-calls' } },
+    ] as Script[number]
+    const { ctx, parent } = await setup([response])
+    let sideEffectRan = false
+    ctx.tools.register({
+      name: 'side_effect',
+      description: 'probe',
+      parameters: { type: 'object', properties: {} },
+      execute(): Promise<ContentBlock[]> {
+        sideEffectRan = true
+        return Promise.resolve([{ type: 'text', text: 'ran' }])
+      },
+    })
+    const run = ctx.subagents.start('spawn', structuredRequest(parent))
+    const result = await run.result
+    expect(result.stopReason).toBe('completed')
+    expect(result.structured).toEqual({ answer: 5 })
+    // The deny skipped dispatch entirely: the probe body never ran.
+    expect(sideEffectRan).toBe(false)
+    await run.dispose()
+  })
+
+  it('leaves tool calls that PRECEDE the capture in the same response untouched', async () => {
+    const response = [
+      { type: 'block-start', index: 0, blockType: 'tool-call' },
+      { type: 'block-end', index: 0, block: { type: 'tool-call', id: CallId('c1'), name: 'side_effect', arguments: '{}' } },
+      ...toolCallResponse('c2', STRUCTURED_OUTPUT_TOOL, { answer: 6 }).map(chunk =>
+        'index' in chunk ? { ...chunk, index: 1 } : chunk),
+    ] as Script[number]
+    const { ctx, parent } = await setup([response])
+    let sideEffectRan = false
+    ctx.tools.register({
+      name: 'side_effect',
+      description: 'probe',
+      parameters: { type: 'object', properties: {} },
+      execute(): Promise<ContentBlock[]> {
+        sideEffectRan = true
+        return Promise.resolve([{ type: 'text', text: 'ran' }])
+      },
+    })
+    const run = ctx.subagents.start('spawn', structuredRequest(parent))
+    const result = await run.result
+    // The call ran BEFORE captured was set: the deny gate only guards the
+    // window after the terminal answer landed.
+    expect(sideEffectRan).toBe(true)
+    expect(result.structured).toEqual({ answer: 6 })
+    await run.dispose()
+  })
+
+  it('snapshots the schema at start(): caller mutation after start cannot drift enforcement', async () => {
+    const mutable: StructuredOutputSchema = {
+      type: 'object',
+      properties: { answer: { type: 'number' } },
+      required: ['answer'],
+      additionalProperties: false,
+    }
+    const pristine = structuredClone(mutable)
+    const { ctx, parent, adapter } = await setup([
+      toolCallResponse('c1', STRUCTURED_OUTPUT_TOOL, { answer: 3 }),
+    ])
+    const run = ctx.subagents.start('spawn', structuredRequest(parent, { outputSchema: mutable }))
+    // Mutate the caller's object AFTER start() returned but before the child's
+    // first request assembles: with a live reference this would reach both the
+    // model-visible parameters and validateStructuredValue.
+    ;(mutable.properties as Record<string, unknown>).answer = { type: 'string' }
+    const result = await run.result
+    expect(result.structured).toEqual({ answer: 3 })
+    // The child's request carried the PRISTINE schema, not the mutated one.
+    const childRequest = adapter.requests.at(-1)
+    const captureTool = (childRequest?.tools ?? []).find(tool => tool.name === STRUCTURED_OUTPUT_TOOL)
+    expect(captureTool?.parameters).toEqual(pristine)
+    await run.dispose()
+  })
+
   it('the captured-turn veto is prepend: an EARLIER force-continue listener cannot short-circuit it', async () => {
     const ctx = new Context()
     await ctx.plugin(SystemPrompt)