mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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.
This commit is contained in:
parent
08b41f2b9e
commit
6ebe2dad0a
13 changed files with 133 additions and 15 deletions
|
|
@ -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<typeof experimentsSchema>
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -540,9 +540,10 @@ export class Task extends EventEmitter<TaskEvents> 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)
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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<ExperimentId, boolean>),
|
||||
}
|
||||
|
||||
// Check if hooks experiment was just enabled
|
||||
const newExperiments = newValue as Record<ExperimentId, boolean>
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string, ExperimentId>
|
||||
|
||||
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
|
||||
|
|
@ -26,6 +27,7 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
|
|||
RUN_SLASH_COMMAND: { enabled: false },
|
||||
MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false },
|
||||
CUSTOM_TOOLS: { enabled: false },
|
||||
HOOKS: { enabled: false },
|
||||
}
|
||||
|
||||
export const experimentDefault = Object.fromEntries(
|
||||
|
|
|
|||
|
|
@ -9,7 +9,12 @@ type SectionHeaderProps = HTMLAttributes<HTMLDivElement> & {
|
|||
|
||||
export const SectionHeader = ({ description, children, className, ...props }: SectionHeaderProps) => {
|
||||
return (
|
||||
<div className={cn("sticky top-0 z-10 text-vscode-sideBar-foreground px-5 pt-6 pb-4", className)} {...props}>
|
||||
<div
|
||||
className={cn(
|
||||
"sticky top-0 z-10 text-vscode-sideBar-foreground px-5 pt-6 pb-4 bg-vscode-sideBar-background/70 backdrop-blur-sm",
|
||||
className,
|
||||
)}
|
||||
{...props}>
|
||||
<h3 className="text-[1.25em] font-semibold text-vscode-foreground m-0">{children}</h3>
|
||||
{description && <p className="text-vscode-descriptionForeground text-sm mt-2 mb-0">{description}</p>}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -521,8 +521,8 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ 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<SettingsViewRef, SettingsViewProps>(({ 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<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
|
||||
return (
|
||||
<Tab>
|
||||
<TabHeader className="flex justify-between items-center gap-2">
|
||||
<TabHeader className="flex justify-between items-center gap-2 bg-vscode-editor-background/95 backdrop-blur-sm sticky top-0 z-10">
|
||||
<div className="flex items-center gap-2 grow">
|
||||
<StandardTooltip content={t("settings:header.doneButtonTooltip")}>
|
||||
<Button variant="ghost" className="px-1.5 -ml-2" onClick={() => checkUnsaveChanges(onDone)}>
|
||||
|
|
@ -884,8 +885,8 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
{/* MCP Section */}
|
||||
{renderTab === "mcp" && <McpView />}
|
||||
|
||||
{/* Hooks Section */}
|
||||
{renderTab === "hooks" && <HooksSettings />}
|
||||
{/* Hooks Section - only render if experiment is enabled */}
|
||||
{renderTab === "hooks" && experiments?.hooks === true && <HooksSettings />}
|
||||
|
||||
{/* Prompts Section */}
|
||||
{renderTab === "prompts" && (
|
||||
|
|
|
|||
|
|
@ -245,6 +245,7 @@ describe("mergeExtensionState", () => {
|
|||
nativeToolCalling: false,
|
||||
multipleNativeToolCalls: false,
|
||||
customTools: false,
|
||||
hooks: false,
|
||||
} as Record<ExperimentId, boolean>,
|
||||
checkpointTimeout: DEFAULT_CHECKPOINT_TIMEOUT_SECONDS + 5,
|
||||
}
|
||||
|
|
@ -269,6 +270,7 @@ describe("mergeExtensionState", () => {
|
|||
nativeToolCalling: false,
|
||||
multipleNativeToolCalls: false,
|
||||
customTools: false,
|
||||
hooks: false,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -900,6 +900,10 @@
|
|||
"refreshSuccess": "Tools refreshed successfully",
|
||||
"refreshError": "Failed to refresh tools",
|
||||
"toolParameters": "Parameters"
|
||||
},
|
||||
"HOOKS": {
|
||||
"name": "Enable Hooks",
|
||||
"description": "Use custom shell commands to automate actions before or after tool execution."
|
||||
}
|
||||
},
|
||||
"promptCaching": {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue