diff --git a/src/main/fish-startup-environment.node-pty.test.ts b/src/main/fish-startup-environment.node-pty.test.ts index e6a64d4d6c2..11bd1e9592f 100644 --- a/src/main/fish-startup-environment.node-pty.test.ts +++ b/src/main/fish-startup-environment.node-pty.test.ts @@ -245,4 +245,42 @@ describe('fish startup environment', () => { rmSync(home, { recursive: true, force: true }) } }) + + itWithFish('drops a repeated shim even when a copy is already first in PATH', () => { + // Why: guarding on `$PATH[1]` alone would take the shim's front position as proof + // there is nothing to do and leave the later copy in place. + const home = mkdtempSync(join(tmpdir(), 'orca fish dedupe ')) + try { + const initPath = join(home, 'init.fish') + writeFileSync(initPath, getFishInitCommand(SHELL_READY_MARKER)) + const result = spawnSync( + FISH.path as string, + [ + '--no-config', + '-c', + [ + `set -gx PATH ${fishQuote(SHIM_DIR)} /usr/bin /bin ${fishQuote(SHIM_DIR)}`, + `source ${fishQuote(initPath)}`, + 'for __probe_entry in $PATH; echo "PATH=$__probe_entry"; end' + ].join('\n') + ], + { + encoding: 'utf8', + env: { + PATH: process.env.PATH ?? '/usr/bin:/bin', + HOME: home, + ORCA_ATTRIBUTION_SHIM_DIR: SHIM_DIR, + 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]) + expect(pathEntries).toEqual([SHIM_DIR, '/usr/bin', '/bin']) + } finally { + rmSync(home, { recursive: true, force: true }) + } + }) }) diff --git a/src/main/shell-templates.ts b/src/main/shell-templates.ts index 6e408392766..0982da3a845 100644 --- a/src/main/shell-templates.ts +++ b/src/main/shell-templates.ts @@ -183,7 +183,7 @@ 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). + * and conf.d). * * 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. @@ -200,10 +200,11 @@ export function getFishInitCommand(escapedMarker: string): string { # 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" - # 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\`, + if test -n "$__orca_shim_dir" + # Why filter unconditionally instead of skipping when the shim is already $PATH[1]: + # the shim can be first *and* repeated later, and that skip would keep the copy. + # Filter-then-prepend also makes a re-run a no-op, where a plain prepend would grow + # PATH by one duplicate per shim. Compared as strings, not with \`string match\`, # because a shim path may contain glob characters. set -l __orca_kept_path for __orca_path_entry in $PATH