From 87df09e3c3766edaa1ceb70c049d46c9cf0e8e7d Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 17 Jun 2026 21:26:12 +0800 Subject: [PATCH] fix(tools): isolate tool schemas and waterfall failures --- packages/system-prompt/src/index.ts | 7 ++-- .../system-prompt/tests/system-prompt.spec.ts | 17 ++++++++++ packages/tools/src/index.ts | 13 +++++++- packages/tools/tests/tools.spec.ts | 32 +++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/packages/system-prompt/src/index.ts b/packages/system-prompt/src/index.ts index ec89a3460c..b2f3bd3db4 100644 --- a/packages/system-prompt/src/index.ts +++ b/packages/system-prompt/src/index.ts @@ -123,8 +123,11 @@ export class SystemPrompt extends Service { */ assemble(): Promise { const assembly: PromptAssembly = { - sections: [...this.sections].sort((a, b) => a.order - b.order), - tools: this.toolProviders.flatMap(provider => provider()), + sections: this.sections + .map(section => ({ ...section })) + .sort((a, b) => a.order - b.order), + tools: this.toolProviders.flatMap(provider => + provider().map(tool => ({ ...tool, parameters: structuredClone(tool.parameters) }))), } return this.ctx.waterfall(this, 'system-prompt/assemble', assembly, () => Promise.resolve(assembly)) } diff --git a/packages/system-prompt/tests/system-prompt.spec.ts b/packages/system-prompt/tests/system-prompt.spec.ts index 43e9240412..cfbf36cbec 100644 --- a/packages/system-prompt/tests/system-prompt.spec.ts +++ b/packages/system-prompt/tests/system-prompt.spec.ts @@ -107,6 +107,23 @@ describe('SystemPrompt', () => { expect(assembly.sections).toHaveLength(0) }) + it('assembles snapshots so one-step mutations do not leak into future assemblies', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 'base', order: 0, text: 'base' }) + ctx.systemPrompt.tools(() => [{ name: 't', description: 'tool', parameters: { type: 'object', properties: {} } }]) + + const first = await ctx.systemPrompt.assemble() + first.sections[0]!.name = 'mutated' + first.tools[0]!.description = 'mutated' + const firstParameters = first.tools[0]!.parameters as { properties: Record } + firstParameters.properties['leak'] = { type: 'string' } + + const second = await ctx.systemPrompt.assemble() + expect(second.sections.map(section => section.name)).toEqual(['base']) + expect(second.tools).toEqual([{ name: 't', description: 'tool', parameters: { type: 'object', properties: {} } }]) + }) + it('filters out empty section text from renderPrompt', () => { // Direct test of renderPrompt: function returning empty string, and empty static text const result = renderPrompt({ diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index 8e8c106147..b09647ffcd 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -173,7 +173,10 @@ export class ToolRegistry extends Service { schemas(): ToolSchema[] { // Rest-destructure to drop `execute`; the unused binding is the idiom. // eslint-disable-next-line @typescript-eslint/unbound-method, @typescript-eslint/no-unused-vars - return [...this.store.values()].map(({ execute, ...schema }) => schema) + return [...this.store.values()].map(({ execute, ...schema }) => ({ + ...schema, + parameters: structuredClone(schema.parameters), + })) } /** @@ -201,6 +204,14 @@ export class ToolRegistry extends Service { ...info ? { error: info } : {}, } } + }).catch((error: unknown): ToolExecutionResult => { + const info = errorInfo(error) + return { + callId: exec.callId, + content: [{ type: 'text', text: `Error: ${errorMessage(error)}` }], + isError: true, + ...info ? { error: info } : {}, + } }) } } diff --git a/packages/tools/tests/tools.spec.ts b/packages/tools/tests/tools.spec.ts index 7338a2c9c8..ac2fd8414c 100644 --- a/packages/tools/tests/tools.spec.ts +++ b/packages/tools/tests/tools.spec.ts @@ -122,6 +122,38 @@ describe('ToolRegistry', () => { expect(order).toEqual(['first:before', 'second:before', 'second:after', 'first:after']) }) + it('returns an isError result when a tools/execute listener throws', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + ctx.on('tools/execute', async () => { + throw new Error('permission hook broke') + }) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + + expect(result).toEqual({ + callId: CallId('c1'), + content: [{ type: 'text', text: 'Error: permission hook broke' }], + isError: true, + }) + }) + + it('schemas() snapshots tool schemas instead of exposing registry objects', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + const first = ctx.tools.schemas() + const firstParameters = first[0]!.parameters as { properties: Record } + firstParameters.properties['mutated'] = { type: 'string' } + first[0]!.description = 'mutated' + + expect(ctx.tools.schemas()).toEqual([{ + name: 'echo', + description: 'echo arguments back', + parameters: { type: 'object', properties: { text: { type: 'string' } } }, + }]) + }) + it('rejects duplicate names and unregisters on fiber dispose (HMR safety)', async () => { const ctx = await setup() ctx.tools.register(echoTool)