From 06556237f4d3b5dd98ccdb4ee01129c8c16b464d Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:24:34 +0800 Subject: [PATCH] fix(tools): close scheduler coverage gaps; regen persistence catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage gate flagged three untaken paths in the bridge scheduler: - the exclusive-head inFlight re-check was dead (the shared guard above already returns for an exclusive head with any in-flight sibling) — removed; - the commit-cursor undefined-dispatched break was structurally unreachable once entries join commitQueue only after start() ran synchronously — reordered the pump so the invariant holds by construction, annotated; - the finish (final-result) commit arm and the pump re-entry guard gain a covering test (throwing tools/pre-execute listener) and a defensive annotation respectively; mid-run unregistration test renamed to match its actual post-result settlement path. Also covers the direct-construction maxParallelSubCalls default (index.ts) and commits the regenerated persistence catalog for the new dispatch pair. --- docs/persistence-catalog.md | 4 +- packages/core/tools/src/code-mode.ts | 18 ++++-- packages/core/tools/tests/code-mode.spec.ts | 70 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/docs/persistence-catalog.md b/docs/persistence-catalog.md index 14b323704c..e7790fa76f 100644 --- a/docs/persistence-catalog.md +++ b/docs/persistence-catalog.md @@ -474,7 +474,7 @@ Source: [`packages/core/session/src/types.ts:283`](../packages/core/session/src/ Types: [CallId](core-data-structures/core.md) · [ContentBlock](core-data-structures/core.md) -Source: [`packages/core/tools/src/code-mode.ts:48`](../packages/core/tools/src/code-mode.ts) +Source: [`packages/core/tools/src/code-mode.ts:49`](../packages/core/tools/src/code-mode.ts) #### `tool/code-dispatch-start` — log-only @@ -497,7 +497,7 @@ Source: [`packages/core/tools/src/code-mode.ts:48`](../packages/core/tools/src/c Types: [CallId](core-data-structures/core.md) -Source: [`packages/core/tools/src/code-mode.ts:32`](../packages/core/tools/src/code-mode.ts) +Source: [`packages/core/tools/src/code-mode.ts:33`](../packages/core/tools/src/code-mode.ts) #### `tool/result` — surface diff --git a/packages/core/tools/src/code-mode.ts b/packages/core/tools/src/code-mode.ts index 54b851232a..c97338518d 100644 --- a/packages/core/tools/src/code-mode.ts +++ b/packages/core/tools/src/code-mode.ts @@ -285,6 +285,7 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => const head = commitQueue[0] /* v8 ignore next -- the loop condition bounds the index. */ if (head === undefined) break + /* v8 ignore next -- entries join commitQueue only after start() set dispatched (see pump). */ if (head.dispatched === undefined) break await head.dispatched commitQueue.shift() @@ -295,7 +296,11 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => } } const pump = (): void => { - // The finally-driven re-entry below would otherwise recurse. + // Defensive re-entry guard: today every caller (binding submission, + // flight.finally, drain) runs off promise callbacks, never while pump + // is on the stack, so this cannot fire — kept against a future + // synchronous caller. + /* v8 ignore next -- see the re-entry note above. */ if (pumping) return pumping = true try { @@ -310,12 +315,10 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => // Reclassify at start time (fail-closed on registry changes). const mode = head.classify() if (exclusiveActive || inFlight.size >= (mode === 'exclusive' ? 1 : maxParallel)) return - if (mode === 'exclusive') { - if (inFlight.size > 0) return - exclusiveActive = true - } + // The guard above already returned for an exclusive head with any + // in-flight sibling, so claiming the barrier here is race-free. + if (mode === 'exclusive') exclusiveActive = true pendingQueue.shift() - commitQueue.push(head) const flight = head.start().finally(() => { inFlight.delete(flight) if (mode === 'exclusive') exclusiveActive = false @@ -325,6 +328,9 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => void commitReady() pump() }) + // Joined AFTER start() ran synchronously, so every commitQueue + // entry already carries its `dispatched` promise. + commitQueue.push(head) inFlight.add(flight) } } finally { diff --git a/packages/core/tools/tests/code-mode.spec.ts b/packages/core/tools/tests/code-mode.spec.ts index 973660a5ce..e227f07544 100644 --- a/packages/core/tools/tests/code-mode.spec.ts +++ b/packages/core/tools/tests/code-mode.spec.ts @@ -478,6 +478,38 @@ describe('the sub-dispatch scheduler (native concurrency contract)', () => { expect(gated.peakLive()).toBe(2) }) + it('a tool unregistered between binding enumeration and dispatch fails as unknown tool', async () => { + const { ctx, runtime } = await setup({ mode: 'code' }) + const calls: unknown[] = [] + const dispose = ctx.tools.register(defineTool({ + name: 'ephemeral', + description: 'Unregistered between binding enumeration and dispatch.', + parameters: {}, + output: { + schema: { type: 'string' }, + render: (_args, value) => [{ type: 'text', text: value }], + }, + execute() { + calls.push('ran') + return Promise.resolve('ok') + }, + })) + runtime.behavior = async (request) => { + // The binding exists (enumerated at run start); the registry mutation + // makes prepare resolve UNKNOWN_TOOL as a final-result, which commits + // through scheduler.finish (no post-execute). + dispose() + const message = await request.bindings[0]!.functions.ephemeral!({}) + .then(() => 'resolved', (error: unknown) => error instanceof Error ? error.message : String(error)) + return { logs: [], value: message } + } + const result = await runCode(ctx, 'program') + expect(result.isError).toBe(false) + if (result.isError) throw new Error('expected success') + expect(result.value).toMatchObject({ result: 'unknown tool "ephemeral"' }) + expect(calls).toEqual([]) + }) + it('post-execute and context commitment stay in submission order under out-of-order completion', async () => { const { ctx, runtime } = await setup({ mode: 'code' }) const gated = registerGated(ctx, 'safe_read', true) @@ -662,6 +694,37 @@ describe('the run_code dispatch bridge', () => { expect(result.content[0]).toEqual({ type: 'text', text: 'caught: deliberate failure' }) }) + it('a throwing tools/pre-execute listener settles the sub-call without post-execute', async () => { + const { ctx, runtime } = await setup({ mode: 'code' }) + const calls = registerEcho(ctx) + const postExecuted: string[] = [] + ctx.on('tools/pre-execute', (exec, next) => { + if (exec.name === 'echo') throw new Error('gate exploded') + return next() + }) + ctx.on('tools/post-execute', (exec, _result, next): Promise => { + if (exec.name === 'echo') postExecuted.push(exec.name) + return next() + }) + const { agent, events } = fakeAgent() + runtime.behavior = async (request) => { + const message = await request.bindings[0]!.functions.echo!({ value: 'x' }) + .then(() => 'resolved', (error: unknown) => error instanceof Error ? error.message : String(error)) + return { logs: [], value: message } + } + const result = await runCode(ctx, 'program', { agent }) + expect(result.isError).toBe(false) + if (result.isError) throw new Error('expected success') + expect(result.value).toMatchObject({ result: 'gate exploded' }) + // The pipeline failure is final: the body never ran and post-execute was + // skipped, yet the settle event still carries the error outcome. + expect(calls).toEqual([]) + expect(postExecuted).toEqual([]) + const settles = events.filter(event => event.type === 'tool/code-dispatch') + expect(settles).toHaveLength(1) + expect(settles[0]?.data).toMatchObject({ name: 'echo', isError: true }) + }) + it('a tools/pre-execute deny reaches the program as a binding rejection', async () => { const { ctx, runtime } = await setup({ mode: 'code' }) registerEcho(ctx) @@ -1230,6 +1293,13 @@ describe('the run_code dispatch bridge', () => { expect(derived[0]?.role).toBe('user') }) + it('direct construction in code mode defaults the parallel sub-call cap', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt, {}) + const registry = new ToolRegistry(ctx, { mode: 'code' }) + expect(registry.get(RUN_CODE_NAME)).toBeDefined() + }) + it('defaults to native mode under direct construction with no config', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt, {})