scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-07 08:26:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(hooks): align tests and handler typing for update flow

Browse source 
- Update hooksSetAllEnabled tests to match global hooksEnabled toggle behavior\n- Add IHookManager.updateHook to test mocks after interface change\n- Narrow hooksUpdateHook events to HookEventType via schema validation
This commit is contained in:
Toray Altas 2026-01-17 19:47:05 -05:00
parent 609ff88969
commit 469d7feedd
4 changed files with 31 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/core/webview/__tests__/ClineProvider.hooks-dynamic-init.spec.ts
View file

@ -102,6 +102,7 @@ const createMockHookManager = (): IHookManager => ({
totalDuration: 0,
}),
setHookEnabled: vi.fn().mockResolvedValue(undefined),
updateHook: vi.fn().mockResolvedValue(undefined),
getHookExecutionHistory: vi.fn().mockReturnValue([]),
getConfigSnapshot: vi.fn().mockReturnValue({
hooksByEvent: new Map(),

70
src/core/webview/__tests__/webviewMessageHandler.hooks.spec.ts
View file

@ -96,6 +96,7 @@ const createMockHookManager = (): IHookManager => ({
totalDuration: 0,
}),
setHookEnabled: vi.fn().mockResolvedValue(undefined),
updateHook: vi.fn().mockResolvedValue(undefined),
getHookExecutionHistory: vi.fn().mockReturnValue([]),
getConfigSnapshot: vi.fn().mockReturnValue({
hooksByEvent: new Map(),
@ -264,45 +265,15 @@ describe("webviewMessageHandler - hooks commands", () => {
})
describe("hooksSetAllEnabled", () => {
it("should call setHookEnabled for all hooks in snapshot and postStateToWebview", async () => {
const hooksById = new Map<string, ResolvedHook>()
hooksById.set("hook-1", {
id: "hook-1",
event: "PreToolUse" as any,
matcher: ".*",
command: "echo 1",
enabled: true,
source: "global" as any,
timeout: 30,
includeConversationHistory: false,
} as any)
hooksById.set("hook-2", {
id: "hook-2",
event: "PostToolUse" as any,
matcher: ".*",
command: "echo 2",
enabled: true,
source: "project" as any,
timeout: 30,
includeConversationHistory: false,
} as any)
vi.mocked(mockHookManager.getConfigSnapshot).mockReturnValue({
hooksByEvent: new Map(),
hooksById,
loadedAt: new Date(),
disabledHookIds: new Set(),
hasProjectHooks: false,
} as HooksConfigSnapshot)
it("should update global hooksEnabled and postStateToWebview", async () => {
await webviewMessageHandler(mockClineProvider, {
type: "hooksSetAllEnabled",
hooksEnabled: false,
})
expect(mockHookManager.setHookEnabled).toHaveBeenCalledTimes(2)
expect(mockHookManager.setHookEnabled).toHaveBeenCalledWith("hook-1", false)
expect(mockHookManager.setHookEnabled).toHaveBeenCalledWith("hook-2", false)
// hooksSetAllEnabled no longer iterates hooks; it toggles global state.
expect(mockHookManager.setHookEnabled).not.toHaveBeenCalled()
expect((mockClineProvider as any).contextProxy.setValue).toHaveBeenCalledWith("hooksEnabled", false)
expect(mockClineProvider.postStateToWebview).toHaveBeenCalledTimes(1)
})
@ -313,39 +284,22 @@ describe("webviewMessageHandler - hooks commands", () => {
} as any)
expect(mockHookManager.setHookEnabled).not.toHaveBeenCalled()
expect((mockClineProvider as any).contextProxy.setValue).not.toHaveBeenCalled()
expect(mockClineProvider.postStateToWebview).not.toHaveBeenCalled()
})
it("should show error message when bulk setHookEnabled fails", async () => {
const hooksById = new Map<string, ResolvedHook>()
hooksById.set("hook-1", {
id: "hook-1",
event: "PreToolUse" as any,
matcher: ".*",
command: "echo 1",
enabled: true,
source: "global" as any,
timeout: 30,
includeConversationHistory: false,
} as any)
vi.mocked(mockHookManager.getConfigSnapshot).mockReturnValue({
hooksByEvent: new Map(),
hooksById,
loadedAt: new Date(),
disabledHookIds: new Set(),
hasProjectHooks: false,
} as HooksConfigSnapshot)
vi.mocked(mockHookManager.setHookEnabled).mockRejectedValueOnce(new Error("boom"))
it("should show error message when updating hooksEnabled global state fails", async () => {
vi.mocked((mockClineProvider as any).contextProxy.setValue).mockRejectedValueOnce(new Error("boom"))
await webviewMessageHandler(mockClineProvider, {
type: "hooksSetAllEnabled",
hooksEnabled: true,
})
expect(mockClineProvider.log).toHaveBeenCalledWith("Failed to set all hooks enabled: boom")
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to enable all hooks")
expect(mockHookManager.setHookEnabled).not.toHaveBeenCalled()
expect(mockClineProvider.postStateToWebview).not.toHaveBeenCalled()
expect(mockClineProvider.log).toHaveBeenCalledWith("Failed to set hooks enabled: boom")
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to enable hooks")
})
})

18
src/core/webview/webviewMessageHandler.ts
View file

@ -3,6 +3,11 @@ import { safeWriteText } from "../../utils/safeWriteText"
import * as path from "path"
import * as os from "os"
import * as fs from "fs/promises"
import {
HookEventType as HookEventTypeSchema,
type HookEventType,
type HookUpdateData,
} from "../../services/hooks/types"
import { getRooDirectoriesForCwd } from "../../services/roo-config/index.js"
import pWaitFor from "p-wait-for"
import * as vscode from "vscode"
@ -3559,7 +3564,18 @@ export const webviewMessageHandler = async (
}
try {
await hookManager.updateHook(message.filePath, message.hookId, message.hookUpdates)
const hookUpdates: HookUpdateData = {
// Webview messages are not strongly typed, so validate and narrow.
events: Array.isArray(message.hookUpdates.events)
? message.hookUpdates.events.filter(
(event): event is HookEventType => HookEventTypeSchema.safeParse(event).success,
)
: undefined,
matcher: typeof message.hookUpdates.matcher === "string" ? message.hookUpdates.matcher : undefined,
timeout: typeof message.hookUpdates.timeout === "number" ? message.hookUpdates.timeout : undefined,
}
await hookManager.updateHook(message.filePath, message.hookId, hookUpdates)
await hookManager.reloadHooksConfig()
await provider.postStateToWebview()
} catch (error) {

1
src/services/hooks/__tests__/ToolExecutionHooks.spec.ts
View file

@ -30,6 +30,7 @@ describe("ToolExecutionHooks", () => {
totalDuration: 100,
} as HooksExecutionResult),
setHookEnabled: vi.fn(),
updateHook: vi.fn(),
getEnabledHooks: vi.fn().mockReturnValue([]),
getHookExecutionHistory: vi.fn().mockReturnValue([]),
})