vrr/kimi-code
2
0
Fork 0
mirror of https://github.com/MoonshotAI/kimi-code.git synced 2026-08-24 16:17:57 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix: align v2 external hook behavior

Browse source
This commit is contained in:
_Kerman 2026-07-07 14:36:02 +08:00
parent b2fe7a7c7c
commit d25cb76138
5 changed files with 220 additions and 96 deletions
Download patch file Download diff file Expand all files Collapse all files

7
packages/agent-core-v2/src/agent/externalHooks/engine.ts
View file

@ -97,13 +97,14 @@ export class HookEngine {
}
private matchingHooks(event: string, matcherValue: string): HookDef[] {
const seenCommands = new Set<string>();
const seen = new Set<string>();
const matched: HookDef[] = [];
for (const hook of this.byEvent.get(event) ?? []) {
if (!matches(hook.matcher ?? '', matcherValue)) continue;
if (seenCommands.has(hook.command)) continue;
seenCommands.add(hook.command);
const key = (hook.cwd ?? '') + '\0' + hook.command;
if (seen.has(key)) continue;
seen.add(key);
matched.push(hook);
}

118
packages/agent-core-v2/src/agent/externalHooks/externalHooksService.ts
View file

@ -17,7 +17,6 @@ import { IInstantiationService } from '#/_base/di/instantiation';
import { Disposable } from '#/_base/di/lifecycle';
import { InstantiationType } from '#/_base/di/extensions';
import { LifecycleScope, registerScopedService } from '#/_base/di/scope';
import { isUserCancellation } from '#/_base/utils/abort';
import { isPlainRecord } from '#/_base/utils/canonical-args';
import { IAgentTaskService, type AgentTaskNotificationContext } from '#/agent/task';
import { IAgentContextMemoryService, USER_PROMPT_ORIGIN } from '#/agent/contextMemory';
@ -71,24 +70,11 @@ declare module '#/app/event/eventBus' {
const SUBAGENT_HOOK_TEXT_PREVIEW_LENGTH = 500;
function fireAndForget(
engine: ExternalHooksServiceOptions['hookEngine'],
event: string,
inputData: Record<string, unknown>,
signal: AbortSignal,
matcherValue?: string,
): void {
// Genuinely fire-and-forget: never throw on an already-aborted signal. A
// cancelled tool still finalizes its result (e.g. the "manually interrupted"
// output), and throwing here would clobber that with a finalize-abort error.
// Matches legacy `fireAndForgetTrigger`, which fires unconditionally.
void engine?.fireAndForgetTrigger(event, { matcherValue, signal, inputData });
}
export class AgentExternalHooksService extends Disposable implements IAgentExternalHooksService {
declare readonly _serviceBrand: undefined;
private dynamicEngine: HookEngine | undefined;
private readonly hooksReady: Promise<void>;
private stopHookContinuationUsed = false;
constructor(
@ -103,12 +89,14 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
super();
if (options.hookEngine === undefined) {
this.dynamicEngine = new HookEngine([], { cwd: this.bootstrap.cwd });
void this.loadDynamicHooks();
this.hooksReady = this.loadDynamicHooksSafe();
this._register(
this.plugins.onDidReload(() => {
void this.loadDynamicHooks();
void this.loadDynamicHooksSafe();
}),
);
} else {
this.hooksReady = Promise.resolve();
}
this.registerListeners();
}
@ -117,6 +105,28 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
return this.options.hookEngine ?? this.dynamicEngine;
}
private async readyEngine(): Promise<ExternalHooksServiceOptions['hookEngine']> {
await this.hooksReady;
return this.engine();
}
private fireAndForget(
event: string,
inputData: Record<string, unknown>,
matcherValue?: string,
signal?: AbortSignal,
): void {
// Genuinely fire-and-forget: never throw on an already-aborted signal. A
// cancelled tool still finalizes its result (e.g. the "manually interrupted"
// output), and throwing here would clobber that with a finalize-abort error.
// Matches legacy `fireAndForgetTrigger`, which fires unconditionally.
void this.readyEngine()
.then((engine) => {
void engine?.fireAndForgetTrigger(event, { matcherValue, signal, inputData });
})
.catch(() => undefined);
}
private registerListeners(): void {
this.registerToolHooks(
this.instantiation.invokeFunction((accessor) => accessor.get(IAgentToolExecutorService)),
@ -167,23 +177,17 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
);
}
private registerPermissionHooks(permission: IAgentPermissionGate): void {
private registerPermissionHooks(_permission: IAgentPermissionGate): void {
this._register(
this.eventBus.subscribe('permission.approval.requested', (e) => {
const { type: _type, ...inputData } = e;
void this.engine()?.fireAndForgetTrigger('PermissionRequest', {
matcherValue: e.toolName,
inputData,
});
this.fireAndForget('PermissionRequest', inputData, e.toolName);
}),
);
this._register(
this.eventBus.subscribe('permission.approval.resolved', (e) => {
const { type: _type, ...inputData } = e;
void this.engine()?.fireAndForgetTrigger('PermissionResult', {
matcherValue: e.toolName,
inputData,
});
this.fireAndForget('PermissionResult', inputData, e.toolName);
}),
);
}
@ -200,7 +204,7 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
);
}
private registerTurnHooks(turn: IAgentTurnService): void {
private registerTurnHooks(_turn: IAgentTurnService): void {
this._register(
this.eventBus.subscribe('turn.ended', (e) => this.notifyTurnEnded(e)),
);
@ -245,7 +249,7 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
);
}
private registerTaskHooks(tasks: IAgentTaskService): void {
private registerTaskHooks(_tasks: IAgentTaskService): void {
this._register(
this.eventBus.subscribe('task.notified', (e) => {
const { type: _type, ...ctx } = e;
@ -263,10 +267,17 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
});
}
private async loadDynamicHooksSafe(): Promise<void> {
try {
await this.loadDynamicHooks();
} catch {}
}
private async runPreToolUse(ctx: ToolWillExecuteContext): Promise<string | undefined> {
ctx.signal.throwIfAborted();
const toolInput = isPlainRecord(ctx.args) ? ctx.args : {};
const block = await this.engine()?.triggerBlock('PreToolUse', {
const engine = await this.readyEngine();
const block = await engine?.triggerBlock('PreToolUse', {
matcherValue: ctx.toolCall.name,
signal: ctx.signal,
inputData: {
@ -282,8 +293,7 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
private notifyPostToolUse(ctx: ToolDidExecuteContext): void {
const output = toolOutputText(ctx.result.output);
const isError = ctx.result.isError === true;
fireAndForget(
this.engine(),
this.fireAndForget(
isError ? 'PostToolUseFailure' : 'PostToolUse',
{
toolName: ctx.toolCall.name,
@ -292,8 +302,8 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
error: isError ? toKimiErrorPayload(output) : undefined,
toolOutput: isError ? undefined : output.slice(0, 2000),
},
ctx.signal,
ctx.toolCall.name,
ctx.signal,
);
}
@ -305,7 +315,8 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
const signal = new AbortController().signal;
const input = ctx.promptMessage.content;
signal.throwIfAborted();
const results = await this.engine()?.trigger('UserPromptSubmit', {
const engine = await this.readyEngine();
const results = await engine?.trigger('UserPromptSubmit', {
matcherValue: input,
signal,
inputData: { prompt: input, isSteer: ctx.isSteer },
@ -356,23 +367,20 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
this.notifyStopFailure(event.error, new AbortController().signal);
}
if (event.reason === 'cancelled') {
void this.engine()?.fireAndForgetTrigger('Interrupt', {
inputData: { turnId: event.turnId, reason: 'cancelled' },
});
this.fireAndForget('Interrupt', { turnId: event.turnId, reason: 'cancelled' });
}
}
private notifyStopFailure(error: unknown, signal: AbortSignal): void {
const payload = toKimiErrorPayload(error);
fireAndForget(
this.engine(),
this.fireAndForget(
'StopFailure',
{
errorType: payload.name,
errorMessage: payload.message,
},
signal,
payload.name,
signal,
);
}
@ -380,7 +388,8 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
ctx.signal.throwIfAborted();
if (this.stopHookContinuationUsed) return undefined;
const block = await this.engine()?.triggerBlock('Stop', {
const engine = await this.readyEngine();
const block = await engine?.triggerBlock('Stop', {
signal: ctx.signal,
inputData: { stopHookActive: false },
});
@ -390,7 +399,8 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
private async runPreCompact(ctx: FullCompactionWillCompactContext): Promise<void> {
ctx.signal.throwIfAborted();
await this.engine()?.trigger('PreCompact', {
const engine = await this.readyEngine();
await engine?.trigger('PreCompact', {
matcherValue: ctx.trigger,
signal: ctx.signal,
inputData: {
@ -402,29 +412,30 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
}
private notifyPostCompact(event: { trigger: CompactionSource; result: CompactionResult }): void {
void this.engine()?.fireAndForgetTrigger('PostCompact', {
matcherValue: event.trigger,
inputData: {
this.fireAndForget(
'PostCompact',
{
trigger: event.trigger,
estimatedTokenCount: event.result.tokensAfter,
},
});
event.trigger,
);
}
private notifyTaskNotification(ctx: AgentTaskNotificationContext): void {
const signal = new AbortController().signal;
fireAndForget(
this.engine(),
this.fireAndForget(
'Notification',
{ sink: 'context', ...ctx },
signal,
ctx.notificationType,
signal,
);
}
async runAgentTaskStart(ctx: AgentTaskStartHookContext): Promise<void> {
ctx.signal.throwIfAborted();
await this.engine()?.trigger('SubagentStart', {
const engine = await this.readyEngine();
await engine?.trigger('SubagentStart', {
matcherValue: ctx.agentName,
signal: ctx.signal,
inputData: {
@ -436,13 +447,14 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter
}
notifyAgentTaskStop(ctx: AgentTaskStopHookContext): void {
void this.engine()?.fireAndForgetTrigger('SubagentStop', {
matcherValue: ctx.agentName,
inputData: {
this.fireAndForget(
'SubagentStop',
{
agentName: ctx.agentName,
response: ctx.response.slice(0, SUBAGENT_HOOK_TEXT_PREVIEW_LENGTH),
},
});
ctx.agentName,
);
}
}

21
packages/agent-core-v2/src/agent/externalHooks/runner.ts
View file

@ -208,8 +208,12 @@ function killProcess(child: ChildProcessWithoutNullStreams): void {
}
function tryKillProcess(child: ChildProcessWithoutNullStreams, signal: NodeJS.Signals): void {
if (process.platform === 'win32') {
killProcessTreeWindows(child, signal === 'SIGKILL');
return;
}
try {
if (process.platform !== 'win32' && child.pid !== undefined) {
if (child.pid !== undefined) {
process.kill(-child.pid, signal);
} else {
child.kill(signal);
@ -221,6 +225,21 @@ function tryKillProcess(child: ChildProcessWithoutNullStreams, signal: NodeJS.Si
}
}
function killProcessTreeWindows(child: ChildProcessWithoutNullStreams, force: boolean): void {
if (child.pid === undefined) return;
const args = force
? ['/T', '/F', '/PID', String(child.pid)]
: ['/T', '/PID', String(child.pid)];
try {
const killer = spawn('taskkill', args, { stdio: 'ignore', windowsHide: true });
killer.once('error', () => {});
} catch {
try {
child.kill('SIGTERM');
} catch {}
}
}
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}

17
packages/agent-core-v2/test/externalHooks/engine.test.ts
View file

@ -7,7 +7,7 @@ import { describe, expect, it, vi } from 'vitest';
import { HookEngine } from '#/agent/externalHooks/engine';
function nodeCommand(source: string): string {
return `node -e ${JSON.stringify(source.replace(/\s*\n\s*/g, ' '))}`;
return `node -e ${JSON.stringify(source.replaceAll(/\s*\n\s*/g, ' '))}`;
}
describe('HookEngine', () => {
@ -289,6 +289,21 @@ describe('HookEngine', () => {
expect(results).toHaveLength(1);
});
it('does not dedupe hooks that share a command but have different cwd', async () => {
const command = nodeCommand('process.stdout.write(process.cwd() + "\\n");');
const engine = new HookEngine([
{ event: 'Stop', command, timeout: 5, cwd: process.cwd() },
{ event: 'Stop', command, timeout: 5, cwd: tmpdir() },
]);
const results = await engine.trigger('Stop', { inputData: {} });
expect(results).toHaveLength(2);
expect(new Set(results.map((result) => result.stdout?.trim()))).toEqual(
new Set([realpathSync(process.cwd()), realpathSync(tmpdir())]),
);
});
it('silently skips hooks whose matcher is not a valid regex', async () => {
const engine = new HookEngine([
{

153
packages/agent-core-v2/test/externalHooks/integration.test.ts
View file

@ -25,6 +25,7 @@ import {
import { HookEngine } from '#/agent/externalHooks/engine';
import {
HookDefSchema,
HOOKS_SECTION,
hooksFromToml,
hooksToToml,
} from '#/agent/externalHooks/configSection';
@ -33,9 +34,11 @@ import { IAgentLoopService, type TurnAfterStepContext } from '#/agent/loop';
import { IAgentPermissionGate } from '#/agent/permissionGate';
import { IAgentPromptService } from '#/agent/prompt';
import { IAgentToolExecutorService } from '#/agent/toolExecutor';
import { IAgentTurnService, type Turn } from '#/agent/turn';
import { IAgentTurnService } from '#/agent/turn';
import { IBootstrapService } from '#/app/bootstrap/bootstrap';
import { IConfigService } from '#/app/config/config';
import { IEventBus } from '#/app/event/eventBus';
import { EventBusService } from '#/app/event/eventBusService';
import { IPluginService } from '#/app/plugin/plugin';
import { createHooks } from '#/hooks';
import { IAgentWireService, WireService } from '#/wire';
@ -58,15 +61,6 @@ 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' }),
};
}
function makeAfterStep(signal: AbortSignal): TurnAfterStepContext {
return {
turnId: 0,
@ -89,7 +83,7 @@ function stubContextMemory(): IAgentContextMemoryService & {
messages.push(...inserted.map(ensureMessageId));
},
clear: () => {
messages.splice(0, messages.length);
messages.splice(0);
},
undo: (count) => {
const cut = computeUndoCut(messages, count);
@ -108,11 +102,15 @@ function stubContextMemory(): IAgentContextMemoryService & {
splice: (start, deleteCount, inserted) => {
messages.splice(start, deleteCount, ...inserted);
},
hooks: createHooks(['onSpliced']) as IAgentContextMemoryService['hooks'],
messages,
};
}
async function flushMicrotasks(): Promise<void> {
await Promise.resolve();
await Promise.resolve();
}
describe('HookEngine integration', () => {
it('blocks a dangerous Bash command and allows a safe one via a PreToolUse script hook', async () => {
const engine = new HookEngine([
@ -167,7 +165,6 @@ describe('HookEngine integration', () => {
let ix: TestInstantiationService | undefined;
try {
const loop = stubLoopWithHooks();
const turnService = stubTurnWithHooks();
const context = stubContextMemory();
const stopInputs: unknown[] = [];
const hookEngine = {
@ -187,20 +184,17 @@ describe('HookEngine integration', () => {
reg.definePartialInstance(IPluginService, {});
reg.defineInstance(IAgentContextMemoryService, context);
reg.defineInstance(IAgentLoopService, loop);
reg.define(IEventBus, EventBusService);
reg.definePartialInstance(IAgentPromptService, {
hooks: createHooks(['onWillSubmitPrompt']),
});
reg.defineInstance(IAgentTurnService, turnService);
reg.defineInstance(IAgentTurnService, stubTurnWithHooks());
reg.defineInstance(IAgentToolExecutorService, stubToolExecutor());
reg.definePartialInstance(IAgentPermissionGate, {
hooks: createHooks(['onDidRequestApproval', 'onDidResolveApproval']),
});
reg.definePartialInstance(IAgentPermissionGate, {});
reg.definePartialInstance(IAgentFullCompactionService, {
hooks: createHooks(['onWillCompact', 'onDidCompact']),
});
reg.definePartialInstance(IAgentTaskService, {
hooks: createHooks(['onDidNotify']),
hooks: createHooks(['onWillCompact']),
});
reg.definePartialInstance(IAgentTaskService, {});
reg.defineInstance(
IAgentWireService,
disposables.add(new WireService({ logScope: 'wire', logKey: 'external-hooks' })),
@ -212,6 +206,7 @@ describe('HookEngine integration', () => {
new SyncDescriptor(AgentExternalHooksService, [{ hookEngine }]),
);
ix.get(IAgentExternalHooksService);
const eventBus = ix.get(IEventBus);
const signal = new AbortController().signal;
const filtered: TurnAfterStepContext = {
@ -239,9 +234,11 @@ describe('HookEngine integration', () => {
expect(second.continue).toBe(false);
expect(stopInputs).toEqual([{ stopHookActive: false }]);
await turnService.hooks.onEnded.run({
turn: makeTurn(0),
result: { reason: 'completed' },
eventBus.publish({
type: 'turn.ended',
turnId: 0,
reason: 'completed',
durationMs: 0,
});
const nextTurn = makeAfterStep(signal);
@ -265,10 +262,6 @@ describe('HookEngine integration', () => {
const disposables = new DisposableStore();
let ix: TestInstantiationService | undefined;
try {
const permissionHooks = createHooks([
'onDidRequestApproval',
'onDidResolveApproval',
]) as IAgentPermissionGate['hooks'];
const fired: Array<{
event: string;
matcherValue?: unknown;
@ -296,22 +289,18 @@ describe('HookEngine integration', () => {
reg.definePartialInstance(IConfigService, {});
reg.definePartialInstance(IPluginService, {});
reg.defineInstance(IAgentContextMemoryService, stubContextMemory());
reg.defineInstance(IAgentRecordService, stubRecord());
reg.defineInstance(IAgentLoopService, stubLoopWithHooks());
reg.define(IEventBus, EventBusService);
reg.definePartialInstance(IAgentPromptService, {
hooks: createHooks(['onWillSubmitPrompt']),
});
reg.defineInstance(IAgentTurnService, stubTurnWithHooks());
reg.defineInstance(IAgentToolExecutorService, stubToolExecutor());
reg.definePartialInstance(IAgentPermissionGate, {
hooks: permissionHooks,
});
reg.definePartialInstance(IAgentPermissionGate, {});
reg.definePartialInstance(IAgentFullCompactionService, {
hooks: createHooks(['onWillCompact', 'onDidCompact']),
});
reg.definePartialInstance(IAgentTaskService, {
hooks: createHooks(['onDidNotify']),
hooks: createHooks(['onWillCompact']),
});
reg.definePartialInstance(IAgentTaskService, {});
},
});
ix.set(
@ -319,6 +308,7 @@ describe('HookEngine integration', () => {
new SyncDescriptor(AgentExternalHooksService, [{ hookEngine }]),
);
ix.get(IAgentExternalHooksService);
const eventBus = ix.get(IEventBus);
const requestContext = {
sessionId: 'session-1',
@ -330,12 +320,17 @@ describe('HookEngine integration', () => {
toolInput: { command: 'pwd' },
display: { kind: 'command' as const, command: 'pwd' },
};
await permissionHooks.onDidRequestApproval.run(requestContext);
await permissionHooks.onDidResolveApproval.run({
eventBus.publish({
type: 'permission.approval.requested',
...requestContext,
});
eventBus.publish({
type: 'permission.approval.resolved',
...requestContext,
decision: 'approved',
selectedLabel: 'Approve once',
});
await flushMicrotasks();
expect(fired).toEqual([
{
@ -359,6 +354,88 @@ describe('HookEngine integration', () => {
}
});
it('waits for dynamic hooks to load before running the first blocking hook', async () => {
const disposables = new DisposableStore();
let ix: TestInstantiationService | undefined;
try {
const loop = stubLoopWithHooks();
const context = stubContextMemory();
let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});
ix = createServices(disposables, {
strict: true,
additionalServices: (reg) => {
reg.defineInstance(IBootstrapService, stubBootstrap());
reg.definePartialInstance(IConfigService, {
ready,
get: <T = unknown>(domain: string): T =>
(domain === HOOKS_SECTION
? [
{
event: 'Stop' as const,
command: nodeCommand('process.stderr.write("loaded stop hook"); process.exit(2);'),
timeout: 5,
},
]
: undefined) as T,
});
reg.definePartialInstance(IPluginService, {
enabledHooks: async () => [],
onDidReload: Event.None as IPluginService['onDidReload'],
});
reg.defineInstance(IAgentContextMemoryService, context);
reg.defineInstance(IAgentLoopService, loop);
reg.define(IEventBus, EventBusService);
reg.definePartialInstance(IAgentPromptService, {
hooks: createHooks(['onWillSubmitPrompt']),
});
reg.defineInstance(IAgentTurnService, stubTurnWithHooks());
reg.defineInstance(IAgentToolExecutorService, stubToolExecutor());
reg.definePartialInstance(IAgentPermissionGate, {});
reg.definePartialInstance(IAgentFullCompactionService, {
hooks: createHooks(['onWillCompact']),
});
reg.definePartialInstance(IAgentTaskService, {});
reg.defineInstance(
IAgentWireService,
disposables.add(new WireService({ logScope: 'wire', logKey: 'external-hooks' })),
);
},
});
ix.set(
IAgentExternalHooksService,
new SyncDescriptor(AgentExternalHooksService, [{}]),
);
ix.get(IAgentExternalHooksService);
const afterStep = makeAfterStep(new AbortController().signal);
let completed = false;
const pending = loop.hooks.afterStep.run(afterStep).then(() => {
completed = true;
});
await flushMicrotasks();
expect(completed).toBe(false);
resolveReady();
await pending;
expect(afterStep.continue).toBe(true);
expect(context.messages.at(-1)).toEqual(
expect.objectContaining({
role: 'user',
content: [{ type: 'text', text: 'loaded stop hook' }],
origin: { kind: 'system_trigger', name: 'stop_hook' },
}),
);
} finally {
ix?.dispose();
disposables.dispose();
}
});
it('fires a Notification hook only when its matcher equals the notification matcher value', async () => {
const engine = new HookEngine([
{