fix(shell): bound PATH repair fallbacks by Chrono-byte · Pull Request #831 · pingdotgg/t3code

Chrono-byte · 2026-03-10T19:22:39Z

What Changed

Tightened desktop/server PATH repair on macOS and Linux by moving recovery through a shared resolver with bounded fallbacks.

Why

Packaged launches can start with an incomplete PATH, which breaks downstream process resolution. We need to recover a login-shell PATH, but doing that by probing too many shells synchronously can stall startup. This change keeps the recovery behavior while capping worst-case startup cost.

Note

Bound PATH repair fallbacks with multi-shell candidates and a platform gate

Replaces single-shell, macOS-only PATH repair in fixPath and os-jank.ts with a shared, cross-platform flow from @t3tools/shared/shell.
Adds shouldRepairPath to gate repair by platform (darwin/linux only) and PATH contents (e.g. missing /opt/homebrew/bin on macOS).
Adds resolvePathFromLoginShells which iterates defaultShellCandidates() within a 2,000ms global deadline, falling back from -ilc to -lc per shell at 750ms each.
Behavioral Change: PATH repair no longer runs on unsupported platforms or when PATH already contains expected entries; per-attempt timeout drops from 5,000ms to 750ms.

^{Macroscope summarized e488a7f.}

coderabbitai · 2026-03-10T19:22:46Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a11ced47-8e06-49e6-873c-aa35669e9246

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR refactors PATH “repair” during startup on macOS and Linux by centralizing login-shell PATH resolution behind a shared resolver with a bounded set of shell candidates and invocation modes, to avoid excessive synchronous probing.

Changes:

Add bounded shell candidate selection (defaultShellCandidates) and a multi-shell resolver (resolvePathFromLoginShells) in packages/shared.
Update desktop + server startup PATH repair to use the shared resolver and enable Linux support.
Expand unit tests for the new fallback behavior and candidate selection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
packages/shared/src/shell.ts	Adds login-shell PATH resolution fallbacks and shared candidate/resolution helpers.
packages/shared/src/shell.test.ts	Adds tests for the new fallback behavior and default candidate sets.
apps/server/src/os-jank.ts	Switches server PATH repair to the shared bounded resolver; enables Linux.
apps/desktop/src/fixPath.ts	Switches desktop PATH repair to the shared bounded resolver; enables Linux.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T19:27:32Z

packages/shared/src/shell.ts

