From f2b70242263312ac37a627b24b85f13e145c601f Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 25 Apr 2026 19:12:11 +0300 Subject: [PATCH] fix(windows): support path mentions and editor launch --- src/main/http/validation.ts | 17 +++- src/main/ipc/config.ts | 80 ++++++++++++++--- src/main/utils/childProcess.ts | 6 +- .../components/chat/UserChatGroup.tsx | 37 ++++---- src/renderer/hooks/useFileSuggestions.ts | 16 ++++ src/renderer/utils/groupTransformer.ts | 86 +++++++++++++------ test/main/utils/childProcess.test.ts | 16 +++- .../renderer/hooks/useFileSuggestions.test.ts | 9 +- test/renderer/utils/fileReferences.test.ts | 21 +++++ 9 files changed, 226 insertions(+), 62 deletions(-) diff --git a/src/main/http/validation.ts b/src/main/http/validation.ts index c9d9ddcd..9b15c606 100644 --- a/src/main/http/validation.ts +++ b/src/main/http/validation.ts @@ -20,11 +20,20 @@ const logger = createLogger('HTTP:validation'); * Prevents path traversal attacks. */ function isPathContained(fullPath: string, basePath: string): boolean { - const normalizedFull = path.normalize(fullPath); - const normalizedBase = path.normalize(basePath); + const normalizedFull = normalizeForContainment(fullPath); + const normalizedBase = normalizeForContainment(basePath); return normalizedFull === normalizedBase || normalizedFull.startsWith(normalizedBase + path.sep); } +function normalizeForContainment(value: string): string { + const resolved = path.resolve(value); + return process.platform === 'win32' ? resolved.toLowerCase() : resolved; +} + +function resolveProjectPath(projectPath: string, requestedPath: string): string { + return path.isAbsolute(requestedPath) ? requestedPath : path.join(projectPath, requestedPath); +} + export function registerValidationRoutes(app: FastifyInstance): void { // Validate path app.post<{ Body: { relativePath: string; projectPath: string } }>( @@ -32,7 +41,7 @@ export function registerValidationRoutes(app: FastifyInstance): void { async (request) => { try { const { relativePath, projectPath } = request.body; - const fullPath = path.join(projectPath, relativePath); + const fullPath = resolveProjectPath(projectPath, relativePath); if (!isPathContained(fullPath, projectPath)) { logger.warn('validate-path blocked path traversal attempt:', relativePath); @@ -57,7 +66,7 @@ export function registerValidationRoutes(app: FastifyInstance): void { // Validate all mentions in parallel with async I/O const entries = await Promise.all( mentions.map(async (mention) => { - const fullPath = path.join(projectPath, mention.value); + const fullPath = resolveProjectPath(projectPath, mention.value); if (!isPathContained(fullPath, projectPath)) { return [`@${mention.value}`, false] as const; } diff --git a/src/main/ipc/config.ts b/src/main/ipc/config.ts index bcc81789..d4b40c2f 100644 --- a/src/main/ipc/config.ts +++ b/src/main/ipc/config.ts @@ -18,10 +18,11 @@ */ import { syncTelemetryFlag } from '@main/sentry'; +import { quoteWindowsCmdArg } from '@main/utils/childProcess'; import { getAutoDetectedClaudeBasePath, getClaudeBasePath } from '@main/utils/pathDecoder'; import { getErrorMessage } from '@shared/utils/errorHandling'; import { createLogger } from '@shared/utils/logger'; -import { execFile } from 'child_process'; +import { execFile, execFileSync, spawn } from 'child_process'; import { BrowserWindow, dialog, type IpcMain, type IpcMainInvokeEvent } from 'electron'; import * as fs from 'fs'; import * as path from 'path'; @@ -57,6 +58,72 @@ let onClaudeRootPathUpdated: ((claudeRootPath: string | null) => Promise | null; let onAgentLanguageUpdated: ((newLangCode: string) => Promise | void) | null = null; +function isPathLikeCommand(command: string): boolean { + return /[\\/]/.test(command) || /^[A-Za-z]:/.test(command); +} + +function resolveWindowsEditorCommand(editor: string): string { + if (process.platform !== 'win32' || isPathLikeCommand(editor)) { + return editor; + } + + try { + const whereExe = path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'where.exe'); + const output = execFileSync(whereExe, [editor], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + }); + return output.trim().split(/\r?\n/)[0] || editor; + } catch { + return editor; + } +} + +function needsWindowsShell(command: string): boolean { + if (process.platform !== 'win32') return false; + const extension = path.extname(command).toLowerCase(); + return extension === '.cmd' || extension === '.bat'; +} + +function launchExternalEditor(editor: string, configPath: string): Promise { + return new Promise((resolve, reject) => { + const resolvedEditor = resolveWindowsEditorCommand(editor); + const launchOptions = { + detached: true, + stdio: 'ignore' as const, + windowsHide: true, + }; + let child: ReturnType; + if (needsWindowsShell(resolvedEditor)) { + const command = [resolvedEditor, configPath].map(quoteWindowsCmdArg).join(' '); + // eslint-disable-next-line sonarjs/os-command -- Windows .cmd launchers require cmd.exe; editor path is resolved via where.exe and args are cmd-escaped. + child = spawn(command, { + ...launchOptions, + shell: true, + }); + } else { + child = spawn(resolvedEditor, [configPath], launchOptions); + } + + let settled = false; + function settle(fn: () => void): void { + if (settled) return; + settled = true; + clearTimeout(timer); + fn(); + } + const timer = setTimeout(() => { + child.unref(); + settle(() => resolve()); + }, 500); + + child.on('error', (err) => { + settle(() => reject(err)); + }); + }); +} + /** * Initializes config handlers with callbacks that require app-level services. */ @@ -607,16 +674,7 @@ async function handleOpenInEditor(_event: IpcMainInvokeEvent): Promise((resolve, reject) => { - const child = execFile(editor, [configPath], { timeout: 5000 }); - // If the process spawns successfully, resolve after a short delay - // (editors typically fork and the parent exits quickly) - const timer = setTimeout(() => resolve(), 500); - child.on('error', (err) => { - clearTimeout(timer); - reject(err); - }); - }); + await launchExternalEditor(editor, configPath); return { success: true }; } catch { // Editor not found, try next diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index b4faa4f5..30319e9c 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -192,13 +192,17 @@ function quoteCmdChunk(chunk: string): string { return `"${escaped}"`; } -function quoteArg(arg: string): string { +export function quoteWindowsCmdArg(arg: string): string { if (/[^A-Za-z0-9_\-/.]/.test(arg)) { return arg.split('%').map(quoteCmdChunk).join('^%'); } return arg; } +function quoteArg(arg: string): string { + return quoteWindowsCmdArg(arg); +} + /** Env vars injected into every spawned Claude CLI process. */ const CLI_ENV_DEFAULTS: Record = { CLAUDE_HOOK_JUDGE_MODE: 'true', diff --git a/src/renderer/components/chat/UserChatGroup.tsx b/src/renderer/components/chat/UserChatGroup.tsx index 4baeff2b..ef7fa9a5 100644 --- a/src/renderer/components/chat/UserChatGroup.tsx +++ b/src/renderer/components/chat/UserChatGroup.tsx @@ -8,6 +8,7 @@ import { useTabUI } from '@renderer/hooks/useTabUI'; import { useTheme } from '@renderer/hooks/useTheme'; import { useStore } from '@renderer/store'; import { selectResolvedMembersForTeamName } from '@renderer/store/slices/teamSlice'; +import { extractFileReferenceTokens } from '@renderer/utils/groupTransformer'; import { REHYPE_PLUGINS } from '@renderer/utils/markdownPlugins'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { linkifyAllMentionsInMarkdown } from '@renderer/utils/mentionLinkify'; @@ -33,9 +34,6 @@ import type { UserGroup } from '@renderer/types/groups'; const logger = createLogger('Component:UserChatGroup'); -// Pattern for @paths only (file references) -const PATH_PATTERN = /@([^\s,)}\]]+)/g; - interface UserChatGroupProps { userGroup: UserGroup; } @@ -46,24 +44,22 @@ interface UserChatGroupProps { */ // eslint-disable-next-line sonarjs/function-return-type -- React child manipulation inherently returns mixed node types function highlightTextNode(text: string, validatedPaths: Record): React.ReactNode { - const pathPattern = /@[^\s,)}\]]+/g; + const pathReferences = extractFileReferenceTokens(text); const parts: React.ReactNode[] = []; let lastIndex = 0; - let match; - pathPattern.lastIndex = 0; - while ((match = pathPattern.exec(text)) !== null) { - if (match.index > lastIndex) { - parts.push(text.slice(lastIndex, match.index)); + for (const reference of pathReferences) { + if (reference.startIndex > lastIndex) { + parts.push(text.slice(lastIndex, reference.startIndex)); } - const fullMatch = match[0]; + const fullMatch = reference.raw; const isValid = validatedPaths[fullMatch] === true; if (isValid) { parts.push( parts.push(fullMatch); } - lastIndex = match.index + fullMatch.length; + lastIndex = reference.endIndex; } if (lastIndex < text.length) { @@ -456,13 +452,10 @@ const UserChatGroupInner = ({ userGroup }: Readonly): React. // Extract @path mentions from text const pathMentions = useMemo(() => { if (!textContent) return []; - const result: { value: string; raw: string }[] = []; - const pathPattern = new RegExp(PATH_PATTERN.source, PATH_PATTERN.flags); - let match; - while ((match = pathPattern.exec(textContent)) !== null) { - result.push({ value: match[1], raw: match[0] }); - } - return result; + return extractFileReferenceTokens(textContent).map((reference) => ({ + value: reference.path, + raw: reference.raw, + })); }, [textContent]); // Validate @path mentions via IPC @@ -475,7 +468,11 @@ const UserChatGroupInner = ({ userGroup }: Readonly): React. const toValidate = pathMentions.map((m) => ({ type: 'path' as const, value: m.value })); const results = await api.validateMentions(toValidate, projectPath); if (isCurrent) { - setValidatedPaths(results); + setValidatedPaths( + Object.fromEntries( + pathMentions.map((mention) => [mention.raw, results[`@${mention.value}`] === true]) + ) + ); } } catch (err) { logger.error('Path validation failed:', err); diff --git a/src/renderer/hooks/useFileSuggestions.ts b/src/renderer/hooks/useFileSuggestions.ts index 605d99db..15b8541e 100644 --- a/src/renderer/hooks/useFileSuggestions.ts +++ b/src/renderer/hooks/useFileSuggestions.ts @@ -19,6 +19,7 @@ import type { QuickOpenFile } from '@shared/types/editor'; const MAX_FILE_SUGGESTIONS = 8; const MAX_FOLDER_SUGGESTIONS = 5; +const MENTION_PATH_QUOTE_NEEDED = /[\s,)}\]"']/; export interface UseFileSuggestionsResult { suggestions: MentionSuggestion[]; @@ -35,6 +36,19 @@ interface DerivedFolder { absolutePath: string; } +export function formatFileMentionPath(relativePath: string): string { + if (!MENTION_PATH_QUOTE_NEEDED.test(relativePath)) { + return relativePath; + } + if (!relativePath.includes('"')) { + return `"${relativePath}"`; + } + if (!relativePath.includes("'")) { + return `'${relativePath}'`; + } + return `"${relativePath.replace(/"/g, '')}"`; +} + /** * Extracts unique directories from a list of file paths. * Returns directories sorted by depth (shallower first), then alphabetically. @@ -94,6 +108,7 @@ export function filterFileSuggestions(files: QuickOpenFile[], query: string): Me type: 'file', filePath: f.path, relativePath: f.relativePath, + insertText: formatFileMentionPath(f.relativePath), }); } } @@ -127,6 +142,7 @@ export function filterFolderSuggestions( type: 'folder', filePath: f.absolutePath, relativePath: f.relativePath, + insertText: formatFileMentionPath(f.relativePath), }); } } diff --git a/src/renderer/utils/groupTransformer.ts b/src/renderer/utils/groupTransformer.ts index 0a975adf..274a0699 100644 --- a/src/renderer/utils/groupTransformer.ts +++ b/src/renderer/utils/groupTransformer.ts @@ -353,17 +353,21 @@ const KNOWN_DIRS = new Set([ 'node_modules', ]); -/** - * Simple pattern for detecting @ mentions that could be file paths. - * The filtering logic in extractFileReferences determines validity. - */ -const FILE_REF_PATTERN = /@([~a-zA-Z0-9._/-]+)/g; +export type FileReferenceToken = FileReference & { + startIndex: number; + endIndex: number; +}; + +const UNQUOTED_FILE_REF_STOP = /[\s,)}\]]/; /** * Checks if a path looks like a valid file reference. * Must start with known dir, contain /, or start with ./ or ../ */ function isValidFileRef(path: string): boolean { + if (/^[A-Za-z]:[\\/]/.test(path) || path.startsWith('\\\\')) { + return true; + } // Check for relative path indicators if (isRelativePath(path)) { return true; @@ -385,6 +389,58 @@ function isValidFileRef(path: string): boolean { return false; } +function readFileRefAt(text: string, atIndex: number): FileReferenceToken | null { + const valueStart = atIndex + 1; + const firstChar = text[valueStart]; + if (!firstChar) return null; + + let path = ''; + let endIndex = valueStart; + + if (firstChar === '"' || firstChar === "'") { + const quote = firstChar; + const quotedStart = valueStart + 1; + const quotedEnd = text.indexOf(quote, quotedStart); + if (quotedEnd < 0) return null; + path = text.slice(quotedStart, quotedEnd); + endIndex = quotedEnd + 1; + } else { + while (endIndex < text.length && !UNQUOTED_FILE_REF_STOP.test(text[endIndex])) { + endIndex += 1; + } + path = text.slice(valueStart, endIndex); + } + + if (!path || !isValidFileRef(path)) return null; + return { + path, + raw: text.slice(atIndex, endIndex), + startIndex: atIndex, + endIndex, + }; +} + +export function extractFileReferenceTokens(text: string): FileReferenceToken[] { + if (!text) return []; + + const references: FileReferenceToken[] = []; + let index = 0; + while (index < text.length) { + const atIndex = text.indexOf('@', index); + if (atIndex < 0) break; + + const reference = readFileRefAt(text, atIndex); + if (reference) { + references.push(reference); + index = reference.endIndex; + } else { + index = atIndex + 1; + } + } + + return references; +} + /** * Extracts file references (@file.ts) from text. * @@ -392,25 +448,7 @@ function isValidFileRef(path: string): boolean { * @returns Array of FileReference objects */ export function extractFileReferences(text: string): FileReference[] { - if (!text) return []; - - const references: FileReference[] = []; - // Reset regex state before use - FILE_REF_PATTERN.lastIndex = 0; - let match: RegExpExecArray | null; - - while ((match = FILE_REF_PATTERN.exec(text)) !== null) { - const [fullMatch, path] = match; - // Only include if it looks like a valid file reference - if (isValidFileRef(path)) { - references.push({ - path, - raw: fullMatch, - }); - } - } - - return references; + return extractFileReferenceTokens(text).map(({ path, raw }) => ({ path, raw })); } // ============================================================================= diff --git a/test/main/utils/childProcess.test.ts b/test/main/utils/childProcess.test.ts index b2bf5f54..ff44665f 100644 --- a/test/main/utils/childProcess.test.ts +++ b/test/main/utils/childProcess.test.ts @@ -19,7 +19,12 @@ vi.mock('child_process', async (importOriginal) => { // Import after the mock call so that the mocked module is returned. import * as child from 'child_process'; -import { execCli, killTrackedCliProcesses, spawnCli } from '@main/utils/childProcess'; +import { + execCli, + killTrackedCliProcesses, + quoteWindowsCmdArg, + spawnCli, +} from '@main/utils/childProcess'; type ExecCallback = (error: Error | null, stdout: string, stderr: string) => void; @@ -68,6 +73,15 @@ describe('cli child process helpers', () => { setPlatform(originalPlatform); }); + describe('quoteWindowsCmdArg', () => { + it('keeps percent signs literal in cmd.exe command strings', () => { + const quoted = quoteWindowsCmdArg('C:\\Users\\Alice\\a%PATH%b.txt'); + expect(quoted).toContain('"C:\\Users\\Alice\\a"^%"PATH"^%"b.txt"'); + expect(quoted).not.toContain('%PATH%'); + expect(quoted).not.toContain('%%PATH%%'); + }); + }); + describe('spawnCli', () => { it('calls spawn directly when path is ascii on windows', () => { setPlatform('win32'); diff --git a/test/renderer/hooks/useFileSuggestions.test.ts b/test/renderer/hooks/useFileSuggestions.test.ts index d6966a8e..1f916937 100644 --- a/test/renderer/hooks/useFileSuggestions.test.ts +++ b/test/renderer/hooks/useFileSuggestions.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { filterFileSuggestions } from '@renderer/hooks/useFileSuggestions'; +import { filterFileSuggestions, formatFileMentionPath } from '@renderer/hooks/useFileSuggestions'; import type { QuickOpenFile } from '@shared/types/editor'; @@ -43,6 +43,7 @@ describe('filterFileSuggestions', () => { expect(results[0].type).toBe('file'); expect(results[0].filePath).toBe('/project/src/test.ts'); expect(results[0].relativePath).toBe('src/test.ts'); + expect(results[0].insertText).toBe('src/test.ts'); }); it('filters by relative path', () => { @@ -94,4 +95,10 @@ describe('filterFileSuggestions', () => { const results = filterFileSuggestions(FILES, '.ts'); expect(results[0].name).toBe('index.ts'); }); + + it('quotes inserted paths that contain spaces', () => { + expect(formatFileMentionPath('src/My Component/App.tsx')).toBe( + '"src/My Component/App.tsx"' + ); + }); }); diff --git a/test/renderer/utils/fileReferences.test.ts b/test/renderer/utils/fileReferences.test.ts index ee7e5c0c..e0f78300 100644 --- a/test/renderer/utils/fileReferences.test.ts +++ b/test/renderer/utils/fileReferences.test.ts @@ -82,6 +82,27 @@ describe('extractFileReferences', () => { expect(refs).toHaveLength(1); expect(refs[0].path).toBe('~/projects/foo'); }); + + it('accepts Windows backslash paths', () => { + const refs = extractFileReferences('Check @src\\components\\App.tsx'); + expect(refs).toHaveLength(1); + expect(refs[0].path).toBe('src\\components\\App.tsx'); + }); + + it('accepts Windows drive absolute paths', () => { + const refs = extractFileReferences('Open @C:\\Users\\Alice\\project\\src\\App.tsx'); + expect(refs).toHaveLength(1); + expect(refs[0].path).toBe('C:\\Users\\Alice\\project\\src\\App.tsx'); + }); + + it('accepts quoted paths with spaces', () => { + const refs = extractFileReferences('Open @"src/My Component/App.tsx" now'); + expect(refs).toHaveLength(1); + expect(refs[0]).toEqual({ + path: 'src/My Component/App.tsx', + raw: '@"src/My Component/App.tsx"', + }); + }); }); describe('rejects npm scoped packages', () => {