From e1c33dcd33949c18eb30986ba513c1334064677c Mon Sep 17 00:00:00 2001 From: Guilherme Barros Date: Sat, 7 Mar 2026 11:14:30 +0100 Subject: [PATCH] Fix Fish shell PATH detection and clean up lint warnings (#323) * Fix Fish shell PATH detection for Codex * refactor(provider): resolve lint warnings in provider layers * refactor(terminal): simplify thread lock release initialization * test(git): simplify fetch release callback initialization --- apps/desktop/src/fixPath.ts | 7 +-- apps/server/src/git/Layers/GitCore.test.ts | 2 +- apps/server/src/os-jank.ts | 7 +-- .../src/provider/Layers/CodexAdapter.test.ts | 1 - .../src/provider/Layers/ProviderService.ts | 18 +++--- apps/server/src/terminal/Layers/Manager.ts | 2 +- packages/shared/package.json | 4 ++ packages/shared/src/shell.test.ts | 55 +++++++++++++++++++ packages/shared/src/shell.ts | 38 +++++++++++++ 9 files changed, 114 insertions(+), 20 deletions(-) create mode 100644 packages/shared/src/shell.test.ts create mode 100644 packages/shared/src/shell.ts diff --git a/apps/desktop/src/fixPath.ts b/apps/desktop/src/fixPath.ts index 150848c14c632530b80b199813f111b06263bc02..8853248b247c2314c1e8675733153486bc0d3c16 100644 --- a/apps/desktop/src/fixPath.ts +++ b/apps/desktop/src/fixPath.ts @@ -1,14 +1,11 @@ -import * as ChildProcess from "node:child_process"; +import { readPathFromLoginShell } from "@t3tools/shared/shell"; export function fixPath(): void { if (process.platform !== "darwin") return; try { const shell = process.env.SHELL ?? "/bin/zsh"; - const result = ChildProcess.execFileSync(shell, ["-ilc", "echo -n $PATH"], { - encoding: "utf8", - timeout: 5000, - }); + const result = readPathFromLoginShell(shell); if (result) { process.env.PATH = result; } diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 0c0e914032438760727a8ccdbd7bdd79499c3ff5..d03ad60615fbd6fedf00d363c28d05aab0bf72fd 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -654,7 +654,7 @@ it.layer(TestLayer)("git integration", (it) => { const realGitService = yield* GitService; let fetchStarted = false; - let releaseFetch: () => void = () => undefined; + let releaseFetch!: () => void; const waitForReleasePromise = new Promise((resolve) => { releaseFetch = resolve; }); diff --git a/apps/server/src/os-jank.ts b/apps/server/src/os-jank.ts index 938b4280c33ce9bf66bb320b0380eb82bdf78f4b..586aca6f79c3dbd8827d12235605dedb1103e964 100644 --- a/apps/server/src/os-jank.ts +++ b/apps/server/src/os-jank.ts @@ -1,16 +1,13 @@ import * as OS from "node:os"; import { Effect, Path } from "effect"; -import { execFileSync } from "node:child_process"; +import { readPathFromLoginShell } from "@t3tools/shared/shell"; export function fixPath(): void { if (process.platform !== "darwin") return; try { const shell = process.env.SHELL ?? "/bin/zsh"; - const result = execFileSync(shell, ["-ilc", "echo -n $PATH"], { - encoding: "utf8", - timeout: 5000, - }); + const result = readPathFromLoginShell(shell); if (result) { process.env.PATH = result; } diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index 09de4aa3d69487e543df316b38b8795ed81c737a..c977f5dadc7a6fdcb4fa6d2f709f5967c34c624c 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -22,7 +22,6 @@ import { type CodexAppServerSendTurnInput, } from "../../codexAppServerManager.ts"; import { ServerConfig } from "../../config.ts"; -import { ProviderAdapterValidationError } from "../Errors.ts"; import { CodexAdapter } from "../Services/CodexAdapter.ts"; import { ProviderSessionDirectory } from "../Services/ProviderSessionDirectory.ts"; import { makeCodexAdapterLive } from "./CodexAdapter.ts"; diff --git a/apps/server/src/provider/Layers/ProviderService.ts b/apps/server/src/provider/Layers/ProviderService.ts index 285e0a870963f5ab7ee6a423926cddd6f71364fc..398a26fb7b9f0182e9825878ca7c5b233e80a538 100644 --- a/apps/server/src/provider/Layers/ProviderService.ts +++ b/apps/server/src/provider/Layers/ProviderService.ts @@ -436,13 +436,17 @@ const makeProviderService = (options?: ProviderServiceLiveOptions) => return session; } - return { - ...session, - ...(session.resumeCursor === undefined && binding.resumeCursor !== undefined - ? { resumeCursor: binding.resumeCursor } - : {}), - ...(binding.runtimeMode !== undefined ? { runtimeMode: binding.runtimeMode } : {}), - }; + const overrides: { + resumeCursor?: ProviderSession["resumeCursor"]; + runtimeMode?: ProviderSession["runtimeMode"]; + } = {}; + if (session.resumeCursor === undefined && binding.resumeCursor !== undefined) { + overrides.resumeCursor = binding.resumeCursor; + } + if (binding.runtimeMode !== undefined) { + overrides.runtimeMode = binding.runtimeMode; + } + return Object.assign({}, session, overrides); }); }); diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index a0e4ad705ccf44a3838a7d936c84570541e46b24..771c56b287e83bfc907daece15e37f55e1e682ef 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -1150,7 +1150,7 @@ export class TerminalManagerRuntime extends EventEmitter private async runWithThreadLock(threadId: string, task: () => Promise): Promise { const previous = this.threadLocks.get(threadId) ?? Promise.resolve(); - let release: () => void = () => {}; + let release!: () => void; const current = new Promise((resolve) => { release = resolve; }); diff --git a/packages/shared/package.json b/packages/shared/package.json index 4ac86444b8c077b21feb97c50b5d3c34474d2c6c..b1a94c76048f6e69b3cf39bc248dda9892064453 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -16,6 +16,10 @@ "types": "./src/logging.ts", "import": "./src/logging.ts" }, + "./shell": { + "types": "./src/shell.ts", + "import": "./src/shell.ts" + }, "./Net": { "types": "./src/Net.ts", "import": "./src/Net.ts" diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..83006988b58b5eedefad3df881ce49b4a9cded3f --- /dev/null +++ b/packages/shared/src/shell.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it, vi } from "vitest"; + +import { extractPathFromShellOutput, readPathFromLoginShell } from "./shell"; + +describe("extractPathFromShellOutput", () => { + it("extracts the path between capture markers", () => { + expect( + extractPathFromShellOutput("__T3CODE_PATH_START__\n/opt/homebrew/bin:/usr/bin\n__T3CODE_PATH_END__\n"), + ).toBe("/opt/homebrew/bin:/usr/bin"); + }); + + it("ignores shell startup noise around the capture markers", () => { + expect( + extractPathFromShellOutput( + "Welcome to fish\n__T3CODE_PATH_START__\n/opt/homebrew/bin:/usr/bin\n__T3CODE_PATH_END__\nBye\n", + ), + ).toBe("/opt/homebrew/bin:/usr/bin"); + }); + + it("returns null when the markers are missing", () => { + expect(extractPathFromShellOutput("/opt/homebrew/bin /usr/bin")).toBeNull(); + }); +}); + +describe("readPathFromLoginShell", () => { + it("uses a shell-agnostic printenv PATH probe", () => { + const execFile = vi.fn< + ( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number }, + ) => string + >(() => "__T3CODE_PATH_START__\n/a:/b\n__T3CODE_PATH_END__\n"); + + expect(readPathFromLoginShell("/opt/homebrew/bin/fish", execFile)).toBe("/a:/b"); + expect(execFile).toHaveBeenCalledTimes(1); + + const firstCall = execFile.mock.calls[0] as + | [string, ReadonlyArray, { encoding: "utf8"; timeout: number }] + | undefined; + expect(firstCall).toBeDefined(); + if (!firstCall) { + throw new Error("Expected execFile to be called"); + } + + const [shell, args, options] = firstCall; + expect(shell).toBe("/opt/homebrew/bin/fish"); + expect(args).toHaveLength(2); + expect(args?.[0]).toBe("-ilc"); + expect(args?.[1]).toContain("printenv PATH"); + expect(args?.[1]).toContain("__T3CODE_PATH_START__"); + expect(args?.[1]).toContain("__T3CODE_PATH_END__"); + expect(options).toEqual({ encoding: "utf8", timeout: 5000 }); + }); +}); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts new file mode 100644 index 0000000000000000000000000000000000000000..e6029c4432b1346cc5591349930889c3fafa0880 --- /dev/null +++ b/packages/shared/src/shell.ts @@ -0,0 +1,38 @@ +import { execFileSync } from "node:child_process"; + +const PATH_CAPTURE_START = "__T3CODE_PATH_START__"; +const PATH_CAPTURE_END = "__T3CODE_PATH_END__"; +const PATH_CAPTURE_COMMAND = [ + `printf '%s\n' '${PATH_CAPTURE_START}'`, + "printenv PATH", + `printf '%s\n' '${PATH_CAPTURE_END}'`, +].join("; "); + +type ExecFileSyncLike = ( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number }, +) => string; + +export function extractPathFromShellOutput(output: string): string | null { + const startIndex = output.indexOf(PATH_CAPTURE_START); + if (startIndex === -1) return null; + + const valueStartIndex = startIndex + PATH_CAPTURE_START.length; + const endIndex = output.indexOf(PATH_CAPTURE_END, valueStartIndex); + if (endIndex === -1) return null; + + const pathValue = output.slice(valueStartIndex, endIndex).trim(); + return pathValue.length > 0 ? pathValue : null; +} + +export function readPathFromLoginShell( + shell: string, + execFile: ExecFileSyncLike = execFileSync, +): string | undefined { + const output = execFile(shell, ["-ilc", PATH_CAPTURE_COMMAND], { + encoding: "utf8", + timeout: 5000, + }); + return extractPathFromShellOutput(output) ?? undefined; +} -- 2.54.0