Skip to content

Conversation

@code-crusher
Copy link
Member

No description provided.

@matter-code-review
Copy link
Contributor

Summary By MatterAI MatterAI logo

🔄 What Changed

This PR introduces a new file editing review system with enhanced tool descriptions and workflow improvements. Key changes include adding a file edit review controller for managing pending file changes, updating system prompts with detailed tool descriptions, and implementing a TODO list management system. The changes also include improvements to file editing tools and enhanced user experience for reviewing code changes.

🔍 Impact of the Change

The introduction of the file edit review controller significantly improves the code review process by providing better visualization and management of pending file changes. The updated system prompts enhance tool usability and understanding. The new TODO list management system improves task tracking and completion workflows.

📁 Total Files Changed

File ChangeLog
mode.ts Added critical instructions for using update_todo_list tool and inline line numbers
presentAssistantMessage.ts Added fileEditTool handling in switch statement
system.ts Added detailed descriptions for file_edit and update_todo_list tools, updated tool documentation
index.ts Added updateTodoList tool import and export
read_file.ts Updated read_file tool description with line range requirements
search_files.ts Added file_pattern parameter to search_files tool
Task.ts Added FileEditReviewController integration and disposal handling
fileEditTool.ts Implemented file edit review controller integration and enhanced tool functionality
FileEditReviewController.ts Added new file for managing file edit reviews with VS Code decorations and commands
ChatRow.tsx Added tool content handling for file edit tool display
useOpenRouterModelProviders.ts Removed 68 lines of unused code

🧪 Test Added/Recommended

Added

N/A

Recommended

  • Add unit tests for FileEditReviewController to verify file edit management functionality
  • Add integration tests for fileEditTool to ensure proper file editing workflow
  • Add UI tests for ChatRow component to verify tool content display

🔒Security Vulnerabilities

N/A

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 PR Review is completed: Review of new file edit review functionality and todo list tool integration

Skipped files
  • README.md: Skipped file pattern
⬇️ Low Priority Suggestions (2)
src/integrations/editor/FileEditReviewController.ts

Location: src/integrations/editor/FileEditReviewController.ts (Lines 34-37)

🟡 Performance

Issue: The refreshDecorations method is called on multiple events (active editor change, visible editors change, document close) without any debouncing. This could cause excessive UI updates.

Fix: Add debouncing to prevent rapid successive calls to refreshDecorations.

Impact: Improves UI responsiveness and prevents unnecessary re-renders.

-  	private readonly disposables: vscode.Disposable[] = []
-  	private pendingEdits = new Map<string, PendingFileEdit>()
-  	private reviewQueue: string[] = []
-  
+  	private readonly disposables: vscode.Disposable[] = []
+  	private pendingEdits = new Map<string, PendingFileEdit>()
+  	private reviewQueue: string[] = []
+  	private decorationTimeout: NodeJS.Timeout | undefined = undefined
src/core/prompts/tools/native-tools/read_file.ts

Location: src/core/prompts/tools/native-tools/read_file.ts (Lines 8-8)

🟡 Documentation

Issue: The description contains a typo 'less thatn' instead of 'less than'.

Fix: Correct the typo in the description.

Impact: Improves documentation quality.

-  			"Read one or more files and return their contents with line numbers for diffing or discussion. Use line ranges always to keep reads efficient and combine related files when possible. You will always read less thatn 100 lines at a time",
+  			"Read one or more files and return their contents with line numbers for diffing or discussion. Use line ranges always to keep reads efficient and combine related files when possible. You will always read less than 100 lines at a time",

Comment on lines 20 to 23
file_pattern: {
type: ["string", "null"],
description: "Optional string glob to limit which files are searched (e.g., '*.ts')",
type: ["string"],
description: "String glob to limit which files are searched (e.g., '*.ts')",
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Type Safety

Issue: The file_pattern parameter type was changed from string to array, but the description still suggests it accepts a single string pattern like '*.ts'.

Fix: Either revert to string type or update the description and validation to handle array of patterns.

Impact: Prevents runtime errors when tools pass string patterns instead of arrays.

Suggested change
file_pattern: {
type: ["string", "null"],
description: "Optional string glob to limit which files are searched (e.g., '*.ts')",
type: ["string"],
description: "String glob to limit which files are searched (e.g., '*.ts')",
},
file_pattern: {
type: "string",
description: "String glob to limit which files are searched (e.g., '*.ts')",
},

Fix in Cursor

Comment on lines +79 to +86
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}

