ソースを参照

Merge pull request #992 from RooVetGit/new_mode_role_validation

Validate custom modes schema before creation from the UI
Matt Rubens 10 ヶ月 前
コミット
c815ea72a8

+ 7 - 0
src/__mocks__/get-folder-size.js

@@ -4,3 +4,10 @@ module.exports = async function getFolderSize() {
 		errors: [],
 		errors: [],
 	}
 	}
 }
 }
+
+module.exports.loose = async function getFolderSizeLoose() {
+	return {
+		size: 1000,
+		errors: [],
+	}
+}

+ 0 - 2
src/core/config/CustomModesManager.ts

@@ -62,8 +62,6 @@ export class CustomModesManager {
 			const settings = JSON.parse(content)
 			const settings = JSON.parse(content)
 			const result = CustomModesSettingsSchema.safeParse(settings)
 			const result = CustomModesSettingsSchema.safeParse(settings)
 			if (!result.success) {
 			if (!result.success) {
-				const errorMsg = `Schema validation failed for ${filePath}`
-				console.error(`[CustomModesManager] ${errorMsg}:`, result.error)
 				return []
 				return []
 			}
 			}
 
 

+ 13 - 16
src/core/config/CustomModesSchema.ts

@@ -29,22 +29,19 @@ const GroupOptionsSchema = z.object({
 const GroupEntrySchema = z.union([ToolGroupSchema, z.tuple([ToolGroupSchema, GroupOptionsSchema])])
 const GroupEntrySchema = z.union([ToolGroupSchema, z.tuple([ToolGroupSchema, GroupOptionsSchema])])
 
 
 // Schema for array of groups
 // Schema for array of groups
-const GroupsArraySchema = z
-	.array(GroupEntrySchema)
-	.min(1, "At least one tool group is required")
-	.refine(
-		(groups) => {
-			const seen = new Set()
-			return groups.every((group) => {
-				// For tuples, check the group name (first element)
-				const groupName = Array.isArray(group) ? group[0] : group
-				if (seen.has(groupName)) return false
-				seen.add(groupName)
-				return true
-			})
-		},
-		{ message: "Duplicate groups are not allowed" },
-	)
+const GroupsArraySchema = z.array(GroupEntrySchema).refine(
+	(groups) => {
+		const seen = new Set()
+		return groups.every((group) => {
+			// For tuples, check the group name (first element)
+			const groupName = Array.isArray(group) ? group[0] : group
+			if (seen.has(groupName)) return false
+			seen.add(groupName)
+			return true
+		})
+	},
+	{ message: "Duplicate groups are not allowed" },
+)
 
 
 // Schema for mode configuration
 // Schema for mode configuration
 export const CustomModeSchema = z.object({
 export const CustomModeSchema = z.object({

+ 0 - 22
src/core/config/__tests__/CustomModesSchema.test.ts

@@ -95,17 +95,6 @@ describe("CustomModeSchema", () => {
 			expect(() => validateCustomMode(invalidGroupMode)).toThrow(ZodError)
 			expect(() => validateCustomMode(invalidGroupMode)).toThrow(ZodError)
 		})
 		})
 
 
-		test("rejects empty groups array", () => {
-			const invalidMode = {
-				slug: "123e4567-e89b-12d3-a456-426614174000",
-				name: "Test Mode",
-				roleDefinition: "Test role definition",
-				groups: [] as const,
-			} satisfies ModeConfig
-
-			expect(() => validateCustomMode(invalidMode)).toThrow("At least one tool group is required")
-		})
-
 		test("handles null and undefined gracefully", () => {
 		test("handles null and undefined gracefully", () => {
 			expect(() => validateCustomMode(null)).toThrow(ZodError)
 			expect(() => validateCustomMode(null)).toThrow(ZodError)
 			expect(() => validateCustomMode(undefined)).toThrow(ZodError)
 			expect(() => validateCustomMode(undefined)).toThrow(ZodError)
@@ -179,16 +168,5 @@ describe("CustomModeSchema", () => {
 
 
 			expect(() => CustomModeSchema.parse(modeWithDuplicates)).toThrow(/Duplicate groups/)
 			expect(() => CustomModeSchema.parse(modeWithDuplicates)).toThrow(/Duplicate groups/)
 		})
 		})
-
-		it("requires at least one group", () => {
-			const modeWithNoGroups = {
-				slug: "test",
-				name: "Test",
-				roleDefinition: "Test",
-				groups: [],
-			}
-
-			expect(() => CustomModeSchema.parse(modeWithNoGroups)).toThrow(/At least one tool group is required/)
-		})
 	})
 	})
 })
 })

