mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-19 13:45:28 +00:00
fix: tool dedupe
This commit is contained in:
_Kerman
parent
3381c44b16
commit
70f936acad
8 changed files with 435 additions and 28 deletions
|
|
@ -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<string, string>();
|
||||
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<ToolDedupResult>());
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> = {
|
||||
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 ||
|
||||
|
|
|
|||
|
|
@ -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([
|
||||
{
|
||||
|
|
|
|||
|
|
@ -60,10 +60,12 @@ function makeTurn(id: number): Turn {
|
|||
async function runGoalStep(loopService: IAgentLoopService, turn: Turn): Promise<boolean> {
|
||||
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<boolean> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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<void>;
|
||||
beginStep(turnId?: number, step?: number): Promise<void>;
|
||||
endStep(): Promise<void>;
|
||||
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<Record<string, unknown>> {
|
||||
readonly name = 'Echo';
|
||||
readonly description = 'Echo input text.';
|
||||
readonly parameters = { type: 'object', additionalProperties: true };
|
||||
readonly calls: Array<ExecutableToolContext & { readonly args: Record<string, unknown> }> = [];
|
||||
|
||||
resolveExecution(args: Record<string, unknown>): 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('<system-reminder>');
|
||||
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('<system-reminder>');
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue