From 70f936acad8e85ee1a57e549ac2f9e15e7d295ee Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 3 Jul 2026 13:32:35 +0800 Subject: [PATCH] fix: tool dedupe --- .../src/agent/toolDedupe/toolDedupeService.ts | 63 ++++++- .../agent/toolExecutor/toolExecutorService.ts | 38 ++++- .../test/externalHooks/integration.test.ts | 104 ++++++++++++ packages/agent-core-v2/test/goal/goal.test.ts | 5 + .../agent-core-v2/test/loop/basic.test.ts | 37 ++++ .../test/toolDedup/tool-dedup.test.ts | 160 +++++++++++++++++- .../test/toolExecutor/tool-executor.test.ts | 31 +++- packages/agent-core-v2/test/turn/turn.test.ts | 25 ++- 8 files changed, 435 insertions(+), 28 deletions(-) diff --git a/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts b/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts index 783de76a5..57c00cc4a 100644 --- a/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts +++ b/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts @@ -8,6 +8,8 @@ * hooks are installed without any other service injecting it. */ +import { createHash } from 'node:crypto'; + import { InstantiationType } from '#/_base/di/extensions'; import { Disposable } from '#/_base/di/lifecycle'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; @@ -70,6 +72,16 @@ function makeKey(toolName: string, args: unknown): string { return `${toolName} ${canonicalTelemetryArgs(args)}`; } +function argsHash(args: unknown): string { + return createHash('sha256').update(canonicalTelemetryArgs(args)).digest('hex').slice(0, 8); +} + +interface CheckedToolCall { + readonly syntheticResult: ToolDedupResult | null; +} + +type ToolCallDupType = 'same_step' | 'cross_step'; + function appendReminder(result: ToolDedupResult, reminderText: string): ToolDedupResult { const output = result.output; let newOutput: string | ContentPart[]; @@ -106,6 +118,8 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu private readonly callKeyByCallId = new Map(); private consecutiveKey: string | null = null; private consecutiveCount = 0; + private activeTurnId: number | undefined; + private activeStep = 0; constructor( @ITelemetryService private readonly telemetry: ITelemetryService, @@ -113,8 +127,8 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu @IAgentToolExecutorService toolExecutor: IAgentToolExecutorService, ) { super(); - loop.hooks.beforeStep.register('toolDedup', async (_ctx, next) => { - this.beginStep(); + loop.hooks.beforeStep.register('toolDedup', async (ctx, next) => { + this.beginStep(ctx.turnId, ctx.step); await next(); }); loop.hooks.afterStep.register('toolDedup', async (_ctx, next) => { @@ -122,9 +136,9 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu await next(); }); toolExecutor.hooks.onWillExecuteTool.register('toolDedup', async (ctx, next) => { - const cached = this.checkSameStep(ctx.toolCall.id, ctx.toolCall.name, ctx.args); - if (cached !== null) { - ctx.decision = { syntheticResult: cached }; + const checked = this.checkToolCall(ctx.toolCall.id, ctx.toolCall.name, ctx.args); + if (checked.syntheticResult !== null) { + ctx.decision = { syntheticResult: checked.syntheticResult }; return; } await next(); @@ -143,7 +157,16 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu }); } - private beginStep(): void { + private beginStep(turnId?: number, step?: number): void { + if (turnId !== undefined && turnId !== this.activeTurnId) { + this.activeTurnId = turnId; + this.consecutiveKey = null; + this.consecutiveCount = 0; + } + if (step !== undefined) { + this.activeStep = step; + } + for (const deferred of this.stepDeferreds.values()) { deferred.resolve({ output: 'Tool call deduplicated but original result was lost', @@ -168,7 +191,8 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu } } - private checkSameStep(toolCallId: string, toolName: string, args: unknown): ToolDedupResult | null { + + private checkToolCall(toolCallId: string, toolName: string, args: unknown): CheckedToolCall { const key = makeKey(toolName, args); const index = this.stepCalls.length; this.stepCalls.push(key); @@ -177,11 +201,32 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu const existing = this.stepDeferreds.get(key); if (existing !== undefined) { this.syntheticCallIds.add(toolCallId); - return DEDUP_PLACEHOLDER_RESULT; + this.recordDupType(toolCallId, toolName, args, 'same_step'); + return { syntheticResult: DEDUP_PLACEHOLDER_RESULT }; } this.stepDeferreds.set(key, makeDeferred()); this.originalCallIndex.set(toolCallId, index); - return null; + if (this.consecutiveKey === key && this.consecutiveCount > 0) { + this.recordDupType(toolCallId, toolName, args, 'cross_step'); + return { syntheticResult: null }; + } + return { syntheticResult: null }; + } + + private recordDupType( + toolCallId: string, + toolName: string, + args: unknown, + dupType: ToolCallDupType, + ): void { + this.telemetry.track('tool_call_dedup_detected', { + turn_id: this.activeTurnId ?? 0, + step_no: this.activeStep, + tool_call_id: toolCallId, + tool_name: toolName, + dup_type: dupType, + args_hash: argsHash(args), + }); } private async finalizeResult( diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts index d895e3358..6a9b517aa 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts @@ -100,14 +100,15 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { const results: ToolResult[] = []; for (let index = 0; index < preparedTasks.length; index += 1) { - const { call } = preparedTasks[index]!; + const prepared = preparedTasks[index]!; + const { call } = prepared; const timedResult = timedResults[index]!; const rawResult = timedResult.result; const finalized = await this.finalizeToolResult(call, rawResult, options); results.push(finalized); await dispatchToolResult(call, finalized, options); - this.trackToolCall(call, finalized, timedResult.durationMs); + this.trackToolCall(call, finalized, timedResult.durationMs, options.turnId); } return results; @@ -117,13 +118,15 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { call: PreflightedToolCall, result: ToolResult, durationMs: number, + turnId: number, ): void { const outcome = toolTelemetryOutcome(result); const properties: Record = { + turn_id: turnId, + tool_call_id: call.toolCall.id, tool_name: call.toolName, outcome, duration_ms: durationMs, - dup_type: 'normal', }; if (result.isError === true) properties['error_type'] = toolTelemetryErrorType(outcome); this.telemetry.track('tool_call', properties); @@ -133,21 +136,29 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { call: PreflightedToolCall, allCalls: readonly ToolCall[], options: ToolExecutorExecuteOptions, - ): Promise<{ task: ToolExecutionTask; stopBatchAfterThis?: boolean }> { + ): Promise<{ + task: ToolExecutionTask; + stopBatchAfterThis?: boolean; + }> { const settleError = ( args: unknown, output: string, displayFields?: ToolCallDisplayFields, ): { task: ToolExecutionTask } => { dispatchToolCall(call, args, options, displayFields); - return { task: makeResolvedTask(makeErrorToolResult(call, args, output)) }; + return { + task: makeResolvedTask(makeErrorToolResult(call, args, output)), + }; }; const settleSynthetic = ( args: unknown, result: ExecutableToolResult, displayFields?: ToolCallDisplayFields, - ): { task: ToolExecutionTask; stopBatchAfterThis?: boolean } => { + ): { + task: ToolExecutionTask; + stopBatchAfterThis?: boolean; + } => { const toolResult = this.normalizeAndMergeResult(result, call.toolName, undefined); dispatchToolCall(call, args, options, displayFields); return { @@ -203,7 +214,11 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { ); } if (decision?.syntheticResult !== undefined) { - return settleSynthetic(call.args, decision.syntheticResult, displayFields); + return settleSynthetic( + call.args, + decision.syntheticResult, + displayFields, + ); } const executionMetadata = decision?.executionMetadata; @@ -346,9 +361,14 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { }; } - const effectiveResult = coerceToolResult(didCtx.result, call.toolName); + const coercedResult = coerceToolResult(didCtx.result, call.toolName); + const effectiveResult = normalizeToolResult(coercedResult); return { - ...result, + ...effectiveResult, + message: coercedResult.message ?? result.message, + description: result.description, + display: result.display, + approvalRule: result.approvalRule, stopTurn: result.stopTurn === true || didCtx.stopTurn === true || diff --git a/packages/agent-core-v2/test/externalHooks/integration.test.ts b/packages/agent-core-v2/test/externalHooks/integration.test.ts index 963bdf72b..c16a8b313 100644 --- a/packages/agent-core-v2/test/externalHooks/integration.test.ts +++ b/packages/agent-core-v2/test/externalHooks/integration.test.ts @@ -1,11 +1,36 @@ import { describe, expect, it } from 'vitest'; +import { SyncDescriptor } from '#/_base/di/descriptors'; +import { DisposableStore } from '#/_base/di/lifecycle'; +import { + createServices, + type TestInstantiationService, +} from '#/_base/di/test'; +import { Event } from '#/_base/event'; +import { IAgentToolService } from '#/agent/agentTool'; +import { IAgentBackgroundService } from '#/agent/background'; +import { + AgentExternalHooksService, + IAgentExternalHooksService, +} from '#/agent/externalHooks'; import { HookEngine } from '#/agent/externalHooks/engine'; import { HookDefSchema, hooksFromToml, hooksToToml, } from '#/agent/externalHooks/configSection'; +import { IAgentFullCompactionService } from '#/agent/fullCompaction'; +import { IAgentLoopService, type TurnWillStopContext } from '#/agent/loop'; +import { IAgentPermissionGate } from '#/agent/permissionGate'; +import { IAgentToolExecutorService } from '#/agent/toolExecutor'; +import { IAgentTurnService, type Turn } from '#/agent/turn'; +import { IBootstrapService } from '#/app/bootstrap'; +import { IConfigService } from '#/app/config'; +import { IPluginService } from '#/app/plugin'; +import { createHooks } from '#/hooks'; + +import { stubBootstrap } from '../bootstrap/stubs'; +import { stubLoopWithHooks, stubToolExecutor, stubTurnWithHooks } from '../turn/stubs'; function nodeCommand(source: string): string { return `node -e ${JSON.stringify(source.replace(/\s*\n\s*/g, ' '))}`; @@ -22,6 +47,15 @@ function stdinScript(body: string): string { ].join('\n')); } +function makeTurn(id: number): Turn { + return { + id, + abortController: new AbortController(), + ready: Promise.resolve(), + result: Promise.resolve({ reason: 'completed' }), + }; +} + describe('HookEngine integration', () => { it('blocks a dangerous Bash command and allows a safe one via a PreToolUse script hook', async () => { const engine = new HookEngine([ @@ -71,6 +105,76 @@ describe('HookEngine integration', () => { expect(results[0]?.reason).toContain('tests not written'); }); + it('limits external Stop hook continuations to once per active turn', async () => { + const disposables = new DisposableStore(); + let ix: TestInstantiationService | undefined; + try { + const loop = stubLoopWithHooks(); + const turnService = stubTurnWithHooks(); + const stopInputs: unknown[] = []; + const hookEngine = { + trigger: async () => [], + fireAndForgetTrigger: async () => [], + triggerBlock: async (_event: string, args: { inputData?: unknown }) => { + stopInputs.push(args.inputData); + return { block: true, reason: `continue ${stopInputs.length}` }; + }, + }; + + ix = createServices(disposables, { + strict: true, + additionalServices: (reg) => { + reg.defineInstance(IBootstrapService, stubBootstrap()); + reg.definePartialInstance(IConfigService, {}); + reg.definePartialInstance(IPluginService, {}); + reg.defineInstance(IAgentLoopService, loop); + reg.defineInstance(IAgentTurnService, turnService); + reg.defineInstance(IAgentToolExecutorService, stubToolExecutor()); + reg.definePartialInstance(IAgentPermissionGate, { + hooks: createHooks(['onDidRequestApproval', 'onDidResolveApproval']), + }); + reg.definePartialInstance(IAgentFullCompactionService, { + hooks: createHooks(['onWillCompact', 'onDidCompact']), + }); + reg.definePartialInstance(IAgentBackgroundService, { + hooks: createHooks(['onDidNotify']), + }); + reg.definePartialInstance(IAgentToolService, { + hooks: createHooks(['onWillRunSubagent', 'onDidRunSubagent']), + }); + }, + }); + ix.set( + IAgentExternalHooksService, + new SyncDescriptor(AgentExternalHooksService, [{ hookEngine }]), + ); + ix.get(IAgentExternalHooksService); + + const signal = new AbortController().signal; + const first: TurnWillStopContext = { signal }; + await loop.hooks.onWillStop.run(first); + expect(first.continuationPrompt).toBe('continue 1'); + + const second: TurnWillStopContext = { signal }; + await loop.hooks.onWillStop.run(second); + expect(second.continuationPrompt).toBeUndefined(); + expect(stopInputs).toEqual([{ stopHookActive: false }]); + + await turnService.hooks.onEnded.run({ + turn: makeTurn(0), + result: { reason: 'completed' }, + }); + + const nextTurn: TurnWillStopContext = { signal }; + await loop.hooks.onWillStop.run(nextTurn); + expect(nextTurn.continuationPrompt).toBe('continue 2'); + expect(stopInputs).toEqual([{ stopHookActive: false }, { stopHookActive: false }]); + } finally { + ix?.dispose(); + disposables.dispose(); + } + }); + it('fires a Notification hook only when its matcher equals the notification matcher value', async () => { const engine = new HookEngine([ { diff --git a/packages/agent-core-v2/test/goal/goal.test.ts b/packages/agent-core-v2/test/goal/goal.test.ts index 79aa36bba..f0a94920f 100644 --- a/packages/agent-core-v2/test/goal/goal.test.ts +++ b/packages/agent-core-v2/test/goal/goal.test.ts @@ -60,10 +60,12 @@ function makeTurn(id: number): Turn { async function runGoalStep(loopService: IAgentLoopService, turn: Turn): Promise { const step = { turnId: turn.id, + step: 1, signal: turn.abortController.signal, }; const afterStep = { turnId: turn.id, + step: 1, signal: turn.abortController.signal, usage: zeroUsage, continueTurn: false, @@ -81,6 +83,7 @@ async function runStepUsageHooks( ): Promise { const afterStep = { turnId: turn.id, + step: 1, signal: turn.abortController.signal, usage, continueTurn: false, @@ -658,10 +661,12 @@ describe('AgentGoalService core workflow hooks', () => { await turnService.hooks.onLaunched.run({ turn }); const step = { turnId: turn.id, + step: 1, signal: turn.abortController.signal, }; const afterStep = { turnId: turn.id, + step: 1, signal: turn.abortController.signal, usage: zeroUsage, continueTurn: false, diff --git a/packages/agent-core-v2/test/loop/basic.test.ts b/packages/agent-core-v2/test/loop/basic.test.ts index df0ed981f..8c5719f51 100644 --- a/packages/agent-core-v2/test/loop/basic.test.ts +++ b/packages/agent-core-v2/test/loop/basic.test.ts @@ -182,6 +182,43 @@ describe('Agent loop', () => { tool[call_lookup]: text "lookup-result" `); }); + + it('lets non-external stop hooks continue a turn more than once', async () => { + profile.update({ activeToolNames: [] }); + let continuations = 0; + loop.hooks.onWillStop.register('test-repeat-stop-continuation', async (hookCtx, next) => { + if (continuations < 2) { + continuations += 1; + hookCtx.continuationPrompt = `continue ${continuations}`; + return; + } + await next(); + }); + + ctx.mockNextResponse({ type: 'text', text: 'First answer.' }); + ctx.mockNextResponse({ type: 'text', text: 'Second answer.' }); + ctx.mockNextResponse({ type: 'text', text: 'Third answer.' }); + + await ctx.rpc.prompt({ input: [{ type: 'text', text: 'hello' }] }); + await ctx.untilTurnEnd(); + + expect(continuations).toBe(2); + expect(ctx.llmCalls).toHaveLength(3); + expect(ctx.contextData().history).toContainEqual( + expect.objectContaining({ + role: 'user', + content: [{ type: 'text', text: 'continue 1' }], + origin: { kind: 'system_trigger', name: 'stop_hook' }, + }), + ); + expect(ctx.contextData().history).toContainEqual( + expect.objectContaining({ + role: 'user', + content: [{ type: 'text', text: 'continue 2' }], + origin: { kind: 'system_trigger', name: 'stop_hook' }, + }), + ); + }); }); describe('step timing split propagation', () => { diff --git a/packages/agent-core-v2/test/toolDedup/tool-dedup.test.ts b/packages/agent-core-v2/test/toolDedup/tool-dedup.test.ts index c387c324a..0eb54ba0b 100644 --- a/packages/agent-core-v2/test/toolDedup/tool-dedup.test.ts +++ b/packages/agent-core-v2/test/toolDedup/tool-dedup.test.ts @@ -3,15 +3,19 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { DisposableStore } from '#/_base/di/lifecycle'; import { createServices } from '#/_base/di/test'; import { ITelemetryService } from '#/app/telemetry'; +import type { ToolCall } from '#/app/llmProtocol'; import { IAgentLoopService } from '#/agent/loop'; +import type { ExecutableTool, ExecutableToolContext, ToolExecution } from '#/agent/tool'; import { IAgentToolDedupeService, AgentToolDedupeService, __testing as toolDedupTesting, } from '#/agent/toolDedupe'; import type { ToolDedupResult } from '#/agent/toolDedupe'; -import { IAgentToolExecutorService } from '#/agent/toolExecutor'; +import { AgentToolExecutorService, IAgentToolExecutorService } from '#/agent/toolExecutor'; +import { AgentToolRegistryService, IAgentToolRegistryService } from '#/agent/toolRegistry'; import { IAgentTurnService } from '#/agent/turn'; +import { registerLogServices } from '../log/stubs'; import { recordingTelemetry, type TelemetryRecord } from '../telemetry/stubs'; import { stubLoopWithHooks, stubToolExecutor, stubTurnWithHooks } from '../turn/stubs'; @@ -46,7 +50,7 @@ function okResult(text: string): ToolDedupResult { } interface ToolDedupeInternals extends IAgentToolDedupeService { - beginStep(): Promise; + beginStep(turnId?: number, step?: number): Promise; endStep(): Promise; checkSameStep( toolCallId: string, @@ -82,6 +86,32 @@ async function runOriginal( return deduper.finalizeResult(callId, tool, args, result); } +function toolCall(id: string, name: string, args: unknown): ToolCall { + return { + type: 'function', + id, + name, + arguments: JSON.stringify(args), + }; +} + +class EchoTool implements ExecutableTool> { + readonly name = 'Echo'; + readonly description = 'Echo input text.'; + readonly parameters = { type: 'object', additionalProperties: true }; + readonly calls: Array }> = []; + + resolveExecution(args: Record): ToolExecution { + return { + approvalRule: this.name, + execute: async (ctx) => { + this.calls.push({ ...ctx, args }); + return { output: String(args['text'] ?? '') }; + }, + }; + } +} + describe('AgentToolDedupeService', () => { describe('same-step dedup', () => { it('returns a placeholder synchronously and resolves to the real result on finalize', async () => { @@ -122,6 +152,53 @@ describe('AgentToolDedupeService', () => { expect(origFinal).toEqual(okResult('A')); expect(dupFinal).toEqual(okResult('A')); }); + + it('wires through ToolExecutor hooks and replaces same-step placeholders', async () => { + const loop = stubLoopWithHooks(); + const ix = createServices(disposables, { + additionalServices: (reg) => { + reg.defineInstance(ITelemetryService, recordingTelemetry(telemetryEvents)); + reg.defineInstance(IAgentLoopService, loop); + reg.defineInstance(IAgentTurnService, stubTurnWithHooks()); + reg.define(IAgentToolRegistryService, AgentToolRegistryService); + reg.define(IAgentToolExecutorService, AgentToolExecutorService); + reg.define(IAgentToolDedupeService, AgentToolDedupeService); + registerLogServices(reg); + }, + strict: true, + }); + const registry = ix.get(IAgentToolRegistryService); + const executor = ix.get(IAgentToolExecutorService); + const tool = new EchoTool(); + registry.register(tool); + ix.get(IAgentToolDedupeService); + + await loop.hooks.beforeStep.run({ + turnId: 3, + step: 1, + signal: new AbortController().signal, + }); + const results = await executor.execute( + [ + toolCall('call_1', 'Echo', { text: 'same' }), + toolCall('call_2', 'Echo', { text: 'same' }), + ], + { turnId: 3, signal: new AbortController().signal }, + ); + + expect(tool.calls).toHaveLength(1); + expect(results.map((result) => result.output)).toEqual(['same', 'same']); + expect(telemetryEvents).toContainEqual({ + event: 'tool_call_dedup_detected', + properties: expect.objectContaining({ + turn_id: 3, + step_no: 1, + tool_call_id: 'call_2', + tool_name: 'Echo', + dup_type: 'same_step', + }), + }); + }); }); describe('cross-step streak', () => { @@ -494,6 +571,67 @@ describe('AgentToolDedupeService', () => { }); describe('repeat telemetry', () => { + it('emits same-step duplicate detection telemetry', async () => { + const dedup = createDeduper(); + await dedup.beginStep(7, 1); + await runOriginal(dedup, 'c1', 'Read', { path: '/a' }, okResult('FILE_A')); + const cached = await dedup.checkSameStep('c2', 'Read', { path: '/a' }); + + expect(cached).not.toBeNull(); + expect(telemetryEvents).toContainEqual({ + event: 'tool_call_dedup_detected', + properties: { + turn_id: 7, + step_no: 1, + tool_call_id: 'c2', + tool_name: 'Read', + dup_type: 'same_step', + args_hash: expect.any(String), + }, + }); + }); + + it('emits cross-step duplicate detection telemetry', async () => { + const dedup = createDeduper(); + await dedup.beginStep(7, 1); + await runOriginal(dedup, 'c1', 'Read', { path: '/a' }, okResult('FILE_A')); + await dedup.endStep(); + telemetryEvents.length = 0; + + await dedup.beginStep(7, 2); + await runOriginal(dedup, 'c2', 'Read', { path: '/a' }, okResult('FILE_A')); + + expect(telemetryEvents).toContainEqual({ + event: 'tool_call_dedup_detected', + properties: { + turn_id: 7, + step_no: 2, + tool_call_id: 'c2', + tool_name: 'Read', + dup_type: 'cross_step', + args_hash: expect.any(String), + }, + }); + }); + + it('does not keep interrupted cross-step history just for duplicate telemetry', async () => { + const dedup = createDeduper(); + await dedup.beginStep(7, 1); + await runOriginal(dedup, 'a1', 'Read', { path: '/a' }, okResult('A')); + await dedup.endStep(); + await dedup.beginStep(7, 2); + await runOriginal(dedup, 'b1', 'Read', { path: '/b' }, okResult('B')); + await dedup.endStep(); + telemetryEvents.length = 0; + + await dedup.beginStep(7, 3); + const result = await runOriginal(dedup, 'a2', 'Read', { path: '/a' }, okResult('A')); + + expect(result.output as string).not.toContain(''); + expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedup_detected')).toHaveLength(0); + expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0); + }); + it('emits tool_call_repeat with the streak count starting at the second occurrence', async () => { const dedup = createDeduper(); for (let i = 0; i < 3; i += 1) { @@ -575,6 +713,24 @@ describe('AgentToolDedupeService', () => { expect(counts).toEqual([2]); }); + it('resets repeat state at turn boundaries', async () => { + const dedup = createDeduper(); + for (let i = 0; i < 2; i += 1) { + await dedup.beginStep(1, i + 1); + await runOriginal(dedup, `a${String(i)}`, 'Read', { p: 1 }, okResult('R')); + await dedup.endStep(); + } + telemetryEvents.length = 0; + + await dedup.beginStep(2, 1); + const firstInNewTurn = await runOriginal(dedup, 'b1', 'Read', { p: 1 }, okResult('R')); + await dedup.endStep(); + + expect(firstInNewTurn.output as string).not.toContain(''); + expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0); + expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedup_detected')).toHaveLength(0); + }); + it('runs with a no-op telemetry service', async () => { const dedup = createDeduper(recordingTelemetry([])); for (let i = 0; i < 3; i += 1) { diff --git a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts index 2d11bcb83..e52e57274 100644 --- a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts +++ b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts @@ -70,10 +70,11 @@ describe('AgentToolExecutorService', () => { expect(telemetryEvents).toContainEqual({ event: 'tool_call', properties: expect.objectContaining({ + turn_id: 0, + tool_call_id: 'call_echo', tool_name: 'echo', outcome: 'success', duration_ms: expect.any(Number), - dup_type: 'normal', }), }); }); @@ -94,10 +95,11 @@ describe('AgentToolExecutorService', () => { expect(telemetryEvents).toContainEqual({ event: 'tool_call', properties: expect.objectContaining({ + turn_id: 0, + tool_call_id: 'call_missing', tool_name: 'missing', outcome: 'error', duration_ms: expect.any(Number), - dup_type: 'normal', error_type: 'error', }), }); @@ -468,6 +470,31 @@ describe('AgentToolExecutorService', () => { }), ]); }); + + it('onDidExecuteTool can replace the final tool result', async () => { + const tool = new TestTool('echo'); + registry.register(tool); + executor.hooks.onDidExecuteTool.register('replace-result', async (ctx) => { + ctx.result = { output: 'hook output', isError: true }; + }); + + const results = await execute([toolCall('call_echo', 'echo', { text: 'raw output' })]); + + expect(results).toEqual([ + expect.objectContaining({ + output: 'hook output', + isError: true, + }), + ]); + expect(events).toContainEqual({ + type: 'tool.result', + toolCallId: 'call_echo', + result: expect.objectContaining({ + output: 'hook output', + isError: true, + }), + }); + }); }); describe('parseToolCallArguments', () => { diff --git a/packages/agent-core-v2/test/turn/turn.test.ts b/packages/agent-core-v2/test/turn/turn.test.ts index 1b8480748..45ff0f392 100644 --- a/packages/agent-core-v2/test/turn/turn.test.ts +++ b/packages/agent-core-v2/test/turn/turn.test.ts @@ -11,6 +11,7 @@ import { APITimeoutError, type ChatProvider, type ModelCapability, + type ProviderRequestAuth, type ToolCall, } from '#/app/llmProtocol/kosong'; import { describe, expect, it, vi } from 'vitest'; @@ -207,6 +208,7 @@ describe('Agent turn flow', () => { properties: { turn_id: 0, step_no: 1, + tool_call_id: 'call_dup_2', tool_name: 'Bash', dup_type: 'same_step', args_hash: expect.any(String), @@ -244,6 +246,7 @@ describe('Agent turn flow', () => { properties: { turn_id: 0, step_no: 2, + tool_call_id: 'call_dup_2', tool_name: 'Bash', dup_type: 'cross_step', args_hash: expect.any(String), @@ -252,9 +255,10 @@ describe('Agent turn flow', () => { expect(records).toContainEqual({ event: 'tool_call', properties: expect.objectContaining({ + turn_id: 0, + tool_call_id: 'call_dup_2', tool_name: 'Bash', outcome: 'success', - dup_type: 'cross_step', duration_ms: expect.any(Number), }), }); @@ -329,9 +333,10 @@ describe('Agent turn flow', () => { expect(records).toContainEqual({ event: 'tool_call', properties: expect.objectContaining({ + turn_id: 0, + tool_call_id: 'call_missing', tool_name: 'MissingTool', outcome: 'error', - dup_type: 'normal', error_type: 'ToolNotFound', duration_ms: expect.any(Number), }), @@ -898,7 +903,7 @@ describe('Agent turn flow', () => { expect(JSON.stringify(ctx.contextData().history)).toContain('Second answer.'); }); - it('removes an unconsumed Stop hook continuation when no step budget remains', async () => { + it('fails with max steps when a Stop hook continuation exceeds step budget', async () => { const hookEngine = new HookEngine([ { event: 'Stop', @@ -919,11 +924,19 @@ describe('Agent turn flow', () => { const events = await ctx.untilTurnEnd(); expect(ctx.llmCalls).toHaveLength(1); - expect(JSON.stringify(ctx.contextData().history)).not.toContain('continue from hook'); + expect(JSON.stringify(ctx.contextData().history)).toContain('continue from hook'); expect(events).toContainEqual( expect.objectContaining({ event: 'turn.ended', - args: expect.objectContaining({ reason: 'completed' }), + args: expect.objectContaining({ + reason: 'failed', + error: expect.objectContaining({ + code: 'loop.max_steps_exceeded', + details: expect.objectContaining({ + maxSteps: 1, + }), + }), + }), }), ); }); @@ -1748,7 +1761,7 @@ describe('Agent turn flow', () => { const withAuth = ctx.modelResolver.resolveAuth?.('kimi-code'); if (withAuth === undefined) throw new Error('OAuth model did not resolve auth wrapper'); const videoUploader: VideoUploader = (input) => - withAuth((auth) => { + withAuth((auth: ProviderRequestAuth) => { const uploadVideo = provider.uploadVideo; if (uploadVideo === undefined) throw new Error('Provider did not expose uploadVideo'); return uploadVideo.call(provider, input, { auth });