mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
Merge remote-tracking branch 'origin/main' into nwparker/sta-4319-ssh-host-key-verification
This commit is contained in:
@@ -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<NodeJS.Platform>(['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)
|
||||
})
|
||||
})
|
||||
@@ -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<CommitMessageGenerationTarget, { kind: 'local' }>
|
||||
|
||||
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 }
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
@@ -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'])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user