Skip to content

Commit 1fab30d

Browse files
juliusmarmingejuliusmarminge
committed
fix(shell): detach environment probes and retain interactive startup
1 parent 3093fe2 commit 1fab30d

5 files changed

Lines changed: 59 additions & 57 deletions

File tree

‎apps/desktop/src/shell/DesktopShellEnvironment.test.ts‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ describe("DesktopShellEnvironment", () => {
158158
userShell: "/usr/local/bin/fish",
159159
handler: (command) => {
160160
if (command._tag !== "StandardCommand") return "";
161-
assert.deepEqual(command.args.slice(0, -1), ["-lc"]);
162161
commands.push(command.command);
163162
return envOutput({ PATH: "/home/test/.local/bin:/usr/bin" });
164163
},
@@ -214,10 +213,7 @@ describe("DesktopShellEnvironment", () => {
214213
});
215214

216215
assert.equal(commands.length, 1);
217-
const probe = commands[0];
218-
assert(probe?._tag === "StandardCommand");
219-
assert.equal(probe.command, "/bin/zsh");
220-
assert.deepEqual(probe.args.slice(0, -1), ["+m", "-ilc"]);
216+
assert.equal(commands[0]?._tag === "StandardCommand" ? commands[0].command : "", "/bin/zsh");
221217
assert.equal(env.PATH, "/opt/homebrew/bin:/usr/bin:/Users/test/.local/bin");
222218
assert.equal(env.SSH_AUTH_SOCK, "/tmp/secretive.sock");
223219
assert.equal(env.HOMEBREW_PREFIX, "/opt/homebrew");
@@ -409,21 +405,13 @@ describe("DesktopShellEnvironment", () => {
409405
yield* runShellEnvironment({
410406
env,
411407
platform: "linux",
412-
handler: (command) => {
413-
assert(command._tag === "StandardCommand");
414-
assert.equal(command.command, "/bin/bash");
415-
assert.deepEqual(command.args.slice(0, -1), ["-lc"]);
416-
return envOutput({
408+
handler: () =>
409+
envOutput({
417410
PATH: "/home/me/.local/bin:/mise/installs/claude/latest:/mise/shims",
418411
__MISE_DIFF: "mise-diff",
419-
});
420-
},
412+
}),
421413
});
422414

423-
assert.equal(
424-
env.PATH,
425-
"/home/me/.local/bin:/mise/installs/claude/latest:/mise/shims:/usr/bin",
426-
);
427415
assert.equal(env.__MISE_DIFF, "mise-diff");
428416
}),
429417
);

‎apps/desktop/src/shell/DesktopShellEnvironment.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
listLoginShellCandidates,
3-
loginShellProbeArgs,
4-
setWindowsPath,
5-
} from "@t3tools/shared/shell";
1+
import { listLoginShellCandidates, setWindowsPath } from "@t3tools/shared/shell";
62
import * as Context from "effect/Context";
73
import * as Duration from "effect/Duration";
84
import * as Effect from "effect/Effect";
@@ -324,7 +320,7 @@ const readLoginShellEnvironment = (
324320
: runCommandOutput({
325321
probe: "login-shell",
326322
command: shell,
327-
args: loginShellProbeArgs(shell, capturePosixEnvironmentCommand(names)),
323+
args: ["-ilc", capturePosixEnvironmentCommand(names)],
328324
timeout: LOGIN_SHELL_TIMEOUT,
329325
}).pipe(Effect.map((output) => extractEnvironment(output, names)));
330326

‎packages/shared/src/shell.test.ts‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,42 @@ const recordCommandTrace = Effect.fnUntraced(function* <A, E, R>(effect: Effect.
5858
});
5959

6060
describe("readEnvironmentFromLoginShell probe", () => {
61-
it.each(["/bin/bash", "/opt/homebrew/bin/fish"])(
62-
"captures PATH with a noninteractive login probe for %s",
63-
(shell) => {
64-
const execFile = vi.fn(() => "__T3CODE_ENV_PATH_START__\n/a:/b\n__T3CODE_ENV_PATH_END__\n");
65-
66-
expect(readEnvironmentFromLoginShell(shell, ["PATH"], execFile)).toEqual({ PATH: "/a:/b" });
67-
expect(execFile).toHaveBeenCalledExactlyOnceWith(
68-
shell,
69-
["-lc", expect.stringContaining("printenv PATH || true")],
70-
{ encoding: "utf8", timeout: 5000 },
71-
);
72-
},
73-
);
61+
it("uses a shell-agnostic printenv PATH probe", () => {
62+
const execFile = vi.fn<
63+
(
64+
file: string,
65+
args: ReadonlyArray<string>,
66+
options: { encoding: "utf8"; timeout: number },
67+
) => string
68+
>(() => "__T3CODE_ENV_PATH_START__\n/a:/b\n__T3CODE_ENV_PATH_END__\n");
69+
70+
expect(readEnvironmentFromLoginShell("/opt/homebrew/bin/fish", ["PATH"], execFile).PATH).toBe(
71+
"/a:/b",
72+
);
73+
expect(execFile).toHaveBeenCalledTimes(1);
74+
75+
const firstCall = execFile.mock.calls[0] as
76+
| [string, ReadonlyArray<string>, { encoding: "utf8"; timeout: number }]
77+
| undefined;
78+
expect(firstCall).toBeDefined();
79+
if (!firstCall) {
80+
throw new Error("Expected execFile to be called");
81+
}
82+
83+
const [shell, args, options] = firstCall;
84+
expect(shell).toBe("/opt/homebrew/bin/fish");
85+
expect(args).toHaveLength(2);
86+
expect(args?.[0]).toBe("-ilc");
87+
expect(args?.[1]).toContain("printenv PATH || true");
88+
expect(args?.[1]).toContain("__T3CODE_ENV_PATH_START__");
89+
expect(args?.[1]).toContain("__T3CODE_ENV_PATH_END__");
90+
expect(options).toEqual({
91+
encoding: "utf8",
92+
timeout: 5000,
93+
detached: true,
94+
stdio: ["ignore", "pipe", "ignore"],
95+
});
96+
});
7497
});
7598

7699
describe("readPathFromLaunchctl", () => {
@@ -106,7 +129,7 @@ describe("readPathFromLaunchctl", () => {
106129
});
107130

108131
describe("readEnvironmentFromLoginShell", () => {
109-
it("captures zsh environment interactively with job control disabled", () => {
132+
it("extracts multiple environment variables from a login shell command", () => {
110133
const execFile = vi.fn<
111134
(
112135
file: string,
@@ -128,11 +151,7 @@ describe("readEnvironmentFromLoginShell", () => {
128151
PATH: "/a:/b",
129152
SSH_AUTH_SOCK: "/tmp/secretive.sock",
130153
});
131-
expect(execFile).toHaveBeenCalledExactlyOnceWith(
132-
"/bin/zsh",
133-
["+m", "-ilc", expect.stringContaining("printenv SSH_AUTH_SOCK || true")],
134-
{ encoding: "utf8", timeout: 5000 },
135-
);
154+
expect(execFile).toHaveBeenCalledTimes(1);
136155
});
137156

138157
it("omits environment variables that are missing or empty", () => {

‎packages/shared/src/shell.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ const WINDOWS_SHELL_CANDIDATES = ["pwsh.exe", "powershell.exe"] as const;
2020
type ExecFileSyncLike = (
2121
file: string,
2222
args: ReadonlyArray<string>,
23-
options: { encoding: "utf8"; timeout: number },
23+
options: {
24+
encoding: "utf8";
25+
timeout: number;
26+
detached?: true;
27+
stdio?: ["ignore", "pipe", "ignore"];
28+
},
2429
) => string;
2530

2631
function canExecuteFile(filePath: string): boolean {
@@ -259,12 +264,6 @@ export type ShellEnvironmentReader = (
259264
execFile?: ExecFileSyncLike,
260265
) => Partial<Record<string, string>>;
261266

262-
// Interactive job control can stop probes with SIGTTOU on an inherited TTY.
263-
// zsh needs -i for .zshrc PATH entries; +m disables its job control before startup.
264-
export function loginShellProbeArgs(shell: string, command: string) {
265-
return NodePath.posix.basename(shell) === "zsh" ? ["+m", "-ilc", command] : ["-lc", command];
266-
}
267-
268267
export const readEnvironmentFromLoginShell: ShellEnvironmentReader = (
269268
shell,
270269
names,
@@ -274,14 +273,14 @@ export const readEnvironmentFromLoginShell: ShellEnvironmentReader = (
274273
return {};
275274
}
276275

277-
const output = execFile(
278-
shell,
279-
loginShellProbeArgs(shell, buildEnvironmentCaptureCommand(names)),
280-
{
281-
encoding: "utf8",
282-
timeout: 5000,
283-
},
284-
);
276+
// Interactive startup loads PATH entries; detaching prevents job control on an
277+
// inherited controlling TTY from stopping the probe while the parent waits.
278+
const output = execFile(shell, ["-ilc", buildEnvironmentCaptureCommand(names)], {
279+
encoding: "utf8",
280+
timeout: 5000,
281+
detached: true,
282+
stdio: ["ignore", "pipe", "ignore"],
283+
});
285284

286285
const environment: Partial<Record<string, string>> = {};
287286
for (const name of names) {

‎scripts/mobile-showcase.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ async function createShowcaseShell(baseDir: string): Promise<string> {
573573
await NodeFSP.writeFile(
574574
shellPath,
575575
`#!/bin/sh
576-
if [ "$1" = "-lc" ] || [ "$1" = "-ilc" ] || [ "$1" = "-lic" ]; then
576+
if [ "$1" = "-ilc" ] || [ "$1" = "-lic" ]; then
577577
exec /bin/sh -c "$2"
578578
fi
579579
exec /bin/cat

0 commit comments

Comments
 (0)