diff --git a/src/cli/computer-format.test.ts b/src/cli/computer-format.test.ts index 2945ba537299..63b62aad1bc4 100644 --- a/src/cli/computer-format.test.ts +++ b/src/cli/computer-format.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { ComputerActionResult } from '../shared/runtime-types' import { formatComputerAction, prepareComputerCliJsonResult } from './computer-format' import { printResult } from './format' @@ -54,6 +54,10 @@ describe('prepareComputerCliJsonResult', () => { }) describe('formatComputerAction', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + it('does not treat legacy action results without metadata as completed', () => { const result: ComputerActionResult = { snapshot: { @@ -104,4 +108,31 @@ describe('formatComputerAction', () => { expect(output).toContain('Inspect with the command above') expect(output).not.toContain('Click completed') }) + + it('quotes worktree selectors so a pasted Windows command keeps dollar signs', () => { + const result: ComputerActionResult = { + snapshot: { + id: 'snap-1', + app: { name: 'App', bundleId: 'com.example.app', pid: 100 }, + window: { title: 'App', id: 42, width: 800, height: 600 }, + coordinateSpace: 'window', + treeText: 'tree', + elementCount: 5, + focusedElementId: null + }, + screenshot: null, + screenshotStatus: { state: 'skipped', reason: 'no_screenshot_flag' }, + action: { + path: 'synthetic', + verification: { state: 'verified', property: 'value' } + } + } + + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const output = formatComputerAction('click', result, { + worktree: 'path:C:/work/$review' + }) + + expect(output).toContain('--worktree path:C:/work/$"r"eview') + }) }) diff --git a/src/cli/shell-command-quote.test.ts b/src/cli/shell-command-quote.test.ts index 06e47245af2a..49b33655c229 100644 --- a/src/cli/shell-command-quote.test.ts +++ b/src/cli/shell-command-quote.test.ts @@ -15,6 +15,22 @@ describe('quoteCliCommandArgument', () => { expect(quoteCliCommandArgument('Text Editor')).toBe("'Text Editor'") vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + // cmd.exe splits on spaces and does not treat single quotes as delimiters. expect(quoteCliCommandArgument('Text Editor')).toBe('"Text Editor"') }) + + it('keeps a dollar sign literal for both PowerShell and cmd.exe', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + expect(quoteCliCommandArgument('path:C:/work/$review')).toBe('path:C:/work/$"r"eview') + expect(quoteCliCommandArgument('a b$c')).toBe('a" "b$"c"') + // cmd.exe splits on an unquoted &; single quotes would not hide it. + expect(quoteCliCommandArgument('a&b$c')).toBe('a"&"b$"c"') + expect(quoteCliCommandArgument('a$$b')).toBe('a$"$"b') + expect(quoteCliCommandArgument('path:C:/work/$$()')).toBe('path:C:/work/$"`$()"') + expect(quoteCliCommandArgument("path:C:/work/$r'x")).toBe(`path:C:/work/$"r'"x`) + expect(quoteCliCommandArgument('path:C:/work/\\\\\\ $review')).toBe( + 'path:C:/work/"\\\\\\ "$"r"eview' + ) + expect(quoteCliCommandArgument(' $review')).toBe('" `$review"') + }) }) diff --git a/src/cli/shell-command-quote.ts b/src/cli/shell-command-quote.ts index c5dff7f62a39..210e02587149 100644 --- a/src/cli/shell-command-quote.ts +++ b/src/cli/shell-command-quote.ts @@ -1,9 +1,106 @@ +// Literal unquoted in cmd.exe, and not a PowerShell token boundary. +const WINDOWS_BARE_SAFE = /^[A-Za-z0-9._:/@\\~*?!=+-]$/ + export function quoteCliCommandArgument(value: string): string { if (/^[a-zA-Z0-9._:/@-]+$/.test(value)) { return value } if (process.platform === 'win32') { - return `"${value.replace(/"/g, '\\"')}"` + return quoteWindowsCliArgument(value) } return `'${value.replaceAll("'", "'\\''")}'` } + +function quoteWindowsCliArgument(value: string): string { + // No `$`: keep the historical double-quoted Windows spelling. + if (!value.includes('$')) { + return `"${value.replace(/"/g, '\\"')}"` + } + const first = value[0] ?? '' + const dotSource = first === '.' && (value[1] === '$' || value[1] === '"' || value[1] === "'") + // Why: `"`, `@`, and a dot-source `.` end the PowerShell token before the rest. + if (dotSource || first === '@' || (first !== '$' && !WINDOWS_BARE_SAFE.test(first))) { + return `"${value.replace(/[`$"]/g, (char) => `\`${char}`)}"` + } + // Why `$"x"`: PowerShell expands `$` only when a variable name follows, and + // cmd concatenates a quoted character onto the surrounding text. + let quoted = '' + for (let index = 0; index < value.length; index += 1) { + const char = value[index] ?? '' + if (char === '$') { + const next = value[index + 1] + if (next && isPowerShellVariableStart(next)) { + let run = next + index += 2 + while ( + index < value.length && + value[index] !== '$' && + !WINDOWS_BARE_SAFE.test(value[index] ?? '') + ) { + run += value[index] + index += 1 + } + index -= 1 + // Why: `$"r""'"` is one quoted run, so the `""` becomes a literal quote. + quoted += `$${quoteWindowsRun(run)}` + continue + } + quoted += '$' + continue + } + if (WINDOWS_BARE_SAFE.test(char)) { + quoted += char + continue + } + let run = '' + while ( + index < value.length && + value[index] !== '$' && + !WINDOWS_BARE_SAFE.test(value[index] ?? '') + ) { + run += value[index] + index += 1 + } + index -= 1 + // Why: a `\` left outside the quotes is doubled for cmd and kept by PowerShell. + const slashes = quoted.match(/\\+$/) + if (slashes) { + quoted = quoted.slice(0, -slashes[0].length) + run = `${slashes[0]}${run}` + } + quoted += quoteWindowsRun(run) + } + return quoted +} + +function quoteWindowsRun(run: string): string { + return `"${escapeWindowsQuotedRun(run)}"` +} + +function escapeWindowsQuotedRun(run: string): string { + let escaped = '' + for (let index = 0; index < run.length; index += 1) { + const char = run[index] ?? '' + const next = run[index + 1] + if (char === '"') { + escaped += '""' + continue + } + if (char === '`') { + escaped += '``' + continue + } + // Why: `"$()"` is a PowerShell subexpression. A final `$` is already literal. + if (char === '$' && next && (next === '(' || next === '{' || isPowerShellVariableStart(next))) { + escaped += '`$' + continue + } + escaped += char + } + return escaped +} + +function isPowerShellVariableStart(char: string): boolean { + // Mirrors PowerShell VarNameFirst, plus other letters and digits. + return /^[A-Za-z0-9_?$^:]$/.test(char) || /[\p{L}\p{N}]/u.test(char) +}