diff --git a/apps/desktop/src/shell/DesktopShellEnvironment.test.ts b/apps/desktop/src/shell/DesktopShellEnvironment.test.ts index 5a76402b1d34..e8bdc3018aca 100644 --- a/apps/desktop/src/shell/DesktopShellEnvironment.test.ts +++ b/apps/desktop/src/shell/DesktopShellEnvironment.test.ts @@ -129,6 +129,65 @@ describe("DesktopShellEnvironment", () => { }), ); + it.effect("probes zsh interactively with job control disabled", () => + Effect.gen(function* () { + const env: NodeJS.ProcessEnv = { + SHELL: "/bin/zsh", + PATH: "/usr/bin", + }; + const commands: ChildProcess.Command[] = []; + + yield* runShellEnvironment({ + env, + platform: "darwin", + handler: (command) => { + commands.push(command); + return envOutput({ PATH: "/opt/homebrew/bin:/usr/bin" }); + }, + }); + + const probe = commands[0]; + assert.equal(probe?._tag, "StandardCommand"); + if (probe?._tag === "StandardCommand") { + // `+m` clears MONITOR before zsh startup, so the interactive probe + // still reads ~/.zshrc (version-manager PATH lives there) without + // tcsetpgrp() on a controlling TTY owned by another process group. + assert.deepEqual(probe.args.slice(0, 2), ["+m", "-ilc"]); + } + assert.equal(env.PATH, "/opt/homebrew/bin:/usr/bin"); + }), + ); + + it.effect("runs the login-shell probe without interactive job control", () => + Effect.gen(function* () { + const env: NodeJS.ProcessEnv = { + SHELL: "/bin/bash", + PATH: "/usr/bin", + }; + const commands: ChildProcess.Command[] = []; + + yield* runShellEnvironment({ + env, + platform: "darwin", + handler: (command) => { + commands.push(command); + return envOutput({ PATH: "/opt/homebrew/bin:/usr/bin" }); + }, + }); + + const probe = commands[0]; + assert.equal(probe?._tag, "StandardCommand"); + if (probe?._tag === "StandardCommand") { + // `-l` without `-i`: interactive job control stops the probe on + // SIGTTOU when the session inherits a controlling TTY owned by + // another process group. bash reads the same files either way. + assert.deepEqual(probe.args.slice(0, 1), ["-lc"]); + assert.notInclude(probe.args, "-i"); + } + assert.equal(env.PATH, "/opt/homebrew/bin:/usr/bin"); + }), + ); + it.effect("preserves inherited POSIX values when present", () => Effect.gen(function* () { const env: NodeJS.ProcessEnv = { diff --git a/apps/desktop/src/shell/DesktopShellEnvironment.ts b/apps/desktop/src/shell/DesktopShellEnvironment.ts index 5c6b67fd58c0..fd39e83c1735 100644 --- a/apps/desktop/src/shell/DesktopShellEnvironment.ts +++ b/apps/desktop/src/shell/DesktopShellEnvironment.ts @@ -331,6 +331,16 @@ const runCommandOutput = Effect.fn("desktop.shellEnvironment.runCommandOutput")( return ""; }); +// zsh only reads ~/.zshrc when interactive, and that file is where version +// managers and custom bin dirs commonly export PATH, so its probe keeps `-i` +// but prepends `+m`: clearing MONITOR before startup means the shell never +// calls tcsetpgrp() on a controlling TTY owned by another process group, which +// is what stopped `-ilc` probes on SIGTTOU. Other shells keep the plain login +// probe: bash login shells never read ~/.bashrc, and fish reads config.fish +// for login shells, so `-i` would only reintroduce the hang. +const loginShellProbeArgs = (shell: string, command: string): ReadonlyArray => + executableName(shell) === "zsh" ? ["+m", "-ilc", command] : ["-lc", command]; + const readLoginShellEnvironment = ( shell: string, names: ReadonlyArray, @@ -340,7 +350,7 @@ const readLoginShellEnvironment = ( : runCommandOutput({ probe: "login-shell", command: shell, - args: ["-ilc", capturePosixEnvironmentCommand(names)], + args: loginShellProbeArgs(shell, capturePosixEnvironmentCommand(names)), timeout: LOGIN_SHELL_TIMEOUT, }).pipe(Effect.map((output) => extractEnvironment(output, names))); diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index 621fe49b3087..edf5b7765f8a 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -39,7 +39,7 @@ const withWindowsEnvironmentMocks = ( ); describe("readPathFromLoginShell", () => { - it("uses a shell-agnostic printenv PATH probe", () => { + it("probes PATH through a login shell without interactive job control", () => { const execFile = vi.fn< ( file: string, @@ -62,12 +62,54 @@ describe("readPathFromLoginShell", () => { const [shell, args, options] = firstCall; expect(shell).toBe("/opt/homebrew/bin/fish"); expect(args).toHaveLength(2); - expect(args?.[0]).toBe("-ilc"); + // `-l` without `-i`: interactive job control stops the probe on SIGTTOU + // when the session inherits a controlling TTY owned by another pgrp. + expect(args?.[0]).toBe("-lc"); + expect(args?.[0]).not.toContain("i"); expect(args?.[1]).toContain("printenv PATH || true"); expect(args?.[1]).toContain("__T3CODE_ENV_PATH_START__"); expect(args?.[1]).toContain("__T3CODE_ENV_PATH_END__"); expect(options).toEqual({ encoding: "utf8", timeout: 5000 }); }); + + it("keeps zsh interactive startup files while disabling job control", () => { + const execFile = vi.fn< + ( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number }, + ) => string + >(() => "__T3CODE_ENV_PATH_START__\n/a:/b\n__T3CODE_ENV_PATH_END__\n"); + + expect(readPathFromLoginShell("/bin/zsh", execFile)).toBe("/a:/b"); + + const args = execFile.mock.calls[0]?.[1]; + // `-i` keeps ~/.zshrc, where version managers commonly export PATH, and + // `+m` clears MONITOR before startup so the shell never tcsetpgrp()s a + // controlling TTY owned by another process group. + expect(args).toHaveLength(3); + expect(args?.[0]).toBe("+m"); + expect(args?.[1]).toBe("-ilc"); + expect(args?.[2]).toContain("printenv PATH || true"); + }); + + it("does not probe bash interactively", () => { + const execFile = vi.fn< + ( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number }, + ) => string + >(() => "__T3CODE_ENV_PATH_START__\n/a:/b\n__T3CODE_ENV_PATH_END__\n"); + + expect(readPathFromLoginShell("/bin/bash", execFile)).toBe("/a:/b"); + + const args = execFile.mock.calls[0]?.[1]; + // Login bash never reads ~/.bashrc, so `-i` adds nothing, and bash still + // grabs the terminal even with `+m` - it must stay non-interactive. + expect(args).toHaveLength(2); + expect(args?.[0]).toBe("-lc"); + }); }); describe("readPathFromLaunchctl", () => { diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 07ac73f8c6a7..bc01c7f86b67 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -289,6 +289,18 @@ export type ShellEnvironmentReader = ( execFile?: ExecFileSyncLike, ) => Partial>; +// zsh only reads ~/.zshrc when interactive, and that file is where version +// managers and custom bin dirs commonly export PATH, so its probe keeps `-i` +// but prepends `+m`: clearing MONITOR before startup means the shell never +// calls tcsetpgrp() on a controlling TTY owned by another process group, which +// is what stopped `-ilc` probes on SIGTTOU (SSH daemons started from a +// terminal hand sessions exactly that). Other shells keep the plain login +// probe: bash login shells never read ~/.bashrc, and fish reads config.fish +// for login shells, so `-i` would only reintroduce the hang. +function loginShellProbeArgs(shell: string, command: string): Array { + return NodePath.posix.basename(shell) === "zsh" ? ["+m", "-ilc", command] : ["-lc", command]; +} + export const readEnvironmentFromLoginShell: ShellEnvironmentReader = ( shell, names, @@ -298,10 +310,14 @@ export const readEnvironmentFromLoginShell: ShellEnvironmentReader = ( return {}; } - const output = execFile(shell, ["-ilc", buildEnvironmentCaptureCommand(names)], { - encoding: "utf8", - timeout: 5000, - }); + const output = execFile( + shell, + loginShellProbeArgs(shell, buildEnvironmentCaptureCommand(names)), + { + encoding: "utf8", + timeout: 5000, + }, + ); const environment: Partial> = {}; for (const name of names) { diff --git a/scripts/mobile-showcase.ts b/scripts/mobile-showcase.ts index 6b6adebeb779..11aac709e1e1 100644 --- a/scripts/mobile-showcase.ts +++ b/scripts/mobile-showcase.ts @@ -573,7 +573,7 @@ async function createShowcaseShell(baseDir: string): Promise { await NodeFSP.writeFile( shellPath, `#!/bin/sh -if [ "$1" = "-ilc" ] || [ "$1" = "-lic" ]; then +if [ "$1" = "-lc" ] || [ "$1" = "-ilc" ] || [ "$1" = "-lic" ]; then exec /bin/sh -c "$2" fi exec /bin/cat