From 6ebe2dad0a9a096e977b05992e6c4997e559bfab Mon Sep 17 00:00:00 2001 From: Toray Altas Date: Sat, 17 Jan 2026 10:22:46 -0500 Subject: [PATCH] feat: gate hooks behind experimental flag Implement hooks experimental flag with conditional UI rendering and backend functionality gating. Fix missing experimental settings translations for hooks feature. Fix hook discovery issue by initializing HookManager when experiment is toggled on. --- packages/types/src/experiment.ts | 2 + ...resentAssistantMessage-custom-tool.spec.ts | 21 +++++++++ ...esentAssistantMessage-unknown-tool.spec.ts | 21 +++++++++ src/core/task/Task.ts | 5 +- src/core/webview/ClineProvider.ts | 10 +++- .../webviewMessageHandler.hooks.spec.ts | 7 ++- src/core/webview/webviewMessageHandler.ts | 47 ++++++++++++++++++- src/shared/__tests__/experiments.spec.ts | 3 ++ src/shared/experiments.ts | 2 + .../src/components/settings/SectionHeader.tsx | 7 ++- .../src/components/settings/SettingsView.tsx | 17 +++---- .../__tests__/ExtensionStateContext.spec.tsx | 2 + webview-ui/src/i18n/locales/en/settings.json | 4 ++ 13 files changed, 133 insertions(+), 15 deletions(-) diff --git a/packages/types/src/experiment.ts b/packages/types/src/experiment.ts index f6f701a25d..4c15acf536 100644 --- a/packages/types/src/experiment.ts +++ b/packages/types/src/experiment.ts @@ -14,6 +14,7 @@ export const experimentIds = [ "runSlashCommand", "multipleNativeToolCalls", "customTools", + "hooks", ] as const export const experimentIdsSchema = z.enum(experimentIds) @@ -32,6 +33,7 @@ export const experimentsSchema = z.object({ runSlashCommand: z.boolean().optional(), multipleNativeToolCalls: z.boolean().optional(), customTools: z.boolean().optional(), + hooks: z.boolean().optional(), }) export type Experiments = z.infer diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts index e90646fd9a..df45532096 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts @@ -52,6 +52,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { diffEnabled: false, consecutiveMistakeCount: 0, clineMessages: [], + cwd: "/mock/project/path", api: { getModel: () => ({ id: "test-model", info: {} }), }, @@ -63,6 +64,26 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { toolRepetitionDetector: { check: vi.fn().mockReturnValue({ allowExecution: true }), }, + toolExecutionHooks: { + executePreToolUse: vi.fn().mockResolvedValue({ + proceed: true, + hookResult: { + results: [], + blocked: false, + totalDuration: 0, + }, + }), + executePostToolUse: vi.fn().mockResolvedValue({ + results: [], + blocked: false, + totalDuration: 0, + }), + executePostToolUseFailure: vi.fn().mockResolvedValue({ + results: [], + blocked: false, + totalDuration: 0, + }), + }, providerRef: { deref: () => ({ getState: vi.fn().mockResolvedValue({ diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts index d4ae2764a0..fe8314dbe2 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts @@ -37,6 +37,7 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { diffEnabled: false, consecutiveMistakeCount: 0, clineMessages: [], + cwd: "/mock/project/path", api: { getModel: () => ({ id: "test-model", info: {} }), }, @@ -48,6 +49,26 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { toolRepetitionDetector: { check: vi.fn().mockReturnValue({ allowExecution: true }), }, + toolExecutionHooks: { + executePreToolUse: vi.fn().mockResolvedValue({ + proceed: true, + hookResult: { + results: [], + blocked: false, + totalDuration: 0, + }, + }), + executePostToolUse: vi.fn().mockResolvedValue({ + results: [], + blocked: false, + totalDuration: 0, + }), + executePostToolUseFailure: vi.fn().mockResolvedValue({ + results: [], + blocked: false, + totalDuration: 0, + }), + }, providerRef: { deref: () => ({ getState: vi.fn().mockResolvedValue({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 64abfab687..7960ca2d56 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -540,9 +540,10 @@ export class Task extends EventEmitter implements TaskLike { } }) - // Initialize tool execution hooks + // Initialize tool execution hooks (only if hooks experiment is enabled) + const hooksEnabled = experiments.isEnabled(experimentsConfig ?? {}, EXPERIMENT_IDS.HOOKS) this.toolExecutionHooks = createToolExecutionHooks( - provider.getHookManager() ?? null, + hooksEnabled ? (provider.getHookManager() ?? null) : null, (status) => provider.postHookStatusToWebview(status), async (type, text) => { await this.say(type as ClineSay, text) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index d6ffa8b73a..57248a8178 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -56,7 +56,7 @@ import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" import { GlobalFileNames } from "../../shared/globalFileNames" import { Mode, defaultModeSlug, getModeBySlug } from "../../shared/modes" -import { experimentDefault } from "../../shared/experiments" +import { experimentDefault, experiments, EXPERIMENT_IDS } from "../../shared/experiments" import { formatLanguage } from "../../shared/language" import { WebviewMessage } from "../../shared/WebviewMessage" import { EMBEDDING_MODEL_PROFILES } from "../../shared/embeddingModels" @@ -2650,6 +2650,7 @@ export class ClineProvider /** * Initialize the Hook Manager for lifecycle hooks. * This loads hooks configuration from project/.roo/hooks/ files. + * Only initializes if the hooks experiment is enabled. */ private async initializeHookManager(): Promise { const cwd = this.currentWorkspacePath || getWorkspacePath() @@ -2660,6 +2661,13 @@ export class ClineProvider try { const state = await this.getState() + + // Check if hooks experiment is enabled + if (!experiments.isEnabled(state?.experiments ?? {}, EXPERIMENT_IDS.HOOKS)) { + this.log("[HookManager] Hooks experiment is disabled, skipping initialization") + return + } + this.hookManager = createHookManager({ cwd, mode: state?.mode, diff --git a/src/core/webview/__tests__/webviewMessageHandler.hooks.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.hooks.spec.ts index 5faf340898..48bf065e9c 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.hooks.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.hooks.spec.ts @@ -128,7 +128,12 @@ const createMockClineProvider = (hookManager?: IHookManager) => { globalStorageUri: { fsPath: "/mock/global/storage" }, }, setValue: vi.fn(), - getValue: vi.fn(), + getValue: vi.fn().mockImplementation((key: string) => { + if (key === "experiments") { + return { hooks: true } // Enable hooks experiment for tests + } + return undefined + }), }, customModesManager: { getCustomModes: vi.fn(), diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 8015a500d5..3aa92e84b7 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -39,7 +39,7 @@ import { type RouterName, toRouterName } from "../../shared/api" import { MessageEnhancer } from "./messageEnhancer" import { checkExistKey } from "../../shared/checkExistApiConfig" -import { experimentDefault } from "../../shared/experiments" +import { experimentDefault, experiments, EXPERIMENT_IDS } from "../../shared/experiments" import { Terminal } from "../../integrations/terminal/Terminal" import { openFile } from "../../integrations/misc/open-file" import { openImage, saveImage } from "../../integrations/misc/image-handler" @@ -635,10 +635,23 @@ export const webviewMessageHandler = async ( continue } + const oldExperiments = getGlobalState("experiments") ?? experimentDefault newValue = { - ...(getGlobalState("experiments") ?? experimentDefault), + ...oldExperiments, ...(value as Record), } + + // Check if hooks experiment was just enabled + const newExperiments = newValue as Record + if ( + !experiments.isEnabled(oldExperiments, EXPERIMENT_IDS.HOOKS) && + experiments.isEnabled(newExperiments, EXPERIMENT_IDS.HOOKS) + ) { + // Initialize HookManager when hooks experiment is enabled + provider.initializeHookManager().catch((error) => { + provider.log(`Failed to initialize Hook Manager after experiment enable: ${error}`) + }) + } } else if (key === "customSupportPrompts") { if (!value) { continue @@ -3342,6 +3355,11 @@ export const webviewMessageHandler = async ( // ===================================================================== case "hooksReloadConfig": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } // Reload hooks configuration from all sources const hookManager = provider.getHookManager() if (hookManager) { @@ -3359,6 +3377,11 @@ export const webviewMessageHandler = async ( } case "hooksSetEnabled": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } // Enable or disable a specific hook const hookManager = provider.getHookManager() if (hookManager && message.hookId && typeof message.hookEnabled === "boolean") { @@ -3376,6 +3399,11 @@ export const webviewMessageHandler = async ( } case "hooksSetAllEnabled": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } // Enable or disable ALL currently known hooks. // This mirrors MCP's "Enable MCP Servers" top-level toggle. const hookManager = provider.getHookManager() @@ -3400,6 +3428,11 @@ export const webviewMessageHandler = async ( } case "hooksOpenConfigFolder": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } // Open the hooks configuration folder in VS Code const source = message.hooksSource ?? "project" try { @@ -3430,6 +3463,11 @@ export const webviewMessageHandler = async ( } case "hooksDeleteHook": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } const hookManager = provider.getHookManager() if (!hookManager || !message.hookId) { break @@ -3536,6 +3574,11 @@ export const webviewMessageHandler = async ( } case "hooksOpenHookFile": { + // Check if hooks experiment is enabled + const hooksExperimentsState = getGlobalState("experiments") ?? experimentDefault + if (!experiments.isEnabled(hooksExperimentsState, EXPERIMENT_IDS.HOOKS)) { + break + } const { filePath: hookFilePath } = message if (!hookFilePath) { return diff --git a/src/shared/__tests__/experiments.spec.ts b/src/shared/__tests__/experiments.spec.ts index 0b43302611..18a3f5a09b 100644 --- a/src/shared/__tests__/experiments.spec.ts +++ b/src/shared/__tests__/experiments.spec.ts @@ -33,6 +33,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + hooks: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) @@ -46,6 +47,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + hooks: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true) }) @@ -59,6 +61,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + hooks: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) diff --git a/src/shared/experiments.ts b/src/shared/experiments.ts index ad3aeca863..3e5f1a7ce2 100644 --- a/src/shared/experiments.ts +++ b/src/shared/experiments.ts @@ -8,6 +8,7 @@ export const EXPERIMENT_IDS = { RUN_SLASH_COMMAND: "runSlashCommand", MULTIPLE_NATIVE_TOOL_CALLS: "multipleNativeToolCalls", CUSTOM_TOOLS: "customTools", + HOOKS: "hooks", } as const satisfies Record type _AssertExperimentIds = AssertEqual>> @@ -26,6 +27,7 @@ export const experimentConfigsMap: Record = { RUN_SLASH_COMMAND: { enabled: false }, MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false }, CUSTOM_TOOLS: { enabled: false }, + HOOKS: { enabled: false }, } export const experimentDefault = Object.fromEntries( diff --git a/webview-ui/src/components/settings/SectionHeader.tsx b/webview-ui/src/components/settings/SectionHeader.tsx index 2b690f5711..bac0844099 100644 --- a/webview-ui/src/components/settings/SectionHeader.tsx +++ b/webview-ui/src/components/settings/SectionHeader.tsx @@ -9,7 +9,12 @@ type SectionHeaderProps = HTMLAttributes & { export const SectionHeader = ({ description, children, className, ...props }: SectionHeaderProps) => { return ( -
+

{children}

{description &&

{description}

}
diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 0c4e2fb09d..b7cd310531 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -521,8 +521,8 @@ const SettingsView = forwardRef(({ onDone, t } }, []) - const sections: { id: SectionName; icon: LucideIcon }[] = useMemo( - () => [ + const sections: { id: SectionName; icon: LucideIcon }[] = useMemo(() => { + const allSections: { id: SectionName; icon: LucideIcon }[] = [ { id: "providers", icon: Plug }, { id: "modes", icon: Users2 }, { id: "mcp", icon: Server }, @@ -539,9 +539,10 @@ const SettingsView = forwardRef(({ onDone, t { id: "experimental", icon: FlaskConical }, { id: "language", icon: Globe }, { id: "about", icon: Info }, - ], - [], // No dependencies needed now - ) + ] + // Filter out hooks section if the experiment is not enabled + return allSections.filter((section) => section.id !== "hooks" || experiments?.hooks === true) + }, [experiments?.hooks]) // Update target section logic to set active tab useEffect(() => { @@ -635,7 +636,7 @@ const SettingsView = forwardRef(({ onDone, t return ( - +