+type ShellPathResolveErrorReporter = (shell: string, error: unknown) => void;
+
+const defaultShellPathErrorReporter: ShellPathResolveErrorReporter | undefined =
+  process.env.T3CODE_DEBUG_SHELL_PATH === "1"
+    ? (shell, error) => {
+        const message = error instanceof Error ? error.message : String(error);
+        console.warn(`[shell] PATH resolution failed for ${shell}: ${message}`);
+      }
+    : undefined;
+
+export function resolvePathFromLoginShells(
+  shells: ReadonlyArray<string>,
+  execFile: ExecFileSyncLike = execFileSync,
+  onError: ShellPathResolveErrorReporter | undefined = defaultShellPathErrorReporter,
+): string | undefined {
+  for (const shell of shells) {
+    try {
+      const result = readPathFromLoginShell(shell, execFile);
+      if (result) {
+        return result;
+      }
+    } catch (error) {
+      onError?.(shell, error);
+      // Try next shell candidate.
+    }
+  }


resolvePathFromLoginShells's onError hook (and the T3CODE_DEBUG_SHELL_PATH logging) is effectively unreachable for normal shell failures because readPathFromLoginShell swallows all execFile errors internally. If the intent is to report per-shell failures, either let errors bubble up from readPathFromLoginShell (after trying its arg fallbacks) or invoke the reporter from within readPathFromLoginShell when each mode fails.

Copilot · 2026-03-10T19:27:32Z

apps/server/src/os-jank.ts

 export function fixPath(): void {
-  if (process.platform !== "darwin") return;
+  if (process.platform !== "darwin" && process.platform !== "linux") return;

-  try {
-    const shell = process.env.SHELL ?? "/bin/zsh";
-    const result = readPathFromLoginShell(shell);
-    if (result) {
-      process.env.PATH = result;
-    }
-  } catch {
-    // Silently ignore — keep default PATH
+  const shells = defaultShellCandidates();
+
+  const resolvedPath = resolvePathFromLoginShells(shells);
+  if (resolvedPath) {
+    process.env.PATH = resolvedPath;
  }


On darwin/linux this runs a synchronous PATH repair that can block startup for up to timeout * argModes * shellCandidates (currently 5000ms * 2 * up to 3 = 30s on macOS; 20s on Linux). If the PR goal is to cap worst-case startup cost, consider lowering the per-invocation timeout and/or adding an overall deadline / gating the repair to cases where PATH looks incomplete.

Copilot · 2026-03-10T19:27:33Z

apps/server/src/os-jank.ts

+
+  const resolvedPath = resolvePathFromLoginShells(shells);


defaultShellCandidates() includes process.env.SHELL, which means on Linux this server startup path repair may execute whatever binary is pointed to by SHELL. If this code can run in environments where SHELL is not trusted/controlled (e.g., service managers, container entrypoints), consider ignoring process.env.SHELL here or validating/allowlisting shell paths before executing them.

Suggested change

const resolvedPath = resolvePathFromLoginShells(shells);

const allowedShells = new Set<string>([

"/bin/bash",

"/usr/bin/bash",

"/bin/zsh",

"/usr/bin/zsh",

"/bin/sh",

"/usr/bin/sh",

"/bin/dash",

"/usr/bin/dash",

"/bin/fish",

"/usr/bin/fish",

]);

const filteredShells = shells.filter((shell) => allowedShells.has(shell));

if (filteredShells.length === 0) return;

const resolvedPath = resolvePathFromLoginShells(filteredShells);

Copilot · 2026-03-10T19:27:33Z

apps/desktop/src/fixPath.ts

 export function fixPath(): void {
-  if (process.platform !== "darwin") return;
+  if (process.platform !== "darwin" && process.platform !== "linux") return;

-  try {
-    const shell = process.env.SHELL ?? "/bin/zsh";
-    const result = readPathFromLoginShell(shell);
-    if (result) {
-      process.env.PATH = result;
-    }
-  } catch {
-    // Keep inherited PATH if shell lookup fails.
+  const result = resolvePathFromLoginShells(defaultShellCandidates());
+  if (result) {
+    process.env.PATH = result;
  }


This runs a synchronous PATH repair on Linux as well as macOS; with the current 5s timeout and two invocation modes per shell, Electron main startup can block for up to ~30s in the worst case. Consider reducing the timeout and/or short-circuiting when PATH is already populated enough to resolve required binaries.

Copilot · 2026-03-10T19:27:33Z

packages/shared/src/shell.test.ts

+    it("returns the first resolved PATH from the provided shells", () => {
+      const execFile = vi.fn<
+        (
+          file: string,
+          args: ReadonlyArray<string>,
+          options: { encoding: "utf8"; timeout: number },
+        ) => string
+      >((file) => {
+        if (file === "/bin/zsh") {
+          throw new Error("zsh unavailable");
+        }
+        return "__T3CODE_PATH_START__\n/a:/b\n__T3CODE_PATH_END__\n";
+      });
+
+      const result = resolvePathFromLoginShells(["/bin/zsh", "/bin/bash"], execFile);
+      expect(result).toBe("/a:/b");
+      expect(execFile).toHaveBeenCalledTimes(2);
+    });


The resolvePathFromLoginShells test undercounts how many execFile calls occur. readPathFromLoginShell now tries both -ilc and -lc per shell candidate, so a shell that always throws (like /bin/zsh in this mock) will be invoked twice before moving on. Update the expectation (or the mock) so the call count matches the two-mode fallback behavior.

packages/shared/src/shell.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(shell): bound PATH repair fallbacks

70ca0c4

Copilot AI review requested due to automatic review settings March 10, 2026 19:22

github-actions bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Mar 10, 2026

Copilot started reviewing on behalf of Chrono-byte March 10, 2026 19:23 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Chrono-byte and others added 2 commits March 10, 2026 15:38

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

4b617ff

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(shell): bound PATH repair startup cost

e488a7f

Chrono-byte added a commit to Chrono-byte/t3code-linux that referenced this pull request Mar 10, 2026

Merge PR pingdotgg#831 branch

5851939

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(shell): bound PATH repair fallbacks#831

fix(shell): bound PATH repair fallbacks#831
Chrono-byte wants to merge 3 commits intopingdotgg:mainfrom
Chrono-byte:split/path-repair

Chrono-byte commented Mar 10, 2026 •

edited by macroscopeapp bot

Loading

Uh oh!

coderabbitai bot commented Mar 10, 2026 •

edited

Loading

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-  const resolvedPath = resolvePathFromLoginShells(shells);
+  const allowedShells = new Set<string>([
+    "/bin/bash",
+    "/usr/bin/bash",
+    "/bin/zsh",
+    "/usr/bin/zsh",
+    "/bin/sh",
+    "/usr/bin/sh",
+    "/bin/dash",
+    "/usr/bin/dash",
+    "/bin/fish",
+    "/usr/bin/fish",
+  ]);
+  const filteredShells = shells.filter((shell) => allowedShells.has(shell));
+  if (filteredShells.length === 0) return;
+  const resolvedPath = resolvePathFromLoginShells(filteredShells);

Conversation

Chrono-byte commented Mar 10, 2026 • edited by macroscopeapp bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Changed

Why

Bound PATH repair fallbacks with multi-shell candidates and a platform gate

Uh oh!

coderabbitai bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Chrono-byte commented Mar 10, 2026 •

edited by macroscopeapp bot

Loading

coderabbitai bot commented Mar 10, 2026 •

edited

Loading