mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 16:03:26 +00:00
fix(fish): give the relay the same fish init command as the local launch sites
src/relay/pty-shell-launch.ts had no fish branch: any shell that was not zsh or bash returned early with a bare `-l`. So on an SSH/relay host whose login shell is fish, everything the previous commit fixes locally still broke — the routed ORCA_OPENCODE_CONFIG_DIR / ORCA_CODEX_HOME / ORCA_MIMOCODE_HOME were never restored after the remote's config.fish, and the shim the relay prepends to PATH was never re-prepended, so remote commits lost the Orca trailer. The relay now reuses getFishInitCommand rather than carrying its own copy; the loop also walks ORCA_REMOTE_CLI_BIN_DIR, which is the relay's name for the shim dir. fish expands an unset variable to nothing, so one text still serves all three sites, and a new parity test fails if any site starts fish differently from the others. Real fish 4.7.1 also showed the PATH block was not idempotent: with more than one shim set, a second evaluation (a re-initialized pane) re-prepended every entry, because the guard only compared against PATH[1] and each prepend displaced the previous one. Drop existing copies before prepending, by string compare rather than `string match` so a shim path containing glob characters cannot take the whole init command down.
This commit is contained in:
@@ -269,7 +269,7 @@ describePosix('daemon shell-ready launch config', () => {
|
||||
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 PATH "$__orca_shim_dir" $__orca_kept_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"')
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
/**
|
||||
* fish is launched from three independent places — the local PTY provider, the
|
||||
* daemon/SSH transport, and the relay running on a remote host. Each one used to grow
|
||||
* its own idea of how to start a shell, and the relay simply had no fish branch at all
|
||||
* (it fell through to bare `-l`), so an SSH user on fish lost the routed agent homes and
|
||||
* the shim PATH entry that the same code fixes locally.
|
||||
*
|
||||
* This file is the drift alarm: it fails if any one site starts fish differently from
|
||||
* the others, or stops sourcing its init text from `getFishInitCommand`.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
getAttributionShellLaunchConfig as getDaemonAttributionShellLaunchConfig,
|
||||
getShellReadyLaunchConfig as getDaemonShellReadyLaunchConfig
|
||||
} from './daemon/shell-ready'
|
||||
import {
|
||||
getAttributionShellLaunchConfig as getLocalPtyAttributionShellLaunchConfig,
|
||||
getShellReadyLaunchConfig as getLocalPtyShellReadyLaunchConfig
|
||||
} from './providers/local-pty-shell-ready'
|
||||
import { getRelayShellLaunchConfig } from '../relay/pty-shell-launch'
|
||||
import { getFishInitCommand } from './shell-templates'
|
||||
|
||||
const FISH_PATH = '/opt/homebrew/bin/fish'
|
||||
const SHELL_READY_MARKER = '\\033]777;orca-shell-ready\\007'
|
||||
const RELAY_ENV = { HOME: '/home/tester' }
|
||||
|
||||
type LaunchShape = { args: string[]; env: Record<string, string> }
|
||||
|
||||
function shape(config: { args: string[] | null; env: Record<string, string> }): LaunchShape {
|
||||
return { args: config.args ?? [], env: config.env }
|
||||
}
|
||||
|
||||
function launchShapes(emitReadyMarker: boolean): Record<string, LaunchShape> {
|
||||
return {
|
||||
local: shape(
|
||||
emitReadyMarker
|
||||
? getLocalPtyShellReadyLaunchConfig(FISH_PATH)
|
||||
: getLocalPtyAttributionShellLaunchConfig(FISH_PATH)
|
||||
),
|
||||
daemon: shape(
|
||||
emitReadyMarker
|
||||
? getDaemonShellReadyLaunchConfig(FISH_PATH)
|
||||
: getDaemonAttributionShellLaunchConfig(FISH_PATH)
|
||||
),
|
||||
relay: shape(getRelayShellLaunchConfig(FISH_PATH, RELAY_ENV, 'linux', { emitReadyMarker }))
|
||||
}
|
||||
}
|
||||
|
||||
describe('fish launch config parity across the three launch sites', () => {
|
||||
for (const emitReadyMarker of [true, false]) {
|
||||
const label = emitReadyMarker ? 'shell-ready' : 'attribution-only'
|
||||
|
||||
it(`starts ${label} fish identically from the local, daemon, and relay sites`, () => {
|
||||
const { local, daemon, relay } = launchShapes(emitReadyMarker)
|
||||
expect(daemon).toEqual(local)
|
||||
expect(relay).toEqual(local)
|
||||
})
|
||||
|
||||
it(`hands ${label} fish the shared init text rather than a per-site copy`, () => {
|
||||
const expected = ['-l', '-C', getFishInitCommand(SHELL_READY_MARKER)]
|
||||
for (const [site, config] of Object.entries(launchShapes(emitReadyMarker))) {
|
||||
expect(config.args, site).toEqual(expected)
|
||||
// Why: the marker block is inert unless this is set, so a site that drops it
|
||||
// would look correct in the args diff and never emit a ready marker.
|
||||
expect(config.env, site).toEqual({
|
||||
ORCA_SHELL_READY_MARKER: emitReadyMarker ? '1' : '0'
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
it('re-prepends every shim variable the three sites can plant', () => {
|
||||
const init = getFishInitCommand(SHELL_READY_MARKER)
|
||||
// ORCA_REMOTE_CLI_BIN_DIR is the relay's own name for the shim dir; the local sites
|
||||
// never set it and fish expands the unset name to nothing, which is what lets one
|
||||
// text serve all three.
|
||||
for (const variable of [
|
||||
'ORCA_ATTRIBUTION_SHIM_DIR',
|
||||
'ORCA_AGENT_TEAMS_SHIM_DIR',
|
||||
'ORCA_REMOTE_CLI_BIN_DIR'
|
||||
]) {
|
||||
expect(init).toContain(`$${variable}`)
|
||||
}
|
||||
for (const variable of ['ORCA_OPENCODE_CONFIG_DIR', 'ORCA_MIMOCODE_HOME', 'ORCA_CODEX_HOME']) {
|
||||
expect(init).toContain(`$${variable}`)
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps non-fish relay shells on their existing launch shapes', () => {
|
||||
// Why: the fish branch sits ahead of the bash/zsh wrapper work, so a basename
|
||||
// mismatch there would silently disarm the overlay wrappers for everyone else.
|
||||
expect(getRelayShellLaunchConfig('/usr/bin/dash', RELAY_ENV, 'linux')).toEqual({
|
||||
args: ['-l'],
|
||||
env: {}
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -11,13 +11,18 @@
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* Every path here is deliberately spacey (and one carries glob characters), because the
|
||||
* dirs this text moves around are really "~/Library/Application Support/...".
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process'
|
||||
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 { getFishInitCommand } from './shell-templates'
|
||||
import { fishRequirementViolation, resolveFishBinary } from '../shared/fish-binary-requirement'
|
||||
|
||||
const FISH = resolveFishBinary()
|
||||
@@ -33,10 +38,25 @@ const TERMINAL_QUERY_REPLIES: readonly (readonly [string, string])[] = [
|
||||
]
|
||||
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'
|
||||
// Spaces on purpose: "~/Library/Application Support/..." is the real shape of an Orca
|
||||
// shim dir on macOS, and an unquoted fish `set` would silently split it into two entries.
|
||||
const SHIM_DIR = '/orca test/attribution shim'
|
||||
const TEAMS_SHIM_DIR = '/orca test/agent teams shim'
|
||||
// Glob characters on purpose: fish treats an unmatched glob as a hard error, so a filter
|
||||
// written with `string match` instead of a plain compare would take the whole init down.
|
||||
const RELAY_SHIM_DIR = '/orca test/relay [cli] bin'
|
||||
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'
|
||||
const SHELL_READY_MARKER = '\\033]777;orca-shell-ready\\007'
|
||||
|
||||
/**
|
||||
* fish single quotes are NOT POSIX single quotes: `\\` and `\'` still escape inside them,
|
||||
* and a trailing backslash before the closing quote is a syntax error. Verified on 4.7.1.
|
||||
*/
|
||||
function fishQuote(value: string): string {
|
||||
return `'${value.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawns fish on a real PTY with the given args and returns what the session looks
|
||||
@@ -48,7 +68,7 @@ async function readFirstPromptEnvironment(args: string[]): Promise<{
|
||||
codexHome: string
|
||||
}> {
|
||||
const pty = await import('node-pty')
|
||||
const home = mkdtempSync(join(tmpdir(), 'orca-fish-startup-'))
|
||||
const home = mkdtempSync(join(tmpdir(), 'orca fish startup '))
|
||||
const resultPath = join(home, 'probe-result')
|
||||
const donePath = join(home, 'probe-done')
|
||||
try {
|
||||
@@ -60,14 +80,14 @@ async function readFirstPromptEnvironment(args: string[]): Promise<{
|
||||
'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}`,
|
||||
`set -gx CODEX_HOME ${fishQuote(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`,
|
||||
`set -gx PATH ${fishQuote(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}"`,
|
||||
` echo "PATH1=$PATH[1]" >${fishQuote(resultPath)}`,
|
||||
` echo "CODEX_HOME=$CODEX_HOME" >>${fishQuote(resultPath)}`,
|
||||
` echo done >${fishQuote(donePath)}`,
|
||||
'end',
|
||||
''
|
||||
].join('\n')
|
||||
@@ -172,4 +192,57 @@ describe('fish startup environment', () => {
|
||||
},
|
||||
45_000
|
||||
)
|
||||
|
||||
itWithFish('runs clean and unchanged when a pane re-runs it', () => {
|
||||
// Why: a re-initialized pane can evaluate this text a second time in the same
|
||||
// session. A plain prepend would grow PATH by one duplicate per shim per run.
|
||||
const home = mkdtempSync(join(tmpdir(), 'orca fish idempotent '))
|
||||
try {
|
||||
const initPath = join(home, 'init.fish')
|
||||
writeFileSync(initPath, getFishInitCommand(SHELL_READY_MARKER))
|
||||
const result = spawnSync(
|
||||
FISH.path as string,
|
||||
[
|
||||
// --no-config so the outcome is the init text alone, not a developer's config.fish.
|
||||
'--no-config',
|
||||
'-c',
|
||||
[
|
||||
'set -gx PATH /usr/bin /bin',
|
||||
`source ${fishQuote(initPath)}`,
|
||||
`source ${fishQuote(initPath)}`,
|
||||
'for __probe_entry in $PATH; echo "PATH=$__probe_entry"; end',
|
||||
'echo "CODEX_HOME=$CODEX_HOME"',
|
||||
// Nothing the init text declares may survive into the user's session.
|
||||
'echo "LEAKED=$__orca_shim_dir$__orca_kept_path$__orca_path_entry"'
|
||||
].join('\n')
|
||||
],
|
||||
{
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
PATH: process.env.PATH ?? '/usr/bin:/bin',
|
||||
HOME: home,
|
||||
// All three shim variables, because the duplication only shows up once a
|
||||
// later shim displaces an earlier one from PATH[1]. ORCA_REMOTE_CLI_BIN_DIR
|
||||
// is the relay's name for it and is only ever set on an SSH host.
|
||||
ORCA_ATTRIBUTION_SHIM_DIR: SHIM_DIR,
|
||||
ORCA_AGENT_TEAMS_SHIM_DIR: TEAMS_SHIM_DIR,
|
||||
ORCA_REMOTE_CLI_BIN_DIR: RELAY_SHIM_DIR,
|
||||
ORCA_CODEX_HOME,
|
||||
ORCA_SHELL_READY_MARKER: '0'
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
expect(result.stderr).toBe('')
|
||||
expect(result.status).toBe(0)
|
||||
const pathEntries = [...result.stdout.matchAll(/^PATH=(.*)$/gm)].map((match) => match[1])
|
||||
// Two `source`s, one entry each — and the relay shim ends up first, matching where
|
||||
// the relay's own bash/zsh wrappers put it.
|
||||
expect(pathEntries).toEqual([RELAY_SHIM_DIR, TEAMS_SHIM_DIR, SHIM_DIR, '/usr/bin', '/bin'])
|
||||
expect(result.stdout).toContain(`CODEX_HOME=${ORCA_CODEX_HOME}`)
|
||||
expect(result.stdout).toContain('LEAKED=\n')
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -391,7 +391,7 @@ describePosix('local PTY shell-ready launch config', () => {
|
||||
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 PATH "$__orca_shim_dir" $__orca_kept_path')
|
||||
expect(init).toContain('set -gx CODEX_HOME "$ORCA_CODEX_HOME"')
|
||||
expect(init).toContain('if test "$ORCA_SHELL_READY_MARKER" = 1')
|
||||
})
|
||||
|
||||
@@ -187,16 +187,29 @@ end`
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* One text serves all three launch sites (local PTY, daemon/SSH, relay): fish expands
|
||||
* an unset variable to nothing, so each site's unused shim variables drop out of the
|
||||
* loop. Forking a per-site copy is how the POSIX call sites drifted apart before.
|
||||
*/
|
||||
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
|
||||
# of the user's session. Order matches the POSIX wrappers: attribution, then teams,
|
||||
# and the relay's own shim (ORCA_REMOTE_CLI_BIN_DIR) last so it ends up first.
|
||||
for __orca_shim_dir in $ORCA_ATTRIBUTION_SHIM_DIR $ORCA_AGENT_TEAMS_SHIM_DIR $ORCA_REMOTE_CLI_BIN_DIR
|
||||
if test -n "$__orca_shim_dir"; and test "$PATH[1]" != "$__orca_shim_dir"
|
||||
set -gx PATH "$__orca_shim_dir" $PATH
|
||||
# Why drop existing copies instead of a plain prepend: this text can run twice in
|
||||
# one session (a re-initialized pane), and a plain prepend grows PATH by one
|
||||
# duplicate per shim per run. Filtered by string compare, not \`string match\`,
|
||||
# because a shim path may contain glob characters.
|
||||
set -l __orca_kept_path
|
||||
for __orca_path_entry in $PATH
|
||||
test "$__orca_path_entry" = "$__orca_shim_dir"; or set -a __orca_kept_path "$__orca_path_entry"
|
||||
end
|
||||
set -gx PATH "$__orca_shim_dir" $__orca_kept_path
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { getFishInitCommand } from '../main/shell-templates'
|
||||
import { getRelayShellLaunchConfig } from './pty-shell-launch'
|
||||
|
||||
const SHELL_READY_MARKER_ESCAPED = '\\033]777;orca-shell-ready\\007'
|
||||
|
||||
const hasBash = process.platform !== 'win32' && spawnSync('bash', ['--version']).status === 0
|
||||
const itWithBash = hasBash ? it : it.skip
|
||||
|
||||
@@ -122,6 +125,42 @@ describe('getRelayShellLaunchConfig', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('restores agent homes and the shim PATH entry on a remote whose shell is fish', () => {
|
||||
// Why: fish fell through to a bare `-l` here, so an SSH host on fish kept none of the
|
||||
// overlay env the relay planted — the account switcher looked like a no-op remotely
|
||||
// and remote commits lost the Orca trailer.
|
||||
const config = getRelayShellLaunchConfig(
|
||||
'/usr/bin/fish',
|
||||
{ HOME: homeDir, ORCA_REMOTE_CLI_BIN_DIR: '/tmp/orca-relay-bin' },
|
||||
'linux'
|
||||
)
|
||||
|
||||
expect(config.args.slice(0, 2)).toEqual(['-l', '-C'])
|
||||
expect(config.args[2]).toBe(getFishInitCommand(SHELL_READY_MARKER_ESCAPED))
|
||||
expect(config.env).toEqual({ ORCA_SHELL_READY_MARKER: '0' })
|
||||
})
|
||||
|
||||
it('arms the fish ready marker only when the caller asks for it', () => {
|
||||
const args = ['/usr/bin/fish', { HOME: homeDir }, 'linux'] as const
|
||||
expect(
|
||||
getRelayShellLaunchConfig(...args, { emitReadyMarker: true }).env.ORCA_SHELL_READY_MARKER
|
||||
).toBe('1')
|
||||
// Markerless is still wrapped: that is the pane where a user types `git commit`.
|
||||
const markerless = getRelayShellLaunchConfig(...args, { emitReadyMarker: false })
|
||||
expect(markerless.env.ORCA_SHELL_READY_MARKER).toBe('0')
|
||||
expect(markerless.args).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('writes no persistent wrapper files for fish', () => {
|
||||
// Why: fish carries everything on `-C`, so it must not create ~/.orca-relay on a
|
||||
// remote or leave stale rc files behind for a shell that never reads them.
|
||||
getRelayShellLaunchConfig('/usr/bin/fish', { HOME: homeDir }, 'linux', {
|
||||
emitReadyMarker: true
|
||||
})
|
||||
|
||||
expect(existsSync(join(homeDir, '.orca-relay'))).toBe(false)
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')('rewrites stale persistent wrapper files', () => {
|
||||
const zshRoot = join(homeDir, '.orca-relay', 'shell-ready', 'zsh')
|
||||
mkdirSync(zshRoot, { recursive: true })
|
||||
|
||||
@@ -3,6 +3,7 @@ import { homedir } from 'node:os'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { getPosixOmpShellWrapper } from '../main/pty/omp-shell-wrapper'
|
||||
import {
|
||||
getFishInitCommand,
|
||||
getZshFinalZdotdirRestoreBlock,
|
||||
getZshShellReadyMarkerRegistrationBlock,
|
||||
SHELL_STARTUP_IDENTITY_MARKER_BLOCK,
|
||||
@@ -288,6 +289,18 @@ export function getRelayShellLaunchConfig(
|
||||
}
|
||||
}
|
||||
|
||||
// Why: mirrors src/main/daemon/shell-ready.ts and providers/local-pty-shell-ready.ts.
|
||||
// A remote whose login shell is fish gets no wrapper rc file, so `-C` is the only hook
|
||||
// that runs after the remote's config.fish — without it the routed agent homes and the
|
||||
// shim PATH entry the relay planted are silently lost on this host. Ungated like the
|
||||
// local sites: the markerless pane is where the user types `git commit` by hand.
|
||||
if (shellName === 'fish') {
|
||||
return {
|
||||
args: [...POSIX_LOGIN_ARGS, '-C', getFishInitCommand(SHELL_READY_MARKER_ESCAPED)],
|
||||
env: { ORCA_SHELL_READY_MARKER: emitReadyMarker ? '1' : '0' }
|
||||
}
|
||||
}
|
||||
|
||||
if (shellName !== 'zsh' && shellName !== 'bash') {
|
||||
return { args: POSIX_LOGIN_ARGS, env: {} }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user