From 04f0435cc95c60359a6ba743fdc7529dae2c749a Mon Sep 17 00:00:00 2001 From: _Kerman Date: Sun, 26 Jul 2026 21:24:16 +0800 Subject: [PATCH] refactor(tools): let the terminal marker ride the nested result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staged-promotion machinery (concludingParents keyed by parent token plus a pendingParentConclusions staging map) spread one fact — this call concluded the turn — across three registry-side collections with manual cleanup. Align it with how additionalContexts already crosses the same boundary: concludeTurn() marks only its own execution, the marker rides that execution's successful result (ToolExecutionFailure types concludesTurn as never, so a policy-converted failure sheds it with the type), and the composite that owns the nested dispatch forwards it — Code Mode's binding does so beside its existing context forwarding. The registry loses both parent-keyed collections and the promotion block; the propagation decision moves to the owning boundary; the structured-output consumer's own two-phase commit is untouched. --- packages/core/tools/src/code-mode.ts | 6 ++++ packages/core/tools/src/index.ts | 36 ++++++------------- packages/core/tools/tests/code-mode.spec.ts | 40 +++++++++++++++++++++ packages/core/tools/tests/tools.spec.ts | 20 ++++++----- 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/packages/core/tools/src/code-mode.ts b/packages/core/tools/src/code-mode.ts index afa09a3e32..63dc5754a0 100644 --- a/packages/core/tools/src/code-mode.ts +++ b/packages/core/tools/src/code-mode.ts @@ -268,6 +268,12 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => for (const context of result.additionalContexts ?? []) { exec.deferContext(context) } + // Like the context forwarding above, cross-boundary facts travel on + // the nested result and the composite forwards them: only a + // successful nested result can carry the terminal marker + // (ToolExecutionFailure types it never), so a policy-converted + // failure cannot stop the turn through a recovering program. + if (result.concludesTurn) exec.concludeTurn() exec.agent?.session.append('tool/code-dispatch', { parentCallId: exec.callId, subCallId, diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 067c854cb9..649ab7c788 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -307,7 +307,14 @@ export interface ToolRunContext extends ToolExecution { * are emitted in call order. */ deferContext(context: UserMessageData): void - /** Mark a successful final result as terminal for the current agent turn. */ + /** + * Mark a successful final result as terminal for the current agent turn. + * The marker rides this execution's own result (`concludesTurn` exists only + * on {@link ToolExecutionSuccess}); a composite that dispatches nested + * calls forwards it from the nested result, exactly like + * `additionalContexts`, so only an authoritative nested success can + * conclude the enclosing run. + */ concludeTurn(): void } @@ -653,16 +660,8 @@ export class ToolRegistry extends Service { /** Context deferred by a running tool body, keyed by its scheduler-owned execution. */ private deferredContexts = new WeakMap() - /** Successful executions whose tool body declared the current turn complete. */ + /** Executions whose tool body declared the current turn complete. */ private concludingExecutions = new WeakSet() - /** Enclosing transport tokens marked terminal by a successful nested call. */ - private concludingParents = new Set() - /** - * Nested conclusions staged until their call's final verdict: a post-execute - * policy may still convert the nested success into an error, and a failed - * terminal operation must not stop the turn through its composite. - */ - private pendingParentConclusions = new WeakMap() /** Original caller cancellation, kept outside the wrapper-mutable execution object. */ private cancellationStates = new WeakMap() /** Definition-owned final content transform snapshotted before policy begins. */ @@ -985,7 +984,6 @@ export class ToolRegistry extends Service { const definition = this.get(name, agent) const finalizeContent = definition?.finalizeContent?.bind(definition) const concludingExecutions = this.concludingExecutions - const pendingParentConclusions = this.pendingParentConclusions const base = { token, callId, @@ -997,10 +995,7 @@ export class ToolRegistry extends Service { deferredContexts.push(context) }, concludeTurn(): void { - if (parent === undefined) concludingExecutions.add(this as unknown as ToolExecution) - // Staged, not propagated: only this nested call's authoritative - // successful result promotes the marker onto its parent. - else pendingParentConclusions.set(this as unknown as ToolRunContext, parent) + concludingExecutions.add(this as unknown as ToolExecution) }, } try { @@ -1214,16 +1209,7 @@ export class ToolRegistry extends Service { } catch (error: unknown) { finalResult = this.materializeFinalResult(toolErrorResult(error)) } - // Promote a staged nested conclusion only on the call's authoritative - // successful verdict — a policy-converted failure must not let the - // composite stop the turn on a failed terminal operation. - const stagedParent = this.pendingParentConclusions.get(exec) - if (stagedParent !== undefined) { - this.pendingParentConclusions.delete(exec) - if (!finalResult.isError) this.concludingParents.add(stagedParent) - } this.notifyResult(exec, finalResult) - this.concludingParents.delete(exec.token) return finalResult } @@ -1394,7 +1380,7 @@ export class ToolRegistry extends Service { } meta = snapshotProjection(tool.name, 'presentationMeta', projected) } - const concludesTurn = this.concludingExecutions.has(exec) || this.concludingParents.has(exec.token) + const concludesTurn = this.concludingExecutions.has(exec) return this.markCanonical(exec, this.materializeFinalResult({ isError: false, value, diff --git a/packages/core/tools/tests/code-mode.spec.ts b/packages/core/tools/tests/code-mode.spec.ts index b19baa5c0d..2a7410078c 100644 --- a/packages/core/tools/tests/code-mode.spec.ts +++ b/packages/core/tools/tests/code-mode.spec.ts @@ -417,6 +417,46 @@ describe('the run_code dispatch bridge', () => { expect(result.content).toEqual([{ type: 'text', text: 'done' }]) }) + it('forwards a nested terminal conclusion onto the successful run_code result', async () => { + const { ctx, runtime } = await setup({ mode: 'code' }) + ctx.tools.register(defineTool({ + name: 'finalize', + description: 'Terminal tool.', + parameters: {}, + output: { + schema: { type: 'string' }, + render: (_args, value) => [{ type: 'text', text: value }], + }, + execute(_args, exec) { + exec.concludeTurn() + return Promise.resolve('done') + }, + })) + runtime.behavior = async (request) => { + await request.bindings[0]!.functions.finalize!({}) + return { logs: [], value: 'program complete' } + } + + const concluded = await runCode(ctx, 'await tools.finalize({})') + expect(concluded.isError).toBe(false) + expect(concluded.concludesTurn).toBe(true) + + // A policy that converts the nested success into an error strips the + // marker with the result type: the recovering program cannot conclude. + const veto = ctx.on('tools/post-execute', async (exec, _result, next): Promise => { + if (exec.name !== 'finalize') return next() + return { kind: 'block', feedback: [{ type: 'text', text: 'terminal rejected' }] } + }) + runtime.behavior = async (request) => { + await request.bindings[0]!.functions.finalize!({}).catch(() => undefined) + return { logs: [], value: 'recovered' } + } + const recovered = await runCode(ctx, 'await tools.finalize({}).catch(() => {})') + veto() + expect(recovered.isError).toBe(false) + expect(recovered.concludesTurn).toBeUndefined() + }) + it('serializes Promise.all dispatches: tool executions never overlap, in submission order', async () => { const { ctx, runtime } = await setup({ mode: 'code' }) const intervals: [string, string][] = [] diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index c704cfbada..743130168b 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -553,18 +553,20 @@ describe('ToolRegistry', () => { expect(nested.isError ? undefined : nested.value).toBe('') }) - it('propagates a nested concludeTurn only when the nested verdict stays successful', async () => { + it('carries a nested conclusion on the nested result for its composite to forward', async () => { const ctx = await setup() ctx.tools.register({ ...echoTool, name: 'terminal-nested', async execute(_args, exec) { exec.concludeTurn() - return 'staged' + return 'terminal' }, }) - // A composite that swallows its nested failure and returns success — the - // Code Mode shape the staged propagation exists for. + // A composite that forwards the marker from the nested result — the Code + // Mode dispatch shape. A recovering composite (nested failure swallowed) + // has no marker to forward: ToolExecutionFailure types concludesTurn as + // never, so only an authoritative nested success can conclude the run. let call = 0 ctx.tools.register({ ...echoTool, @@ -574,13 +576,13 @@ describe('ToolRegistry', () => { const nested = await ctx.tools.execute({ signal: exec.signal, callId: CallId(`nested-${call}`), name: 'terminal-nested', arguments: {}, parent: exec.token, }) + if (nested.concludesTurn) exec.concludeTurn() return nested.isError ? 'nested failed, composite recovered' : 'nested succeeded' }, }) - // A policy converts the nested success into an error: the staged - // conclusion must NOT reach the composite, or the loop would stop the - // turn on a failed terminal operation. + // A policy converts the nested success into an error: the failed result + // carries no marker, so the recovering composite does not conclude. const veto = ctx.on('tools/post-execute', async (exec, _result, next): Promise => { if (exec.name !== 'terminal-nested') return next() return { kind: 'block', feedback: [{ type: 'text', text: 'nested success rejected' }] } @@ -592,8 +594,8 @@ describe('ToolRegistry', () => { expect(recovered.concludesTurn).toBeUndefined() veto() - // The same nested call succeeding promotes the marker: the composite's - // own successful result now carries concludesTurn. + // The same nested call succeeding carries the marker; the composite + // forwards it onto its own successful result. const concluded = await ctx.tools.execute({ signal: testToolSignal, callId: CallId('composite-ok'), name: 'composite', arguments: {}, })