+ 0 - 9
src/core/config/__tests__/GroupConfigSchema.test.ts

@@ -45,15 +45,6 @@ describe("GroupConfigSchema", () => {
 			expect(() => CustomModeSchema.parse(mode)).toThrow()
 			expect(() => CustomModeSchema.parse(mode)).toThrow()
 		})
 		})
 
 
-		test("rejects empty groups array", () => {
-			const mode = {
-				...validBaseMode,
-				groups: [] as const,
-			} satisfies ModeConfig
-
-			expect(() => CustomModeSchema.parse(mode)).toThrow("At least one tool group is required")
-		})
-
 		test("rejects invalid group names", () => {
 		test("rejects invalid group names", () => {
 			const mode = {
 			const mode = {
 				...validBaseMode,
 				...validBaseMode,

+ 25 - 21
src/core/webview/__tests__/ClineProvider.test.ts

@@ -246,7 +246,7 @@ describe("ClineProvider", () => {
 		// Mock CustomModesManager
 		// Mock CustomModesManager
 		const mockCustomModesManager = {
 		const mockCustomModesManager = {
 			updateCustomMode: jest.fn().mockResolvedValue(undefined),
 			updateCustomMode: jest.fn().mockResolvedValue(undefined),
-			getCustomModes: jest.fn().mockResolvedValue({}),
+			getCustomModes: jest.fn().mockResolvedValue({ customModes: [] }),
 			dispose: jest.fn(),
 			dispose: jest.fn(),
 		}
 		}
 
 
@@ -1049,7 +1049,7 @@ describe("ClineProvider", () => {
 				"900x600", // browserViewportSize
 				"900x600", // browserViewportSize
 				"code", // mode
 				"code", // mode
 				{}, // customModePrompts
 				{}, // customModePrompts
-				{}, // customModes
+				{ customModes: [] }, // customModes
 				undefined, // effectiveInstructions
 				undefined, // effectiveInstructions
 				undefined, // preferredLanguage
 				undefined, // preferredLanguage
 				true, // diffEnabled
 				true, // diffEnabled
@@ -1102,7 +1102,7 @@ describe("ClineProvider", () => {
 				"900x600", // browserViewportSize
 				"900x600", // browserViewportSize
 				"code", // mode
 				"code", // mode
 				{}, // customModePrompts
 				{}, // customModePrompts
-				{}, // customModes
+				{ customModes: [] }, // customModes
 				undefined, // effectiveInstructions
 				undefined, // effectiveInstructions
 				undefined, // preferredLanguage
 				undefined, // preferredLanguage
 				false, // diffEnabled
 				false, // diffEnabled
@@ -1220,12 +1220,14 @@ describe("ClineProvider", () => {
 			provider.customModesManager = {
 			provider.customModesManager = {
 				updateCustomMode: jest.fn().mockResolvedValue(undefined),
 				updateCustomMode: jest.fn().mockResolvedValue(undefined),
 				getCustomModes: jest.fn().mockResolvedValue({
 				getCustomModes: jest.fn().mockResolvedValue({
-					"test-mode": {
-						slug: "test-mode",
-						name: "Test Mode",
-						roleDefinition: "Updated role definition",
-						groups: ["read"] as const,
-					},
+					customModes: [
+						{
+							slug: "test-mode",
+							name: "Test Mode",
+							roleDefinition: "Updated role definition",
+							groups: ["read"] as const,
+						},
+					],
 				}),
 				}),
 				dispose: jest.fn(),
 				dispose: jest.fn(),
 			} as any
 			} as any
@@ -1251,27 +1253,29 @@ describe("ClineProvider", () => {
 			)
 			)
 
 
 			// Verify state was updated
 			// Verify state was updated
-			expect(mockContext.globalState.update).toHaveBeenCalledWith(
-				"customModes",
-				expect.objectContaining({
-					"test-mode": expect.objectContaining({
+			expect(mockContext.globalState.update).toHaveBeenCalledWith("customModes", {
+				customModes: [
+					expect.objectContaining({
 						slug: "test-mode",
 						slug: "test-mode",
 						roleDefinition: "Updated role definition",
 						roleDefinition: "Updated role definition",
 					}),
 					}),
-				}),
-			)
+				],
+			})
 
 
 			// Verify state was posted to webview
 			// Verify state was posted to webview
+			// Verify state was posted to webview with correct format
 			expect(mockPostMessage).toHaveBeenCalledWith(
 			expect(mockPostMessage).toHaveBeenCalledWith(
 				expect.objectContaining({
 				expect.objectContaining({
 					type: "state",
 					type: "state",
 					state: expect.objectContaining({
 					state: expect.objectContaining({
-						customModes: expect.objectContaining({
-							"test-mode": expect.objectContaining({
-								slug: "test-mode",
-								roleDefinition: "Updated role definition",
-							}),
-						}),
+						customModes: {
+							customModes: [
+								expect.objectContaining({
+									slug: "test-mode",
+									roleDefinition: "Updated role definition",
+								}),
+							],
+						},
 					}),
 					}),
 				}),
 				}),
 			)
 			)

+ 79 - 20
webview-ui/src/components/prompts/PromptsView.tsx

@@ -19,6 +19,7 @@ import {
 	ModeConfig,
 	ModeConfig,
 	GroupEntry,
 	GroupEntry,
 } from "../../../../src/shared/modes"
 } from "../../../../src/shared/modes"
+import { CustomModeSchema } from "../../../../src/core/config/CustomModesSchema"
 import {
 import {
 	supportPrompt,
 	supportPrompt,
 	SupportPromptType,
 	SupportPromptType,
@@ -157,15 +158,34 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 	const [newModeGroups, setNewModeGroups] = useState<GroupEntry[]>(availableGroups)
 	const [newModeGroups, setNewModeGroups] = useState<GroupEntry[]>(availableGroups)
 	const [newModeSource, setNewModeSource] = useState<ModeSource>("global")
 	const [newModeSource, setNewModeSource] = useState<ModeSource>("global")
 
 
+	// Field-specific error states
+	const [nameError, setNameError] = useState<string>("")
+	const [slugError, setSlugError] = useState<string>("")
+	const [roleDefinitionError, setRoleDefinitionError] = useState<string>("")
+	const [groupsError, setGroupsError] = useState<string>("")
+
+	// Helper to reset form state
+	const resetFormState = useCallback(() => {
+		// Reset form fields
+		setNewModeName("")
+		setNewModeSlug("")
+		setNewModeGroups(availableGroups)
+		setNewModeRoleDefinition("")
+		setNewModeCustomInstructions("")
+		setNewModeSource("global")
+		// Reset error states
+		setNameError("")
+		setSlugError("")
+		setRoleDefinitionError("")
+		setGroupsError("")
+	}, [])
+
 	// Reset form fields when dialog opens
 	// Reset form fields when dialog opens
 	useEffect(() => {
 	useEffect(() => {
 		if (isCreateModeDialogOpen) {
 		if (isCreateModeDialogOpen) {
-			setNewModeGroups(availableGroups)
-			setNewModeRoleDefinition("")
-			setNewModeCustomInstructions("")
-			setNewModeSource("global")
+			resetFormState()
 		}
 		}
-	}, [isCreateModeDialogOpen])
+	}, [isCreateModeDialogOpen, resetFormState])
 
 
 	// Helper function to generate a unique slug from a name
 	// Helper function to generate a unique slug from a name
 	const generateSlug = useCallback((name: string, attempt = 0): string => {
 	const generateSlug = useCallback((name: string, attempt = 0): string => {
@@ -186,26 +206,52 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 	)
 	)
 
 
 	const handleCreateMode = useCallback(() => {
 	const handleCreateMode = useCallback(() => {
-		if (!newModeName.trim() || !newModeSlug.trim()) return
+		// Clear previous errors
+		setNameError("")
+		setSlugError("")
+		setRoleDefinitionError("")
+		setGroupsError("")
 
 
 		const source = newModeSource
 		const source = newModeSource
 		const newMode: ModeConfig = {
 		const newMode: ModeConfig = {
 			slug: newModeSlug,
 			slug: newModeSlug,
 			name: newModeName,
 			name: newModeName,
-			roleDefinition: newModeRoleDefinition.trim() || "",
+			roleDefinition: newModeRoleDefinition.trim(),
 			customInstructions: newModeCustomInstructions.trim() || undefined,
 			customInstructions: newModeCustomInstructions.trim() || undefined,
 			groups: newModeGroups,
 			groups: newModeGroups,
 			source,
 			source,
 		}
 		}
+
+		// Validate the mode against the schema
+		const result = CustomModeSchema.safeParse(newMode)
+		if (!result.success) {
+			// Map Zod errors to specific fields
+			result.error.errors.forEach((error) => {
+				const field = error.path[0] as string
+				const message = error.message
+
+				switch (field) {
+					case "name":
+						setNameError(message)
+						break
+					case "slug":
+						setSlugError(message)
+						break
+					case "roleDefinition":
+						setRoleDefinitionError(message)
+						break
+					case "groups":
+						setGroupsError(message)
+						break
+				}
+			})
+			return
+		}
+
 		updateCustomMode(newModeSlug, newMode)
 		updateCustomMode(newModeSlug, newMode)
 		switchMode(newModeSlug)
 		switchMode(newModeSlug)
 		setIsCreateModeDialogOpen(false)
 		setIsCreateModeDialogOpen(false)
-		setNewModeName("")
-		setNewModeSlug("")
-		setNewModeRoleDefinition("")
-		setNewModeCustomInstructions("")
-		setNewModeGroups(availableGroups)
-		setNewModeSource("global")
+		resetFormState()
 		// eslint-disable-next-line react-hooks/exhaustive-deps
 		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [
 	}, [
 		newModeName,
 		newModeName,
@@ -431,7 +477,7 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 
 
 				<div className="mt-5">
 				<div className="mt-5">
 					<div onClick={(e) => e.stopPropagation()} className="flex justify-between items-center mb-3">
 					<div onClick={(e) => e.stopPropagation()} className="flex justify-between items-center mb-3">
-						<h3 className="text-vscode-foreground m-0">Mode-Specific Prompts</h3>
+						<h3 className="text-vscode-foreground m-0">Modes</h3>
 						<div className="flex gap-2">
 						<div className="flex gap-2">
 							<VSCodeButton appearance="icon" onClick={openCreateModeDialog} title="Create new mode">
 							<VSCodeButton appearance="icon" onClick={openCreateModeDialog} title="Create new mode">
 								<span className="codicon codicon-add"></span>
 								<span className="codicon codicon-add"></span>
@@ -727,7 +773,7 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 								alignItems: "center",
 								alignItems: "center",
 								marginBottom: "4px",
 								marginBottom: "4px",
 							}}>
 							}}>
-							<div style={{ fontWeight: "bold" }}>Mode-specific Custom Instructions</div>
+							<div style={{ fontWeight: "bold" }}>Mode-specific Custom Instructions (optional)</div>
 							{!findModeBySlug(mode, customModes) && (
 							{!findModeBySlug(mode, customModes) && (
 								<VSCodeButton
 								<VSCodeButton
 									appearance="icon"
 									appearance="icon"
@@ -1069,6 +1115,9 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 									}}
 									}}
 									style={{ width: "100%" }}
 									style={{ width: "100%" }}
 								/>
 								/>
+								{nameError && (
+									<div className="text-xs text-vscode-errorForeground mt-1">{nameError}</div>
+								)}
 							</div>
 							</div>
 							<div style={{ marginBottom: "16px" }}>
 							<div style={{ marginBottom: "16px" }}>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Slug</div>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Slug</div>
@@ -1091,6 +1140,9 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 									The slug is used in URLs and file names. It should be lowercase and contain only
 									The slug is used in URLs and file names. It should be lowercase and contain only
 									letters, numbers, and hyphens.
 									letters, numbers, and hyphens.
 								</div>
 								</div>
+								{slugError && (
+									<div className="text-xs text-vscode-errorForeground mt-1">{slugError}</div>
+								)}
 							</div>
 							</div>
 							<div style={{ marginBottom: "16px" }}>
 							<div style={{ marginBottom: "16px" }}>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Save Location</div>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Save Location</div>
@@ -1147,6 +1199,11 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 									resize="vertical"
 									resize="vertical"
 									style={{ width: "100%" }}
 									style={{ width: "100%" }}
 								/>
 								/>
+								{roleDefinitionError && (
+									<div className="text-xs text-vscode-errorForeground mt-1">
+										{roleDefinitionError}
+									</div>
+								)}
 							</div>
 							</div>
 							<div style={{ marginBottom: "16px" }}>
 							<div style={{ marginBottom: "16px" }}>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Available Tools</div>
 								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Available Tools</div>
@@ -1184,9 +1241,14 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 										</VSCodeCheckbox>
 										</VSCodeCheckbox>
 									))}
 									))}
 								</div>
 								</div>
+								{groupsError && (
+									<div className="text-xs text-vscode-errorForeground mt-1">{groupsError}</div>
+								)}
 							</div>
 							</div>
 							<div style={{ marginBottom: "16px" }}>
 							<div style={{ marginBottom: "16px" }}>
-								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>Custom Instructions</div>
+								<div style={{ fontWeight: "bold", marginBottom: "4px" }}>
+									Custom Instructions (optional)
+								</div>
 								<div
 								<div
 									style={{
 									style={{
 										fontSize: "13px",
 										fontSize: "13px",
@@ -1219,10 +1281,7 @@ const PromptsView = ({ onDone }: PromptsViewProps) => {
 								backgroundColor: "var(--vscode-editor-background)",
 								backgroundColor: "var(--vscode-editor-background)",
 							}}>
 							}}>
 							<VSCodeButton onClick={() => setIsCreateModeDialogOpen(false)}>Cancel</VSCodeButton>
 							<VSCodeButton onClick={() => setIsCreateModeDialogOpen(false)}>Cancel</VSCodeButton>
-							<VSCodeButton
-								appearance="primary"
-								onClick={handleCreateMode}
-								disabled={!newModeName.trim() || !newModeSlug.trim()}>
+							<VSCodeButton appearance="primary" onClick={handleCreateMode}>
 								Create Mode
 								Create Mode
 							</VSCodeButton>
 							</VSCodeButton>
 						</div>
 						</div>

+ 1 - 0
webview-ui/src/index.css

@@ -84,6 +84,7 @@
 	--color-vscode-notifications-background: var(--vscode-notifications-background);
 	--color-vscode-notifications-background: var(--vscode-notifications-background);
 	--color-vscode-notifications-border: var(--vscode-notifications-border);
 	--color-vscode-notifications-border: var(--vscode-notifications-border);
 	--color-vscode-descriptionForeground: var(--vscode-descriptionForeground);
 	--color-vscode-descriptionForeground: var(--vscode-descriptionForeground);
+	--color-vscode-errorForeground: var(--vscode-errorForeground);
 }
 }
 
 
 @layer base {
 @layer base {