From 27cf3096a05aae92c5feed21893aee75026340df Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 02:10:00 -0700 Subject: [PATCH] test(fish): grade the teardown control on status, not on fish's error text Co-authored-by: Orca --- ...inal-history-fish-session.node-pty.test.ts | 2 + ...sh-draft-prefill-teardown.node-pty.test.ts | 48 +++++++++++++------ ...fish-startup-arg-quoting.live-fish.test.ts | 42 +++++++--------- 3 files changed, 53 insertions(+), 39 deletions(-) diff --git a/src/main/terminal-history-fish-session.node-pty.test.ts b/src/main/terminal-history-fish-session.node-pty.test.ts index 7be40ffa9e3..a317d77904c 100644 --- a/src/main/terminal-history-fish-session.node-pty.test.ts +++ b/src/main/terminal-history-fish-session.node-pty.test.ts @@ -85,7 +85,9 @@ describe('fish keeps per-worktree history under the session Orca names', () => { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: home, TERM: 'xterm-256color', + // LC_ALL wins over any LANG/LC_* a host might contribute, pinning fish's locale. LANG: 'en_US.UTF-8', + LC_ALL: 'en_US.UTF-8', XDG_CONFIG_HOME: home, XDG_DATA_HOME: dataHome, // The production injection under test (terminal-history.ts). diff --git a/src/shared/fish-draft-prefill-teardown.node-pty.test.ts b/src/shared/fish-draft-prefill-teardown.node-pty.test.ts index e5f676d313e..16082f12029 100644 --- a/src/shared/fish-draft-prefill-teardown.node-pty.test.ts +++ b/src/shared/fish-draft-prefill-teardown.node-pty.test.ts @@ -6,6 +6,14 @@ * no `unset`: the sh spelling errors out and leaves the variable exported, which is * only observable in the shell — a string assertion cannot tell the two apart. * + * The negative control is graded on exit status and variable liveness, never on the + * error text. fish only installs its own "Unknown command" printer when NON-interactive + * (share/config.fish); an interactive fish autoloads `fish_command_not_found`, which + * delegates to the distro handler when one exists — `/usr/lib/command-not-found` on + * Ubuntu, where the CI fish comes from. That handler prints its own wording, and the + * fish fallback is gettext-translated on top of that, so the text varies by host and + * locale while the status (127) and the surviving variable do not. + * * DA1/CPR/OSC-11 probes are answered here because no real xterm is attached; * without them fish stalls ~10s on its DA1 read sentinel before the first prompt. */ @@ -76,7 +84,10 @@ describe('fish clears an agent draft prefill variable', () => { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: home, TERM: 'xterm-256color', + // LC_ALL wins over any LANG/LC_* a host might otherwise contribute, so + // fish's locale is fixed even though nothing here reads its messages. LANG: 'en_US.UTF-8', + LC_ALL: 'en_US.UTF-8', XDG_CONFIG_HOME: home, XDG_DATA_HOME: path.join(home, 'data'), [VAR]: 'draft text' @@ -101,28 +112,37 @@ describe('fish clears an agent draft prefill variable', () => { exited = true }) - // `set -q` exits 0 while the variable exists, so the echoed status is the verdict. - const probe = (label: string): Promise => { - term.write(`set -q ${VAR}; echo ${label}=$status\r`) - return waitUntil(() => new RegExp(`${label}=[01]`).test(rendered), 5_000) + // \b so a label is never read out of a longer one (`FISHRC` vs `SHRC`), and the + // echoed keystrokes (`...=$status`) can never satisfy the digit capture. + const statusPattern = (label: string): RegExp => new RegExp(String.raw`\b${label}=(\d+)`) + /** Runs one line and labels the `$status` its LAST statement left behind. */ + // Generous: an unknown command routes through the distro `fish_command_not_found`, + // and Ubuntu's is a Python process that opens the apt database before fish moves on. + const run = (label: string, command: string): Promise => { + term.write(`${command}; echo ${label}=$status\r`) + return waitUntil(() => statusPattern(label).test(rendered), 15_000) } const statusOf = (label: string): string => - new RegExp(`${label}=([01])`).exec(rendered)?.[1] ?? 'missing' + statusPattern(label).exec(rendered)?.[1] ?? 'missing' + // `set -q` exits 0 while the variable exists, so its status is the liveness verdict. + const liveness = (label: string): Promise => run(label, `set -q ${VAR}`) try { expect(await waitUntil(() => rendered.includes(PROMPT_MARK), 15_000)).toBe(true) - expect(await probe('BEFORE')).toBe(true) + expect(await liveness('BEFORE')).toBe(true) expect(statusOf('BEFORE')).toBe('0') - // The sh spelling Orca emitted before this dialect was threaded through. - term.write(`${clearEnvCommand(VAR, 'posix')}\r`) - expect(await probe('AFTERSH')).toBe(true) - expect(rendered).toContain('Unknown command') - expect(statusOf('AFTERSH')).toBe('0') + // The sh spelling Orca emitted before this dialect was threaded through: fish + // must reject the command outright AND leave the export standing. + expect(await run('POSIXRC', clearEnvCommand(VAR, 'posix'))).toBe(true) + expect(statusOf('POSIXRC')).not.toBe('0') + expect(await liveness('POSIXLEFT')).toBe(true) + expect(statusOf('POSIXLEFT')).toBe('0') - term.write(`${clearEnvCommand(VAR, 'fish')}\r`) - expect(await probe('AFTERFISH')).toBe(true) - expect(statusOf('AFTERFISH')).toBe('1') + expect(await run('FISHRC', clearEnvCommand(VAR, 'fish'))).toBe(true) + expect(statusOf('FISHRC')).toBe('0') + expect(await liveness('FISHLEFT')).toBe(true) + expect(statusOf('FISHLEFT')).toBe('1') } finally { term.write('exit\r') await waitUntil(() => exited, 5_000) diff --git a/src/shared/fish-startup-arg-quoting.live-fish.test.ts b/src/shared/fish-startup-arg-quoting.live-fish.test.ts index 707b81ef1bd..6ed1e3ddf4f 100644 --- a/src/shared/fish-startup-arg-quoting.live-fish.test.ts +++ b/src/shared/fish-startup-arg-quoting.live-fish.test.ts @@ -25,6 +25,20 @@ const itWithFish = FISH.available ? it : it.skip let home: string | null = null +/** + * Fully pinned: nothing ambient reaches fish, and LC_ALL wins over any LANG/LC_* + * a host might contribute, so locale-sensitive parsing cannot vary by machine. + */ +const fishEnv = (root: string): NodeJS.ProcessEnv => ({ + 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', + LC_ALL: 'en_US.UTF-8' +}) + /** Runs `commandLine` in a config-free fish and returns the argv it produced. */ function fishArgv(commandLine: string): string[] { const root = home as string @@ -37,14 +51,7 @@ function fishArgv(commandLine: string): string[] { 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' - } + env: fishEnv(root) }) const raw = readFileSync(outPath, 'utf8') // join0 terminates the last element too, so the trailing empty split is not an argument. @@ -168,14 +175,7 @@ describe('quoteStartupArg round-trips through real fish', () => { 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' - } + env: fishEnv(root) }) expect(JSON.parse(readFileSync(capturePath, 'utf8'))).toEqual([prompt]) @@ -215,15 +215,7 @@ describe('quoteStartupArg round-trips through real fish', () => { 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 - } + env: { ...fishEnv(root), ...plan?.env } }) expect(JSON.parse(readFileSync(capturePath, 'utf8'))).toContain(`--query=${prompt}`)