Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions apps/desktop/src/shell/DesktopShellEnvironment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
12 changes: 11 additions & 1 deletion apps/desktop/src/shell/DesktopShellEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> =>
executableName(shell) === "zsh" ? ["+m", "-ilc", command] : ["-lc", command];

const readLoginShellEnvironment = (
shell: string,
names: ReadonlyArray<string>,
Expand All @@ -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)));

Expand Down
46 changes: 44 additions & 2 deletions packages/shared/src/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const withWindowsEnvironmentMocks = <A, E, R>(
);

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,
Expand All @@ -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<string>,
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<string>,
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", () => {
Expand Down
24 changes: 20 additions & 4 deletions packages/shared/src/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,18 @@ export type ShellEnvironmentReader = (
execFile?: ExecFileSyncLike,
) => Partial<Record<string, string>>;

// 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<string> {
return NodePath.posix.basename(shell) === "zsh" ? ["+m", "-ilc", command] : ["-lc", command];
}

export const readEnvironmentFromLoginShell: ShellEnvironmentReader = (
shell,
names,
Expand All @@ -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<Record<string, string>> = {};
for (const name of names) {
Expand Down
2 changes: 1 addition & 1 deletion scripts/mobile-showcase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ async function createShowcaseShell(baseDir: string): Promise<string> {
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
Expand Down
Loading