Skip to content

Commit 526ff40

Browse files
committed
fix: defer launchctl subprocess, remove dead resolveLoginShell, capture error in outer catch
- Defer readPathFromLaunchctl() on macOS so it only spawns when the login shell did not provide a PATH (avoids unnecessary synchronous subprocess) - Remove resolveLoginShell which had no production callers after refactor - Capture error object in outer catch of syncShellEnvironment.ts to match the os-jank.ts pattern and preserve error details in log output
1 parent 0d649e0 commit 526ff40

File tree

4 files changed

+8
-21
lines changed

4 files changed

+8
-21
lines changed

apps/desktop/src/syncShellEnvironment.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ export function syncShellEnvironment(
5050
}
5151

5252
const launchctlPath =
53-
platform === "darwin" ? (options.readLaunchctlPath ?? readPathFromLaunchctl)() : undefined;
53+
platform === "darwin" && !shellEnvironment.PATH
54+
? (options.readLaunchctlPath ?? readPathFromLaunchctl)()
55+
: undefined;
5456
const mergedPath = mergePathEntries(shellEnvironment.PATH ?? launchctlPath, env.PATH, platform);
5557
if (mergedPath) {
5658
env.PATH = mergedPath;
@@ -71,7 +73,7 @@ export function syncShellEnvironment(
7173
env[name] = shellEnvironment[name];
7274
}
7375
}
74-
} catch {
75-
logWarning("Failed to synchronize the desktop shell environment.");
76+
} catch (error) {
77+
logWarning("Failed to synchronize the desktop shell environment.", error);
7678
}
7779
}

apps/server/src/os-jank.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ export function fixPath(
4343
}
4444

4545
const launchctlPath =
46-
platform === "darwin" ? (options.readLaunchctlPath ?? readPathFromLaunchctl)() : undefined;
46+
platform === "darwin" && !shellPath
47+
? (options.readLaunchctlPath ?? readPathFromLaunchctl)()
48+
: undefined;
4749
const mergedPath = mergePathEntries(shellPath ?? launchctlPath, env.PATH, platform);
4850
if (mergedPath) {
4951
env.PATH = mergedPath;

packages/shared/src/shell.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
readEnvironmentFromLoginShell,
88
readPathFromLaunchctl,
99
readPathFromLoginShell,
10-
resolveLoginShell,
1110
} from "./shell";
1211

1312
describe("extractPathFromShellOutput", () => {
@@ -176,14 +175,6 @@ describe("listLoginShellCandidates", () => {
176175
});
177176
});
178177

179-
describe("resolveLoginShell", () => {
180-
it("returns the first available login shell candidate", () => {
181-
expect(resolveLoginShell("darwin", undefined, "/opt/homebrew/bin/fish")).toBe(
182-
"/opt/homebrew/bin/fish",
183-
);
184-
});
185-
});
186-
187178
describe("mergePathEntries", () => {
188179
it("prefers login-shell PATH entries and keeps inherited extras", () => {
189180
expect(

packages/shared/src/shell.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ export function listLoginShellCandidates(
4545
return candidates;
4646
}
4747

48-
export function resolveLoginShell(
49-
platform: NodeJS.Platform,
50-
shell: string | undefined,
51-
userShell = readUserLoginShell(),
52-
): string | undefined {
53-
return listLoginShellCandidates(platform, shell, userShell)[0];
54-
}
55-
5648
export function extractPathFromShellOutput(output: string): string | null {
5749
const startIndex = output.indexOf(PATH_CAPTURE_START);
5850
if (startIndex === -1) return null;

0 commit comments

Comments
 (0)