Quellcode durchsuchen

feat: Allow escaping of context mentions (#4362)

* allow escaping of context mentions

* refactor: update comment

---------

Co-authored-by: cannuri <[email protected]>
Co-authored-by: Daniel <[email protected]>
KJ7LNW vor 6 Monaten
Ursprung
Commit
53e7e6beaa

+ 186 - 0
src/core/tools/__tests__/newTaskTool.test.ts

@@ -0,0 +1,186 @@
+import { jest } from "@jest/globals"
+import type { AskApproval, HandleError } from "../../../shared/tools" // Import the types
+
+// Mock dependencies before importing the module under test
+// Explicitly type the mock functions
+const mockAskApproval = jest.fn<AskApproval>()
+const mockHandleError = jest.fn<HandleError>() // Explicitly type HandleError
+const mockPushToolResult = jest.fn()
+const mockRemoveClosingTag = jest.fn((_name: string, value: string | undefined) => value ?? "") // Simple mock
+const mockGetModeBySlug = jest.fn()
+// Define a minimal type for the resolved value
+type MockClineInstance = { taskId: string }
+// Make initClineWithTask return a mock Cline-like object with taskId, providing type hint
+const mockInitClineWithTask = jest
+	.fn<() => Promise<MockClineInstance>>()
+	.mockResolvedValue({ taskId: "mock-subtask-id" })
+const mockEmit = jest.fn()
+const mockRecordToolError = jest.fn()
+const mockSayAndCreateMissingParamError = jest.fn()
+
+// Mock the Cline instance and its methods/properties
+const mockCline = {
+	ask: jest.fn(),
+	sayAndCreateMissingParamError: mockSayAndCreateMissingParamError,
+	emit: mockEmit,
+	recordToolError: mockRecordToolError,
+	consecutiveMistakeCount: 0,
+	isPaused: false,
+	pausedModeSlug: "ask", // Default or mock value
+	providerRef: {
+		deref: jest.fn(() => ({
+			getState: jest.fn(() => ({ customModes: [], mode: "ask" })), // Mock provider state
+			handleModeSwitch: jest.fn(),
+			initClineWithTask: mockInitClineWithTask,
+		})),
+	},
+}
+
+// Mock other modules
+jest.mock("delay", () => jest.fn(() => Promise.resolve())) // Mock delay to resolve immediately
+jest.mock("../../../shared/modes", () => ({
+	// Corrected path
+	getModeBySlug: mockGetModeBySlug,
+	defaultModeSlug: "ask",
+}))
+jest.mock("../../prompts/responses", () => ({
+	// Corrected path
+	formatResponse: {
+		toolError: jest.fn((msg: string) => `Tool Error: ${msg}`), // Simple mock
+	},
+}))
+
+// Import the function to test AFTER mocks are set up
+import { newTaskTool } from "../newTaskTool"
+import type { ToolUse } from "../../../shared/tools"
+
+describe("newTaskTool", () => {
+	beforeEach(() => {
+		// Reset mocks before each test
+		jest.clearAllMocks()
+		mockAskApproval.mockResolvedValue(true) // Default to approved
+		mockGetModeBySlug.mockReturnValue({ slug: "code", name: "Code Mode" }) // Default valid mode
+		mockCline.consecutiveMistakeCount = 0
+		mockCline.isPaused = false
+	})
+
+	it("should correctly un-escape \\\\@ to \\@ in the message passed to the new task", async () => {
+		const block: ToolUse = {
+			type: "tool_use", // Add required 'type' property
+			name: "new_task", // Correct property name
+			params: {
+				mode: "code",
+				message: "Review this: \\\\@file1.txt and also \\\\\\\\@file2.txt", // Input with \\@ and \\\\@
+			},
+			partial: false,
+		}
+
+		await newTaskTool(
+			mockCline as any, // Use 'as any' for simplicity in mocking complex type
+			block,
+			mockAskApproval, // Now correctly typed
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		// Verify askApproval was called
+		expect(mockAskApproval).toHaveBeenCalled()
+
+		// Verify the message passed to initClineWithTask reflects the code's behavior in unit tests
+		expect(mockInitClineWithTask).toHaveBeenCalledWith(
+			"Review this: \\@file1.txt and also \\\\\\@file2.txt", // Unit Test Expectation: \\@ -> \@, \\\\@ -> \\\\@
+			undefined,
+			mockCline,
+		)
+
+		// Verify side effects
+		expect(mockCline.emit).toHaveBeenCalledWith("taskSpawned", expect.any(String)) // Assuming initCline returns a mock task ID
+		expect(mockCline.isPaused).toBe(true)
+		expect(mockCline.emit).toHaveBeenCalledWith("taskPaused")
+		expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
+	})
+
+	it("should not un-escape single escaped \@", async () => {
+		const block: ToolUse = {
+			type: "tool_use", // Add required 'type' property
+			name: "new_task", // Correct property name
+			params: {
+				mode: "code",
+				message: "This is already unescaped: \\@file1.txt",
+			},
+			partial: false,
+		}
+
+		await newTaskTool(
+			mockCline as any,
+			block,
+			mockAskApproval, // Now correctly typed
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		expect(mockInitClineWithTask).toHaveBeenCalledWith(
+			"This is already unescaped: \\@file1.txt", // Expected: \@ remains \@
+			undefined,
+			mockCline,
+		)
+	})
+
+	it("should not un-escape non-escaped @", async () => {
+		const block: ToolUse = {
+			type: "tool_use", // Add required 'type' property
+			name: "new_task", // Correct property name
+			params: {
+				mode: "code",
+				message: "A normal mention @file1.txt",
+			},
+			partial: false,
+		}
+
+		await newTaskTool(
+			mockCline as any,
+			block,
+			mockAskApproval, // Now correctly typed
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		expect(mockInitClineWithTask).toHaveBeenCalledWith(
+			"A normal mention @file1.txt", // Expected: @ remains @
+			undefined,
+			mockCline,
+		)
+	})
+
+	it("should handle mixed escaping scenarios", async () => {
+		const block: ToolUse = {
+			type: "tool_use", // Add required 'type' property
+			name: "new_task", // Correct property name
+			params: {
+				mode: "code",
+				message: "Mix: @file0.txt, \\@file1.txt, \\\\@file2.txt, \\\\\\\\@file3.txt",
+			},
+			partial: false,
+		}
+
+		await newTaskTool(
+			mockCline as any,
+			block,
+			mockAskApproval, // Now correctly typed
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		expect(mockInitClineWithTask).toHaveBeenCalledWith(
+			"Mix: @file0.txt, \\@file1.txt, \\@file2.txt, \\\\\\@file3.txt", // Unit Test Expectation: @->@, \@->\@, \\@->\@, \\\\@->\\\\@
+			undefined,
+			mockCline,
+		)
+	})
+
+	// Add more tests for error handling (missing params, invalid mode, approval denied) if needed
+})

+ 5 - 2
src/core/tools/newTaskTool.ts

@@ -42,6 +42,9 @@ export async function newTaskTool(
 			}
 			}
 
 
 			cline.consecutiveMistakeCount = 0
 			cline.consecutiveMistakeCount = 0
+			// Un-escape one level of backslashes before '@' for hierarchical subtasks
+// Un-escape one level: \\@ -> \@ (removes one backslash for hierarchical subtasks)
+			const unescapedMessage = message.replace(/\\\\@/g, "\\@")
 
 
 			// Verify the mode exists
 			// Verify the mode exists
 			const targetMode = getModeBySlug(mode, (await cline.providerRef.deref()?.getState())?.customModes)
 			const targetMode = getModeBySlug(mode, (await cline.providerRef.deref()?.getState())?.customModes)
@@ -82,10 +85,10 @@ export async function newTaskTool(
 			// Delay to allow mode change to take effect before next tool is executed.
 			// Delay to allow mode change to take effect before next tool is executed.
 			await delay(500)
 			await delay(500)
 
 
-			const newCline = await provider.initClineWithTask(message, undefined, cline)
+			const newCline = await provider.initClineWithTask(unescapedMessage, undefined, cline)
 			cline.emit("taskSpawned", newCline.taskId)
 			cline.emit("taskSpawned", newCline.taskId)
 
 
-			pushToolResult(`Successfully created new task in ${targetMode.name} mode with message: ${message}`)
+			pushToolResult(`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage}`)
 
 
 			// Set the isPaused flag to true so the parent
 			// Set the isPaused flag to true so the parent
 			// task can wait for the sub-task to finish.
 			// task can wait for the sub-task to finish.

+ 8 - 1
src/shared/__tests__/context-mentions.test.ts

@@ -41,8 +41,15 @@ describe("mentionRegex and mentionRegexGlobal", () => {
 		{ input: "mention@", expected: null }, // Trailing @
 		{ input: "mention@", expected: null }, // Trailing @
 		{ input: "@/path/trailing\\", expected: null }, // Trailing backslash (invalid escape)
 		{ input: "@/path/trailing\\", expected: null }, // Trailing backslash (invalid escape)
 		{ input: "@/path/to/file\\not-a-space", expected: null }, // Backslash not followed by space
 		{ input: "@/path/to/file\\not-a-space", expected: null }, // Backslash not followed by space
+		// Escaped mentions (should not match due to negative lookbehind)
+		{ input: "This is not a mention: \\@/path/to/file.txt", expected: null },
+		{ input: "Escaped \\@problems word", expected: null },
+		{ input: "Text with \\@https://example.com", expected: null },
+		{ input: "Another \\@a1b2c3d hash", expected: null },
+		{ input: "Not escaped @terminal", expected: ["@terminal"] }, // Ensure non-escaped still works nearby
+		{ input: "Double escape \\\\@/should/match", expected: null }, // Double backslash escapes the backslash, currently incorrectly fails to match
+		{ input: "Text with \\@/escaped/path\\ with\\ spaces.txt", expected: null }, // Escaped mention with escaped spaces within the path part
 	]
 	]
-
 	testCases.forEach(({ input, expected }) => {
 	testCases.forEach(({ input, expected }) => {
 		it(`should handle input: "${input}"`, () => {
 		it(`should handle input: "${input}"`, () => {
 			// Test mentionRegex (first match)
 			// Test mentionRegex (first match)

+ 1 - 1
src/shared/context-mentions.ts

@@ -54,7 +54,7 @@ Mention regex:
 
 
 */
 */
 export const mentionRegex =
 export const mentionRegex =
-	/@((?:\/|\w+:\/\/)(?:[^\s\\]|\\ )+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
+	/(?<!\\)@((?:\/|\w+:\/\/)(?:[^\s\\]|\\ )+?|[a-f0-9]{7,40}\b|problems\b|git-changes\b|terminal\b)(?=[.,;:!?]?(?=[\s\r\n]|$))/
 export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g")
 export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g")
 
 
 export interface MentionSuggestion {
 export interface MentionSuggestion {