remove hooks experimental and refactor hook Config

This commit is contained in:
DennisYu07 2026-04-01 11:50:23 +08:00
parent 1b1a029fd7
commit 5221002831
33 changed files with 722 additions and 322 deletions

View file

@ -347,7 +347,7 @@ describe('Server Config (config.ts)', () => {
const mockMessageBus = { request: vi.fn() };
const config = new Config({
...baseParams,
enableHooks: true,
disableAllHooks: false,
});
// Set messageBus using the setter
config.setMessageBus(mockMessageBus as unknown as MessageBus);
@ -378,7 +378,7 @@ describe('Server Config (config.ts)', () => {
it('should not fire notification hook when hooks are disabled', async () => {
const config = new Config({
...baseParams,
enableHooks: false,
disableAllHooks: true,
});
const authType = AuthType.USE_GEMINI;
const mockContentConfig = {

View file

@ -418,12 +418,10 @@ export interface ConfigParameters {
modelProvidersConfig?: ModelProvidersConfig;
/** Multi-agent collaboration settings (Arena, Team, Swarm) */
agents?: AgentsCollabSettings;
/** Enable hook system for lifecycle events */
enableHooks?: boolean;
/** Disable all hooks (default: false, hooks enabled) */
disableAllHooks?: boolean;
/** Hooks configuration from settings */
hooks?: Record<string, unknown>;
/** Hooks config settings (enabled, disabled list) */
hooksConfig?: Record<string, unknown>;
/** Warnings generated during configuration resolution */
warnings?: string[];
/**
@ -593,9 +591,8 @@ export class Config {
private readonly eventEmitter?: EventEmitter;
private readonly channel: string | undefined;
private readonly defaultFileEncoding: FileEncodingType | undefined;
private readonly enableHooks: boolean;
private readonly disableAllHooks: boolean;
private readonly hooks?: Record<string, unknown>;
private readonly hooksConfig?: Record<string, unknown>;
private hookSystem?: HookSystem;
private messageBus?: MessageBus;
@ -759,9 +756,8 @@ export class Config {
enabledExtensionOverrides: this.overrideExtensions,
isWorkspaceTrusted: this.isTrustedFolder(),
});
this.enableHooks = params.enableHooks ?? false;
this.disableAllHooks = params.disableAllHooks ?? false;
this.hooks = params.hooks;
this.hooksConfig = params.hooksConfig;
}
/**
@ -786,7 +782,7 @@ export class Config {
this.debugLogger.debug('Extension manager initialized');
// Initialize hook system if enabled
if (this.enableHooks) {
if (!this.disableAllHooks) {
this.hookSystem = new HookSystem(this);
await this.hookSystem.initialize();
this.debugLogger.debug('Hook system initialized');
@ -1072,7 +1068,7 @@ export class Config {
// Fire auth_success notification hook (supports both interactive & non-interactive)
const messageBus = this.getMessageBus();
const hooksEnabled = this.getEnableHooks();
const hooksEnabled = !this.getDisableAllHooks();
if (hooksEnabled && messageBus) {
fireNotificationHook(
messageBus,
@ -1779,10 +1775,10 @@ export class Config {
}
/**
* Check if hooks are enabled.
* Check if all hooks are disabled.
*/
getEnableHooks(): boolean {
return this.enableHooks;
getDisableAllHooks(): boolean {
return this.disableAllHooks;
}
/**
@ -1801,17 +1797,6 @@ export class Config {
this.messageBus = messageBus;
}
/**
* Get the list of disabled hook names.
* This is used by the HookRegistry to filter out disabled hooks.
*/
getDisabledHooks(): string[] {
const hooksConfig = this.hooksConfig;
if (!hooksConfig) return [];
const disabled = hooksConfig['disabled'];
return Array.isArray(disabled) ? (disabled as string[]) : [];
}
/**
* Get project-level hooks configuration.
* This is used by the HookRegistry to load project-specific hooks.

View file

@ -2415,7 +2415,7 @@ Other open files:
request: vi.fn(),
response: vi.fn(),
};
vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
vi.spyOn(client['config'], 'getDisableAllHooks').mockReturnValue(false);
vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
);
@ -2439,7 +2439,7 @@ Other open files:
request: vi.fn(),
response: vi.fn(),
};
vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
vi.spyOn(client['config'], 'getDisableAllHooks').mockReturnValue(false);
vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
);
@ -2463,7 +2463,7 @@ Other open files:
request: vi.fn().mockResolvedValue({ modifiedPrompt: undefined }),
response: vi.fn(),
};
vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
vi.spyOn(client['config'], 'getDisableAllHooks').mockReturnValue(false);
vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
);

View file

@ -463,7 +463,7 @@ export class GeminiClient {
}
// Fire UserPromptSubmit hook through MessageBus (only if hooks are enabled)
const hooksEnabled = this.config.getEnableHooks();
const hooksEnabled = !this.config.getDisableAllHooks();
const messageBus = this.config.getMessageBus();
if (
messageType !== SendMessageType.Retry &&

View file

@ -1009,7 +1009,7 @@ export class CoreToolScheduler {
const messageBus = this.config.getMessageBus() as
| MessageBus
| undefined;
const hooksEnabled = this.config.getEnableHooks();
const hooksEnabled = !this.config.getDisableAllHooks();
if (hooksEnabled && messageBus) {
const permissionMode = String(this.config.getApprovalMode());
@ -1326,7 +1326,7 @@ export class CoreToolScheduler {
// Get MessageBus for hook execution
const messageBus = this.config.getMessageBus() as MessageBus | undefined;
const hooksEnabled = this.config.getEnableHooks();
const hooksEnabled = !this.config.getDisableAllHooks();
// PreToolUse Hook
if (hooksEnabled && messageBus) {

View file

@ -28,7 +28,6 @@ describe('HookRegistry', () => {
isTrustedFolder: vi.fn().mockReturnValue(true),
getHooks: vi.fn().mockReturnValue(undefined),
getProjectHooks: vi.fn().mockReturnValue(undefined),
getDisabledHooks: vi.fn().mockReturnValue([]),
getExtensions: vi.fn().mockReturnValue([]),
};
mockFeedbackEmitter = {
@ -123,21 +122,20 @@ describe('HookRegistry', () => {
expect(postHooks[0].config.name).toBe('post-hook');
});
it('should filter out disabled hooks', async () => {
mockConfig.getDisabledHooks = vi.fn().mockReturnValue(['disabled-hook']);
it('should register all hooks as enabled by default', async () => {
const hooksConfig = {
[HookEventName.PreToolUse]: [
{
hooks: [
{
type: HookType.Command,
command: 'echo enabled',
name: 'enabled-hook',
command: 'echo first',
name: 'first-hook',
},
{
type: HookType.Command,
command: 'echo disabled',
name: 'disabled-hook',
command: 'echo second',
name: 'second-hook',
},
],
},
@ -149,8 +147,9 @@ describe('HookRegistry', () => {
await registry.initialize();
const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
expect(hooks).toHaveLength(1);
expect(hooks[0].config.name).toBe('enabled-hook');
expect(hooks).toHaveLength(2);
expect(hooks[0].enabled).toBe(true);
expect(hooks[1].enabled).toBe(true);
});
it('should sort hooks by source priority', async () => {
@ -181,37 +180,6 @@ describe('HookRegistry', () => {
});
describe('setHookEnabled', () => {
it('should enable a disabled hook', async () => {
mockConfig.getDisabledHooks = vi.fn().mockReturnValue(['test-hook']);
const hooksConfig = {
[HookEventName.PreToolUse]: [
{
hooks: [
{
type: HookType.Command,
command: 'echo test',
name: 'test-hook',
},
],
},
],
};
mockConfig.getHooks = vi.fn().mockReturnValue(hooksConfig);
const registry = new HookRegistry(mockConfig);
await registry.initialize();
expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
0,
);
registry.setHookEnabled('test-hook', true);
const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
expect(hooks).toHaveLength(1);
expect(hooks[0].enabled).toBe(true);
});
it('should disable an enabled hook', async () => {
const hooksConfig = {
[HookEventName.PreToolUse]: [
@ -237,9 +205,40 @@ describe('HookRegistry', () => {
registry.setHookEnabled('test-hook', false);
const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
expect(hooks).toHaveLength(0);
});
it('should enable a disabled hook', async () => {
const hooksConfig = {
[HookEventName.PreToolUse]: [
{
hooks: [
{
type: HookType.Command,
command: 'echo test',
name: 'test-hook',
},
],
},
],
};
mockConfig.getHooks = vi.fn().mockReturnValue(hooksConfig);
const registry = new HookRegistry(mockConfig);
await registry.initialize();
// First disable the hook
registry.setHookEnabled('test-hook', false);
expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
0,
);
// Then enable it again
registry.setHookEnabled('test-hook', true);
expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
1,
);
});
it('should update all hooks with matching name', async () => {

View file

@ -32,7 +32,6 @@ export interface HookRegistryConfig {
isTrustedFolder(): boolean;
getHooks(): { [K in HookEventName]?: HookDefinition[] } | undefined;
getProjectHooks(): { [K in HookEventName]?: HookDefinition[] } | undefined;
getDisabledHooks(): string[];
getExtensions(): ExtensionWithHooks[];
}
@ -250,18 +249,13 @@ please review the project settings (.qwen/settings.json) and remove them.`;
return;
}
// Get disabled hooks list from settings
const disabledHooks = this.config.getDisabledHooks();
for (const hookConfig of definition.hooks) {
if (
hookConfig &&
typeof hookConfig === 'object' &&
this.validateHookConfig(hookConfig, eventName, source)
) {
// Check if this hook is in the disabled list
const hookName = this.getHookName({ config: hookConfig });
const isDisabled = disabledHooks.includes(hookName);
// Check for duplicate hooks (same name+command+source+eventName+matcher+sequential)
const isDuplicate = this.entries.some(
@ -288,7 +282,7 @@ please review the project settings (.qwen/settings.json) and remove them.`;
eventName,
matcher: definition.matcher,
sequential: definition.sequential,
enabled: !isDisabled,
enabled: true,
});
} else {
// Invalid hooks are logged and discarded here, they won't reach HookRunner