Procházet zdrojové kódy

fix: reach quiescence even when the runtime rejects (agent review)

[P1] review finding: the run-scoped abort + queue drain ran only after
runtime.run() FULFILLED, so a backend that starts a binding call and
then throws left the sub-dispatch running past run_code's settlement —
its tool/code-dispatch event could append after the parent call
returned, breaking the drain-before-return contract. The quiescence
pair now lives in a finally around runtime.run(); the folded queue tail
keeps the drain from masking the thrown error. Pinned by a test whose
fake runtime fails mid-flight: pre-fix it returns in milliseconds with
the slow tool still running.
Tianyi Cui před 2 měsíci
rodič
revize
1b29273f12

+ 23 - 15
packages/core/tools/src/code-mode.ts

@@ -250,21 +250,29 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () =>
       }
 
       try {
-        const result = await runtime.run({
-          program: args.code,
-          bindings: [{ global: 'tools', functions }],
-          signal: runController.signal,
-        })
-        // Quiescence before returning: fire the run-scoped abort (cancelling
-        // an in-flight sub-dispatch, abandoning queued ones), then await the
-        // queue's drain — an aborted sub-call still settles and logs its
-        // event INSIDE the open turn; nothing can append after we return.
-        // `queue` is the FOLDED tail (every link swallows its rejection into
-        // undefined), so this await cannot itself reject — an abandoned
-        // queued call can never mask the runtime's own `result.error` below;
-        // rejections surface only on the per-call promises the program holds.
-        runController.abort('run_code settled')
-        await queue
+        let result: CodeRunResult
+        try {
+          result = await runtime.run({
+            program: args.code,
+            bindings: [{ global: 'tools', functions }],
+            signal: runController.signal,
+          })
+        } finally {
+          // Quiescence before returning, whether the runtime fulfilled or
+          // REJECTED (a backend that starts a binding call and then throws
+          // must not leak a live sub-dispatch past this settlement): fire
+          // the run-scoped abort (cancelling an in-flight sub-dispatch,
+          // abandoning queued ones), then await the queue's drain — an
+          // aborted sub-call still settles and logs its event INSIDE the
+          // open turn; nothing can append after we return. `queue` is the
+          // FOLDED tail (every link swallows its rejection into undefined),
+          // so this await cannot itself reject — an abandoned queued call
+          // can never mask the runtime's own failure, returned or thrown;
+          // rejections surface only on the per-call promises the program
+          // holds.
+          runController.abort('run_code settled')
+          await queue
+        }
 
         if (result.error) {
           const logsText = result.logs.length > 0 ? `\nCaptured output:\n${result.logs.map(entry => entry.text).join('\n')}` : ''

+ 38 - 0
packages/core/tools/tests/code-mode.spec.ts

@@ -385,6 +385,44 @@ describe('the run_code dispatch bridge', () => {
     expect(sawAbort).toBe(true)
   })
 
+  it('a runtime that starts a binding call and then REJECTS still reaches quiescence before returning', async () => {
+    const { ctx, runtime } = await setup({ mode: 'code' })
+    const { agent, events } = fakeAgent()
+    let sawAbort = false
+    let started!: () => void
+    const inFlight = new Promise<void>((resolve) => { started = resolve })
+    ctx.tools.register(defineTool({
+      name: 'slow',
+      description: 'Slow tool observing its signal.',
+      parameters: { id: { type: 'string', required: true } },
+      async execute(args, exec) {
+        started()
+        await new Promise<void>((resolve) => {
+          const timer = setTimeout(resolve, 500)
+          exec.signal?.addEventListener('abort', () => { sawAbort = true; clearTimeout(timer); resolve() }, { once: true })
+        })
+        return [{ type: 'text' as const, text: args.id }]
+      },
+    }))
+    runtime.behavior = async (request) => {
+      // Start a sub-dispatch, keep its rejection held, and fail the run once
+      // the tool is genuinely in flight — a seam error AFTER work has begun.
+      // The bridge's settlement still owes quiescence: without the finally,
+      // run_code would return now and the slow tool would finish (and log)
+      // afterwards.
+      request.bindings[0]!.functions.slow!({ id: 'orphan' }).catch(() => 'held')
+      await inFlight
+      throw new Error('backend exploded')
+    }
+    const result = await runCode(ctx, 'program', { agent })
+    expect(result.isError).toBe(true)
+    expect((result.content[0] as { text: string }).text).toContain('backend exploded')
+    // Quiescence held: the in-flight sub-dispatch was aborted and its event
+    // logged INSIDE the run_code execution, not after it returned.
+    expect(sawAbort).toBe(true)
+    expect(events.filter(event => event.type === 'tool/code-dispatch').map(event => (event.data as { name: string }).name)).toEqual(['slow'])
+  })
+
   it('runs without an owning agent: dispatches work, event logging is skipped', async () => {
     const { ctx, runtime } = await setup({ mode: 'code' })
     const calls = registerEcho(ctx)