mirror of
https://github.com/stablyai/orca.git
synced 2026-10-29 16:17:51 +00:00
fix(fish): quote and tokenize with fish rules, not sh ones
Threading the `fish` dialect to real callers made quoting a correctness problem: `quoteStartupArg` had no fish branch, so prompts and CLI args were quoted with sh rules and handed to fish. fish single quotes are not literal — `\\` and `\'` are escapes inside them — so every backslash silently halved and a trailing backslash was a hard syntax error that killed the launch. - `quoteStartupArg` gains a fish branch escaping `\` and `'`. - New `fish-command-tokenizer.ts`: fish's single/double-quote and unquoted escape rules, plus divergence flags for command substitution, brace expansion and globs (an unmatched wildcard is a hard error in fish). - `agent-resume-argv-drop` adds fish to the quoting candidates, so a fish-quoted resume suffix is recognised instead of refusing the launch. - `agent-resume-launch-command` skips `NAME=value` prefixes for fish too. - `ai-vault-resume-command` quotes its cd/CODEX_HOME prefix for the live shell. - `hermes-startup-query` quotes the `sh -c` wrapper for the real dialect. - `isPosixStartupShell`'s doc no longer claims fish shares sh quoting. fish history deletion now uses the path resolved from the PTY's spawn env (recorded in meta.json) rather than the main process env, and warns explicitly when no file is found instead of leaking one per deleted worktree in silence.
This commit is contained in:
@@ -249,6 +249,7 @@ jobs:
|
||||
src/main/terminal-history-fish-session.node-pty.test.ts \
|
||||
src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
|
||||
src/shared/fish-draft-prefill-teardown.node-pty.test.ts \
|
||||
src/shared/fish-startup-arg-quoting.live-fish.test.ts \
|
||||
src/shared/posix-command-path-lookup.test.ts
|
||||
|
||||
test:
|
||||
@@ -287,6 +288,7 @@ jobs:
|
||||
--exclude=src/main/terminal-history-fish-session.node-pty.test.ts \
|
||||
--exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
|
||||
--exclude=src/shared/fish-draft-prefill-teardown.node-pty.test.ts \
|
||||
--exclude=src/shared/fish-startup-arg-quoting.live-fish.test.ts \
|
||||
--exclude=src/shared/posix-command-path-lookup.test.ts \
|
||||
--exclude=tests/e2e/cross-version-wire/** \
|
||||
--shard=${{ matrix.shard }}/${{ matrix.shard_total }}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { homedir } from 'node:os'
|
||||
import { isAbsolute, join } from 'node:path'
|
||||
import { rmSync } from 'node:fs'
|
||||
import { basename, isAbsolute, join } from 'node:path'
|
||||
import { existsSync, rmSync } from 'node:fs'
|
||||
|
||||
/**
|
||||
* Per-worktree fish history, which fish models as a session NAME rather than a path.
|
||||
@@ -45,17 +45,62 @@ export function resolveFishHistoryFilePath(
|
||||
return join(dataHome, 'fish', `${session}_history`)
|
||||
}
|
||||
|
||||
/** Best-effort removal of one worktree's fish history file. */
|
||||
export function deleteFishHistoryFile(session: string, env: NodeJS.ProcessEnv = process.env): void {
|
||||
const path = resolveFishHistoryFilePath(session, env)
|
||||
if (!path) {
|
||||
/**
|
||||
* Accepts a path recorded at spawn time only if it still names this session's own
|
||||
* history file, so a tampered meta.json cannot steer `rmSync` somewhere else.
|
||||
*/
|
||||
function isTrustedRecordedPath(session: string, path: string | null | undefined): path is string {
|
||||
return Boolean(
|
||||
path &&
|
||||
SAFE_SESSION_NAME.test(session) &&
|
||||
isAbsolute(path) &&
|
||||
basename(path) === `${session}_history`
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes one worktree's fish history file.
|
||||
*
|
||||
* `recordedPath` is the path resolved from the PTY's own spawn env when the
|
||||
* session was minted; the main process env is only a fallback, because the two
|
||||
* disagree whenever Orca was launched with a different `XDG_DATA_HOME`/`HOME`
|
||||
* than the shells it spawns.
|
||||
*
|
||||
* Neither can see a `set -gx XDG_DATA_HOME …` that lives inside the user's
|
||||
* config.fish — fish resolves that after we have handed off. That file cannot be
|
||||
* found from here, so it is reported instead of being dropped silently.
|
||||
*/
|
||||
export function deleteFishHistoryFile(
|
||||
session: string,
|
||||
options: { recordedPath?: string | null; env?: NodeJS.ProcessEnv } = {}
|
||||
): void {
|
||||
const candidates = new Set<string>()
|
||||
if (isTrustedRecordedPath(session, options.recordedPath)) {
|
||||
candidates.add(options.recordedPath)
|
||||
}
|
||||
const fromEnv = resolveFishHistoryFilePath(session, options.env ?? process.env)
|
||||
if (fromEnv) {
|
||||
candidates.add(fromEnv)
|
||||
}
|
||||
if (candidates.size === 0) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
rmSync(path, { force: true })
|
||||
} catch (err) {
|
||||
let removed = false
|
||||
for (const path of candidates) {
|
||||
try {
|
||||
removed ||= existsSync(path)
|
||||
rmSync(path, { force: true })
|
||||
} catch (err) {
|
||||
console.warn(
|
||||
`[pty:history] Failed to delete fish history ${path}: ${err instanceof Error ? err.message : String(err)}`
|
||||
)
|
||||
}
|
||||
}
|
||||
if (!removed) {
|
||||
// Explicit: a config.fish-set XDG_DATA_HOME leaves a file we cannot locate,
|
||||
// and silence would hide one leaked history file per deleted worktree.
|
||||
console.warn(
|
||||
`[pty:history] Failed to delete fish history ${session}: ${err instanceof Error ? err.message : String(err)}`
|
||||
`[pty:history] No fish history file found for session ${session}; if fish keeps history outside ${[...candidates].join(' or ')} (e.g. XDG_DATA_HOME set in config.fish) that file is left behind.`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,9 +97,11 @@ function scheduleHistoryTreeRemoval(dir: string): void {
|
||||
export function scheduleWorktreeHistoryTreeDeletion(dir: string, historyRoot: string): boolean {
|
||||
// Why first: fish keeps its history in the user's fish data dir, outside this tree,
|
||||
// so the meta.json naming the session must still be readable when we look it up.
|
||||
const fishSession = readHistoryMeta(dir)?.fishSession
|
||||
if (fishSession) {
|
||||
deleteFishHistoryFile(fishSession)
|
||||
const meta = readHistoryMeta(dir)
|
||||
if (meta?.fishSession) {
|
||||
deleteFishHistoryFile(meta.fishSession, {
|
||||
recordedPath: meta.fishHistoryPath
|
||||
})
|
||||
}
|
||||
const tombstone = tombstoneHistoryTree(dir, historyRoot)
|
||||
if (!tombstone) {
|
||||
|
||||
@@ -247,6 +247,32 @@ describe('terminal-history', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('records the history path from the SPAWN env, not this process env', () => {
|
||||
// Pinned so the assertion cannot accidentally read the developer's own value.
|
||||
const originalDataHome = process.env.XDG_DATA_HOME
|
||||
process.env.XDG_DATA_HOME = ['', 'main', 'process', 'data'].join(sep)
|
||||
try {
|
||||
const env: Record<string, string> = {
|
||||
XDG_DATA_HOME: ['', 'spawn', 'data'].join(sep)
|
||||
}
|
||||
const result = injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt')
|
||||
const meta = JSON.parse(writeFileSyncMock.mock.calls.at(-1)?.[1] as string) as Record<
|
||||
string,
|
||||
string
|
||||
>
|
||||
|
||||
expect(meta.fishHistoryPath).toBe(
|
||||
['', 'spawn', 'data', 'fish', `${result.fishSession}_history`].join(sep)
|
||||
)
|
||||
} finally {
|
||||
if (originalDataHome === undefined) {
|
||||
delete process.env.XDG_DATA_HOME
|
||||
} else {
|
||||
process.env.XDG_DATA_HOME = originalDataHome
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('gives two fish worktrees different history sessions', () => {
|
||||
const envA: Record<string, string> = {}
|
||||
injectHistoryEnv(envA, 'repo-1::/path/wt-a', '/usr/bin/fish', '/path/wt-a')
|
||||
@@ -387,6 +413,79 @@ describe('terminal-history', () => {
|
||||
await flushPendingWorktreeHistoryDeletions()
|
||||
})
|
||||
|
||||
it('prefers the spawn-env path recorded in meta.json over this process env', async () => {
|
||||
const originalDataHome = process.env.XDG_DATA_HOME
|
||||
process.env.XDG_DATA_HOME = ['', 'main', 'data'].join(sep)
|
||||
const recorded = ['', 'spawn', 'data', 'fish', 'orca_abc123_history'].join(sep)
|
||||
try {
|
||||
existsSyncMock.mockReturnValue(true)
|
||||
readFileSyncMock.mockReturnValue(
|
||||
JSON.stringify({
|
||||
worktreeId: 'repo-1::/path/wt',
|
||||
fishSession: 'orca_abc123',
|
||||
fishHistoryPath: recorded
|
||||
})
|
||||
)
|
||||
deleteWorktreeHistoryDir('repo-1::/path/wt')
|
||||
expect(rmSyncMock).toHaveBeenCalledWith(recorded, expect.objectContaining({ force: true }))
|
||||
// The main-process guess stays as a fallback for pre-existing meta.json files.
|
||||
expect(rmSyncMock).toHaveBeenCalledWith(
|
||||
['', 'main', 'data', 'fish', 'orca_abc123_history'].join(sep),
|
||||
expect.objectContaining({ force: true })
|
||||
)
|
||||
} finally {
|
||||
if (originalDataHome === undefined) {
|
||||
delete process.env.XDG_DATA_HOME
|
||||
} else {
|
||||
process.env.XDG_DATA_HOME = originalDataHome
|
||||
}
|
||||
}
|
||||
await flushPendingWorktreeHistoryDeletions()
|
||||
})
|
||||
|
||||
it('refuses a recorded path that does not name its own session file', async () => {
|
||||
const originalDataHome = process.env.XDG_DATA_HOME
|
||||
process.env.XDG_DATA_HOME = ['', 'main', 'data'].join(sep)
|
||||
try {
|
||||
existsSyncMock.mockReturnValue(true)
|
||||
readFileSyncMock.mockReturnValue(
|
||||
JSON.stringify({
|
||||
worktreeId: 'repo-1::/path/wt',
|
||||
fishSession: 'orca_abc123',
|
||||
fishHistoryPath: ['', 'etc', 'passwd'].join(sep)
|
||||
})
|
||||
)
|
||||
deleteWorktreeHistoryDir('repo-1::/path/wt')
|
||||
expect(rmSyncMock).not.toHaveBeenCalledWith(
|
||||
['', 'etc', 'passwd'].join(sep),
|
||||
expect.anything()
|
||||
)
|
||||
} finally {
|
||||
if (originalDataHome === undefined) {
|
||||
delete process.env.XDG_DATA_HOME
|
||||
} else {
|
||||
process.env.XDG_DATA_HOME = originalDataHome
|
||||
}
|
||||
}
|
||||
await flushPendingWorktreeHistoryDeletions()
|
||||
})
|
||||
|
||||
it('warns instead of going quiet when no fish history file is found', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
try {
|
||||
readFileSyncMock.mockReturnValue(
|
||||
JSON.stringify({ worktreeId: 'repo-1::/path/wt', fishSession: 'orca_abc123' })
|
||||
)
|
||||
// The history dir exists (so the tree is deleted) but the fish file does not.
|
||||
existsSyncMock.mockImplementation((path: string) => !path.includes('orca_abc123_history'))
|
||||
deleteWorktreeHistoryDir('repo-1::/path/wt')
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('No fish history file found'))
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
}
|
||||
await flushPendingWorktreeHistoryDeletions()
|
||||
})
|
||||
|
||||
it('leaves the live directory alone when the tombstone rename fails', async () => {
|
||||
existsSyncMock.mockReturnValue(true)
|
||||
renameSyncMock.mockImplementation(() => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { join, basename } from 'node:path'
|
||||
import { mkdirSync, existsSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { fishHistorySessionName } from './fish-history-session'
|
||||
import { fishHistorySessionName, resolveFishHistoryFilePath } from './fish-history-session'
|
||||
import { parseWslPath, toLinuxPath } from './wsl'
|
||||
import { getHistoryRoot, getHistoryRootWsl, hashWorktreeId } from './terminal-history-paths'
|
||||
|
||||
@@ -74,11 +74,20 @@ export function ensureHistoryDir(worktreeHash: string, wslDistro?: string): stri
|
||||
/** Write meta.json alongside history files, for debuggability and for GC.
|
||||
* `fishSession` is load-bearing, not diagnostic: fish history lives outside this
|
||||
* directory, so deletion can only find it by the name recorded here. */
|
||||
function writeMetaFile(dir: string, worktreeId: string, fishSession?: string): void {
|
||||
function writeMetaFile(
|
||||
dir: string,
|
||||
worktreeId: string,
|
||||
fish?: { session: string; historyPath: string | null }
|
||||
): void {
|
||||
try {
|
||||
const metaPath = join(dir, 'meta.json')
|
||||
const existing = existsSync(metaPath) ? readHistoryMeta(dir) : null
|
||||
if (existing && (!fishSession || existing.fishSession === fishSession)) {
|
||||
if (
|
||||
existing &&
|
||||
(!fish ||
|
||||
(existing.fishSession === fish.session &&
|
||||
existing.fishHistoryPath === (fish.historyPath ?? undefined)))
|
||||
) {
|
||||
return
|
||||
}
|
||||
writeFileSync(
|
||||
@@ -86,7 +95,8 @@ function writeMetaFile(dir: string, worktreeId: string, fishSession?: string): v
|
||||
JSON.stringify({
|
||||
worktreeId,
|
||||
createdAt: existing?.createdAt ?? new Date().toISOString(),
|
||||
...(fishSession ? { fishSession } : {})
|
||||
...(fish ? { fishSession: fish.session } : {}),
|
||||
...(fish?.historyPath ? { fishHistoryPath: fish.historyPath } : {})
|
||||
}),
|
||||
{ mode: 0o600 }
|
||||
)
|
||||
@@ -100,6 +110,8 @@ export type HistoryDirMeta = {
|
||||
createdAt?: string
|
||||
/** fish session name whose history file lives in the user's fish data dir. */
|
||||
fishSession?: string
|
||||
/** That file's path resolved from the PTY's spawn env, which the main process env can contradict. */
|
||||
fishHistoryPath?: string
|
||||
}
|
||||
|
||||
/** Read one history directory's meta.json, or null when it is absent or unparseable. */
|
||||
@@ -173,7 +185,12 @@ export function injectHistoryEnv(
|
||||
// find the session file fish keeps in its own data dir. fish never runs as the
|
||||
// inner WSL shell, so histDir needs no /mnt conversion here.
|
||||
const session = fishHistorySessionName(worktreeHash)
|
||||
writeMetaFile(histDir, worktreeId, session)
|
||||
// Resolve from the SPAWN env: that is the XDG_DATA_HOME/HOME fish will see,
|
||||
// which need not match the one this process was launched with.
|
||||
writeMetaFile(histDir, worktreeId, {
|
||||
session,
|
||||
historyPath: resolveFishHistoryFilePath(session, spawnEnv)
|
||||
})
|
||||
spawnEnv.fish_history = session
|
||||
result.fishSession = session
|
||||
result.historyDir = histDir
|
||||
|
||||
@@ -13,7 +13,9 @@ export type AgentResumeArgvDrop =
|
||||
| { status: 'absent' }
|
||||
| { status: 'unrecognized' }
|
||||
|
||||
const RESUME_ARGV_SHELLS: readonly AgentStartupShell[] = ['posix', 'powershell', 'cmd']
|
||||
// fish is listed because its quoting diverges from sh; without it a fish-quoted
|
||||
// suffix reads as `unrecognized` and the caller refuses the launch outright.
|
||||
const RESUME_ARGV_SHELLS: readonly AgentStartupShell[] = ['posix', 'fish', 'powershell', 'cmd']
|
||||
|
||||
function resumeArgvSuffixCandidates(resumeArgs: readonly string[]): string[] {
|
||||
const candidates = new Set<string>([resumeArgs.join(' ')])
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { ResumableTuiAgent } from './agent-session-resume'
|
||||
import {
|
||||
isPosixStartupShell,
|
||||
quoteStartupArg,
|
||||
tokenizeStartupCommand,
|
||||
type AgentStartupShell
|
||||
@@ -37,9 +38,9 @@ function findClaudeExecutableIndex(tokens: readonly string[], shell: AgentStartu
|
||||
return i
|
||||
}
|
||||
if (
|
||||
// Why: `NAME=value cmd` is posix-only syntax; on cmd/PowerShell such a
|
||||
// token is just a bogus executable name, not a prefix to skip.
|
||||
(shell === 'posix' && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) ||
|
||||
// Why: `NAME=value cmd` is sh-family syntax (fish included, 3.1+); on
|
||||
// cmd/PowerShell such a token is a bogus executable name, not a prefix.
|
||||
(isPosixStartupShell(shell) && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) ||
|
||||
(shell === 'powershell' && token === '&' && i === 0)
|
||||
) {
|
||||
continue
|
||||
|
||||
@@ -72,7 +72,7 @@ export function buildAiVaultResumeShellCommand(args: {
|
||||
})
|
||||
}
|
||||
|
||||
const resumeCommand = `${codexHomeEnvPrefix(codexHome?.trim() || null, platform)}${
|
||||
const resumeCommand = `${codexHomeEnvPrefix(codexHome?.trim() || null, platform, shell)}${
|
||||
args.resumeCommand
|
||||
}`
|
||||
if (platform === 'win32' && shell === 'cmd') {
|
||||
@@ -89,7 +89,7 @@ export function buildAiVaultResumeShellCommand(args: {
|
||||
return `cmd /d /s /c ${quoteWindowsCmdArg(inner)}`
|
||||
}
|
||||
|
||||
return `cd ${quoteShellArg(cwd, platform)} && ${resumeCommand}`
|
||||
return `cd ${quoteResumeArg(cwd, platform, shell)} && ${resumeCommand}`
|
||||
}
|
||||
|
||||
function buildResumeShellCommandForShell(args: {
|
||||
@@ -184,14 +184,28 @@ function buildAgentResumeInvocation(
|
||||
}
|
||||
}
|
||||
|
||||
function codexHomeEnvPrefix(codexHome: string | null, platform: NodeJS.Platform): string {
|
||||
function codexHomeEnvPrefix(
|
||||
codexHome: string | null,
|
||||
platform: NodeJS.Platform,
|
||||
shell?: AgentStartupShell
|
||||
): string {
|
||||
if (!codexHome) {
|
||||
return ''
|
||||
}
|
||||
if (platform === 'win32') {
|
||||
return `set ${quoteWindowsCmdArg(`CODEX_HOME=${codexHome}`)} && `
|
||||
}
|
||||
return `CODEX_HOME=${quoteShellArg(codexHome, platform)} `
|
||||
// fish accepts the `NAME=value cmd` prefix (3.1+), but not sh's quoting.
|
||||
return `CODEX_HOME=${quoteResumeArg(codexHome, platform, shell)} `
|
||||
}
|
||||
|
||||
/** Quotes for the live shell when one is known, else for the platform's default. */
|
||||
function quoteResumeArg(
|
||||
value: string,
|
||||
platform: NodeJS.Platform,
|
||||
shell?: AgentStartupShell
|
||||
): string {
|
||||
return shell ? quoteStartupArg(value, shell) : quoteShellArg(value, platform)
|
||||
}
|
||||
|
||||
function quoteShellArg(value: string, platform: NodeJS.Platform): string {
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { tokenizeFishStartupCommand } from './fish-command-tokenizer'
|
||||
import { tokenizeStartupCommand } from './tui-agent-startup-shell'
|
||||
|
||||
function tokens(value: string): string[] {
|
||||
const result = tokenizeFishStartupCommand(value)
|
||||
if (!result.ok) {
|
||||
throw new Error(result.error)
|
||||
}
|
||||
return result.tokens
|
||||
}
|
||||
|
||||
function spanFlags(value: string): boolean[] {
|
||||
const result = tokenizeFishStartupCommand(value)
|
||||
if (!result.ok) {
|
||||
throw new Error(result.error)
|
||||
}
|
||||
return result.spans.map((span) => span.divergesFromShell)
|
||||
}
|
||||
|
||||
describe('tokenizeFishStartupCommand', () => {
|
||||
it('treats a backslash inside single quotes as an escape, unlike sh', () => {
|
||||
expect(tokens(String.raw`'a\\b'`)).toEqual([String.raw`a\b`])
|
||||
expect(tokens(String.raw`'a\'b'`)).toEqual([`a'b`])
|
||||
// Every other backslash stays literal inside single quotes.
|
||||
expect(tokens(String.raw`'a\nb'`)).toEqual([String.raw`a\nb`])
|
||||
})
|
||||
|
||||
it('rejects a trailing backslash before the closing single quote', () => {
|
||||
// fish reads the escaped quote as literal, so the string never closes.
|
||||
expect(tokenizeFishStartupCommand(String.raw`'ends\'`)).toEqual({
|
||||
ok: false,
|
||||
error: 'Unclosed quote in command template.'
|
||||
})
|
||||
})
|
||||
|
||||
it('escapes only \\ $ " and a continuation inside double quotes', () => {
|
||||
expect(tokens(String.raw`"a\\b" "a\$b" "a\"b" "a\nb" "a\zb"`)).toEqual([
|
||||
String.raw`a\b`,
|
||||
'a$b',
|
||||
'a"b',
|
||||
String.raw`a\nb`,
|
||||
String.raw`a\zb`
|
||||
])
|
||||
})
|
||||
|
||||
it('decodes the unquoted escape set fish actually implements', () => {
|
||||
expect(tokens(String.raw`a\nb`)).toEqual(['a\nb'])
|
||||
expect(tokens(String.raw`a\tb`)).toEqual(['a\tb'])
|
||||
expect(tokens(String.raw`\x41 \X41 \101 A \U00000041`)).toEqual(['A', 'A', 'A', 'A', 'A'])
|
||||
expect(tokens(String.raw`\cA`)).toEqual([''])
|
||||
// An unknown escape drops the backslash and keeps the byte.
|
||||
expect(tokens(String.raw`a\zb`)).toEqual(['azb'])
|
||||
})
|
||||
|
||||
it('keeps quoted regions attached to the surrounding token', () => {
|
||||
expect(tokens(`a"b"c'd'e`)).toEqual(['abcde'])
|
||||
expect(tokens(`--flag='has space'`)).toEqual(['--flag=has space'])
|
||||
})
|
||||
|
||||
it('reports an unclosed quote of either kind', () => {
|
||||
expect(tokenizeFishStartupCommand(`a 'b`).ok).toBe(false)
|
||||
expect(tokenizeFishStartupCommand(`a "b`).ok).toBe(false)
|
||||
})
|
||||
|
||||
it('flags live fish syntax a span splice cannot model', () => {
|
||||
// Command substitution, brace expansion and globs are all live in fish; an
|
||||
// unmatched wildcard is a hard error rather than a literal.
|
||||
expect(spanFlags('claude (echo hi)')).toEqual([false, true, true])
|
||||
expect(spanFlags('claude {a,b}')).toEqual([false, true])
|
||||
expect(spanFlags('claude *.ts')).toEqual([false, true])
|
||||
expect(spanFlags('claude a?b')).toEqual([false, true])
|
||||
expect(spanFlags('claude x; y')).toEqual([false, true, false])
|
||||
expect(spanFlags('claude trailing\\')).toEqual([false, true])
|
||||
// Plain words and quoted metacharacters stay modelable.
|
||||
expect(spanFlags(`claude --resume 'a*b' "c;d"`)).toEqual([false, false, false, false])
|
||||
})
|
||||
|
||||
it('is what tokenizeStartupCommand uses for the fish dialect', () => {
|
||||
const line = String.raw`claude --prompt 'match \\d+'`
|
||||
expect(tokenizeStartupCommand(line, 'fish')).toEqual(tokenizeFishStartupCommand(line))
|
||||
// The sh tokenizer keeps both backslashes, which is not what fish does.
|
||||
const posix = tokenizeStartupCommand(line, 'posix')
|
||||
expect(posix.ok && posix.tokens.at(-1)).toBe(String.raw`match \\d+`)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,247 @@
|
||||
import type { CommandTokenSpan } from './commit-message-prompt'
|
||||
|
||||
/**
|
||||
* Splits a command line the way fish does, which is NOT the way sh does.
|
||||
*
|
||||
* fish single quotes are not literal: `\\` and `\'` are escapes inside them, so
|
||||
* a trailing backslash before the closing quote is a syntax error rather than a
|
||||
* literal byte. Unquoted backslashes decode the full C-style escape set
|
||||
* (`\n`, `\t`, `\x41`, `\101`, `A`, `\cA`, …) instead of just protecting the
|
||||
* next byte, and inside double quotes only `\\`, `\$`, `\"` and a line
|
||||
* continuation are escapes — every other backslash stays literal.
|
||||
*
|
||||
* Verified against fish 4.7.1. Feeding these strings through the sh tokenizer
|
||||
* silently drops or invents backslashes, which is how a regex or a UNC path in a
|
||||
* user's CLI-args string gets corrupted before it ever reaches the agent.
|
||||
*/
|
||||
export type FishStartupCommandTokens =
|
||||
| { ok: true; tokens: string[]; spans: CommandTokenSpan[] }
|
||||
| { ok: false; error: string }
|
||||
|
||||
const UNCLOSED_QUOTE_ERROR = 'Unclosed quote in command template.'
|
||||
|
||||
const SIMPLE_ESCAPES: Record<string, string> = {
|
||||
a: '\x07',
|
||||
b: '\b',
|
||||
e: '\x1b',
|
||||
f: '\f',
|
||||
n: '\n',
|
||||
r: '\r',
|
||||
t: '\t',
|
||||
v: '\v',
|
||||
'\\': '\\'
|
||||
}
|
||||
|
||||
type EscapeDecode = { text: string; next: number; diverges: boolean }
|
||||
|
||||
function readRadix(value: string, start: number, pattern: RegExp): string {
|
||||
return pattern.exec(value.slice(start))?.[0] ?? ''
|
||||
}
|
||||
|
||||
/** Decodes the escape whose backslash sits at `start - 1`. */
|
||||
function decodeFishEscape(value: string, start: number): EscapeDecode {
|
||||
const char = value[start]
|
||||
const simple = SIMPLE_ESCAPES[char]
|
||||
if (simple !== undefined) {
|
||||
return { text: simple, next: start + 1, diverges: false }
|
||||
}
|
||||
const fromCode = (digits: string, radix: number, prefix: number): EscapeDecode => {
|
||||
const code = Number.parseInt(digits, radix)
|
||||
// Why: fish rejects an out-of-range escape outright, and a NUL truncates the
|
||||
// token; neither is something this tokenizer's value can represent.
|
||||
if (!Number.isFinite(code) || code === 0 || code > 0x10ffff) {
|
||||
return {
|
||||
text: value.slice(start, start + prefix + digits.length),
|
||||
next: start + prefix + digits.length,
|
||||
diverges: true
|
||||
}
|
||||
}
|
||||
return {
|
||||
text: String.fromCodePoint(code),
|
||||
next: start + prefix + digits.length,
|
||||
diverges: false
|
||||
}
|
||||
}
|
||||
if (char === 'x' || char === 'X') {
|
||||
const digits = readRadix(value, start + 1, /^[0-9a-fA-F]{1,2}/)
|
||||
return digits ? fromCode(digits, 16, 1) : { text: char, next: start + 1, diverges: false }
|
||||
}
|
||||
if (char === 'u' || char === 'U') {
|
||||
const digits = readRadix(
|
||||
value,
|
||||
start + 1,
|
||||
char === 'u' ? /^[0-9a-fA-F]{1,4}/ : /^[0-9a-fA-F]{1,8}/
|
||||
)
|
||||
return digits ? fromCode(digits, 16, 1) : { text: char, next: start + 1, diverges: false }
|
||||
}
|
||||
if (char >= '0' && char <= '7') {
|
||||
const digits = readRadix(value, start, /^[0-7]{1,3}/)
|
||||
// fish caps an octal escape at one byte and errors above it.
|
||||
return Number.parseInt(digits, 8) > 0xff
|
||||
? {
|
||||
text: value.slice(start, start + digits.length),
|
||||
next: start + digits.length,
|
||||
diverges: true
|
||||
}
|
||||
: fromCode(digits, 8, 0)
|
||||
}
|
||||
if (char === 'c' && value[start + 1]) {
|
||||
return {
|
||||
text: String.fromCharCode(value.charCodeAt(start + 1) & 0x1f),
|
||||
next: start + 2,
|
||||
diverges: false
|
||||
}
|
||||
}
|
||||
return { text: char, next: start + 1, diverges: false }
|
||||
}
|
||||
|
||||
export function tokenizeFishStartupCommand(value: string): FishStartupCommandTokens {
|
||||
const tokens: string[] = []
|
||||
const spans: CommandTokenSpan[] = []
|
||||
let token = ''
|
||||
let tokenStart = 0
|
||||
let started = false
|
||||
let diverges = false
|
||||
let index = 0
|
||||
|
||||
const begin = (at: number): void => {
|
||||
if (!started) {
|
||||
tokenStart = at
|
||||
}
|
||||
started = true
|
||||
}
|
||||
|
||||
while (index < value.length) {
|
||||
const char = value[index]
|
||||
|
||||
if (char === "'") {
|
||||
begin(index)
|
||||
index += 1
|
||||
let closed = false
|
||||
while (index < value.length) {
|
||||
const inner = value[index]
|
||||
if (inner === '\\' && (value[index + 1] === '\\' || value[index + 1] === "'")) {
|
||||
token += value[index + 1]
|
||||
index += 2
|
||||
continue
|
||||
}
|
||||
if (inner === "'") {
|
||||
closed = true
|
||||
index += 1
|
||||
break
|
||||
}
|
||||
token += inner
|
||||
index += 1
|
||||
}
|
||||
if (!closed) {
|
||||
return { ok: false, error: UNCLOSED_QUOTE_ERROR }
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
if (char === '"') {
|
||||
begin(index)
|
||||
index += 1
|
||||
let closed = false
|
||||
while (index < value.length) {
|
||||
const inner = value[index]
|
||||
if (inner === '\\') {
|
||||
const next = value[index + 1]
|
||||
if (next === '\\' || next === '$' || next === '"') {
|
||||
token += next
|
||||
index += 2
|
||||
continue
|
||||
}
|
||||
if (next === '\n') {
|
||||
// A continuation joins words the caller's span splice cannot model.
|
||||
diverges = true
|
||||
index += 2
|
||||
continue
|
||||
}
|
||||
if (next === undefined) {
|
||||
break
|
||||
}
|
||||
token += inner
|
||||
index += 1
|
||||
continue
|
||||
}
|
||||
if (inner === '"') {
|
||||
closed = true
|
||||
index += 1
|
||||
break
|
||||
}
|
||||
// Why: `$(…)` and `${…}` reopen a nested parsing context this loop does not model.
|
||||
diverges ||= inner === '$' && '({'.includes(value[index + 1] ?? '\0')
|
||||
token += inner
|
||||
index += 1
|
||||
}
|
||||
if (!closed) {
|
||||
return { ok: false, error: UNCLOSED_QUOTE_ERROR }
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
if (char === '\\') {
|
||||
const next = value[index + 1]
|
||||
if (next === undefined) {
|
||||
// A trailing backslash swallows whatever a consumer appends after the base.
|
||||
diverges = true
|
||||
begin(index)
|
||||
token += char
|
||||
index += 1
|
||||
continue
|
||||
}
|
||||
if (next === '\n') {
|
||||
diverges = true
|
||||
begin(index)
|
||||
index += 2
|
||||
continue
|
||||
}
|
||||
begin(index)
|
||||
const decoded = decodeFishEscape(value, index + 1)
|
||||
diverges ||= decoded.diverges
|
||||
token += decoded.text
|
||||
index = decoded.next
|
||||
continue
|
||||
}
|
||||
|
||||
if (/\s/.test(char)) {
|
||||
if (started) {
|
||||
tokens.push(token)
|
||||
spans.push({
|
||||
start: tokenStart,
|
||||
end: index,
|
||||
divergesFromShell: diverges
|
||||
})
|
||||
token = ''
|
||||
started = false
|
||||
diverges = false
|
||||
}
|
||||
index += 1
|
||||
continue
|
||||
}
|
||||
|
||||
const atTokenStart = !started
|
||||
begin(index)
|
||||
diverges ||=
|
||||
';&|<>'.includes(char) ||
|
||||
// Command substitution `(…)`/`$(…)` and brace expansion, all live fish syntax.
|
||||
'(){}'.includes(char) ||
|
||||
// An unmatched wildcard is a HARD ERROR in fish, and a matched one splits the word.
|
||||
'*?['.includes(char) ||
|
||||
(char === '#' && atTokenStart) ||
|
||||
(char === '$' && '({'.includes(value[index + 1] ?? '\0'))
|
||||
token += char
|
||||
index += 1
|
||||
}
|
||||
|
||||
if (started) {
|
||||
tokens.push(token)
|
||||
spans.push({
|
||||
start: tokenStart,
|
||||
end: value.length,
|
||||
divergesFromShell: diverges
|
||||
})
|
||||
}
|
||||
return { ok: true, tokens, spans }
|
||||
}
|
||||
@@ -0,0 +1,231 @@
|
||||
/**
|
||||
* Round-trips `quoteStartupArg(value, 'fish')` and `tokenizeStartupCommand(…, 'fish')`
|
||||
* through a REAL fish, because fish's quoting is only observable in fish.
|
||||
*
|
||||
* fish single quotes are not literal the way sh's are: `\\` and `\'` are escapes,
|
||||
* so sh's `'\''` idiom silently halves every backslash and a trailing backslash is
|
||||
* a hard syntax error. A string assertion against a hand-written expectation would
|
||||
* happily encode the same wrong rule, so the shell itself is the oracle.
|
||||
*
|
||||
* fish reads each case from a script file and echoes its argv back NUL-separated
|
||||
* (`string join0`), which is the only separator no test input can forge.
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { chmodSync, copyFileSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
import { fishRequirementViolation, resolveFishBinary } from './fish-binary-requirement'
|
||||
import { planHermesStartupQuery } from './hermes-startup-query'
|
||||
import { buildAgentStartupPlan } from './tui-agent-startup'
|
||||
import { quoteStartupArg, tokenizeStartupCommand } from './tui-agent-startup-shell'
|
||||
|
||||
const FISH = resolveFishBinary(4)
|
||||
const itWithFish = FISH.available ? it : it.skip
|
||||
|
||||
let home: string | null = null
|
||||
|
||||
/** Runs `commandLine` in a config-free fish and returns the argv it produced. */
|
||||
function fishArgv(commandLine: string): string[] {
|
||||
const root = home as string
|
||||
const scriptPath = path.join(root, 'probe.fish')
|
||||
const outPath = path.join(root, 'argv.bin')
|
||||
// Removed first so a run that dies before writing surfaces as ENOENT rather
|
||||
// than as the previous case's argv. `exit 0` only masks `string join`'s
|
||||
// nothing-to-join status 1; a syntax error still aborts the file with 127.
|
||||
rmSync(outPath, { force: true })
|
||||
writeFileSync(scriptPath, `string join0 -- ${commandLine} > ${outPath}\nexit 0\n`)
|
||||
execFileSync(FISH.path as string, [scriptPath], {
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
env: {
|
||||
PATH: process.env.PATH ?? '/usr/bin:/bin',
|
||||
HOME: root,
|
||||
XDG_CONFIG_HOME: root,
|
||||
XDG_DATA_HOME: path.join(root, 'data'),
|
||||
TERM: 'dumb',
|
||||
LANG: 'en_US.UTF-8'
|
||||
}
|
||||
})
|
||||
const raw = readFileSync(outPath, 'utf8')
|
||||
// join0 terminates the last element too, so the trailing empty split is not an argument.
|
||||
return raw.length === 0 ? [] : raw.split('\0').slice(0, -1)
|
||||
}
|
||||
|
||||
const ADVERSARIAL_ARGS: readonly [name: string, value: string][] = [
|
||||
['regex backslashes', 'use \\d+ or \\\\d and \\w'],
|
||||
['UNC path', '\\\\server\\share\\dir'],
|
||||
['windows drive path', 'C:\\Users\\nw\\Application Data'],
|
||||
['apostrophes', "it's the user's own prompt"],
|
||||
['trailing backslash', 'ends with a backslash\\'],
|
||||
['lone backslash', '\\'],
|
||||
['mixed quotes', `he said "hi" and 'bye'`],
|
||||
['newlines', 'first line\nsecond line\n'],
|
||||
['dollar expansions', '$PATH and ${HOME} and $(id -u)'],
|
||||
['glob characters', '*.ts ?? [a-z] **/*.zzz'],
|
||||
['command substitution parens', 'call (echo hi) and `echo hi`'],
|
||||
['operators', 'a; b && c || d | e > f < g & h'],
|
||||
['comment and tilde', '# not a comment ~ not a home'],
|
||||
['tabs and unicode', 'tab\there 🐟 é'],
|
||||
['escaped-quote soup', `a\\'b'c\\\\d"e`]
|
||||
]
|
||||
|
||||
describe('quoteStartupArg round-trips through real fish', () => {
|
||||
beforeAll(() => {
|
||||
home = mkdtempSync(path.join(tmpdir(), 'orca-fish-quote-'))
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
if (home) {
|
||||
rmSync(home, { recursive: true, force: true })
|
||||
home = null
|
||||
}
|
||||
})
|
||||
|
||||
// Always runs, so the CI lane cannot report green with everything below skipped.
|
||||
it('has the fish this suite needs when CI requires one', () => {
|
||||
expect(fishRequirementViolation(FISH)).toBeNull()
|
||||
})
|
||||
|
||||
for (const [name, value] of ADVERSARIAL_ARGS) {
|
||||
itWithFish(`survives ${name}`, () => {
|
||||
expect(fishArgv(quoteStartupArg(value, 'fish'))).toEqual([value])
|
||||
})
|
||||
}
|
||||
|
||||
itWithFish('keeps multiple arguments separate', () => {
|
||||
const args = ADVERSARIAL_ARGS.map(([, value]) => value)
|
||||
expect(fishArgv(args.map((arg) => quoteStartupArg(arg, 'fish')).join(' '))).toEqual(args)
|
||||
})
|
||||
|
||||
itWithFish('proves the sh quoting this replaced was broken in fish', () => {
|
||||
// Corrupted, not rejected: the backslashes silently halve.
|
||||
expect(fishArgv(quoteStartupArg('\\\\server\\share', 'posix'))).toEqual(['\\server\\share'])
|
||||
// And a trailing backslash is a hard syntax error that would kill the launch.
|
||||
expect(() => fishArgv(quoteStartupArg('ends with a backslash\\', 'posix'))).toThrow()
|
||||
})
|
||||
|
||||
itWithFish('proves the sh tokenizer this replaced disagreed with fish', () => {
|
||||
const line = String.raw`claude --prompt 'match \d+ in \\server\share'`
|
||||
const posix = tokenizeStartupCommand(line, 'posix')
|
||||
expect(posix.ok).toBe(true)
|
||||
if (posix.ok) {
|
||||
expect(posix.tokens).not.toEqual(fishArgv(line))
|
||||
}
|
||||
})
|
||||
|
||||
itWithFish('tokenizes a fish command line the way fish splits it', () => {
|
||||
const lines = [
|
||||
String.raw`claude --append-system-prompt 'match \d+ in \\server\share'`,
|
||||
String.raw`codex --cd "C:\Users\nw" --note 'it\'s mine'`,
|
||||
String.raw`hermes chat --query a\nb --flag "keep \n literal"`,
|
||||
String.raw`pi --path 'C:\\' --other "a\$b" --third x\ty`,
|
||||
String.raw`agent --hex \x41 --octal \101 --ctrl \cA`
|
||||
]
|
||||
for (const line of lines) {
|
||||
const tokenized = tokenizeStartupCommand(line, 'fish')
|
||||
expect(tokenized.ok, line).toBe(true)
|
||||
if (!tokenized.ok) {
|
||||
continue
|
||||
}
|
||||
expect(tokenized.tokens, line).toEqual(fishArgv(line))
|
||||
// Re-quoting the parsed tokens must reproduce the same argv.
|
||||
expect(
|
||||
fishArgv(tokenized.tokens.map((token) => quoteStartupArg(token, 'fish')).join(' ')),
|
||||
line
|
||||
).toEqual(tokenized.tokens)
|
||||
}
|
||||
})
|
||||
|
||||
// The user-facing break: a composer prompt is quoted here and parsed by fish.
|
||||
itWithFish('delivers a composer prompt to the agent argv intact', () => {
|
||||
const root = home as string
|
||||
const capturePath = path.join(root, 'claude-argv.json')
|
||||
const claudePath = path.join(root, 'claude')
|
||||
const prompt = [
|
||||
String.raw`Use \d+ for digits and read \\server\share\notes.`,
|
||||
String.raw`Keep 'single' and "double" quotes, $PATH, *.ts and a ; too.`,
|
||||
'This last line ends in a backslash\\'
|
||||
].join('\n')
|
||||
copyFileSync(process.execPath, claudePath)
|
||||
chmodSync(claudePath, 0o755)
|
||||
writeFileSync(
|
||||
path.join(root, 'capture.js'),
|
||||
`require('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)))`
|
||||
)
|
||||
rmSync(capturePath, { force: true })
|
||||
|
||||
const plan = buildAgentStartupPlan({
|
||||
agent: 'claude',
|
||||
prompt,
|
||||
cmdOverrides: { claude: `${quoteStartupArg(claudePath, 'fish')} capture.js` },
|
||||
platform: 'darwin',
|
||||
shell: 'fish'
|
||||
})
|
||||
expect(plan).not.toBeNull()
|
||||
|
||||
const scriptPath = path.join(root, 'launch.fish')
|
||||
writeFileSync(scriptPath, `${plan?.launchCommand}\n`)
|
||||
execFileSync(FISH.path as string, [scriptPath], {
|
||||
cwd: root,
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
env: {
|
||||
PATH: process.env.PATH ?? '/usr/bin:/bin',
|
||||
HOME: root,
|
||||
XDG_CONFIG_HOME: root,
|
||||
XDG_DATA_HOME: path.join(root, 'data'),
|
||||
TERM: 'dumb',
|
||||
LANG: 'en_US.UTF-8'
|
||||
}
|
||||
})
|
||||
|
||||
expect(JSON.parse(readFileSync(capturePath, 'utf8'))).toEqual([prompt])
|
||||
})
|
||||
|
||||
// The `sh -c` wrapper's OUTER quoting is parsed by fish, and its payload is a
|
||||
// solid run of `\0NNN` octal escapes — the single worst case for the sh spelling.
|
||||
itWithFish('delivers a hermes startup query through the fish-quoted sh wrapper', () => {
|
||||
const root = home as string
|
||||
const capturePath = path.join(root, 'hermes-argv.json')
|
||||
const hermesPath = path.join(root, 'hermes')
|
||||
const prompt = [
|
||||
String.raw`Match \d+ inside \\server\share and keep 'quotes' plus "doubles".`,
|
||||
'A blank $PATH stays literal, as does *.ts.',
|
||||
'And this line ends with a backslash\\'
|
||||
].join('\n')
|
||||
// A copy of node named `hermes` runs the `chat` script and records its argv.
|
||||
copyFileSync(process.execPath, hermesPath)
|
||||
chmodSync(hermesPath, 0o755)
|
||||
writeFileSync(
|
||||
path.join(root, 'chat'),
|
||||
`require('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)))`
|
||||
)
|
||||
rmSync(capturePath, { force: true })
|
||||
|
||||
const plan = planHermesStartupQuery({
|
||||
baseCommand: quoteStartupArg(hermesPath, 'fish'),
|
||||
agentArgs: '--yolo',
|
||||
prompt,
|
||||
platform: 'darwin',
|
||||
shell: 'fish'
|
||||
})
|
||||
expect(plan).not.toBeNull()
|
||||
|
||||
const scriptPath = path.join(root, 'hermes.fish')
|
||||
writeFileSync(scriptPath, `${plan?.command}\n`)
|
||||
execFileSync(FISH.path as string, [scriptPath], {
|
||||
cwd: root,
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
env: {
|
||||
PATH: process.env.PATH ?? '/usr/bin:/bin',
|
||||
HOME: root,
|
||||
XDG_CONFIG_HOME: root,
|
||||
XDG_DATA_HOME: path.join(root, 'data'),
|
||||
TERM: 'dumb',
|
||||
LANG: 'en_US.UTF-8',
|
||||
...plan?.env
|
||||
}
|
||||
})
|
||||
|
||||
expect(JSON.parse(readFileSync(capturePath, 'utf8'))).toContain(`--query=${prompt}`)
|
||||
})
|
||||
})
|
||||
@@ -159,7 +159,11 @@ function buildQueryCommand(argv: string[], shell: AgentStartupShell): string {
|
||||
// Why: a fixed single-quote-safe wrapper parses in POSIX shells and pwsh;
|
||||
// the dynamic argv is decoded only after entering the known `sh` grammar.
|
||||
const script = `${POSIX_QUERY_VARIABLE}="\${${ORCA_HERMES_STARTUP_QUERY_ENV}}"; unset ${ORCA_HERMES_STARTUP_QUERY_ENV}; eval "$(printf %b "${encodedInvocation}")"`
|
||||
return `sh -c ${quoteStartupArg(script, 'posix')}`
|
||||
// Why `shell` and not 'posix': the body runs under `sh`, but the quoting around
|
||||
// it is parsed by the shell that types the line. Today's payload survives sh
|
||||
// quoting in fish only because its escapes happen to be `\0NNN` and never `\\`
|
||||
// — quote for the real dialect so that stays an implementation detail.
|
||||
return `sh -c ${quoteStartupArg(script, shell)}`
|
||||
}
|
||||
|
||||
export function planHermesStartupQuery(args: {
|
||||
@@ -181,7 +185,10 @@ export function planHermesStartupQuery(args: {
|
||||
return null
|
||||
}
|
||||
const command = buildQueryCommand(argv, args.shell)
|
||||
const env = { ...args.agentEnv, [ORCA_HERMES_STARTUP_QUERY_ENV]: args.prompt }
|
||||
const env = {
|
||||
...args.agentEnv,
|
||||
[ORCA_HERMES_STARTUP_QUERY_ENV]: args.prompt
|
||||
}
|
||||
const envSize = Object.entries(env).reduce((total, [key, value]) => {
|
||||
if (args.platform === 'win32') {
|
||||
return total + key.length + value.length + 2
|
||||
|
||||
@@ -76,18 +76,43 @@ describe('fish startup shell dialect', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('shares POSIX quoting, tokenizing and chaining', () => {
|
||||
expect(quoteStartupArg("it's", 'fish')).toBe(quoteStartupArg("it's", 'posix'))
|
||||
it('shares POSIX word splitting and chaining but NOT quoting', () => {
|
||||
// fish single quotes are not literal: `\\` and `\'` are escapes inside them.
|
||||
expect(quoteStartupArg("it's", 'fish')).toBe(String.raw`'it\'s'`)
|
||||
expect(quoteStartupArg(String.raw`\\server\share`, 'fish')).toBe(
|
||||
String.raw`'\\\\server\\share'`
|
||||
)
|
||||
// The sh spelling would have halved those backslashes when fish read it back.
|
||||
expect(quoteStartupArg(String.raw`\\server\share`, 'posix')).toBe(String.raw`'\\server\share'`)
|
||||
// A trailing backslash is a hard syntax error in fish unless it is escaped.
|
||||
expect(quoteStartupArg('ends\\', 'fish')).toBe(String.raw`'ends\\'`)
|
||||
expect(buildShellCommandFromArgv(['codex', 'resume', 'a b'], 'fish')).toBe(
|
||||
buildShellCommandFromArgv(['codex', 'resume', 'a b'], 'posix')
|
||||
)
|
||||
expect(tokenizeStartupCommand('codex --arg "a b"', 'fish')).toEqual(
|
||||
tokenizeStartupCommand('codex --arg "a b"', 'posix')
|
||||
)
|
||||
expect(commandSeparator('fish')).toBe('; ')
|
||||
expect(isPosixStartupShell('fish')).toBe(true)
|
||||
})
|
||||
|
||||
it('tokenizes with fish escape rules rather than sh ones', () => {
|
||||
const line = String.raw`codex --arg 'a\\b'`
|
||||
expect(tokenizeStartupCommand(line, 'fish')).not.toEqual(tokenizeStartupCommand(line, 'posix'))
|
||||
const fishTokens = tokenizeStartupCommand(line, 'fish')
|
||||
expect(fishTokens.ok && fishTokens.tokens.at(-1)).toBe(String.raw`a\b`)
|
||||
})
|
||||
|
||||
it('round-trips an adversarial argument through quote then tokenize', () => {
|
||||
for (const value of [
|
||||
String.raw`use \d+ and \\server\share`,
|
||||
"it's mine",
|
||||
'ends\\',
|
||||
'a "b" c',
|
||||
'$PATH *.ts'
|
||||
]) {
|
||||
const tokenized = tokenizeStartupCommand(quoteStartupArg(value, 'fish'), 'fish')
|
||||
expect(tokenized.ok && tokenized.tokens).toEqual([value])
|
||||
}
|
||||
})
|
||||
|
||||
it('maps login shell paths to their dialect', () => {
|
||||
expect(resolveLoginShellStartupDialect('/opt/homebrew/bin/fish')).toBe('fish')
|
||||
expect(resolveLoginShellStartupDialect('/usr/local/bin/FISH')).toBe('fish')
|
||||
|
||||
@@ -1,12 +1,20 @@
|
||||
import { tokenizeCustomCommandTemplate, type CommandTokenSpan } from './commit-message-prompt'
|
||||
import { tokenizeFishStartupCommand } from './fish-command-tokenizer'
|
||||
|
||||
// Why: fish shares POSIX word splitting, quoting and `;` chaining, so it is a
|
||||
// separate dialect only where its grammar actually diverges (env clearing).
|
||||
// Why: fish shares POSIX word splitting and `;` chaining, but NOT quoting or
|
||||
// escaping — see quoteStartupArg and fish-command-tokenizer.ts.
|
||||
export type AgentStartupShell = 'posix' | 'fish' | 'powershell' | 'cmd'
|
||||
|
||||
type WindowsStartupShell = Extract<AgentStartupShell, 'powershell' | 'cmd'>
|
||||
|
||||
/** True for shells parsed with POSIX quoting/word rules (sh family + fish). */
|
||||
/**
|
||||
* True for the sh-family grammar: `NAME=value cmd` prefixes, `sh -c` wrapping and
|
||||
* `;` chaining all parse. fish qualifies for those.
|
||||
*
|
||||
* NOT a claim that quoting matches — fish's does not. Anything that builds an
|
||||
* argument must pass the real dialect to `quoteStartupArg`/`tokenizeStartupCommand`,
|
||||
* never hardcode `'posix'` behind this check.
|
||||
*/
|
||||
export function isPosixStartupShell(shell: AgentStartupShell): boolean {
|
||||
return shell === 'posix' || shell === 'fish'
|
||||
}
|
||||
@@ -154,9 +162,10 @@ export function tokenizeStartupCommand(
|
||||
value: string,
|
||||
shell: AgentStartupShell
|
||||
): StartupCommandTokens {
|
||||
return isWindowsStartupShell(shell)
|
||||
? tokenizeWindowsStartupCommand(value, shell)
|
||||
: tokenizeCustomCommandTemplate(value)
|
||||
if (isWindowsStartupShell(shell)) {
|
||||
return tokenizeWindowsStartupCommand(value, shell)
|
||||
}
|
||||
return shell === 'fish' ? tokenizeFishStartupCommand(value) : tokenizeCustomCommandTemplate(value)
|
||||
}
|
||||
|
||||
export function resolveStartupShell(
|
||||
@@ -173,6 +182,14 @@ export function quoteStartupArg(value: string, shell: AgentStartupShell): string
|
||||
if (shell === 'cmd') {
|
||||
return `"${value.replace(/([\^&|<>()%!"])/g, '^$1')}"`
|
||||
}
|
||||
// Why: fish single quotes are NOT literal. `\\` and `\'` are escapes inside
|
||||
// them, so the sh `'\''` idiom collapses every backslash (a `\d+` regex, a
|
||||
// `\\server\share` UNC path) and a trailing backslash is a hard syntax error
|
||||
// that kills the launch. Escaping both metacharacters round-trips instead;
|
||||
// verified against fish 4.7.1.
|
||||
if (shell === 'fish') {
|
||||
return `'${value.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`
|
||||
}
|
||||
return `'${value.replace(/'/g, `'\\''`)}'`
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user