await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")
await vscode.workspace.openTextDocument(entry.absolutePath).then((doc) => doc.save())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Security

Issue: The handleUndo method writes to the file system using the absolute path directly without validating that the path is within the workspace, potentially allowing writes outside the intended directory.

Fix: Validate that the absolute path is within the cwd before writing.

Impact: Prevents potential path traversal attacks when undoing edits.

Suggested change
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}
await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")
await vscode.workspace.openTextDocument(entry.absolutePath).then((doc) => doc.save())
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}
// Validate path is within workspace
if (!entry.absolutePath.startsWith(path.resolve(this.cwd))) {
console.error('Attempted to write outside workspace:', entry.absolutePath)
return
}
await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")

Fix in Cursor

Comment on lines +140 to +146
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Error Handling

Issue: The fileEditTool doesn't handle the case where cline.fileEditReviewController might be undefined or null, which could cause runtime errors.

Fix: Add a null check before calling addEdit on the controller.

Impact: Prevents crashes when the review controller is not properly initialized.

Suggested change
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
if (cline.fileEditReviewController) {
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
}

Fix in Cursor

@code-crusher
Copy link
Member Author

/matter review-full

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 PR Review is completed: Identified critical security, type safety, and performance issues in the new FileEditReviewController and related tools. All previously reported issues remain unresolved and require immediate attention.

Skipped files
  • README.md: Skipped file pattern
⬇️ Low Priority Suggestions (5)
src/integrations/editor/FileEditReviewController.ts

Location: src/integrations/editor/FileEditReviewController.ts (Lines 34-37)

🟡 Performance

Issue: The refreshDecorations method is called on multiple events without debouncing, which could cause excessive UI updates and impact performance.

Fix: Add a decorationTimeout property to enable debouncing of decoration updates.

Impact: Improves UI responsiveness and reduces unnecessary re-renders.

-  	private readonly disposables: vscode.Disposable[] = []
-  	private pendingEdits = new Map<string, PendingFileEdit>()
-  	private reviewQueue: string[] = []
-  
+  	private readonly disposables: vscode.Disposable[] = []
+  	private pendingEdits = new Map<string, PendingFileEdit>()
+  	private reviewQueue: string[] = []
+  	private decorationTimeout: NodeJS.Timeout | undefined = undefined
src/integrations/editor/FileEditReviewController.ts

Location: src/integrations/editor/FileEditReviewController.ts (Lines 164-205)

🟡 Performance

Issue: The refreshDecorations method executes immediately on every editor change, which can cause performance issues with rapid updates.

Fix: Implement debouncing using the decorationTimeout property to limit update frequency.

Impact: Significantly reduces UI update overhead during rapid file operations.

