Merge remote-tracking branch 'upstream/main' into see-all-task-settings

Author: omagoduck

CHANGELOG.md

@@ -1,5 +1,27 @@
 # Roo Code Changelog
 
+## [3.23.5] - 2025-07-09
+
+- Fix: use decodeURIComponent in openFile (thanks @vivekfyi!)
+- Fix(embeddings): Translate error messages before sending to UI (thanks @daniel-lxs!)
+- Make account tab visible
+
+## [3.23.4] - 2025-07-09
+
+- Update chat area icons for better discoverability & consistency
+- Fix a bug that allowed `list_files` to return directory results that should be excluded by .gitignore
+- Add an overflow header menu to make the UI a little tidier (thanks @dlab-anton)
+- Fix a bug the issue where null custom modes configuration files cause a 'Cannot read properties of null' error (thanks @daniel-lxs!)
+- Replace native title attributes with StandardTooltip component for consistency (thanks @daniel-lxs!)
+
+## [3.23.3] - 2025-07-09
+
+- Remove erroneous line from announcement modal
+
+## [3.23.2] - 2025-07-09
+
+- Fix bug where auto-approval was intermittently failing
+
 ## [3.23.1] - 2025-07-09
 
 - Always show the code indexing dot under the chat text area

src/activate/registerCommands.ts

