fix(tools): isolate tool schemas and waterfall failures
This commit is contained in:
@@ -123,8 +123,11 @@ export class SystemPrompt extends Service {
|
|||||||
*/
|
*/
|
||||||
assemble(): Promise<PromptAssembly> {
|
assemble(): Promise<PromptAssembly> {
|
||||||
const assembly: PromptAssembly = {
|
const assembly: PromptAssembly = {
|
||||||
sections: [...this.sections].sort((a, b) => a.order - b.order),
|
sections: this.sections
|
||||||
tools: this.toolProviders.flatMap(provider => provider()),
|
.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))
|
return this.ctx.waterfall(this, 'system-prompt/assemble', assembly, () => Promise.resolve(assembly))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -107,6 +107,23 @@ describe('SystemPrompt', () => {
|
|||||||
expect(assembly.sections).toHaveLength(0)
|
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<string, unknown> }
|
||||||
|
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', () => {
|
it('filters out empty section text from renderPrompt', () => {
|
||||||
// Direct test of renderPrompt: function returning empty string, and empty static text
|
// Direct test of renderPrompt: function returning empty string, and empty static text
|
||||||
const result = renderPrompt({
|
const result = renderPrompt({
|
||||||
|
|||||||
@@ -173,7 +173,10 @@ export class ToolRegistry extends Service {
|
|||||||
schemas(): ToolSchema[] {
|
schemas(): ToolSchema[] {
|
||||||
// Rest-destructure to drop `execute`; the unused binding is the idiom.
|
// Rest-destructure to drop `execute`; the unused binding is the idiom.
|
||||||
// eslint-disable-next-line @typescript-eslint/unbound-method, @typescript-eslint/no-unused-vars
|
// 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 } : {},
|
...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 } : {},
|
||||||
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -122,6 +122,38 @@ describe('ToolRegistry', () => {
|
|||||||
expect(order).toEqual(['first:before', 'second:before', 'second:after', 'first:after'])
|
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<string, unknown> }
|
||||||
|
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 () => {
|
it('rejects duplicate names and unregisters on fiber dispose (HMR safety)', async () => {
|
||||||
const ctx = await setup()
|
const ctx = await setup()
|
||||||
ctx.tools.register(echoTool)
|
ctx.tools.register(echoTool)
|
||||||
|
|||||||
Reference in New Issue
Block a user