-  	private refreshDecorations() {
-  		const hasPending = this.pendingEdits.size > 0
-  		for (const editor of vscode.window.visibleTextEditors) {
-  			const readableEditorPath = getReadablePath(this.cwd, path.relative(this.cwd, editor.document.uri.fsPath))
-  			const entry = this.pendingEdits.get(readableEditorPath)
-  
-  			const labelDecorations: vscode.DecorationOptions[] = []
-  			const highlightDecorations: vscode.DecorationOptions[] = []
-  
-  			if (entry) {
-  				const labelRange = new vscode.Range(editor.document.lineCount - 1, 0, editor.document.lineCount - 1, 0)
-  				labelDecorations.push({
-  					range: labelRange,
-  					hoverMessage: this.getOverlayMarkdown(entry),
-  					renderOptions: {
-  						after: {
-  							contentText: ` $(tools) Review changes`,
-  						},
-  					},
-  				})
-  
-  				highlightDecorations.push({
-  					range: entry.diffAnchor,
-  					hoverMessage: this.getOverlayMarkdown(entry),
-  				})
-  			} else if (hasPending) {
-  				const labelRange = new vscode.Range(editor.document.lineCount - 1, 0, editor.document.lineCount - 1, 0)
-  				labelDecorations.push({
-  					range: labelRange,
-  					hoverMessage: this.getOverlayMarkdown(),
-  					renderOptions: {
-  						after: {
-  							contentText: ` $(arrow-right) Review next file`,
-  						},
-  					},
-  				})
-  			}
-  
-  			editor.setDecorations(labelDecorationType, labelDecorations)
-  			editor.setDecorations(highlightDecorationType, highlightDecorations)
-  		}
-  	}
+  	private refreshDecorations() {
+  		if (this.decorationTimeout) {
+  			clearTimeout(this.decorationTimeout)
+  		}
+  		this.decorationTimeout = setTimeout(() => {
+  			const hasPending = this.pendingEdits.size > 0
+  			for (const editor of vscode.window.visibleTextEditors) {
+  				const readableEditorPath = getReadablePath(this.cwd, path.relative(this.cwd, editor.document.uri.fsPath))
+  				const entry = this.pendingEdits.get(readableEditorPath)
+  				const labelDecorations: vscode.DecorationOptions[] = []
+  				const highlightDecorations: vscode.DecorationOptions[] = []
+  				if (entry) {
+  					const labelRange = new vscode.Range(editor.document.lineCount - 1, 0, editor.document.lineCount - 1, 0)
+  					labelDecorations.push({
+  						range: labelRange,
+  						hoverMessage: this.getOverlayMarkdown(entry),
+  						renderOptions: {
+  							after: {
+  								contentText: ` $(tools) Review changes`,
+  							},
+  						},
+  					})
+  					highlightDecorations.push({
+  						range: entry.diffAnchor,
+  						hoverMessage: this.getOverlayMarkdown(entry),
+  					})
+  				} else if (hasPending) {
+  					const labelRange = new vscode.Range(editor.document.lineCount - 1, 0, editor.document.lineCount - 1, 0)
+  					labelDecorations.push({
+  						range: labelRange,
+  						hoverMessage: this.getOverlayMarkdown(),
+  						renderOptions: {
+  							after: {
+  								contentText: ` $(arrow-right) Review next file`,
+  							},
+  						},
+  					})
+  				}
+  				editor.setDecorations(labelDecorationType, labelDecorations)
+  				editor.setDecorations(highlightDecorationType, highlightDecorations)
+  			}
+  		}, 100)
+  	}
src/core/prompts/tools/native-tools/read_file.ts

Location: src/core/prompts/tools/native-tools/read_file.ts (Lines 8-8)

🔵 Documentation

Issue: The description contains a typo "thatn" instead of "than".

Fix: Correct the typo to improve documentation clarity.

Impact: Improves code documentation quality.

-  			"Read one or more files and return their contents with line numbers for diffing or discussion. Use line ranges always to keep reads efficient and combine related files when possible. You will always read less thatn 100 lines at a time",
+  			"Read one or more files and return their contents with line numbers for diffing or discussion. Use line ranges always to keep reads efficient and combine related files when possible. You will always read less than 100 lines at a time",
src/core/prompts/tools/native-tools/read_file.ts

Location: src/core/prompts/tools/native-tools/read_file.ts (Lines 26-26)

🔵 Documentation

Issue: The description contains duplicated word "about about".

Fix: Remove the duplication to improve readability.

Impact: Improves documentation clarity.

-  									"Always required line ranges to read (format: start-end). If you are unsure about about the what line numbers to query, you can perform a search on the file to determine line numbers. You will never read more than 100 lines at a time!",
+  					"Always required line ranges to read (format: start-end). If you are unsure about the what line numbers to query, you can perform a search on the file to determine line numbers. You will never read more than 100 lines at a time!",
packages/types/src/mode.ts

Location: packages/types/src/mode.ts (Lines 376-376)

🔵 Documentation

Issue: The comment contains "upto" which should be "up to".

Fix: Correct the spacing to follow standard English conventions.

Impact: Minor documentation improvement.

-  CRITICAL: For any task, small or big, you will always and always use the update_todo_list tool to create the TODO list, always keep is upto date with updates to the status and updating/editing the list as needed.
+  CRITICAL: For any task, small or big, you will always and always use the update_todo_list tool to create the TODO list, always keep is up to date with updates to the status and updating/editing the list as needed.

