From daff742741ed394111ad52ca9274cc80408d7358 Mon Sep 17 00:00:00 2001 From: Jimmy Stridh Date: Thu, 28 May 2026 15:07:45 +0200 Subject: [PATCH 1/2] fix(git): use shell PATH for GUI subprocesses --- .../domains/terminal/src/main/shell-env.ts | 41 ++++++++++++++----- .../domains/worktrees/src/main/exec-async.ts | 9 ++++ packages/domains/worktrees/src/main/gh-cli.ts | 14 +++---- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/packages/domains/terminal/src/main/shell-env.ts b/packages/domains/terminal/src/main/shell-env.ts index 47fa470e..bedd8cfc 100644 --- a/packages/domains/terminal/src/main/shell-env.ts +++ b/packages/domains/terminal/src/main/shell-env.ts @@ -39,28 +39,43 @@ export { * (chat-handlers) so subprocess Bash tools find the same binaries the user has. */ let cachedEnrichedPath: string | null | undefined +const PATH_OUTPUT_MARKER = '__SLAYZONE_PATH__' + export function getEnrichedPath(): string | null { if (cachedEnrichedPath !== undefined) return cachedEnrichedPath cachedEnrichedPath = resolveEnrichedPath() return cachedEnrichedPath } +function extractPathProbeOutput(output: string): string | null { + for (const line of output.split(/\r?\n/).reverse()) { + if (!line.startsWith(PATH_OUTPUT_MARKER)) continue + const value = line.slice(PATH_OUTPUT_MARKER.length).trim() + return value || null + } + return null +} + function resolveEnrichedPath(): string | null { if (platform() === 'win32') return null const shell = resolveUserShell() // Fish: $PATH is a list — `echo $PATH` prints space-separated, invalid as OS PATH. // `string join ":" $PATH` produces colon-joined. const isFish = shell.endsWith('/fish') - const cmd = isFish ? 'string join ":" $PATH' : 'echo $PATH' + const cmd = isFish + ? `printf "\\n${PATH_OUTPUT_MARKER}%s\\n" (string join ":" $PATH)` + : `printf '\\n${PATH_OUTPUT_MARKER}%s\\n' "$PATH"` // Interactive login shell first — sources .zshrc/.bashrc where pnpm/nvm add PATH try { const args = getShellStartupArgs(shell) const result = execFileSync(shell, [...args, '-c', cmd], { timeout: 5000, - encoding: 'utf-8' - }).trim() - if (result) return result + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'] + }) + const pathValue = extractPathProbeOutput(result) + if (pathValue) return pathValue } catch { /* fall through */ } @@ -69,9 +84,11 @@ function resolveEnrichedPath(): string | null { try { const result = execFileSync(shell, ['-l', '-c', cmd], { timeout: 5000, - encoding: 'utf-8' - }).trim() - if (result) return result + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'] + }) + const pathValue = extractPathProbeOutput(result) + if (pathValue) return pathValue } catch { /* fall through */ } @@ -152,10 +169,14 @@ export async function whichBinary(name: string): Promise { } try { - const shell = resolveUserShell() - const shellArgs = getShellStartupArgs(shell) + const env = { ...process.env } + const enrichedPath = getEnrichedPath() + if (enrichedPath) env.PATH = enrichedPath const checkCmd = `command -v ${quoteForShell(name)}` - const { stdout } = await execFileAsync(shell, [...shellArgs, '-c', checkCmd], { timeout: 3000 }) + const { stdout } = await execFileAsync('/bin/sh', ['-c', checkCmd], { + timeout: 3000, + env + }) const found = stdout.trim().split('\n')[0] return found || null } catch { diff --git a/packages/domains/worktrees/src/main/exec-async.ts b/packages/domains/worktrees/src/main/exec-async.ts index 8910677b..6cc9a02a 100644 --- a/packages/domains/worktrees/src/main/exec-async.ts +++ b/packages/domains/worktrees/src/main/exec-async.ts @@ -1,5 +1,6 @@ import { spawn } from 'child_process' import { recordDiagnosticEvent } from '@slayzone/diagnostics/main' +import { getEnrichedPath } from '@slayzone/terminal/main' import type { DiagnosticSource } from '@slayzone/diagnostics/shared' export function trimOutput(value: unknown, maxLength = 1200): string | null { @@ -16,6 +17,13 @@ export interface ExecResult { status: number | null } +function subprocessEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env } + const enrichedPath = getEnrichedPath() + if (enrichedPath) env.PATH = enrichedPath + return env +} + /** Async subprocess execution — won't block the main process. */ export function execAsync( command: string, @@ -28,6 +36,7 @@ export function execAsync( const startedAt = Date.now() const child = spawn(command, args, { cwd: opts.cwd, + env: subprocessEnv(), stdio: ['pipe', 'pipe', 'pipe'] }) const stdout: string[] = [] diff --git a/packages/domains/worktrees/src/main/gh-cli.ts b/packages/domains/worktrees/src/main/gh-cli.ts index 43d0b4b0..7a7dcc63 100644 --- a/packages/domains/worktrees/src/main/gh-cli.ts +++ b/packages/domains/worktrees/src/main/gh-cli.ts @@ -1,4 +1,4 @@ -import { whichBinary, resolveUserShell, getShellStartupArgs } from '@slayzone/terminal/main' +import { whichBinary } from '@slayzone/terminal/main' import type { GhPullRequest, GhPrComment, @@ -22,16 +22,12 @@ async function resolveGhPath(): Promise { return ghPath } -/** Run gh with the user's shell environment so PATH is correct. */ +/** Run gh directly so shell startup output cannot corrupt JSON stdout. */ async function spawnGh(args: string[], opts: { cwd?: string; timeout?: number } = {}) { - if (!ghPath) await resolveGhPath() - if (!ghPath) throw new Error('gh CLI not found') + const resolvedGhPath = await resolveGhPath() + if (!resolvedGhPath) throw new Error('gh CLI not found') - const shell = resolveUserShell() - const shellArgs = getShellStartupArgs(shell) - const cmd = [ghPath, ...args].map((a) => `'${a.replace(/'/g, `'"'"'`)}'`).join(' ') - - return execAsync(shell, [...shellArgs, '-c', cmd], { ...opts, source: 'gh' }) + return execAsync(resolvedGhPath, args, { ...opts, source: 'gh' }) } interface RawGhPr { From a5483d5a54e209fe14f753e2698fde3bbfca31ed Mon Sep 17 00:00:00 2001 From: Jimmy Stridh Date: Thu, 28 May 2026 15:13:33 +0200 Subject: [PATCH 2/2] fix(platform): pass Windows platform tests --- packages/shared/platform/src/cli-install.ts | 17 +++++++++++------ packages/shared/platform/src/migrations.test.ts | 3 +++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/shared/platform/src/cli-install.ts b/packages/shared/platform/src/cli-install.ts index b2f9d94f..8061581d 100644 --- a/packages/shared/platform/src/cli-install.ts +++ b/packages/shared/platform/src/cli-install.ts @@ -15,24 +15,29 @@ export interface CliInstallResult { pathNotInPATH?: boolean } +function platformPath() { + return process.platform === 'win32' ? path.win32 : path.posix +} + export function getCliBinDir(): string { + const pathForPlatform = platformPath() switch (process.platform) { case 'darwin': return '/usr/local/bin' case 'win32': - return path.join( - process.env.LOCALAPPDATA ?? path.join(os.homedir(), 'AppData', 'Local'), + return pathForPlatform.join( + process.env.LOCALAPPDATA ?? pathForPlatform.join(os.homedir(), 'AppData', 'Local'), 'SlayZone', 'bin' ) default: - return path.join(os.homedir(), '.local', 'bin') + return pathForPlatform.join(os.homedir(), '.local', 'bin') } } export function getCliBinTarget(): string { const name = process.platform === 'win32' ? 'slay.cmd' : 'slay' - return path.join(getCliBinDir(), name) + return platformPath().join(getCliBinDir(), name) } export function checkCliInstalled(): { installed: boolean; path?: string } { @@ -142,8 +147,8 @@ function installWindows(cliSrcPath: string, binDir: string, target: string): Cli } // Copy slay.js next to the .cmd shim // TODO: In dev mode, slay.js is at ../cli/dist/slay.js, not ../cli/bin/slay.js — shim won't work in dev on Windows - const srcJs = cliSrcPath.replace(/[/\\]slay$/, path.sep + 'slay.js') - const destJs = path.join(binDir, 'slay.js') + const srcJs = cliSrcPath.replace(/([/\\])slay$/, '$1slay.js') + const destJs = path.win32.join(binDir, 'slay.js') if (fs.existsSync(srcJs)) { fs.copyFileSync(srcJs, destJs) } else { diff --git a/packages/shared/platform/src/migrations.test.ts b/packages/shared/platform/src/migrations.test.ts index d9b0581a..c048a42d 100644 --- a/packages/shared/platform/src/migrations.test.ts +++ b/packages/shared/platform/src/migrations.test.ts @@ -118,6 +118,9 @@ describe('migrateStateDir', () => { // --- Failure / rollback --- test('fails and rolls back when newDir parent is not writable', () => { + if (process.platform === 'win32') { + return // Windows chmod does not make the directory unwritable for this test. + } if (process.getuid?.() === 0) { return // skip when running as root }