From 71bbab72e14e822fd483d1a4f01e301a4379f2e7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:12:14 -0700 Subject: [PATCH] fix(commit-message): keep Windows paths intact in agent command overrides (#14984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(commit-message): keep Windows paths intact in agent command overrides `tokenizeCustomCommandTemplate` applies POSIX backslash-escape rules on every platform. On Windows `\` is the path separator, so a native absolute path in an agent command override is silently destroyed: C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -> C:WindowsSystem32WindowsPowerShellv1.0powershell.exe which is then reported as not found on PATH. The agent *startup* path already routes Windows shells to the Windows tokenizer, but the commit-message AI path still calls the generic tokenizer directly, so overrides, extra CLI args and custom commands there are all affected. The tokenizer gains an explicit `'escape' | 'literal'` mode rather than reading `process.platform`, because the same template can be parsed on one host and executed on another. `'escape'` stays the default, so POSIX behaviour — where `foo\ bar` is deliberately one token — is unchanged. `'literal'` is selected only where the command provably runs on native Windows: a LOCAL target, on win32, with no WSL distro. A WSL target runs a Linux binary inside the distro, and a remote target runs on a host whose platform this process cannot see; both keep POSIX escaping. Fixes #11375 * test: pin the platform decision for literal-backslash parsing commandBackslashMode is the only place that reads the platform, so it is where this can be wrong in the direction that matters — applying Windows rules to a command that will actually run under a POSIX shell. WSL and remote targets are pinned explicitly; both were previously untested. --- ...mit-message-command-backslash-mode.test.ts | 41 ++++++++++++++ .../commit-message-text-generation.ts | 48 ++++++++++++++-- src/shared/commit-message-plan.test.ts | 46 +++++++++++++++- src/shared/commit-message-plan.ts | 23 +++++--- src/shared/commit-message-prompt.test.ts | 55 +++++++++++++++++++ src/shared/commit-message-prompt.ts | 32 +++++++++-- 6 files changed, 224 insertions(+), 21 deletions(-) create mode 100644 src/main/text-generation/commit-message-command-backslash-mode.test.ts diff --git a/src/main/text-generation/commit-message-command-backslash-mode.test.ts b/src/main/text-generation/commit-message-command-backslash-mode.test.ts new file mode 100644 index 00000000000..db828261817 --- /dev/null +++ b/src/main/text-generation/commit-message-command-backslash-mode.test.ts @@ -0,0 +1,41 @@ +/** + * The decision that turns on literal-backslash parsing (#11375). + * + * Why its own suite: this is the only place that reads the platform, so it is + * where the fix can be wrong in the direction that matters — applying Windows + * rules to a command that will actually run under a POSIX shell. + */ +import { describe, expect, it } from 'vitest' +import { commandBackslashMode } from './commit-message-text-generation' + +const LOCAL = { kind: 'local' as const, cwd: 'C:\\repo' } +const REMOTE = { + kind: 'remote' as const, + cwd: '/repo', + execute: async () => ({ ok: true }) as never, + missingBinaryLocation: 'the remote host' +} + +describe('commandBackslashMode', () => { + it('reads backslashes literally only for a native Windows local target', () => { + expect(commandBackslashMode(LOCAL, 'win32')).toBe('literal') + }) + + it.each(['darwin', 'linux'])('keeps POSIX escaping on %s', (platform) => { + expect(commandBackslashMode(LOCAL, platform)).toBe('escape') + }) + + it('keeps POSIX escaping for a WSL target, which runs a Linux binary', () => { + expect(commandBackslashMode({ ...LOCAL, wslDistro: 'Ubuntu' }, 'win32')).toBe('escape') + }) + + it('keeps POSIX escaping for a remote target, whose platform we cannot see', () => { + // A Windows client driving a Linux host is the case this protects. + expect(commandBackslashMode(REMOTE, 'win32')).toBe('escape') + }) + + it('defaults to this process platform when none is given', () => { + const expected = process.platform === 'win32' ? 'literal' : 'escape' + expect(commandBackslashMode(LOCAL)).toBe(expected) + }) +}) diff --git a/src/main/text-generation/commit-message-text-generation.ts b/src/main/text-generation/commit-message-text-generation.ts index 655c6de0e7d..6370117066c 100644 --- a/src/main/text-generation/commit-message-text-generation.ts +++ b/src/main/text-generation/commit-message-text-generation.ts @@ -43,6 +43,7 @@ import { planCommitMessageGeneration, type CommitMessagePlan } from '../../shared/commit-message-plan' +import type { CommandTemplateBackslash } from '../../shared/commit-message-prompt' import { LOCAL_COMMIT_MESSAGE_HOST_KEY } from '../../shared/commit-message-host-key' import { resolveSourceControlAiForOperation, @@ -295,13 +296,14 @@ function finalizeModelDiscoveryOutput( function planModelDiscovery( spec: AgentModelProbeSpec, - agentCommandOverride?: string + agentCommandOverride?: string, + backslash: CommandTemplateBackslash = 'escape' ): { ok: true; plan: CommitMessagePlan } | { ok: false; error: string } { const modelDiscovery = spec.modelDiscovery if (!modelDiscovery) { return { ok: false, error: `${spec.label} does not support dynamic model discovery.` } } - const command = planAgentBinary(modelDiscovery.binary, agentCommandOverride) + const command = planAgentBinary(modelDiscovery.binary, agentCommandOverride, backslash) if (!command.ok) { return command } @@ -341,7 +343,15 @@ export async function discoverCommitMessageModelsLocal( const spawnEnv = env ?? process.env let discoveryStdin: string | null = null try { - const planned = planModelDiscovery(spec, agentCommandOverride) + const planned = planModelDiscovery( + spec, + agentCommandOverride, + commandBackslashMode({ + kind: 'local', + cwd: options.cwd ?? '', + wslDistro: options.wslDistro + }) + ) if (!planned.ok) { markProcessClosed() resolve({ success: false, error: planned.error }) @@ -823,6 +833,23 @@ function runLocalPlan( return { result, processClosed } } +/** + * How the user's command override should read `\`. + * + * `'literal'` only when the command provably runs on native Windows: a LOCAL + * target, on win32, with no WSL distro. A WSL target runs a Linux binary inside + * the distro, and a remote target runs on a host whose platform this process + * cannot see — POSIX escaping stays the default for both. + */ +export function commandBackslashMode( + target: CommitMessageGenerationTarget, + platform: NodeJS.Platform = process.platform +): CommandTemplateBackslash { + return platform === 'win32' && target.kind === 'local' && !target.wslDistro + ? 'literal' + : 'escape' +} + type LocalGenerationTarget = Extract function runLocalPlanForAgent( @@ -1101,7 +1128,10 @@ export async function generateCommitMessageFromContext( linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildCommitMessagePrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration(params, prompt) + const planned = planCommitMessageGeneration( + { ...params, backslash: commandBackslashMode(target) }, + prompt + ) if (!planned.ok) { return { success: false, error: planned.error } } @@ -1173,7 +1203,10 @@ export async function generatePullRequestFieldsFromContext( linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildPullRequestFieldsPrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration(params, prompt) + const planned = planCommitMessageGeneration( + { ...params, backslash: commandBackslashMode(target) }, + prompt + ) if (!planned.ok) { return { success: false, @@ -1223,7 +1256,10 @@ export async function generateBranchNameFromContext( assistantMessage: context.assistantMessage ?? '' }) : buildBranchNamePrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration(params, prompt) + const planned = planCommitMessageGeneration( + { ...params, backslash: commandBackslashMode(target) }, + prompt + ) if (!planned.ok) { return { success: false, error: planned.error } } diff --git a/src/shared/commit-message-plan.test.ts b/src/shared/commit-message-plan.test.ts index 4a9ad1e22b7..0b728307bf7 100644 --- a/src/shared/commit-message-plan.test.ts +++ b/src/shared/commit-message-plan.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { planCommitMessageGeneration } from './commit-message-plan' +import { planCommitMessageGeneration, planAgentBinary } from './commit-message-plan' describe('planCommitMessageGeneration', () => { it('plans Claude non-interactive generation with the prompt on stdin only', () => { @@ -584,3 +584,47 @@ describe('planCommitMessageGeneration', () => { }) }) }) + +describe('backslash mode reaches every command the user can type (#11375)', () => { + const WINDOWS_BINARY = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' + + it('keeps an agent command override intact in literal mode', () => { + const posix = planAgentBinary('claude', WINDOWS_BINARY) + const literal = planAgentBinary('claude', WINDOWS_BINARY, 'literal') + + // The bug: POSIX escaping eats every separator, so the binary is not found. + expect(posix.ok && posix.binary).toBe('C:WindowsSystem32WindowsPowerShellv1.0powershell.exe') + expect(literal.ok && literal.binary).toBe(WINDOWS_BINARY) + }) + + it('keeps a quoted path containing spaces intact in literal mode', () => { + const literal = planAgentBinary('claude', '"C:\\Program Files\\nodejs\\node.exe"', 'literal') + + expect(literal.ok && literal.binary).toBe('C:\\Program Files\\nodejs\\node.exe') + }) + + it('keeps extra CLI args intact through planCommitMessageGeneration', () => { + const plan = planCommitMessageGeneration( + { + agentId: 'claude', + model: 'sonnet', + agentCommandOverride: WINDOWS_BINARY, + agentArgs: '--config C:\\Users\\me\\.claude.json', + backslash: 'literal' + }, + 'prompt' + ) + + expect(plan.ok && plan.plan.binary).toBe(WINDOWS_BINARY) + expect(plan.ok && plan.plan.args).toContain('C:\\Users\\me\\.claude.json') + }) + + it('defaults to POSIX escaping when no mode is given', () => { + const plan = planCommitMessageGeneration( + { agentId: 'claude', model: 'sonnet', agentArgs: '--dir /my\\ dir' }, + 'prompt' + ) + + expect(plan.ok && plan.plan.args).toContain('/my dir') + }) +}) diff --git a/src/shared/commit-message-plan.ts b/src/shared/commit-message-plan.ts index a44f2fc3c22..83760f00ae1 100644 --- a/src/shared/commit-message-plan.ts +++ b/src/shared/commit-message-plan.ts @@ -1,3 +1,4 @@ +import type { CommandTemplateBackslash } from './commit-message-prompt' import { getCommitMessageAgentSpec, getCommitMessageModel, @@ -14,6 +15,10 @@ import type { TuiAgent } from './tui-agent' export type CommitMessagePlanInput = { agentId: TuiAgent | 'custom' + /** How to read `\` in the user's command override / args / custom command. + * Defaults to POSIX escaping; pass `'literal'` only when the command is known + * to run on native Windows, where `\` is the path separator (#11375). */ + backslash?: CommandTemplateBackslash model: string thinkingLevel?: string customAgentCommand?: string @@ -36,14 +41,15 @@ export type CommitMessagePlanResult = export function planAgentBinary( defaultBinary: string, - commandOverride: string | undefined + commandOverride: string | undefined, + backslash: CommandTemplateBackslash = 'escape' ): { ok: true; binary: string; prefixArgs: string[] } | { ok: false; error: string } { const command = commandOverride?.trim() if (!command) { return { ok: true, binary: defaultBinary, prefixArgs: [] } } - const tokenized = tokenizeCustomCommandTemplate(command) + const tokenized = tokenizeCustomCommandTemplate(command, backslash) if (!tokenized.ok) { return { ok: false, error: `Agent command override is invalid: ${tokenized.error}` } } @@ -55,13 +61,14 @@ export function planAgentBinary( } function planAdditionalAgentArgs( - agentArgs: string | null | undefined + agentArgs: string | null | undefined, + backslash: CommandTemplateBackslash = 'escape' ): { ok: true; args: string[] } | { ok: false; error: string } { const trimmed = agentArgs?.trim() if (!trimmed) { return { ok: true, args: [] } } - const tokenized = tokenizeCustomCommandTemplate(trimmed) + const tokenized = tokenizeCustomCommandTemplate(trimmed, backslash) if (!tokenized.ok) { return { ok: false, error: `CLI arguments are invalid: ${tokenized.error}` } } @@ -239,11 +246,11 @@ export function planCommitMessageGeneration( error: 'Custom command is empty. Add one in Settings → Git → AI Commit Messages.' } } - const planned = planCustomCommand(command, prompt) + const planned = planCustomCommand(command, prompt, input.backslash) if (!planned.ok) { return { ok: false, error: planned.error } } - const agentArgs = planAdditionalAgentArgs(input.agentArgs) + const agentArgs = planAdditionalAgentArgs(input.agentArgs, input.backslash) if (!agentArgs.ok) { return agentArgs } @@ -294,11 +301,11 @@ export function planCommitMessageGeneration( model: input.model, thinkingLevel: input.thinkingLevel }) - const agentArgs = planAdditionalAgentArgs(input.agentArgs) + const agentArgs = planAdditionalAgentArgs(input.agentArgs, input.backslash) if (!agentArgs.ok) { return agentArgs } - const command = planAgentBinary(spec.binary, input.agentCommandOverride) + const command = planAgentBinary(spec.binary, input.agentCommandOverride, input.backslash) if (!command.ok) { return { ok: false, error: command.error } } diff --git a/src/shared/commit-message-prompt.test.ts b/src/shared/commit-message-prompt.test.ts index ba2a975190f..f080aabc5a2 100644 --- a/src/shared/commit-message-prompt.test.ts +++ b/src/shared/commit-message-prompt.test.ts @@ -367,3 +367,58 @@ describe('planCustomCommand', () => { } }) }) + +describe('Windows command overrides keep native path separators (#11375)', () => { + const WINDOWS_PATH = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' + + it('eats backslashes under the POSIX default, which is what broke Windows paths', () => { + // Pinned as the reason 'literal' exists, not as desired behavior. + const posix = tokenizeCustomCommandTemplate(WINDOWS_PATH) + + expect(posix.ok && posix.tokens).toEqual([ + 'C:WindowsSystem32WindowsPowerShellv1.0powershell.exe' + ]) + }) + + it('keeps a native absolute path intact in literal mode', () => { + const literal = tokenizeCustomCommandTemplate(WINDOWS_PATH, 'literal') + + expect(literal.ok && literal.tokens).toEqual([WINDOWS_PATH]) + }) + + it('still splits on whitespace and honours quotes in literal mode', () => { + const quoted = tokenizeCustomCommandTemplate( + '"C:\\Program Files\\Git\\bin\\bash.exe" --login -i', + 'literal' + ) + + expect(quoted.ok && quoted.tokens).toEqual([ + 'C:\\Program Files\\Git\\bin\\bash.exe', + '--login', + '-i' + ]) + }) + + it('leaves a trailing backslash alone instead of swallowing the delimiter', () => { + const trailing = tokenizeCustomCommandTemplate('C:\\tools\\ --flag', 'literal') + + expect(trailing.ok && trailing.tokens).toEqual(['C:\\tools\\', '--flag']) + }) + + it('keeps POSIX escaping the default so `foo\\ bar` stays one token', () => { + const posix = tokenizeCustomCommandTemplate('/usr/local/my\\ agent/bin --flag') + + expect(posix.ok && posix.tokens).toEqual(['/usr/local/my agent/bin', '--flag']) + }) + + it('substitutes {prompt} as one argument with a Windows binary path', () => { + const plan = planCustomCommand( + `${WINDOWS_PATH} -Command {prompt}`, + 'write a commit message', + 'literal' + ) + + expect(plan.ok && plan.binary).toBe(WINDOWS_PATH) + expect(plan.ok && plan.args).toEqual(['-Command', 'write a commit message']) + }) +}) diff --git a/src/shared/commit-message-prompt.ts b/src/shared/commit-message-prompt.ts index efd42f36e5a..164f4da6327 100644 --- a/src/shared/commit-message-prompt.ts +++ b/src/shared/commit-message-prompt.ts @@ -149,7 +149,23 @@ export type TokenizeCustomCommandResult = // "spawn this exact CLI" — adding shell semantics on top would create // surprising behavior across platforms (especially Windows) and a security // surface we don't need. -export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomCommandResult { +/** + * `'escape'` (default) is POSIX: a backslash quotes the next byte, so `foo\ bar` + * is one token. `'literal'` is for a command that will run on native Windows, + * where `\` is the path separator — eating it turns + * `C:\Windows\System32\powershell.exe` into `C:WindowsSystem32powershell.exe`, + * a path that then "cannot be found" (#11375). + * + * Opt-in rather than sniffed from `process.platform` here, because the same + * template can be parsed on one host and executed on another. + */ +export type CommandTemplateBackslash = 'escape' | 'literal' + +export function tokenizeCustomCommandTemplate( + template: string, + backslash: CommandTemplateBackslash = 'escape' +): TokenizeCustomCommandResult { + const backslashEscapes = backslash === 'escape' const tokens: string[] = [] const spans: CommandTokenSpan[] = [] let current = '' @@ -162,7 +178,7 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC while (i < template.length) { const ch = template[i] if (quote) { - if (ch === '\\' && quote === '"' && i + 1 < template.length) { + if (backslashEscapes && ch === '\\' && quote === '"' && i + 1 < template.length) { // Why: inside double quotes the shell only consumes the backslash // before these; elsewhere it stays a literal byte this tokenizer drops. divergesFromShell ||= !'$`"\\'.includes(template[i + 1]) @@ -197,7 +213,7 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC continue } - if (ch === '\\' && i + 1 < template.length) { + if (backslashEscapes && ch === '\\' && i + 1 < template.length) { // Why: an unquoted line continuation joins words the shell splits, so a // selector can hide inside the joined token and skip the gap check. divergesFromShell ||= template[i + 1] === '\n' @@ -227,7 +243,7 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC } // Why: a trailing unpaired escape swallows whatever a consumer appends // after the base, so the base is not safe to build on. - divergesFromShell ||= ch === '\\' && i + 1 >= template.length + divergesFromShell ||= backslashEscapes && ch === '\\' && i + 1 >= template.length divergesFromShell ||= ';&|<>`'.includes(ch) || (ch === '#' && !inToken) || @@ -260,8 +276,12 @@ export type CustomCommandPlan = * substituted prompt is always passed as a single argument regardless of * whether the template wrote `{prompt}` or `"{prompt}"`. */ -export function planCustomCommand(template: string, prompt: string): CustomCommandPlan { - const tokenized = tokenizeCustomCommandTemplate(template) +export function planCustomCommand( + template: string, + prompt: string, + backslash: CommandTemplateBackslash = 'escape' +): CustomCommandPlan { + const tokenized = tokenizeCustomCommandTemplate(template, backslash) if (!tokenized.ok) { return { ok: false, error: tokenized.error } }