file_pattern: {
type: ["string", "null"],
description: "Optional string glob to limit which files are searched (e.g., '*.ts')",
type: ["string"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Type Safety

Issue: The file_pattern parameter type was changed to an array ["string"], but the description and usage pattern indicate it should be a single string. This is a breaking change that could cause runtime errors.

Fix: Revert the type back to "string" to match the description and expected usage.

Impact: Prevents API compatibility issues and maintains consistent tool behavior.

Suggested change
type: ["string"],
type: "string",

Fix in Cursor

Comment on lines +79 to +91
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}

await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")
await vscode.workspace.openTextDocument(entry.absolutePath).then((doc) => doc.save())

this.pendingEdits.delete(entry.readablePath)
this.reviewQueue = this.reviewQueue.filter((path) => path !== entry.readablePath)
this.refreshDecorations()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Security

Issue: The handleUndo method writes to the file system without validating that the path is within the workspace directory, creating a potential path traversal vulnerability.

Fix: Add path validation to ensure writes are restricted to the workspace directory.

Impact: Prevents potential security vulnerabilities and unauthorized file system access.

Suggested change
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}
await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")
await vscode.workspace.openTextDocument(entry.absolutePath).then((doc) => doc.save())
this.pendingEdits.delete(entry.readablePath)
this.reviewQueue = this.reviewQueue.filter((path) => path !== entry.readablePath)
this.refreshDecorations()
}
async handleUndo(relPath?: string) {
const entry = this.getEntryForPath(relPath)
if (!entry) {
return
}
if (!entry.absolutePath.startsWith(path.resolve(this.cwd))) {
console.error('Attempted to write outside workspace:', entry.absolutePath)
return
}
await fs.writeFile(entry.absolutePath, entry.originalContent, "utf-8")
await vscode.workspace.openTextDocument(entry.absolutePath).then((doc) => doc.save())
this.pendingEdits.delete(entry.readablePath)
this.reviewQueue = this.reviewQueue.filter((path) => path !== entry.readablePath)
this.refreshDecorations()
}

Fix in Cursor

Comment on lines +140 to +146
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Error Handling

Issue: The code calls cline.fileEditReviewController.addEdit() without checking if the controller is initialized, which could cause runtime errors.

Fix: Add a null check before calling addEdit to ensure the controller exists.

Impact: Prevents potential crashes and improves error resilience.

Suggested change
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
await cline.say("tool" as any, JSON.stringify(sayMessageProps))
if (cline.fileEditReviewController) {
cline.fileEditReviewController.addEdit({
relPath,
absolutePath,
originalContent,
newContent,
})
}

Fix in Cursor

@code-crusher code-crusher merged commit cf3b640 into main Nov 19, 2025
7 of 14 checks passed
@code-crusher code-crusher deleted the release/v4.118.0 branch November 19, 2025 08:16
@code-crusher
Copy link
Member Author

/matter release-notes

@matter-code-review
Copy link
Contributor

What's Changed

Summary

This release introduces significant enhancements to the file editing workflow with the addition of a file edit review controller, refined tool descriptions, and improved system prompts. It also includes critical updates to native tools and their integration within the assistant's messaging system.

New Features

  • File Edit Review Controller: A new FileEditReviewController has been added to manage and review pending file edits, allowing users to keep or undo changes directly from the editor with visual decorations and command integration.
  • Enhanced Tool Descriptions: The system prompts now include detailed descriptions and guidance for the file_edit and update_todo_list tools, improving clarity on when and how to use them effectively.
  • New Native Tool Integration: The update_todo_list tool has been integrated into the native tools index, enabling better task tracking and management within the assistant.
  • Improved File Edit Tool: The fileEditTool now supports direct saving with diagnostics and includes content truncation for previews, enhancing performance and usability.

Bug fixes

  • Tool Invocation Fix: Corrected the invocation of the fileEditTool within the assistant message handler to ensure proper execution and error handling.
  • Read File Tool Enhancement: Updated the read_file tool description to enforce line range limits and improve efficiency by combining related file reads.
  • Search Files Tool Update: Modified the search_files tool to include a file pattern parameter, allowing more precise file searches using glob patterns.

Commits

Dependencies added

  • vscode: Integrated extensively within FileEditReviewController for text editor decorations, command registration, and document handling.
  • fs/promises: Utilized in FileEditReviewController for asynchronous file system operations like writing original content back to files during undo operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants