diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index da524b34f7..e9be4a8070 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -229,6 +229,12 @@ export interface HookInfo { filePath?: string /** The event type this hook is registered for */ event: string + /** + * All event types this hook ID is registered for. + * + * Backwards-compatible: older extensions may only provide `event`. + */ + events?: string[] /** Tool name filter (regex/glob pattern) */ matcher?: string /** Preview of the command (truncated for display) */ diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4208e58b4e..5c41038a2e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2250,15 +2250,17 @@ export class ClineProvider // Build a list of *all* known hooks (including currently disabled ones) // so the webview can show per-hook toggles and a global "Enable Hooks" toggle. - const allHooks = snapshot - ? Array.from(snapshot.hooksByEvent.values()).reduce((acc, hooks) => acc.concat(hooks), [] as any[]) - : [] + // + // IMPORTANT: Use hooksById to avoid duplicating the same hook ID when it is + // registered for multiple events. Runtime execution still uses hooksByEvent. + const allHooks = snapshot ? Array.from(snapshot.hooksById.values()) : [] // Convert ResolvedHook[] to HookInfo[] const hookInfos = allHooks.map((hook) => ({ id: hook.id, filePath: hook.filePath, event: hook.event, + events: (hook.events && hook.events.length > 0 ? hook.events : [hook.event]).map(String), matcher: hook.matcher, commandPreview: hook.command, enabled: (hook.enabled ?? true) && !(snapshot?.disabledHookIds?.has(hook.id) ?? false), diff --git a/src/core/webview/__tests__/ClineProvider.hooks-state.spec.ts b/src/core/webview/__tests__/ClineProvider.hooks-state.spec.ts new file mode 100644 index 0000000000..1946aeb87a --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.hooks-state.spec.ts @@ -0,0 +1,251 @@ +// npx vitest run core/webview/__tests__/ClineProvider.hooks-state.spec.ts + +import type { IHookManager, HooksConfigSnapshot, HookEventType, ResolvedHook } from "../../../services/hooks/types" + +import { ClineProvider } from "../ClineProvider" + +vi.mock("fs/promises", () => ({ + default: { + mkdir: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue(""), + writeFile: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), + readdir: vi.fn().mockResolvedValue([]), + rm: vi.fn().mockResolvedValue(undefined), + }, + mkdir: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue(""), + writeFile: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), + readdir: vi.fn().mockResolvedValue([]), + rm: vi.fn().mockResolvedValue(undefined), +})) + +vi.mock("os", () => ({ + default: { + homedir: vi.fn().mockReturnValue("/mock/home"), + tmpdir: vi.fn().mockReturnValue("/mock/tmp"), + }, + homedir: vi.fn().mockReturnValue("/mock/home"), + tmpdir: vi.fn().mockReturnValue("/mock/tmp"), +})) + +vi.mock("vscode", () => ({ + workspace: { + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn(), + update: vi.fn(), + }), + }, + window: { + createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }), + showInformationMessage: vi.fn(), + showWarningMessage: vi.fn(), + showErrorMessage: vi.fn(), + showTextDocument: vi.fn().mockResolvedValue(undefined), + createWebviewPanel: vi.fn(), + }, + commands: { + executeCommand: vi.fn().mockResolvedValue(undefined), + }, + env: { + uriScheme: "vscode", + language: "en", + appName: "Visual Studio Code", + }, + ExtensionMode: { + Production: 1, + Development: 2, + Test: 3, + }, +})) + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + setProvider: vi.fn(), + captureCodeActionUsed: vi.fn(), + captureModeSwitch: vi.fn(), + captureTelemetrySettingsChanged: vi.fn(), + updateTelemetryState: vi.fn(), + }, + hasInstance: vi.fn().mockReturnValue(true), + createInstance: vi.fn(), + }, +})) + +vi.mock("@roo-code/cloud", () => ({ + CloudService: { + hasInstance: vi.fn().mockReturnValue(false), + }, + BridgeOrchestrator: { + isEnabled: vi.fn().mockReturnValue(false), + }, + getRooCodeApiUrl: vi.fn().mockReturnValue("https://app.roocode.com"), +})) + +vi.mock("../../../integrations/workspace/WorkspaceTracker", () => ({ + default: vi.fn().mockImplementation(() => ({ + initializeFilePaths: vi.fn(), + dispose: vi.fn(), + })), +})) + +vi.mock("../../../services/mcp/McpServerManager", () => ({ + McpServerManager: { + getInstance: vi.fn().mockResolvedValue({ + registerClient: vi.fn(), + unregisterClient: vi.fn(), + }), + unregisterProvider: vi.fn(), + }, +})) + +vi.mock("../../../services/skills/SkillsManager", () => ({ + SkillsManager: vi.fn().mockImplementation(() => ({ + initialize: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockResolvedValue(undefined), + })), +})) + +vi.mock("../../../services/hooks", () => ({ + createHookManager: vi.fn().mockImplementation(() => null), + HookManager: vi.fn(), +})) + +vi.mock("../../../utils/path", () => ({ + getWorkspacePath: vi.fn().mockReturnValue("/mock/workspace"), +})) + +vi.mock("../../config/ProviderSettingsManager", () => ({ + ProviderSettingsManager: vi.fn().mockImplementation(() => ({ + listConfig: vi.fn().mockResolvedValue([]), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + activateProfile: vi.fn().mockResolvedValue({ name: "default", apiProvider: "openrouter" }), + getProfile: vi.fn().mockResolvedValue({ apiProvider: "openrouter" }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("id"), + })), +})) + +vi.mock("../../config/CustomModesManager", () => ({ + CustomModesManager: vi.fn().mockImplementation(() => ({ + getCustomModes: vi.fn().mockResolvedValue([]), + dispose: vi.fn(), + })), +})) + +vi.mock("../../../services/marketplace", () => ({ + MarketplaceManager: vi.fn().mockImplementation(() => ({ + cleanup: vi.fn(), + getMarketplaceItems: vi.fn().mockResolvedValue({ organizationMcps: [], marketplaceItems: [], errors: [] }), + getInstallationMetadata: vi.fn().mockResolvedValue({ project: {}, global: {} }), + })), +})) + +vi.mock("../../../activate/registerCommands", () => ({ + setPanel: vi.fn(), +})) + +vi.mock("../../../shared/package", () => ({ + Package: { name: "roo-code" }, +})) + +vi.mock("../../../i18n", () => ({ + t: vi.fn().mockImplementation((key: string) => key), +})) + +vi.mock("../../../api", () => ({ + buildApiHandler: vi.fn(), +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(false), +})) + +vi.mock("../../../utils/tts", () => ({ + setTtsEnabled: vi.fn(), + setTtsSpeed: vi.fn(), +})) + +vi.mock("../../../utils/git", () => ({ + getWorkspaceGitInfo: vi.fn().mockResolvedValue(undefined), +})) + +describe("ClineProvider.getHooksStateForWebview", () => { + it("returns a single HookInfo per id and includes aggregated events", () => { + const mkHook = (id: string, events: HookEventType[], event: HookEventType): ResolvedHook => + ({ + id, + event, + events, + filePath: "/mock/hooks.yaml", + source: "project", + command: "echo hi", + enabled: true, + timeout: 60, + }) as ResolvedHook + + const hookA = mkHook("hook-a", ["PreToolUse", "PostToolUse"], "PreToolUse") + + // hooksByEvent duplicates the same ID across events (runtime model) + const snapshot: HooksConfigSnapshot = { + hooksByEvent: new Map([ + ["PreToolUse", [hookA]], + ["PostToolUse", [hookA]], + ]), + hooksById: new Map([[hookA.id, hookA]]), + loadedAt: new Date("2026-01-01T00:00:00.000Z"), + disabledHookIds: new Set(), + hasProjectHooks: true, + } + + const mockHookManager: IHookManager = { + loadHooksConfig: vi.fn().mockResolvedValue(snapshot), + reloadHooksConfig: vi.fn().mockResolvedValue(undefined), + executeHooks: vi.fn() as any, + getEnabledHooks: vi.fn().mockReturnValue([]), + setHookEnabled: vi.fn().mockResolvedValue(undefined), + updateHook: vi.fn().mockResolvedValue(undefined), + getHookExecutionHistory: vi.fn().mockReturnValue([]), + getConfigSnapshot: vi.fn().mockReturnValue(snapshot), + } + + const mockContext: any = { + extensionUri: {}, + extension: { packageJSON: { version: "0.0.0" } }, + globalState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn(), + store: vi.fn(), + delete: vi.fn(), + }, + globalStorageUri: { fsPath: "/mock/storage" }, + } + + const provider = new ClineProvider(mockContext, { appendLine: vi.fn() } as any, "sidebar", { + getValues: vi.fn().mockReturnValue({}), + getValue: vi.fn().mockReturnValue(undefined), + setValue: vi.fn().mockResolvedValue(undefined), + setValues: vi.fn().mockResolvedValue(undefined), + getProviderSettings: vi.fn().mockReturnValue({}), + setProviderSettings: vi.fn().mockResolvedValue(undefined), + extensionMode: 3, + extensionUri: {}, + globalStorageUri: { fsPath: "/mock/storage" }, + } as any) + + ;(provider as any).hookManager = mockHookManager + + const hooksState = (provider as any).getHooksStateForWebview() + expect(hooksState).toBeDefined() + expect(hooksState.enabledHooks).toHaveLength(1) + expect(hooksState.enabledHooks[0].id).toBe("hook-a") + expect(hooksState.enabledHooks[0].event).toBe("PreToolUse") + expect(hooksState.enabledHooks[0].events).toEqual(["PreToolUse", "PostToolUse"]) + }) +}) diff --git a/webview-ui/src/components/settings/HooksSettings.tsx b/webview-ui/src/components/settings/HooksSettings.tsx index be43ae5116..8e101603bd 100644 --- a/webview-ui/src/components/settings/HooksSettings.tsx +++ b/webview-ui/src/components/settings/HooksSettings.tsx @@ -318,9 +318,13 @@ const HookItem: React.FC = ({ hook, onToggle, autoExpandHookId, o }, [hooks?.enabledHooks, hook.id]) const selectedEvents = useMemo(() => { - const events = hooksForId.map((h) => h.event).filter(Boolean) + // Prefer the aggregated `hook.events` when present (newer extension state). + // Fallback to the legacy state shape where the same ID appeared once per event. + const rawEvents = (hook.events && hook.events.length > 0 ? hook.events : hooksForId.map((h) => h.event)).filter( + Boolean, + ) // Preserve stable order based on HOOK_EVENT_OPTIONS - return HOOK_EVENT_OPTIONS.filter((e) => events.includes(e)) + return HOOK_EVENT_OPTIONS.filter((e) => rawEvents.includes(e)) }, [hooksForId]) const eventTooltipText = useCallback(