fix(fish): restore agent homes and the attribution PATH shim after config.fish

fish gets no wrapper rc file, so the only hook that runs after the user's own
startup files is `--init-command`. Both halves of the POSIX wrapper contract
were missing there:

- a config.fish that sets its own CODEX_HOME/OPENCODE_CONFIG_DIR/MIMOCODE_HOME
  beat Orca's routed home, so agents launched against the wrong account and the
  account switcher looked like a no-op
- macOS fish's bundled config.fish runs path_helper, which rebuilds PATH and
  demotes Orca's prepended attribution shim to near-last, so commits made in
  fish panes lost the Orca trailer / PR footer

Fold both into the single `-C` string that already carries the shell-ready
marker (multi-`-C` support varies by fish version), and give it to the
markerless attribution path too — that is the pane where a user types
`git commit` by hand. The marker itself stays gated on ORCA_SHELL_READY_MARKER.
This commit is contained in:
Neil
2026-08-12 18:49:15 -07:00
parent f1ee3088a5
commit 7821f222de
8 changed files with 253 additions and 18 deletions
+2
View File
@@ -246,6 +246,7 @@ jobs:
src/main/providers/local-pty-shell-ready.test.ts \
src/main/providers/__tests__/shell-ready-framework-example.test.ts \
src/main/pty/omp-shell-wrapper.node-pty.test.ts \
src/main/fish-startup-environment.node-pty.test.ts \
src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
src/shared/posix-command-path-lookup.test.ts
@@ -282,6 +283,7 @@ jobs:
--exclude=src/main/providers/local-pty-shell-ready.test.ts \
--exclude=src/main/providers/__tests__/shell-ready-framework-example.test.ts \
--exclude=src/main/pty/omp-shell-wrapper.node-pty.test.ts \
--exclude=src/main/fish-startup-environment.node-pty.test.ts \
--exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
--exclude=src/shared/posix-command-path-lookup.test.ts \
--exclude=tests/e2e/cross-version-wire/** \
@@ -16,7 +16,8 @@ const shellContractFiles = [
]
const patchedNodePtyContractFiles = [
'src/main/daemon/node-pty-fd-leak.test.ts',
'src/main/pty/omp-shell-wrapper.node-pty.test.ts'
'src/main/pty/omp-shell-wrapper.node-pty.test.ts',
'src/main/fish-startup-environment.node-pty.test.ts'
]
const nativeShellContractFiles = [...shellContractFiles, ...patchedNodePtyContractFiles]
const testFilePatterns = [
+12 -2
View File
@@ -258,12 +258,22 @@ describePosix('daemon shell-ready launch config', () => {
expect(init).toContain('functions -e __orca_shell_ready_marker')
})
it('keeps attribution-only fish spawns unwrapped', async () => {
it('gives attribution-only fish the shim/agent-home init command without the marker', async () => {
// Why: the markerless pane is where the user types commits by hand, so it needs the
// PATH shim most; only the marker stays gated so nothing is emitted into the pane.
const { getAttributionShellLaunchConfig } = await importFreshShellReady()
const config = getAttributionShellLaunchConfig('/opt/homebrew/bin/fish')
expect(config).toEqual({ args: null, env: {}, supportsReadyMarker: false })
expect(config.supportsReadyMarker).toBe(false)
expect(config.env).toEqual({ ORCA_SHELL_READY_MARKER: '0' })
expect(config.args?.slice(0, 2)).toEqual(['-l', '-C'])
const init = config.args?.[2] ?? ''
expect(init).toContain('set -gx PATH "$__orca_shim_dir" $PATH')
expect(init).toContain('set -gx OPENCODE_CONFIG_DIR "$ORCA_OPENCODE_CONFIG_DIR"')
expect(init).toContain('set -gx MIMOCODE_HOME "$ORCA_MIMOCODE_HOME"')
expect(init).toContain('set -gx CODEX_HOME "$ORCA_CODEX_HOME"')
expect(init).toContain('if test "$ORCA_SHELL_READY_MARKER" = 1')
})
itWithFish(
+8 -6
View File
@@ -12,7 +12,7 @@ import {
} from '../powershell-osc133-bootstrap'
import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper'
import {
getFishShellReadyInitCommand,
getFishInitCommand,
getZshEnvTemplate,
getZshFinalZdotdirRestoreBlock,
getZshShellReadyMarkerRegistrationBlock,
@@ -419,12 +419,14 @@ function getWrappedShellLaunchConfig(
}
}
// Why: mirrors local-pty-shell-ready.ts; attribution-only fish stays unwrapped.
if (shellName === 'fish' && options.emitReadyMarker) {
// Why: mirrors local-pty-shell-ready.ts. Markerless fish still needs the init command —
// that is the pane where the user types commits by hand, so the shim and agent homes
// must survive config.fish there too; only the marker itself stays gated.
if (shellName === 'fish') {
return {
args: ['-l', '-C', getFishShellReadyInitCommand(SHELL_READY_MARKER)],
env: { ORCA_SHELL_READY_MARKER: '1' },
supportsReadyMarker: true
args: ['-l', '-C', getFishInitCommand(SHELL_READY_MARKER)],
env: { ORCA_SHELL_READY_MARKER: options.emitReadyMarker ? '1' : '0' },
supportsReadyMarker: options.emitReadyMarker
}
}
@@ -0,0 +1,175 @@
/**
* Real-fish coverage for the `-C` init command Orca hands every fish PTY.
*
* fish has no ZDOTDIR-style wrapper dir, so the only hook that runs AFTER the user's
* config.fish is `--init-command`. Two things depend on it:
* - Orca's routed agent home must beat a `set -gx CODEX_HOME ...` in config.fish,
* or the account switcher silently launches the agent against the wrong account.
* - the attribution shim must be FIRST in PATH, or commits from fish panes lose the
* Orca trailer. macOS fish's bundled config.fish runs /usr/libexec/path_helper,
* which rebuilds PATH and demotes the inherited shim to near-last.
*
* Both are asserted against a control spawn with the init command removed, so a
* regression that makes the init command a no-op cannot pass this file.
*/
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { describe, expect, it } from 'vitest'
import { getAttributionShellLaunchConfig } from './daemon/shell-ready'
import { getAttributionShellLaunchConfig as getLocalPtyAttributionShellLaunchConfig } from './providers/local-pty-shell-ready'
import { fishRequirementViolation, resolveFishBinary } from '../shared/fish-binary-requirement'
const FISH = resolveFishBinary()
const itWithFish = FISH.available ? it : it.skip
/** Minimal xterm-shaped answers to the probes fish blocks its first prompt on
* (~10s on the DA1 wait alone), so this suite settles in milliseconds. */
const TERMINAL_QUERY_REPLIES: readonly (readonly [string, string])[] = [
['\x1b[0c', '\x1b[?6c'],
['\x1b[?u', '\x1b[?0u'],
['\x1b[6n', '\x1b[1;1R'],
['\x1b]11;?', '\x1b]11;rgb:0000/0000/0000\x1b\\']
]
const QUERY_CARRY_LEN = Math.max(...TERMINAL_QUERY_REPLIES.map(([query]) => query.length))
const SHIM_DIR = '/orca-test/attribution-shim'
const USER_BIN_DIR = '/orca-test/user-bin'
const ORCA_CODEX_HOME = '/orca-test/routed-codex-home'
const USER_CODEX_HOME = '/orca-test/user-codex-home'
/**
* Spawns fish on a real PTY with the given args and returns what the session looks
* like at its first prompt. Nothing is typed: a fish_prompt handler in config.fish
* writes the answer to disk, so the result never races the line editor.
*/
async function readFirstPromptEnvironment(args: string[]): Promise<{
pathFirstEntry: string
codexHome: string
}> {
const pty = await import('node-pty')
const home = mkdtempSync(join(tmpdir(), 'orca-fish-startup-'))
const resultPath = join(home, 'probe-result')
const donePath = join(home, 'probe-done')
try {
mkdirSync(join(home, '.config', 'fish'), { recursive: true })
writeFileSync(
join(home, '.config', 'fish', 'config.fish'),
[
'set -g fish_greeting ""',
'function fish_prompt; printf "> "; end',
'function fish_right_prompt; end',
'# A user who routes their own agent home: Orca must still win.',
`set -gx CODEX_HOME ${USER_CODEX_HOME}`,
'# Stands in for macOS path_helper (which really does run here on macOS):',
'# whatever Orca prepended at spawn is no longer first once config.fish ran.',
`set -gx PATH ${USER_BIN_DIR} $PATH`,
'function __orca_probe --on-event fish_prompt',
` echo "PATH1=$PATH[1]" >"${resultPath}"`,
` echo "CODEX_HOME=$CODEX_HOME" >>"${resultPath}"`,
` echo done >"${donePath}"`,
'end',
''
].join('\n')
)
const proc = pty.spawn(FISH.path as string, args, {
name: 'xterm-256color',
cols: 80,
rows: 24,
cwd: home,
// Why fully specified and never spread from process.env: an inherited
// ORCA_* or XDG_* from the developer's own shell would change the outcome.
env: {
// The inherited PATH only follows so the child can still resolve the fish
// binary; both assertions read position 1, which is pinned here and then
// displaced by config.fish, so no ambient entry can decide them.
PATH: `${SHIM_DIR}:${process.env.PATH ?? '/usr/bin:/bin'}`,
HOME: home,
TERM: 'xterm-256color',
XDG_CONFIG_HOME: join(home, '.config'),
XDG_DATA_HOME: join(home, '.local', 'share'),
ORCA_ATTRIBUTION_SHIM_DIR: SHIM_DIR,
ORCA_CODEX_HOME
}
})
let queryCarry = ''
let settle = (): void => {}
const done = new Promise<void>((resolve) => {
settle = resolve
})
const deadline = setTimeout(settle, 15_000)
const poll = setInterval(() => {
if (existsSync(donePath)) {
settle()
}
}, 25)
proc.onData((chunk) => {
// Why the carry: a query split across two chunks would otherwise go unanswered
// and re-stall the prompt, while re-answering one already handled would land in
// fish's stdin as typed input.
const carriedLength = queryCarry.length
const scan = queryCarry + chunk
queryCarry = scan.slice(-QUERY_CARRY_LEN)
for (const [query, reply] of TERMINAL_QUERY_REPLIES) {
for (let at = scan.indexOf(query); at !== -1; at = scan.indexOf(query, at + query.length)) {
if (at + query.length > carriedLength) {
proc.write(reply)
}
}
}
})
await done
clearTimeout(deadline)
clearInterval(poll)
proc.kill()
const result = existsSync(resultPath) ? readFileSync(resultPath, 'utf8') : ''
return {
pathFirstEntry: /^PATH1=(.*)$/m.exec(result)?.[1] ?? '',
codexHome: /^CODEX_HOME=(.*)$/m.exec(result)?.[1] ?? ''
}
} finally {
rmSync(home, { recursive: true, force: true })
}
}
describe('fish startup environment', () => {
// Always runs, so the CI lane cannot report green with the live tests skipped.
it('has the fish this suite needs when CI requires one', () => {
expect(fishRequirementViolation(FISH)).toBeNull()
})
it('hands both PTY transports the same fish attribution launch config', () => {
// Why: the daemon and local-PTY copies drifted before; a fix landing in one
// transport only is invisible until a user on the other one reports it.
expect(getAttributionShellLaunchConfig('fish')).toEqual(
getLocalPtyAttributionShellLaunchConfig('fish')
)
})
itWithFish(
'overrides a config.fish agent home and re-prepends the attribution shim',
async () => {
// Why the attribution config and not the shell-ready one: the markerless pane is
// where a user types `git commit` by hand, and it got none of this before.
const config = getAttributionShellLaunchConfig(FISH.path as string)
expect(config.env.ORCA_SHELL_READY_MARKER).toBe('0')
const withOrcaInit = await readFirstPromptEnvironment(config.args ?? [])
// Control: same fish, same config.fish, no init command. Proves the two
// assertions below are actually produced by the init command.
const withoutOrcaInit = await readFirstPromptEnvironment(['-l'])
expect(withOrcaInit.codexHome).toBe(ORCA_CODEX_HOME)
expect(withoutOrcaInit.codexHome).toBe(USER_CODEX_HOME)
expect(withOrcaInit.pathFirstEntry).toBe(SHIM_DIR)
// Why the exact value and not `not.toBe(SHIM_DIR)`: a probe that never ran reads
// as an empty string, which would satisfy a negative assertion for free.
expect(withoutOrcaInit.pathFirstEntry).toBe(USER_BIN_DIR)
},
45_000
)
})
@@ -380,12 +380,20 @@ describePosix('local PTY shell-ready launch config', () => {
expect(init).toContain('functions -e __orca_shell_ready_marker')
})
it('keeps attribution-only fish spawns unwrapped', async () => {
it('gives attribution-only fish the shim/agent-home init command without the marker', async () => {
// Why: the markerless pane is where the user types commits by hand, so it needs the
// PATH shim most; only the marker stays gated so nothing is emitted into the pane.
const { getAttributionShellLaunchConfig } = await importFreshLocalPtyShellReady()
const config = getAttributionShellLaunchConfig('/opt/homebrew/bin/fish')
expect(config).toEqual({ args: null, env: {}, supportsReadyMarker: false })
expect(config.supportsReadyMarker).toBe(false)
expect(config.env).toEqual({ ORCA_SHELL_READY_MARKER: '0' })
expect(config.args?.slice(0, 2)).toEqual(['-l', '-C'])
const init = config.args?.[2] ?? ''
expect(init).toContain('set -gx PATH "$__orca_shim_dir" $PATH')
expect(init).toContain('set -gx CODEX_HOME "$ORCA_CODEX_HOME"')
expect(init).toContain('if test "$ORCA_SHELL_READY_MARKER" = 1')
})
it('falls back to HOME for ORCA_ORIG_ZDOTDIR when inherited ZDOTDIR points at a wrapper dir', async () => {
+8 -6
View File
@@ -17,7 +17,7 @@ import {
import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper'
import { buildStartupCommandSubmission } from '../../shared/startup-command-submission'
import {
getFishShellReadyInitCommand,
getFishInitCommand,
getZshEnvTemplate,
getZshFinalZdotdirRestoreBlock,
getZshShellReadyMarkerRegistrationBlock,
@@ -402,12 +402,14 @@ function getWrappedShellLaunchConfig(
}
}
// Why: mirrors daemon/shell-ready.ts; attribution-only fish stays unwrapped.
if (shellName === 'fish' && options.emitReadyMarker) {
// Why: mirrors daemon/shell-ready.ts. Markerless fish still needs the init command —
// that is the pane where the user types commits by hand, so the shim and agent homes
// must survive config.fish there too; only the marker itself stays gated.
if (shellName === 'fish') {
return {
args: ['-l', '-C', getFishShellReadyInitCommand(SHELL_READY_MARKER_ESCAPED)],
env: { ORCA_SHELL_READY_MARKER: '1' },
supportsReadyMarker: true
args: ['-l', '-C', getFishInitCommand(SHELL_READY_MARKER_ESCAPED)],
env: { ORCA_SHELL_READY_MARKER: options.emitReadyMarker ? '1' : '0' },
supportsReadyMarker: options.emitReadyMarker
}
}
+36 -1
View File
@@ -171,7 +171,7 @@ fi
// zle-line-init this lands just *before* fish arms `?2004h`, which PostReadyFlushGate
// absorbs. `builtin printf` so a user-defined printf can't silently swallow the marker
// and send every launch to the ready timeout.
export function getFishShellReadyInitCommand(escapedMarker: string): string {
function getFishShellReadyMarkerBlock(escapedMarker: string): string {
return `if test "$ORCA_SHELL_READY_MARKER" = 1
function __orca_shell_ready_marker --on-event fish_prompt
builtin printf "${escapedMarker}"
@@ -180,6 +180,41 @@ export function getFishShellReadyInitCommand(escapedMarker: string): string {
end`
}
/**
* Everything Orca must apply to a fish session *after* the user's own startup
* files, delivered as one `--init-command` (fish evaluates -C after config.fish
* and conf.d, and multi-`-C` support varies by version).
*
* The POSIX wrappers do this from their rc files; fish has no such hook, so
* without this a fish pane silently loses both halves of the contract.
*/
export function getFishInitCommand(escapedMarker: string): string {
return `begin
# Why: macOS fish's bundled config.fish runs path_helper, which rebuilds PATH and
# appends inherited entries last — the attribution shim lands near-last and commits
# from fish panes lose the Orca trailer. The begin block keeps the loop variable out
# of the user's session. Order matches the POSIX wrappers: attribution, then teams.
for __orca_shim_dir in $ORCA_ATTRIBUTION_SHIM_DIR $ORCA_AGENT_TEAMS_SHIM_DIR
if test -n "$__orca_shim_dir"; and test "$PATH[1]" != "$__orca_shim_dir"
set -gx PATH "$__orca_shim_dir" $PATH
end
end
end
# Why: a config.fish that sets its own agent home (\`set -gx CODEX_HOME ...\`) would
# otherwise beat Orca's routed home and launch the agent against the wrong account,
# which reads in the UI as the account switcher doing nothing.
if test -n "$ORCA_OPENCODE_CONFIG_DIR"
set -gx OPENCODE_CONFIG_DIR "$ORCA_OPENCODE_CONFIG_DIR"
end
if test -n "$ORCA_MIMOCODE_HOME"
set -gx MIMOCODE_HOME "$ORCA_MIMOCODE_HOME"
end
if test -n "$ORCA_CODEX_HOME"
set -gx CODEX_HOME "$ORCA_CODEX_HOME"
end
${getFishShellReadyMarkerBlock(escapedMarker)}`
}
export function getZshFinalZdotdirRestoreBlock(homeExpression = '"${ORCA_ORIG_ZDOTDIR:-$HOME}"') {
return `_orca_home=${homeExpression}
case "\${_orca_home%/}" in