migrate fireToolNotificationHook to hookSystem (#17398)

Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
Vedant Mahajan
2026-01-24 18:52:08 +05:30
committed by GitHub
parent a894c88656
commit abb4fb006c
6 changed files with 109 additions and 168 deletions
@@ -32,17 +32,12 @@ import type { ValidatingToolCall, WaitingToolCall } from './types.js';
import type { Config } from '../config/config.js';
import type { EditorType } from '../utils/editor.js';
import { randomUUID } from 'node:crypto';
import { fireToolNotificationHook } from '../core/coreToolHookTriggers.js';
// Mock Dependencies
vi.mock('node:crypto', () => ({
randomUUID: vi.fn(),
}));
vi.mock('../core/coreToolHookTriggers.js', () => ({
fireToolNotificationHook: vi.fn(),
}));
describe('confirmation.ts', () => {
let mockMessageBus: MessageBus;
@@ -140,15 +135,19 @@ describe('confirmation.ts', () => {
configurable: true,
});
const mockHookSystem = {
fireToolNotificationEvent: vi.fn().mockResolvedValue(undefined),
};
mockConfig = {
getEnableHooks: vi.fn().mockReturnValue(true),
getHookSystem: vi.fn().mockReturnValue(mockHookSystem),
} as unknown as Mocked<Config>;
mockModifier = {
handleModifyWithEditor: vi.fn(),
applyInlineModify: vi.fn(),
} as unknown as Mocked<ToolModificationHandler>;
mockConfig = {
getEnableHooks: vi.fn().mockReturnValue(true),
} as unknown as Mocked<Config>;
getPreferredEditor = vi.fn().mockReturnValue('vim');
invocationMock = {
@@ -263,8 +262,9 @@ describe('confirmation.ts', () => {
});
await promise;
expect(fireToolNotificationHook).toHaveBeenCalledWith(
mockMessageBus,
expect(
mockConfig.getHookSystem()?.fireToolNotificationEvent,
).toHaveBeenCalledWith(
expect.objectContaining({
type: details.type,
prompt: details.prompt,
+2 -3
View File
@@ -23,7 +23,6 @@ import type { SchedulerStateManager } from './state-manager.js';
import type { ToolModificationHandler } from './tool-modifier.js';
import type { EditorType } from '../utils/editor.js';
import type { DiffUpdateResult } from '../ide/ide-client.js';
import { fireToolNotificationHook } from '../core/coreToolHookTriggers.js';
import { debugLogger } from '../utils/debugLogger.js';
export interface ConfirmationResult {
@@ -168,8 +167,8 @@ async function notifyHooks(
deps: { config: Config; messageBus: MessageBus },
details: ToolCallConfirmationDetails,
): Promise<void> {
if (deps.config.getEnableHooks()) {
await fireToolNotificationHook(deps.messageBus, {
if (deps.config.getHookSystem()) {
await deps.config.getHookSystem()?.fireToolNotificationEvent({
...details,
// Pass no-op onConfirm to satisfy type definition; side-effects via
// callbacks are disallowed.
@@ -35,10 +35,6 @@ vi.mock('../telemetry/types.js', () => ({
ToolCallEvent: vi.fn().mockImplementation((call) => ({ ...call })),
}));
vi.mock('../core/coreToolHookTriggers.js', () => ({
fireToolNotificationHook: vi.fn(),
}));
import { SchedulerStateManager } from './state-manager.js';
import { resolveConfirmation } from './confirmation.js';
import { checkPolicy, updatePolicy } from './policy.js';