code review and small improvement

Author: liwilliam2021

src/core/prompts/instructions/__tests__/generate-rules.test.ts

@@ -116,22 +116,6 @@ describe("ruleTypeDefinitions", () => {
 		expect(result).toContain("If found, incorporate and improve upon their content")
 	})
 
-	it("should include instructions to open files after generation", () => {
-		const ruleInstructions: RuleInstruction[] = [ruleTypeDefinitions.general]
-		const options: RulesGenerationOptions = {
-			selectedRuleTypes: ["general"],
-			addToGitignore: false,
-			alwaysAllowWriteProtected: false,
-			includeCustomRules: false,
-			customRulesText: "",
-		}
-
-		const result = generateRulesInstructions(ruleInstructions, options)
-
-		expect(result).toContain("Open the generated files")
-		expect(result).toContain("in the editor for review after creation")
-	})
-
 	it("should include proper formatting instructions", () => {
 		const ruleInstructions: RuleInstruction[] = [ruleTypeDefinitions.general]
 		const options: RulesGenerationOptions = {

src/core/prompts/instructions/generate-rules.ts

@@ -49,11 +49,9 @@ ${ruleInstructions
 
 5. **Keep rules concise** - aim for 20 lines per file, focusing on the most important guidelines
 
-6. **Open the generated files** in the editor for review after creation
-
 ${
 	addToGitignore
-		? `7. **Add the generated files to .gitignore**:
+		? `6. **Add the generated files to .gitignore**:
    - After generating all rule files, add entries to .gitignore to prevent them from being committed
    - Add each generated file path to .gitignore (e.g., .roo/rules/coding-standards.md)
    - If .gitignore doesn't exist, create it

src/core/webview/webviewMessageHandler.ts

@@ -1864,51 +1864,23 @@ export const webviewMessageHandler = async (
 		case "generateRules":
 			// Generate rules for the current workspace by spawning a new task
 			try {
-				const workspacePath = getWorkspacePath()
-				if (!workspacePath) {
-					vscode.window.showErrorMessage("No workspace folder open. Please open a folder to generate rules.")
-					break
-				}
-
 				// Import the rules generation service
-				const { createRulesGenerationTaskMessage } = await import("../../services/rules/rulesGenerator")
-
-				// Get selected rule types and options from the message
-				const selectedRuleTypes = message.selectedRuleTypes || ["general"]
-				const addToGitignore = message.addToGitignore || false
-				const alwaysAllowWriteProtected = message.alwaysAllowWriteProtected || false
-				const apiConfigName = message.apiConfigName
-				const includeCustomRules = message.includeCustomRules || false
-				const customRulesText = message.customRulesText || ""
-
-				// Switch to the selected API config if provided
-				if (apiConfigName) {
-					const currentApiConfig = getGlobalState("currentApiConfigName")
-					if (apiConfigName !== currentApiConfig) {
-						await updateGlobalState("currentApiConfigName", apiConfigName)
-						await provider.activateProviderProfile({ name: apiConfigName })
-					}
-				}
-
-				// Create a comprehensive message for the rules generation task using existing analysis logic
-				const rulesGenerationMessage = await createRulesGenerationTaskMessage(
-					workspacePath,
-					selectedRuleTypes,
-					addToGitignore,
-					alwaysAllowWriteProtected,
-					includeCustomRules,
-					customRulesText,
+				const { handleGenerateRules } = await import("../../services/rules/rulesGenerator")
+
+				// Call the refactored function with all necessary parameters
+				await handleGenerateRules(
+					provider,
+					{
+						selectedRuleTypes: message.selectedRuleTypes,
+						addToGitignore: message.addToGitignore,
+						alwaysAllowWriteProtected: message.alwaysAllowWriteProtected,
+						apiConfigName: message.apiConfigName,
+						includeCustomRules: message.includeCustomRules,
+						customRulesText: message.customRulesText,
+					},
+					getGlobalState,
+					updateGlobalState,
 				)
-
-				// Spawn a new task in code mode to generate the rules
-				await provider.initClineWithTask(rulesGenerationMessage)
-
-				// Automatically navigate to the chat tab to show the new task
-				await provider.postMessageToWebview({
-					type: "action",
-					action: "switchTab",
-					tab: "chat",
-				})
 			} catch (error) {
 				// Show error message to user
 				const errorMessage = error instanceof Error ? error.message : String(error)

src/services/rules/__tests__/rulesGenerator.test.ts

@@ -1,13 +1,27 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
 import * as fs from "fs/promises"
 import * as path from "path"
-import { createRulesGenerationTaskMessage } from "../rulesGenerator"
+import * as vscode from "vscode"
+import { createRulesGenerationTaskMessage, handleGenerateRules } from "../rulesGenerator"
+import { ClineProvider } from "../../../core/webview/ClineProvider"
 
 // Mock fs module
 vi.mock("fs/promises", () => ({
 	mkdir: vi.fn(),
 }))
 
+// Mock vscode module
+vi.mock("vscode", () => ({
+	window: {
+		showErrorMessage: vi.fn(),
+	},
+}))
+
+// Mock getWorkspacePath
+vi.mock("../../../utils/path", () => ({
+	getWorkspacePath: vi.fn(),
+}))
+
 describe("rulesGenerator", () => {
 	const mockWorkspacePath = "/test/workspace"
 
@@ -178,4 +192,110 @@ describe("rulesGenerator", () => {
 			expect(message).toContain(".roo/rules-docs-extractor/documentation-rules.md")
 		})
 	})
+
+	describe("handleGenerateRules", () => {
+		let mockProvider: ClineProvider
+		let mockGetGlobalState: any
+		let mockUpdateGlobalState: any
+		let mockGetWorkspacePath: any
+
+		beforeEach(async () => {
+			// Mock provider
+			mockProvider = {
+				activateProviderProfile: vi.fn(),
+				initClineWithTask: vi.fn(),
+				postMessageToWebview: vi.fn(),
+			} as any
+
+			// Mock global state functions
+			mockGetGlobalState = vi.fn()
+			mockUpdateGlobalState = vi.fn()
+
+			// Import and mock getWorkspacePath
+			const pathModule = await import("../../../utils/path")
+			mockGetWorkspacePath = vi.spyOn(pathModule, "getWorkspacePath")
+			mockGetWorkspacePath.mockReturnValue(mockWorkspacePath)
+		})
+
+		it("should show error when no workspace is open", async () => {
+			mockGetWorkspacePath.mockReturnValue(undefined)
+
+			await handleGenerateRules(mockProvider, {}, mockGetGlobalState, mockUpdateGlobalState)
+
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
+				"No workspace folder open. Please open a folder to generate rules.",
+			)
+			expect(mockProvider.initClineWithTask).not.toHaveBeenCalled()
+		})
+
+		it("should switch API config when different from current", async () => {
+			mockGetGlobalState.mockReturnValue("current-config")
+
+			await handleGenerateRules(
+				mockProvider,
+				{ apiConfigName: "new-config" },
+				mockGetGlobalState,
+				mockUpdateGlobalState,
+			)
+
+			expect(mockUpdateGlobalState).toHaveBeenCalledWith("currentApiConfigName", "new-config")
+			expect(mockProvider.activateProviderProfile).toHaveBeenCalledWith({ name: "new-config" })
+		})
+
+		it("should not switch API config when same as current", async () => {
+			mockGetGlobalState.mockReturnValue("current-config")
+
+			await handleGenerateRules(
+				mockProvider,
+				{ apiConfigName: "current-config" },
+				mockGetGlobalState,
+				mockUpdateGlobalState,
+			)
+
+			expect(mockUpdateGlobalState).not.toHaveBeenCalled()
+			expect(mockProvider.activateProviderProfile).not.toHaveBeenCalled()
+		})
+
+		it("should create task and switch to chat tab", async () => {
+			await handleGenerateRules(
+				mockProvider,
+				{ selectedRuleTypes: ["general"] },
+				mockGetGlobalState,
+				mockUpdateGlobalState,
+			)
+
+			expect(mockProvider.initClineWithTask).toHaveBeenCalled()
+			expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
+				type: "action",
+				action: "switchTab",
+				tab: "chat",
+			})
+		})
+
+		it("should pass all options to createRulesGenerationTaskMessage", async () => {
+			const options = {
+				selectedRuleTypes: ["general", "code"],
+				addToGitignore: true,
+				alwaysAllowWriteProtected: true,
+				includeCustomRules: true,
+				customRulesText: "Custom rules text",
+			}
+
+			await handleGenerateRules(mockProvider, options, mockGetGlobalState, mockUpdateGlobalState)
+
+			// Verify the task was created with the correct message
+			expect(mockProvider.initClineWithTask).toHaveBeenCalled()
+			const taskMessage = vi.mocked(mockProvider.initClineWithTask).mock.calls[0][0]
+			expect(taskMessage).toContain("Custom rules text")
+		})
+
+		it("should use default values when options are not provided", async () => {
+			await handleGenerateRules(mockProvider, {}, mockGetGlobalState, mockUpdateGlobalState)
+
+			expect(mockProvider.initClineWithTask).toHaveBeenCalled()
+			// The default selectedRuleTypes should be ["general"]
+			const taskMessage = vi.mocked(mockProvider.initClineWithTask).mock.calls[0][0]
+			expect(taskMessage).toContain(".roo/rules/coding-standards.md")
+		})
+	})
 })

src/services/rules/rulesGenerator.ts

@@ -1,11 +1,15 @@
 import * as fs from "fs/promises"
 import * as path from "path"
+import * as vscode from "vscode"
+import { GlobalState } from "@roo-code/types"
 import {
 	generateRulesInstructions,
 	ruleTypeDefinitions,
 	RulesGenerationOptions,
 	RuleInstruction,
 } from "../../core/prompts/instructions/generate-rules"
+import { getWorkspacePath } from "../../utils/path"
+import { ClineProvider } from "../../core/webview/ClineProvider"
 
 /**
  * Creates a comprehensive task message for rules generation that can be used with initClineWithTask
@@ -55,3 +59,69 @@ export async function createRulesGenerationTaskMessage(
 
 	return generateRulesInstructions(ruleInstructions, options)
 }
+
+/**
+ * Options for generating rules
+ */
+export interface GenerateRulesOptions {
+	selectedRuleTypes?: string[]
+	addToGitignore?: boolean
+	alwaysAllowWriteProtected?: boolean
+	apiConfigName?: string
+	includeCustomRules?: boolean
+	customRulesText?: string
+}
+
+/**
+ * Handles the complete rules generation process including API config switching,
+ * task creation, and UI navigation
+ */
+export async function handleGenerateRules(
+	provider: ClineProvider,
+	options: GenerateRulesOptions,
+	getGlobalState: <K extends keyof GlobalState>(key: K) => GlobalState[K],
+	updateGlobalState: <K extends keyof GlobalState>(key: K, value: GlobalState[K]) => Promise<void>,
+): Promise<void> {
+	const workspacePath = getWorkspacePath()
+	if (!workspacePath) {
+		vscode.window.showErrorMessage("No workspace folder open. Please open a folder to generate rules.")
+		return
+	}
+
+	// Extract options with defaults
+	const selectedRuleTypes = options.selectedRuleTypes || ["general"]
+	const addToGitignore = options.addToGitignore || false
+	const alwaysAllowWriteProtected = options.alwaysAllowWriteProtected || false
+	const apiConfigName = options.apiConfigName
+	const includeCustomRules = options.includeCustomRules || false
+	const customRulesText = options.customRulesText || ""
+
+	// Switch to the selected API config if provided
+	if (apiConfigName) {
+		const currentApiConfig = getGlobalState("currentApiConfigName")
+		if (apiConfigName !== currentApiConfig) {
+			await updateGlobalState("currentApiConfigName", apiConfigName)
+			await provider.activateProviderProfile({ name: apiConfigName })
+		}
+	}
+
+	// Create a comprehensive message for the rules generation task
+	const rulesGenerationMessage = await createRulesGenerationTaskMessage(
+		workspacePath,
+		selectedRuleTypes,
+		addToGitignore,
+		alwaysAllowWriteProtected,
+		includeCustomRules,
+		customRulesText,
+	)
+
+	// Spawn a new task in code mode to generate the rules
+	await provider.initClineWithTask(rulesGenerationMessage)
+
+	// Automatically navigate to the chat tab to show the new task
+	await provider.postMessageToWebview({
+		type: "action",
+		action: "switchTab",
+		tab: "chat",
+	})
+}

webview-ui/src/components/settings/ExperimentalSettings.tsx

@@ -70,7 +70,7 @@ export const ExperimentalSettings = ({
 					})}
 			</Section>
 
-			<RulesSettings className="mt-6" hasUnsavedChanges={hasUnsavedChanges} />
+			<RulesSettings className="mt-3" hasUnsavedChanges={hasUnsavedChanges} />
 		</div>
 	)
 }