@@ -14,6 +14,7 @@ import { registerHumanRelayCallback, unregisterHumanRelayCallback, handleHumanRe
 import { handleNewTask } from "./handleTask"
 import { CodeIndexManager } from "../services/code-index/manager"
 import { importSettingsWithFeedback } from "../core/config/importExport"
+import { MdmService } from "../services/mdm/MdmService"
 import { t } from "../i18n"
 
 /**
@@ -226,7 +227,17 @@ export const openClineInNewTab = async ({ context, outputChannel }: Omit<Registe
 	// https://github.com/microsoft/vscode-extension-samples/blob/main/webview-sample/src/extension.ts
 	const contextProxy = await ContextProxy.getInstance(context)
 	const codeIndexManager = CodeIndexManager.getInstance(context)
-	const tabProvider = new ClineProvider(context, outputChannel, "editor", contextProxy, codeIndexManager)
+
+	// Get the existing MDM service instance to ensure consistent policy enforcement
+	let mdmService: MdmService | undefined
+	try {
+		mdmService = MdmService.getInstance()
+	} catch (error) {
+		// MDM service not initialized, which is fine - extension can work without it
+		mdmService = undefined
+	}
+
+	const tabProvider = new ClineProvider(context, outputChannel, "editor", contextProxy, codeIndexManager, mdmService)
 	const lastCol = Math.max(...vscode.window.visibleTextEditors.map((editor) => editor.viewColumn || 0))
 
 	// Check if there are any visible text editors, otherwise open a new group

src/core/config/CustomModesManager.ts

@@ -148,7 +148,9 @@ export class CustomModesManager {
 		cleanedContent = this.cleanInvisibleCharacters(cleanedContent)
 
 		try {
-			return yaml.parse(cleanedContent)
+			const parsed = yaml.parse(cleanedContent)
+			// Ensure we never return null or undefined
+			return parsed ?? {}
 		} catch (yamlError) {
 			// For .roomodes files, try JSON as fallback
 			if (filePath.endsWith(ROOMODES_FILENAME)) {
@@ -180,6 +182,12 @@ export class CustomModesManager {
 		try {
 			const content = await fs.readFile(filePath, "utf-8")
 			const settings = this.parseYamlSafely(content, filePath)
+
+			// Ensure settings has customModes property
+			if (!settings || typeof settings !== "object" || !settings.customModes) {
+				return []
+			}
+
 			const result = customModesSettingsSchema.safeParse(settings)
 
 			if (!result.success) {
@@ -458,7 +466,15 @@ export class CustomModesManager {
 			settings = { customModes: [] }
 		}
 
-		settings.customModes = operation(settings.customModes || [])
+		// Ensure settings is an object and has customModes property
+		if (!settings || typeof settings !== "object") {
+			settings = { customModes: [] }
+		}
+		if (!settings.customModes) {
+			settings.customModes = []
+		}
+
+		settings.customModes = operation(settings.customModes)
 		await fs.writeFile(filePath, yaml.stringify(settings, { lineWidth: 0 }), "utf-8")
 	}
 

src/integrations/misc/__tests__/open-file.spec.ts

@@ -0,0 +1,240 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
+import * as vscode from "vscode"
+import * as path from "path"
+import * as os from "os"
+import { openFile } from "../open-file"
+
+// Mock vscode module
+vi.mock("vscode", () => ({
+	Uri: {
+		file: vi.fn((path: string) => ({ fsPath: path })),
+	},
+	workspace: {
+		fs: {
+			stat: vi.fn(),
+			writeFile: vi.fn(),
+		},
+		openTextDocument: vi.fn(),
+	},
+	window: {
+		showTextDocument: vi.fn(),
+		showErrorMessage: vi.fn(),
+		tabGroups: {
+			all: [],
+		},
+		activeTextEditor: undefined,
+	},
+	commands: {
+		executeCommand: vi.fn(),
+	},
+	FileType: {
+		Directory: 2,
+		File: 1,
+	},
+	Selection: vi.fn((startLine: number, startChar: number, endLine: number, endChar: number) => ({
+		start: { line: startLine, character: startChar },
+		end: { line: endLine, character: endChar },
+	})),
+	TabInputText: vi.fn(),
+}))
+
+// Mock utils
+vi.mock("../../utils/path", () => {
+	const nodePath = require("path")
+	return {
+		arePathsEqual: vi.fn((a: string, b: string) => a === b),
+		getWorkspacePath: vi.fn(() => {
+			// In tests, we need to return a consistent workspace path
+			// The actual workspace is /Users/roocode/rc2 in local, but varies in CI
+			const cwd = process.cwd()
+			// If we're in the src directory, go up one level to get workspace root
+			if (cwd.endsWith("/src")) {
+				return nodePath.dirname(cwd)
+			}
+			return cwd
+		}),
+	}
+})
+
+// Mock i18n
+vi.mock("../../i18n", () => ({
+	t: vi.fn((key: string, params?: any) => {
+		// Return the key without namespace prefix to match actual behavior
+		if (key.startsWith("common:")) {
+			return key.replace("common:", "")
+		}
+		return key
+	}),
+}))
+
+describe("openFile", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+		vi.spyOn(console, "warn").mockImplementation(() => {})
+	})
+
+	afterEach(() => {
+		vi.restoreAllMocks()
+	})
+
+	describe("decodeURIComponent error handling", () => {
+		it("should handle invalid URI encoding gracefully", async () => {
+			const invalidPath = "test%ZZinvalid.txt" // Invalid percent encoding
+			const mockDocument = { uri: { fsPath: invalidPath } }
+
+			vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
+				type: vscode.FileType.File,
+				ctime: 0,
+				mtime: 0,
+				size: 0,
+			})
+			vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
+			vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
+
+			await openFile(invalidPath)
+
+			// Should log a warning about decode failure
+			expect(console.warn).toHaveBeenCalledWith(
+				"Failed to decode file path: URIError: URI malformed. Using original path.",
+			)
+
+			// Should still attempt to open the file with the original path
+			expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
+			expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
+		})
+
+		it("should successfully decode valid URI-encoded paths", async () => {
+			const encodedPath = "./%5Btest%5D/file.txt" // [test] encoded
+			const decodedPath = "./[test]/file.txt"
+			const mockDocument = { uri: { fsPath: decodedPath } }
+
+			vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
+				type: vscode.FileType.File,
+				ctime: 0,
+				mtime: 0,
+				size: 0,
+			})
+			vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
+			vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
+
+			await openFile(encodedPath)
+
+			// Should not log any warnings
+			expect(console.warn).not.toHaveBeenCalled()
+
+			// Should use the decoded path - verify it contains the decoded brackets
+			// On Windows, the path will include backslashes instead of forward slashes
+			const expectedPathSegment = process.platform === "win32" ? "[test]\\file.txt" : "[test]/file.txt"
+			expect(vscode.Uri.file).toHaveBeenCalledWith(expect.stringContaining(expectedPathSegment))
+			expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
+			expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
+		})
+
+		it("should handle paths with special characters that need encoding", async () => {
+			const pathWithSpecialChars = "./[brackets]/file with spaces.txt"
+			const mockDocument = { uri: { fsPath: pathWithSpecialChars } }
+
+			vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
+				type: vscode.FileType.File,
+				ctime: 0,
+				mtime: 0,
+				size: 0,
+			})
+			vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
+			vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
+
+			await openFile(pathWithSpecialChars)
+
+			// Should work without errors
+			expect(console.warn).not.toHaveBeenCalled()
+			expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
+			expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
+		})
+
+		it("should handle already decoded paths without double-decoding", async () => {
+			const normalPath = "./normal/file.txt"
+			const mockDocument = { uri: { fsPath: normalPath } }
+
+			vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
+				type: vscode.FileType.File,
+				ctime: 0,
+				mtime: 0,
+				size: 0,
+			})
+			vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
+			vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
+
+			await openFile(normalPath)
+
+			// Should work without errors
+			expect(console.warn).not.toHaveBeenCalled()
+			expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
+			expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
+		})
+	})
+
+	describe("error handling", () => {
+		it("should show error message when file does not exist", async () => {
+			const nonExistentPath = "./does/not/exist.txt"
+
+			vi.mocked(vscode.workspace.fs.stat).mockRejectedValue(new Error("File not found"))
+
+			await openFile(nonExistentPath)
+
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.could_not_open_file")
+		})
+
+		it("should handle generic errors", async () => {
+			const testPath = "./test.txt"
+
+			vi.mocked(vscode.workspace.fs.stat).mockRejectedValue("Not an Error object")
+
+			await openFile(testPath)
+
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.could_not_open_file")
+		})
+	})
+
+	describe("directory handling", () => {
+		it("should reveal directories in explorer", async () => {
+			const dirPath = "./components"
+
+			vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
+				type: vscode.FileType.Directory,
+				ctime: 0,
+				mtime: 0,
+				size: 0,
+			})
+
+			await openFile(dirPath)
+
+			expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
+				"revealInExplorer",
+				expect.objectContaining({ fsPath: expect.stringContaining("components") }),
+			)
+			expect(vscode.commands.executeCommand).toHaveBeenCalledWith("list.expand")
+			expect(vscode.workspace.openTextDocument).not.toHaveBeenCalled()
+		})
+	})
+
+	describe("file creation", () => {
+		it("should create new files when create option is true", async () => {
+			const newFilePath = "./new/file.txt"
+			const content = "Hello, world!"
+
+			vi.mocked(vscode.workspace.fs.stat).mockRejectedValue(new Error("File not found"))
+			vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue({} as any)
+			vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
+
+			await openFile(newFilePath, { create: true, content })
+
+			// On Windows, the path will include backslashes instead of forward slashes
+			const expectedPathSegment = process.platform === "win32" ? "new\\file.txt" : "new/file.txt"
+			expect(vscode.workspace.fs.writeFile).toHaveBeenCalledWith(
+				expect.objectContaining({ fsPath: expect.stringContaining(expectedPathSegment) }),
+				Buffer.from(content, "utf8"),
+			)
+			expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
+		})
+	})
+})

src/integrations/misc/open-file.ts

@@ -12,9 +12,19 @@ interface OpenFileOptions {
 
 export async function openFile(filePath: string, options: OpenFileOptions = {}) {
 	try {
+		// Store the original path for error messages before any modifications
+		const originalFilePathForError = filePath
+
+		// Try to decode the URI component, but if it fails, use the original path
+		try {
+			filePath = decodeURIComponent(filePath)
+		} catch (decodeError) {
+			// If decoding fails (e.g., invalid escape sequences), continue with the original path
+			console.warn(`Failed to decode file path: ${decodeError}. Using original path.`)
+		}
+
 		const workspaceRoot = getWorkspacePath()
 		const homeDir = os.homedir()
-		const originalFilePathForError = filePath // Keep original for error messages
 
 		const attemptPaths: string[] = []