fix(coding-agent): run fish user shell interactively instead of as a login shell
The interactive !/!! shortcut wrapped commands as `fish -l -c '…'`: resolveUserShellConfig swaps in $SHELL but inherits the bash-oriented ["-l", "-c"] args, and ensureInteractiveShellArgs injected -i only for zsh. A login fish fires `status is-login` blocks in user config (agent/keychain setup, PATH mutation) on every command. fish sources the same config.fish/conf.d files for interactive shells as for login shells, so give fish -i and strip the inherited -l: user aliases and functions (#1816) keep working without login-shell side effects. zsh keeps -l -i since .zprofile is login-only.
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the interactive `!`/`!!` shell shortcut spawning fish as a login shell (`fish -l -c …`), which fired `status is-login` blocks in user config (agent/keychain setup, PATH mutation) on every command. fish is now started with `-i` instead — interactive shells source the same `config.fish`/`conf.d` files (so aliases and functions from #1816 keep working) without login-shell side effects. zsh behavior (`-l -i`) is unchanged.
|
||||
|
||||
## [17.0.5] - 2026-07-18
|
||||
|
||||
### Added
|
||||
|
||||
@@ -152,7 +152,7 @@ function isBashShell(shell: string): boolean {
|
||||
|
||||
function needsInteractiveShellArg(shell: string): boolean {
|
||||
const basename = shellBasename(shell);
|
||||
return basename.includes("zsh");
|
||||
return basename.includes("zsh") || basename.includes("fish");
|
||||
}
|
||||
|
||||
function supportsAutoUserShell(shell: string): boolean {
|
||||
@@ -165,19 +165,31 @@ function hasInteractiveShellArg(args: string[]): boolean {
|
||||
}
|
||||
|
||||
function ensureInteractiveShellArgs(shell: string, args: string[]): string[] {
|
||||
if (!needsInteractiveShellArg(shell) || hasInteractiveShellArg(args)) return args;
|
||||
if (!needsInteractiveShellArg(shell)) return args;
|
||||
|
||||
const commandIndex = args.findIndex(arg => arg === "-c" || arg === "--command");
|
||||
// fish sources the same config files (config.fish + conf.d) for interactive
|
||||
// shells as for login shells, so the inherited `-l` adds nothing — it only
|
||||
// marks the shell as login, firing `status is-login` blocks in user config
|
||||
// (agent/keychain setup, path mutation) on every `!` command. zsh keeps `-l`
|
||||
// because .zprofile is login-only. Args originate from procmgr's
|
||||
// getShellArgs(), so login only ever appears as a standalone `-l`/`--login`.
|
||||
const effectiveArgs = shellBasename(shell).includes("fish")
|
||||
? args.filter(arg => arg !== "-l" && arg !== "--login")
|
||||
: args;
|
||||
|
||||
if (hasInteractiveShellArg(effectiveArgs)) return effectiveArgs;
|
||||
|
||||
const commandIndex = effectiveArgs.findIndex(arg => arg === "-c" || arg === "--command");
|
||||
if (commandIndex !== -1) {
|
||||
return [...args.slice(0, commandIndex), "-i", ...args.slice(commandIndex)];
|
||||
return [...effectiveArgs.slice(0, commandIndex), "-i", ...effectiveArgs.slice(commandIndex)];
|
||||
}
|
||||
|
||||
const compactCommandIndex = args.findIndex(arg => /^-[^-]*c[^-]*$/.test(arg));
|
||||
const compactCommandIndex = effectiveArgs.findIndex(arg => /^-[^-]*c[^-]*$/.test(arg));
|
||||
if (compactCommandIndex !== -1) {
|
||||
return args.map((arg, index) => (index === compactCommandIndex ? arg.replace("c", "ic") : arg));
|
||||
return effectiveArgs.map((arg, index) => (index === compactCommandIndex ? arg.replace("c", "ic") : arg));
|
||||
}
|
||||
|
||||
return [...args, "-i"];
|
||||
return [...effectiveArgs, "-i"];
|
||||
}
|
||||
|
||||
function quoteShellArg(value: string): string {
|
||||
|
||||
@@ -276,8 +276,10 @@ exit 64
|
||||
expect(result.cancelled).toBe(false);
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.output.trim()).toBe("env-shell-ok");
|
||||
expect(fs.readFileSync(marker, "utf8")).toContain("-l -c");
|
||||
expect(fs.readFileSync(marker, "utf8")).not.toContain("-i");
|
||||
// fish gets `-i` (interactive loads config.fish too) instead of `-l`,
|
||||
// so `status is-login` blocks in user config don't fire on `!` commands.
|
||||
expect(fs.readFileSync(marker, "utf8")).toContain("-i -c");
|
||||
expect(fs.readFileSync(marker, "utf8")).not.toContain("-l");
|
||||
} finally {
|
||||
if (originalShell === undefined) {
|
||||
delete Bun.env.SHELL;
|
||||
@@ -330,6 +332,58 @@ exit 64
|
||||
}
|
||||
});
|
||||
|
||||
it("runs fish user-shell commands without login-shell side effects", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const fishPath = ["/usr/bin/fish", "/bin/fish", "/usr/local/bin/fish", "/opt/homebrew/bin/fish"].find(
|
||||
candidate => fs.existsSync(candidate),
|
||||
);
|
||||
if (!fishPath) {
|
||||
return;
|
||||
}
|
||||
|
||||
const shellDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-fish-shellpath-"));
|
||||
const configDir = path.join(shellDir, ".config", "fish");
|
||||
fs.mkdirSync(path.join(configDir, "conf.d"), { recursive: true });
|
||||
fs.writeFileSync(path.join(configDir, "config.fish"), "function pi_fish_fn; echo fish-fn-ok; end\n");
|
||||
// Login-gated snippet: fires only when the spawned fish is a login shell.
|
||||
fs.writeFileSync(
|
||||
path.join(configDir, "conf.d", "pi-login.fish"),
|
||||
"if status is-login; echo fish-login-side-effect; end\n",
|
||||
);
|
||||
Settings.instance.set("shellPath", fishPath);
|
||||
|
||||
vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
|
||||
shell: fishPath,
|
||||
args: ["-l", "-c"],
|
||||
env: {
|
||||
PATH: Bun.env.PATH ?? "",
|
||||
HOME: shellDir,
|
||||
},
|
||||
prefix: undefined,
|
||||
});
|
||||
|
||||
try {
|
||||
const result = await executeBash("pi_fish_fn", {
|
||||
cwd: tempDir,
|
||||
timeout: 5000,
|
||||
sessionKey: "fish-shell-path",
|
||||
useUserShell: true,
|
||||
});
|
||||
|
||||
expect(result.cancelled).toBe(false);
|
||||
expect(result.exitCode).toBe(0);
|
||||
// config.fish must still load (#1816 contract)…
|
||||
expect(result.output).toContain("fish-fn-ok");
|
||||
// …but the shell must not be a login shell.
|
||||
expect(result.output).not.toContain("fish-login-side-effect");
|
||||
} finally {
|
||||
removeSyncWithRetries(shellDir);
|
||||
}
|
||||
});
|
||||
|
||||
it("invokes onChunk with command output", async () => {
|
||||
let seenChunk: string | null = null;
|
||||
const result = await executeBash("echo hello", {
|
||||
|
||||
Reference in New Issue
Block a user