From 74d5b0c8608b7069db6ce94516dc3e57ccb82524 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 11 Aug 2026 12:14:36 -0700 Subject: [PATCH] fix(windows): show first window before shell PATH hydration (#13799) (cherry picked from commit e790266546a11df480baffd0ae1a982d90a912af) --- src/main/git/runner-command-exec.test.ts | 2 +- .../runner-windows-host-environment.test.ts | 266 ++++++++++++++++++ src/main/git/runner.ts | 149 +++++++++- src/main/index.ts | 76 +++-- ...list-files-git-directory-expansion.test.ts | 106 ++++++- .../ipc/filesystem-list-files-git-fallback.ts | 46 +-- src/main/ipc/filesystem-search-git.test.ts | 3 + src/main/ipc/filesystem-search-git.ts | 14 +- src/main/ipc/filesystem.ts | 4 +- src/main/ipc/repos-remote.test.ts | 92 +++++- src/main/ipc/repos.ts | 61 ++-- src/main/plugins/plugin-git-repository.ts | 15 +- src/main/runtime/orca-runtime-git.ts | 4 +- src/main/runtime/orca-runtime.test.ts | 62 ++-- src/main/runtime/orca-runtime.ts | 38 ++- .../startup/desktop-startup-ordering.test.ts | 42 ++- .../serve-desktop-activation-wiring.test.ts | 2 +- ...windows-desktop-shell-path-startup.test.ts | 37 +++ .../windows-desktop-shell-path-startup.ts | 18 ++ src/main/window/createMainWindow.test.ts | 24 ++ src/main/window/createMainWindow.ts | 5 + src/renderer/src/App.tsx | 9 +- src/renderer/src/app-startup-routing.test.ts | 7 +- 23 files changed, 930 insertions(+), 152 deletions(-) create mode 100644 src/main/git/runner-windows-host-environment.test.ts create mode 100644 src/main/startup/windows-desktop-shell-path-startup.test.ts create mode 100644 src/main/startup/windows-desktop-shell-path-startup.ts diff --git a/src/main/git/runner-command-exec.test.ts b/src/main/git/runner-command-exec.test.ts index dd24b8c8fb0..8d07bd5c555 100644 --- a/src/main/git/runner-command-exec.test.ts +++ b/src/main/git/runner-command-exec.test.ts @@ -179,8 +179,8 @@ describe('commandExecFileAsync Windows command shims', () => { describe('runner execFile timeout handling', () => { beforeEach(() => { execFileMock.mockReset() - execFileSyncMock.mockReset() spawnMock.mockReset() + execFileSyncMock.mockReset() vi.useFakeTimers() }) diff --git a/src/main/git/runner-windows-host-environment.test.ts b/src/main/git/runner-windows-host-environment.test.ts new file mode 100644 index 00000000000..e441b023ff4 --- /dev/null +++ b/src/main/git/runner-windows-host-environment.test.ts @@ -0,0 +1,266 @@ +import { EventEmitter } from 'node:events' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock, spawnMock } = vi.hoisted(() => ({ + execFileMock: vi.fn(), + spawnMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ + execFile: execFileMock, + execFileSync: vi.fn(), + spawn: spawnMock +})) + +import { + awaitWindowsHostGitEnvironmentReady, + configureWindowsHostGitEnvironmentReadiness, + gitExecFileAsync, + gitExecFileAsyncBuffer, + gitSpawnAfterWindowsEnvironmentReady, + gitStreamStdout +} from './runner' + +type MockChildProcess = EventEmitter & { + stdout: EventEmitter + stderr: EventEmitter + pid: number + kill: ReturnType +} + +function createMockChildProcess(pid: number): MockChildProcess { + const child = new EventEmitter() as MockChildProcess + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.pid = pid + child.kill = vi.fn() + return child +} + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +async function withPlatform(platform: NodeJS.Platform, fn: () => Promise): Promise { + const original = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return await fn() + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: original }) + } +} + +describe('Windows host Git environment readiness', () => { + const originalPath = process.env.Path + + beforeEach(() => { + execFileMock.mockReset() + spawnMock.mockReset() + }) + + afterEach(() => { + configureWindowsHostGitEnvironmentReadiness(null) + if (originalPath === undefined) { + delete process.env.Path + } else { + process.env.Path = originalPath + } + }) + + it('waits for each native operation and refreshes a snapshotted PATH', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const waitUntilReady = vi.fn(() => ready.promise) + const child = createMockChildProcess(1234) + execFileMock.mockImplementation((_cmd, _args, _options, callback) => { + callback(null, 'ok', '') + return child + }) + configureWindowsHostGitEnvironmentReadiness(waitUntilReady) + process.env.Path = 'hydrating-path' + + const firstExec = gitExecFileAsync(['status'], { + cwd: String.raw`C:\repo`, + env: { Path: 'stale-path' } + }) + + expect(waitUntilReady).toHaveBeenCalledOnce() + expect(execFileMock).not.toHaveBeenCalled() + process.env.Path = 'hydrated-path' + ready.resolve() + await expect(firstExec).resolves.toEqual({ stdout: 'ok', stderr: '' }) + expect(execFileMock.mock.calls[0]?.[2]?.env?.Path).toBe('hydrated-path') + + await expect(gitExecFileAsync(['status'], { cwd: String.raw`C:\repo` })).resolves.toEqual({ + stdout: 'ok', + stderr: '' + }) + expect(waitUntilReady).toHaveBeenCalledTimes(2) + }) + }) + + it('waits before native spawned Git and refreshes a snapshotted PATH', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const waitUntilReady = vi.fn(() => ready.promise) + const child = createMockChildProcess(1234) + spawnMock.mockReturnValue(child) + configureWindowsHostGitEnvironmentReadiness(waitUntilReady) + process.env.Path = 'hydrating-path' + + const spawned = gitSpawnAfterWindowsEnvironmentReady(['clone', 'url', 'repo'], { + cwd: String.raw`C:\repo`, + env: { Path: 'stale-path' }, + stdio: ['ignore', 'ignore', 'pipe'] + }) + + expect(waitUntilReady).toHaveBeenCalledOnce() + expect(spawnMock).not.toHaveBeenCalled() + process.env.Path = 'hydrated-path' + ready.resolve() + + await expect(spawned).resolves.toBe(child) + expect(spawnMock.mock.calls[0]?.[2]?.env?.Path).toBe('hydrated-path') + }) + }) + + it('does not gate spawned Git routed through WSL on the Windows host profile', async () => { + await withPlatform('win32', async () => { + const waitUntilReady = vi.fn(() => new Promise(() => {})) + const child = createMockChildProcess(1234) + spawnMock.mockReturnValue(child) + configureWindowsHostGitEnvironmentReadiness(waitUntilReady) + + await expect( + gitSpawnAfterWindowsEnvironmentReady(['clone', 'url', 'repo'], { + cwd: String.raw`C:\repo`, + wslDistro: 'Ubuntu', + stdio: ['ignore', 'ignore', 'pipe'] + }) + ).resolves.toBe(child) + + expect(waitUntilReady).not.toHaveBeenCalled() + expect(spawnMock).toHaveBeenCalled() + }) + }) + + it('does not spawn Git when canceled during Windows environment readiness', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const controller = new AbortController() + configureWindowsHostGitEnvironmentReadiness(() => ready.promise) + + const spawned = gitSpawnAfterWindowsEnvironmentReady(['status'], { + cwd: String.raw`C:\repo`, + signal: controller.signal, + stdio: ['ignore', 'ignore', 'pipe'] + }) + controller.abort() + + await expect(spawned).rejects.toMatchObject({ name: 'AbortError' }) + expect(spawnMock).not.toHaveBeenCalled() + ready.resolve() + }) + }) + + it('cancels async Git immediately while Windows environment readiness is pending', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const controller = new AbortController() + configureWindowsHostGitEnvironmentReadiness(() => ready.promise) + + const operation = gitExecFileAsync(['status'], { + cwd: String.raw`C:\repo`, + signal: controller.signal + }) + controller.abort() + + await expect(operation).rejects.toMatchObject({ name: 'AbortError' }) + expect(execFileMock).not.toHaveBeenCalled() + ready.resolve() + }) + }) + + it('does not gate WSL paths at a synchronous Git consumer boundary', async () => { + await withPlatform('win32', async () => { + const waitUntilReady = vi.fn(() => new Promise(() => {})) + configureWindowsHostGitEnvironmentReadiness(waitUntilReady) + + await expect( + awaitWindowsHostGitEnvironmentReady({ + cwd: String.raw`\\wsl.localhost\Ubuntu\home\me\repo` + }) + ).resolves.toBeUndefined() + + expect(waitUntilReady).not.toHaveBeenCalled() + }) + }) + + it('does not gate Git routed through WSL on the Windows host profile', async () => { + await withPlatform('win32', async () => { + const waitUntilReady = vi.fn(() => new Promise(() => {})) + const child = createMockChildProcess(1234) + execFileMock.mockImplementation((_cmd, _args, _options, callback) => { + callback(null, 'ok', '') + return child + }) + configureWindowsHostGitEnvironmentReadiness(waitUntilReady) + + await expect( + gitExecFileAsync(['status'], { + cwd: String.raw`C:\repo`, + wslDistro: 'Ubuntu' + }) + ).resolves.toEqual({ stdout: 'ok', stderr: '' }) + + expect(waitUntilReady).not.toHaveBeenCalled() + expect(execFileMock).toHaveBeenCalled() + }) + }) + + it('gates buffered native Git reads on the current generation', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const child = createMockChildProcess(1234) + execFileMock.mockImplementation((_cmd, _args, _options, callback) => { + callback(null, Buffer.from('blob'), Buffer.alloc(0)) + return child + }) + configureWindowsHostGitEnvironmentReadiness(() => ready.promise) + + const read = gitExecFileAsyncBuffer(['show', 'HEAD:file'], { cwd: String.raw`C:\repo` }) + expect(execFileMock).not.toHaveBeenCalled() + ready.resolve() + + await expect(read).resolves.toEqual({ stdout: Buffer.from('blob') }) + }) + }) + + it('gates native Git streams and refreshes their snapshotted PATH', async () => { + await withPlatform('win32', async () => { + const ready = deferred() + const child = createMockChildProcess(1234) + spawnMock.mockReturnValue(child) + configureWindowsHostGitEnvironmentReadiness(() => ready.promise) + process.env.Path = 'hydrated-stream-path' + + const stream = gitStreamStdout(['status'], { + cwd: String.raw`C:\repo`, + env: { Path: 'stale-stream-path' }, + onStdout: () => {} + }) + expect(spawnMock).not.toHaveBeenCalled() + ready.resolve() + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + expect(spawnMock.mock.calls[0]?.[2]?.env?.Path).toBe('hydrated-stream-path') + child.emit('close', 0) + + await expect(stream).resolves.toEqual({ stoppedEarly: false }) + }) + }) +}) diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 2bca4a4e034..264bd7cbfca 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -182,12 +182,100 @@ function resolveHostGitHubCli(command: 'gh', args: string[]): ResolvedCommand { } let defaultWslDistroOverride: string | null = null +let waitForWindowsHostGitEnvironment: (() => Promise) | null = null // Why: allow host commands fallback to route through the user's pinned WSL distro when host execution fails. export function setDefaultWslDistroOverride(distro: string | null): void { defaultWslDistroOverride = distro } +export function configureWindowsHostGitEnvironmentReadiness( + waitUntilReady: (() => Promise) | null +): void { + waitForWindowsHostGitEnvironment = waitUntilReady +} + +export async function awaitWindowsHostGitEnvironmentReady(options: { + cwd: string + wslDistro?: string + signal?: AbortSignal +}): Promise { + const resolved = resolveGitCommand(['--version'], options) + await prepareWindowsHostGitEnvironment(resolved, undefined, options.signal) +} + +function refreshWindowsHostPath(env: NodeJS.ProcessEnv | undefined): NodeJS.ProcessEnv | undefined { + if (!env) { + return undefined + } + const currentPath = process.env.Path ?? process.env.PATH + if (currentPath === undefined) { + return env + } + const next = { ...env } + const pathKeys = Object.keys(next).filter((key) => key.toLowerCase() === 'path') + if (pathKeys.length === 0) { + next[process.env.Path === undefined ? 'PATH' : 'Path'] = currentPath + } else { + for (const key of pathKeys) { + next[key] = currentPath + } + } + return next +} + +function prepareWindowsHostGitEnvironment( + resolved: ResolvedCommand, + env: NodeJS.ProcessEnv | undefined, + signal?: AbortSignal +): Promise | null { + if ( + process.platform !== 'win32' || + resolved.wsl !== null || + waitForWindowsHostGitEnvironment === null + ) { + return null + } + const ready = waitForWindowsHostGitEnvironment().then(() => refreshWindowsHostPath(env)) + if (!signal) { + return ready + } + if (signal.aborted) { + return Promise.reject(createAbortError()) + } + return new Promise((resolve, reject) => { + let settled = false + const cleanup = (): void => signal.removeEventListener('abort', onAbort) + const onAbort = (): void => { + if (settled) { + return + } + settled = true + cleanup() + reject(createAbortError()) + } + signal.addEventListener('abort', onAbort, { once: true }) + ready.then( + (value) => { + if (settled) { + return + } + settled = true + cleanup() + resolve(value) + }, + (error) => { + if (settled) { + return + } + settled = true + cleanup() + reject(error) + } + ) + }) +} + function resolveDefaultWslCli(command: 'gh' | 'glab', args: string[]): ResolvedCommand | null { const distro = defaultWslDistroOverride ?? getDefaultWslDistro() return distro ? resolveCommand(command, args, undefined, distro) : null @@ -972,10 +1060,18 @@ export async function gitExecFileAsync( signal: options.signal }) } - const resolved = resolveGitCommand(args, options) - const policy = options.useConfiguredSshCommandForNetwork - ? await buildNetworkSshPolicyEnv(options) - : { env: nonInteractiveGitEnv(options.env), mode: 'default' as const } + let resolved = resolveGitCommand(args, options) + const environmentReady = prepareWindowsHostGitEnvironment( + resolved, + options.env, + options.signal + ) + const env = environmentReady ? await environmentReady : options.env + const effectiveOptions = env === options.env ? options : { ...options, env } + resolved = resolveGitCommand(args, effectiveOptions) + const policy = effectiveOptions.useConfiguredSshCommandForNetwork + ? await buildNetworkSshPolicyEnv(effectiveOptions) + : { env: nonInteractiveGitEnv(effectiveOptions.env), mode: 'default' as const } const capture = ( command: ResolvedCommand ): Promise<{ stdout: string | Buffer; stderr: string | Buffer }> => @@ -994,7 +1090,7 @@ export async function gitExecFileAsync( } catch (error) { if (directWslGitExitCode(error, resolved) !== null && !options.signal?.aborted) { const wasMissing = invalidateMissingDirectWslGit(error, resolved) - result = await capture(resolveGitCommand(args, options, true)) + result = await capture(resolveGitCommand(args, effectiveOptions, true)) // Why: matching failures can be normal Git control flow; only a successful login retry proves the direct environment was insufficient. disableDirectWslGitAfterSuccessfulFallback(wasMissing, resolved) const { stdout, stderr } = result @@ -1066,7 +1162,12 @@ export async function gitExecFileAsyncBuffer( if (isWslLinkedWorktreeGitRoutingCandidate(options.cwd, options.wslDistro)) { await prepareWslLinkedWorktreeGitRouting(options.cwd, options.wslDistro) } - const resolved = resolveGitCommand(args, options, true) + let resolved = resolveGitCommand(args, options, true) + const environmentReady = prepareWindowsHostGitEnvironment(resolved, undefined) + if (environmentReady) { + await environmentReady + } + resolved = resolveGitCommand(args, options, true) const { stdout } = (await execFileCapture(resolved.binary, resolved.args, { cwd: resolved.cwd, encoding: 'buffer', @@ -1120,6 +1221,15 @@ export async function gitStreamStdout( ...(options.signal ? { signal: options.signal } : {}) } let resolved = resolveGitCommand(args, gitOptions) + const environmentReady = prepareWindowsHostGitEnvironment( + resolved, + gitOptions.env, + options.signal + ) + if (environmentReady) { + gitOptions.env = await environmentReady + } + resolved = resolveGitCommand(args, gitOptions) const stream = (command: ResolvedCommand): Promise => new Promise((resolve, reject) => { if (options.signal?.aborted) { @@ -1129,7 +1239,7 @@ export async function gitStreamStdout( const stdio: SpawnOptions['stdio'] = ['ignore', 'pipe', 'pipe'] const spawnOptions = { cwd: options.cwd, - env: nonInteractiveGitEnv(options.env), + env: nonInteractiveGitEnv(gitOptions.env), stdio, wslDistro: options.wslDistro, windowsHide: true @@ -1303,10 +1413,29 @@ export function gitExecFileSync( * Spawn a git child process. Drop-in replacement for * `spawn('git', args, { cwd, stdio, ... })`. */ -export function gitSpawn( +type GitSpawnOptions = SpawnOptions & { cwd: string; wslDistro?: string } + +export async function gitSpawnAfterWindowsEnvironmentReady( args: string[], - options: SpawnOptions & { cwd: string; wslDistro?: string } -): ChildProcess { + options: GitSpawnOptions +): Promise { + if (options.signal?.aborted) { + throw createAbortError() + } + const resolved = resolveGitCommand(args, { + cwd: options.cwd, + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), + ...(options.env ? { env: options.env } : {}) + }) + const env = await (prepareWindowsHostGitEnvironment(resolved, options.env, options.signal) ?? + options.env) + if (options.signal?.aborted) { + throw createAbortError() + } + return gitSpawn(args, env === options.env ? options : { ...options, env }) +} + +export function gitSpawn(args: string[], options: GitSpawnOptions): ChildProcess { const { wslDistro, ...spawnOptions } = options const resolved = resolveGitCommand(args, { cwd: options.cwd, diff --git a/src/main/index.ts b/src/main/index.ts index f49e07988e9..3f076c2e908 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -163,6 +163,10 @@ import { createWslCliReconciliationStartupBarrier } from './startup/wsl-cli-reco import { getDevInstanceIdentity } from './startup/dev-instance-identity' import { hydrateShellPath, mergePathSegments } from './startup/hydrate-shell-path' import { createWindowsShellPathHydration } from './startup/windows-shell-path-hydration' +import { + startWindowsDesktopBeforeShellPathReady, + type WindowsDesktopStartupServices +} from './startup/windows-desktop-shell-path-startup' import { acquireSingleInstanceLock, logSingleInstanceLockBypass, @@ -257,7 +261,10 @@ import { maybeAutoRenameBranchOnFirstWork } from './agent-hooks/first-work-branc import { rememberBranchRenameFailureOutput } from './agent-hooks/branch-rename-failure-output' import { renameWorktreeFolderOnFirstWork } from './agent-hooks/first-work-folder-rename' import { moveWorktree } from './git/worktree' -import { setDefaultWslDistroOverride } from './git/runner' +import { + configureWindowsHostGitEnvironmentReadiness, + setDefaultWslDistroOverride +} from './git/runner' import { getRepoIdFromWorktreeId } from '../shared/worktree-id' import { parseWorkspaceKey } from '../shared/workspace-scope' import { setMigrationUnsupportedPtyListener } from './agent-hooks/migration-unsupported-pty-state' @@ -923,7 +930,7 @@ async function reapRestoredSubagentsWithoutLiveAgent(): Promise { }) } -function startTerminalRuntimeStartupServices(): Promise { +function startTerminalRuntimeStartupServices(): WindowsDesktopStartupServices { logStartupMilestone('first-window-startup-services-start') const startupServices = startFirstWindowStartupServices({ // Why: both desktop and headless serve must adopt the same persistent provider before creating terminals or a renderer. @@ -980,19 +987,24 @@ function startTerminalRuntimeStartupServices(): Promise { console.error('[agent-hooks] Failed to start local hook server:', error) } }) - firstWindowStartupServicesReady = startupServices.firstWindowReady - localPtyStartupReady = startupServices.localPtyReady - localPtyProviderStartupReady = startupServices.localPtyProviderReady - void firstWindowStartupServicesReady.then(() => { + void startupServices.firstWindowReady.then(() => { logStartupMilestone('first-window-startup-services-ready') }) - void localPtyStartupReady.then(() => { + void startupServices.localPtyReady.then(() => { logStartupMilestone('local-pty-startup-ready') void reapRestoredSubagentsWithoutLiveAgent().catch((error) => { console.warn('[agent-hooks] restored-subagent liveness probe failed:', error) }) }) - return firstWindowStartupServicesReady + return startupServices +} + +function bindTerminalRuntimeStartupServices( + services: Promise +): void { + firstWindowStartupServicesReady = services.then((value) => value.firstWindowReady) + localPtyStartupReady = services.then((value) => value.localPtyReady) + localPtyProviderStartupReady = services.then((value) => value.localPtyProviderReady) } function prepareCodexRuntimeHomeForLaunch( @@ -1248,7 +1260,7 @@ function syncMacMenuBarIcon(showMenuBarIcon: boolean): Tray | null { return options ? setMacMenuBarIconVisible(showMenuBarIcon, options) : null } -function openMainWindow(): BrowserWindow { +function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {}): BrowserWindow { logStartupMilestone('open-main-window-start') if (!store) { throw new Error('Store must be initialized before opening the main window') @@ -1337,6 +1349,7 @@ function openMainWindow(): BrowserWindow { void presentRendererRecoveryPrompt(recentRecoveryCount) }, deferLoad: true, + ...(options.revealOnDidFinishLoad === true ? { revealOnDidFinishLoad: true } : {}), title: devInstanceIdentity.name, getKeybindings: () => keybindings?.getOverrides(), onBeforeReload: ({ ignoreCache, webContentsId }) => { @@ -1376,6 +1389,9 @@ function openMainWindow(): BrowserWindow { logStartupMilestone('ready-to-show') setImmediate(createSystemTrayDeferred) }) + window.once('show', () => { + logStartupMilestone('window-shown') + }) const trayCreateFallback = setTimeout(createSystemTrayDeferred, TRAY_CREATE_FALLBACK_MS) trayCreateFallback.unref?.() @@ -2153,6 +2169,9 @@ void app.whenReady().then(async () => { const activeOrcaProfile = ensureActiveOrcaProfile() store = new Store({ dataFile: activeOrcaProfile.dataFile }) const windowsShellPathHydration = createWindowsShellPathHydration() + configureWindowsHostGitEnvironmentReadiness( + process.platform === 'win32' ? windowsShellPathHydration.whenReady : null + ) if (process.platform === 'win32') { const settings = store.getSettings() if (app.isPackaged) { @@ -2980,9 +2999,20 @@ void app.whenReady().then(async () => { } }) - // Why: Git hooks inherit process.env, so Source Control must not open before profile PATH settles. - await windowsShellPathHydration.whenReady() - startTerminalRuntimeStartupServices() + const shellPathReady = windowsShellPathHydration.whenReady() + let desktopWindow: BrowserWindow | null = null + if (process.platform === 'win32' && app.isPackaged && !serveOptions) { + const desktopStartup = startWindowsDesktopBeforeShellPathReady({ + openWindow: () => openMainWindow({ revealOnDidFinishLoad: true }), + shellPathReady, + startServices: startTerminalRuntimeStartupServices + }) + desktopWindow = desktopStartup.window + bindTerminalRuntimeStartupServices(desktopStartup.services) + } else { + await shellPathReady + bindTerminalRuntimeStartupServices(Promise.resolve(startTerminalRuntimeStartupServices())) + } app.on('activate', handleMacAppActivation) if (serveOptions) { @@ -3066,15 +3096,21 @@ void app.whenReady().then(async () => { } // Why: window and RPC startup run in parallel; registerPtyHandlers gates PTY spawns so RPC binds without racing the daemon provider swap. + const desktopRuntimeRpc = runtimeRpc + if (!desktopRuntimeRpc) { + throw new Error('runtime_rpc_unavailable') + } const [win, runtimeRpcStartResult] = await Promise.all([ - Promise.resolve(openMainWindow()), - runtimeRpc.start().then( - () => ({ ok: true as const }), - (error: unknown) => { - recordRuntimeRpcStartFailure(error) - return { ok: false as const, error } - } - ) + Promise.resolve(desktopWindow ?? openMainWindow()), + shellPathReady + .then(() => desktopRuntimeRpc.start()) + .then( + () => ({ ok: true as const }), + (error: unknown) => { + recordRuntimeRpcStartFailure(error) + return { ok: false as const, error } + } + ) ]) if (!runtimeRpcStartResult.ok) { void showRuntimeRpcStartupFailureDialog(win, runtimeRpcStartResult.error) diff --git a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts index 5d5cf163746..5d7507a5615 100644 --- a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts +++ b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts @@ -10,7 +10,7 @@ const { gitSpawnMock } = vi.hoisted(() => ({ })) vi.mock('../git/runner', () => ({ - gitSpawn: gitSpawnMock + gitSpawnAfterWindowsEnvironmentReady: gitSpawnMock })) import { listFilesWithGit } from './filesystem-list-files-git-fallback' @@ -56,11 +56,13 @@ describe('main Quick Open git directory expansion', () => { const primary = createMockProcess() const ignored = createMockProcess() gitSpawnMock - .mockReturnValueOnce(revParse) - .mockReturnValueOnce(primary) - .mockReturnValueOnce(ignored) + .mockResolvedValueOnce(revParse) + .mockResolvedValueOnce(primary) + .mockResolvedValueOnce(ignored) const promise = listFilesWithGit(root, [], {}) + await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(revParse.listenerCount('close')).toBeGreaterThan(0)) revParse.emit('close', 0, null) await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3)) ;(primary.stdout as unknown as EventEmitter).emit( @@ -86,12 +88,14 @@ describe('main Quick Open git directory expansion', () => { const primary = createMockProcess() const ignored = createMockProcess() gitSpawnMock - .mockReturnValueOnce(revParse) - .mockReturnValueOnce(primary) - .mockReturnValueOnce(ignored) + .mockResolvedValueOnce(revParse) + .mockResolvedValueOnce(primary) + .mockResolvedValueOnce(ignored) const controller = new AbortController() const promise = listFilesWithGit(root, [], {}, controller.signal) + await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(revParse.listenerCount('close')).toBeGreaterThan(0)) revParse.emit('close', 0, null) await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3)) controller.abort() @@ -100,4 +104,92 @@ describe('main Quick Open git directory expansion', () => { expect(primary.kill).toHaveBeenCalled() expect(ignored.kill).toHaveBeenCalled() }) + + it('kills a repository probe returned after cancellation', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-main-git-probe-race-')) + tempDirs.push(root) + const revParse = createMockProcess() + let resolveRevParse!: (child: ChildProcess) => void + gitSpawnMock.mockReturnValueOnce( + new Promise((resolve) => { + resolveRevParse = resolve + }) + ) + + const controller = new AbortController() + const promise = listFilesWithGit(root, [], {}, controller.signal) + await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledOnce()) + controller.abort() + resolveRevParse(revParse) + + await expect(promise).rejects.toSatisfy(isFileListingCancellation) + expect(revParse.kill).toHaveBeenCalled() + expect(revParse.listenerCount('close')).toBe(0) + }) + + it('kills file scans returned after cancellation', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-main-git-scan-race-')) + tempDirs.push(root) + const revParse = createMockProcess() + const primary = createMockProcess() + const ignored = createMockProcess() + let resolvePrimary!: (child: ChildProcess) => void + let resolveIgnored!: (child: ChildProcess) => void + gitSpawnMock + .mockResolvedValueOnce(revParse) + .mockReturnValueOnce( + new Promise((resolve) => { + resolvePrimary = resolve + }) + ) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveIgnored = resolve + }) + ) + + const controller = new AbortController() + const promise = listFilesWithGit(root, [], {}, controller.signal) + await vi.waitFor(() => expect(revParse.listenerCount('close')).toBeGreaterThan(0)) + revParse.emit('close', 0, null) + await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3)) + controller.abort() + resolvePrimary(primary) + resolveIgnored(ignored) + + await expect(promise).rejects.toSatisfy(isFileListingCancellation) + expect(primary.kill).toHaveBeenCalled() + expect(ignored.kill).toHaveBeenCalled() + }) + + it('cancels a pending sibling spawn when the primary scan fails', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-main-git-sibling-failure-')) + tempDirs.push(root) + const revParse = createMockProcess() + const primary = createMockProcess() + let pendingSignal: AbortSignal | undefined + gitSpawnMock + .mockResolvedValueOnce(revParse) + .mockResolvedValueOnce(primary) + .mockImplementationOnce( + (_args: string[], options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + pendingSignal = options.signal + options.signal?.addEventListener( + 'abort', + () => reject(Object.assign(new Error('aborted'), { name: 'AbortError' })), + { once: true } + ) + }) + ) + + const promise = listFilesWithGit(root, [], {}) + await vi.waitFor(() => expect(revParse.listenerCount('close')).toBeGreaterThan(0)) + revParse.emit('close', 0, null) + await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3)) + primary.emit('error', new Error('primary failed')) + + await expect(promise).rejects.toThrow('primary failed') + expect(pendingSignal?.aborted).toBe(true) + }) }) diff --git a/src/main/ipc/filesystem-list-files-git-fallback.ts b/src/main/ipc/filesystem-list-files-git-fallback.ts index 78e03f56065..bebab27808d 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback.ts @@ -1,5 +1,5 @@ import type { ChildProcess } from 'node:child_process' -import { gitSpawn } from '../git/runner' +import { gitSpawnAfterWindowsEnvironmentReady } from '../git/runner' import { isWslLinkedWorktreeGitRoutingCandidate, prepareWslLinkedWorktreeGitRouting @@ -45,12 +45,13 @@ async function isInsideGitWorkTree( if (signal?.aborted) { throw fileListingCancellationError(signal) } + const child = await gitSpawnAfterWindowsEnvironmentReady(['rev-parse', '--is-inside-work-tree'], { + cwd: rootPath, + ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), + ...(signal ? { signal } : {}), + stdio: ['ignore', 'ignore', 'ignore'] + }) return new Promise((resolve, reject) => { - const child = gitSpawn(['rev-parse', '--is-inside-work-tree'], { - cwd: rootPath, - ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), - stdio: ['ignore', 'ignore', 'ignore'] - }) let done = false let timer: ReturnType const cleanup = (): void => { @@ -88,6 +89,9 @@ async function isInsideGitWorkTree( child.kill() finish(false) }, 10_000) + if (signal?.aborted) { + cancel() + } }) } @@ -121,8 +125,17 @@ export async function listFilesWithGit( reject: (error: Error) => void resolve: () => void }[] = [] + const scanController = new AbortController() - const runGitLsFiles = (args: string[]): Promise => { + const runGitLsFiles = async (args: string[]): Promise => { + // Why: git ls-files outputs paths relative to cwd, so we set cwd to + // rootPath and use the output directly — no prefix stripping needed. + const child = await gitSpawnAfterWindowsEnvironmentReady(['ls-files', ...args], { + cwd: rootPath, + ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), + signal: scanController.signal, + stdio: ['ignore', 'pipe', 'pipe'] + }) return new Promise((resolve, reject) => { let buf = '' let done = false @@ -155,13 +168,6 @@ export async function listFilesWithGit( return maxResults !== undefined && directFileCandidates.size >= maxResults } - // Why: git ls-files outputs paths relative to cwd, so we set cwd to - // rootPath and use the output directly — no prefix stripping needed. - const child = gitSpawn(['ls-files', ...args], { - cwd: rootPath, - ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), - stdio: ['ignore', 'pipe', 'pipe'] - }) let timer: ReturnType const cleanup = (): void => { clearTimeout(timer) @@ -246,6 +252,10 @@ export async function listFilesWithGit( child.kill() rejectPass(new Error('git ls-files timed out')) }, 10000) + if (scanController.signal.aborted) { + child.kill() + rejectPass(fileListingCancellationError(scanController.signal)) + } }) } @@ -275,14 +285,17 @@ export async function listFilesWithGit( } } - const onAbort = (): void => killSurvivors('git ls-files cancelled') + const onAbort = (): void => { + scanController.abort(signal?.reason) + killSurvivors('git ls-files cancelled') + } signal?.addEventListener('abort', onAbort, { once: true }) try { const runIgnoredPass = () => // Why: ignored files are supplementary — a failed or timed-out ignored // pass must not discard the primary listing the user actually needs. runGitLsFiles(ignoredPass).catch((err: Error) => { - if (!signal?.aborted) { + if (!scanController.signal.aborted) { console.warn('[quick-open] git ignored-file pass failed; keeping primary results:', err) } }) @@ -297,6 +310,7 @@ export async function listFilesWithGit( } } } catch (err) { + scanController.abort(err) killSurvivors() if (signal?.aborted) { throw fileListingCancellationError(signal) diff --git a/src/main/ipc/filesystem-search-git.test.ts b/src/main/ipc/filesystem-search-git.test.ts index e6d29acb3cd..ae73b0aa2e2 100644 --- a/src/main/ipc/filesystem-search-git.test.ts +++ b/src/main/ipc/filesystem-search-git.test.ts @@ -184,6 +184,9 @@ describe('filesystem-search-git', () => { const promise = searchWithGitGrep('/mock/root', { query: 'ok', rootPath: '/mock/root' }, 100) + await vi.waitFor(() => + expect((proc.stdout as unknown as EventEmitter).listenerCount('data')).toBeGreaterThan(0) + ) ;(proc.stdout as unknown as EventEmitter).emit('data', 'valid.ts\x001\x00ok\npartial') await vi.runOnlyPendingTimersAsync() diff --git a/src/main/ipc/filesystem-search-git.ts b/src/main/ipc/filesystem-search-git.ts index 9b0c91a6c65..e997ddd2e02 100644 --- a/src/main/ipc/filesystem-search-git.ts +++ b/src/main/ipc/filesystem-search-git.ts @@ -7,7 +7,7 @@ import { ingestGitGrepLine, SEARCH_TIMEOUT_MS } from '../../shared/text-search' -import { gitSpawn } from '../git/runner' +import { gitSpawnAfterWindowsEnvironmentReady } from '../git/runner' import { isWslLinkedWorktreeGitRoutingCandidate, prepareWslLinkedWorktreeGitRouting @@ -29,18 +29,18 @@ export async function searchWithGitGrep( if (isWslLinkedWorktreeGitRoutingCandidate(rootPath, localGitOptions.wslDistro)) { await prepareWslLinkedWorktreeGitRouting(rootPath, localGitOptions.wslDistro) } + const gitArgs = buildGitGrepArgs(args.query, args) + const child = await gitSpawnAfterWindowsEnvironmentReady(gitArgs, { + cwd: rootPath, + ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), + stdio: ['ignore', 'pipe', 'pipe'] + }) return new Promise((resolve) => { - const gitArgs = buildGitGrepArgs(args.query, args) const matchRegex = buildSubmatchRegex(args.query, args) const acc = createAccumulator() let stdoutBuffer = '' let done = false - const child = gitSpawn(gitArgs, { - cwd: rootPath, - ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), - stdio: ['ignore', 'pipe', 'pipe'] - }) let killTimeout: ReturnType function resolveOnce(): void { diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 887acaf539f..40701203e06 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -5,7 +5,7 @@ import type { FileHandle } from 'node:fs/promises' import { randomUUID } from 'node:crypto' import { dirname, extname, join, resolve } from 'node:path' import type { ChildProcess } from 'node:child_process' -import { gitExecFileAsync, wslAwareSpawn } from '../git/runner' +import { awaitWindowsHostGitEnvironmentReady, gitExecFileAsync, wslAwareSpawn } from '../git/runner' import { parseWslPath, toWindowsWslPath } from '../wsl' import { tryDeleteWslUncPath } from '../wsl-unc-delete' import type { Store } from '../persistence' @@ -2322,6 +2322,7 @@ export function registerFilesystemHandlers( return provider.getRemoteFileUrl(args.worktreePath, args.relativePath, args.line) } const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) + await awaitWindowsHostGitEnvironmentReady({ cwd: worktreePath }) return getRemoteFileUrl(worktreePath, args.relativePath, args.line) } ) @@ -2342,6 +2343,7 @@ export function registerFilesystemHandlers( return provider.getRemoteCommitUrl(args.worktreePath, sha) } const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) + await awaitWindowsHostGitEnvironmentReady({ cwd: worktreePath }) return getRemoteCommitUrl(worktreePath, sha) } ) diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index b6ec0118a61..d3440ed4579 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -18,6 +18,7 @@ const { mockFilesystemProvider, mockMultiplexer, gitExecFileAsyncMock, + gitSpawnAfterWindowsEnvironmentReadyMock, gitSpawnMock, listWorktreeGraphMock, invalidateAuthorizedRootsCacheMock, @@ -69,6 +70,7 @@ const { notify: vi.fn() }, gitSpawnMock: vi.fn(), + gitSpawnAfterWindowsEnvironmentReadyMock: vi.fn(), gitExecFileAsyncMock: vi.fn(), listWorktreeGraphMock: vi.fn(), invalidateAuthorizedRootsCacheMock: vi.fn(), @@ -104,7 +106,8 @@ vi.mock('../git/runner', async () => ({ gitExecFileAsync: gitExecFileAsyncMock, gitExecFileAsyncBuffer: vi.fn(), gitStreamStdout: vi.fn(), - gitSpawn: gitSpawnMock + gitSpawn: gitSpawnMock, + gitSpawnAfterWindowsEnvironmentReady: gitSpawnAfterWindowsEnvironmentReadyMock })) vi.mock('../git/worktree', () => ({ @@ -1149,7 +1152,7 @@ describe('repos:addRemote', () => { gitSpawnMock.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => proc.emit('close', 0, null)) + setImmediate(() => proc.emit('close', 0, null)) return proc }) mockWindow.webContents.send.mockReset() @@ -1849,11 +1852,14 @@ describe('repos:add + repos:clone', () => { mockStore.updateProjectHostSetup.mockReset() mockWindow.webContents.send.mockReset() gitSpawnMock.mockReset() + gitSpawnAfterWindowsEnvironmentReadyMock + .mockReset() + .mockImplementation(async (...args) => gitSpawnMock(...args)) invalidateAuthorizedRootsCacheMock.mockReset() prepareLocalWorktreeRootForRepoMock.mockReset().mockResolvedValue(undefined) gitSpawnMock.mockImplementation(() => { const proc = createMockCloneProcess() - queueMicrotask(() => proc.emit('close', 0, null)) + setImmediate(() => proc.emit('close', 0, null)) return proc }) @@ -2243,7 +2249,7 @@ describe('repos:add + repos:clone', () => { ) gitSpawnMock.mockImplementationOnce(() => { const proc = createMockCloneProcess() - queueMicrotask(() => { + setImmediate(() => { cloned = true proc.emit('close', 0, null) }) @@ -2390,6 +2396,84 @@ describe('repos:add + repos:clone', () => { await expect(handlers.get('repos:cloneAbort')!(null, undefined)).resolves.toBeUndefined() }) + it('cancels a local clone before Git starts when environment readiness is pending', async () => { + const destination = await createTempRoot() + const clonePath = join(destination, 'orca') + gitSpawnAfterWindowsEnvironmentReadyMock.mockImplementation( + (_args: string[], options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + const abort = (): void => { + reject(Object.assign(new Error('The operation was aborted'), { name: 'AbortError' })) + } + if (options.signal?.aborted) { + abort() + } else { + options.signal?.addEventListener('abort', abort, { once: true }) + } + }) + ) + + const clonePromise = Promise.resolve( + handlers.get('repos:clone')!(null, { + url: 'https://example.com/orca.git', + destination + }) + ) + const rejection = expect(clonePromise).rejects.toThrow('Clone failed') + await waitForAssertion(() => + expect(gitSpawnAfterWindowsEnvironmentReadyMock).toHaveBeenCalledOnce() + ) + expect(gitSpawnMock).not.toHaveBeenCalled() + + await handlers.get('repos:cloneAbort')!(null, undefined) + await rejection + expect(gitSpawnMock).not.toHaveBeenCalled() + expect(existsSync(clonePath)).toBe(false) + }) + + it('cancels every concurrent local clone waiting for environment readiness', async () => { + const firstDestination = await createTempRoot() + const secondDestination = await createTempRoot() + gitSpawnAfterWindowsEnvironmentReadyMock.mockImplementation( + (_args: string[], options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + const abort = (): void => { + reject(Object.assign(new Error('The operation was aborted'), { name: 'AbortError' })) + } + if (options.signal?.aborted) { + abort() + } else { + options.signal?.addEventListener('abort', abort, { once: true }) + } + }) + ) + + const firstClone = Promise.resolve( + handlers.get('repos:clone')!(null, { + url: 'https://example.com/first.git', + destination: firstDestination + }) + ) + const secondClone = Promise.resolve( + handlers.get('repos:clone')!(null, { + url: 'https://example.com/second.git', + destination: secondDestination + }) + ) + const firstRejection = expect(firstClone).rejects.toThrow('Clone failed') + const secondRejection = expect(secondClone).rejects.toThrow('Clone failed') + await waitForAssertion(() => + expect(gitSpawnAfterWindowsEnvironmentReadyMock).toHaveBeenCalledTimes(2) + ) + + await handlers.get('repos:cloneAbort')!(null, undefined) + + await Promise.all([firstRejection, secondRejection]) + expect(gitSpawnMock).not.toHaveBeenCalled() + expect(existsSync(join(firstDestination, 'first'))).toBe(false) + expect(existsSync(join(secondDestination, 'second'))).toBe(false) + }) + it('does not remove an existing target directory when aborting a pending clone', async () => { const destination = await createTempRoot() const clonePath = join(destination, 'orca') diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 0e422d4df77..558ed1d5a22 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -44,7 +44,12 @@ import { isWorkspaceLinkedItemSourceContextMatch } from '../../shared/workspace- import { invalidateAuthorizedRootsCache } from './filesystem-auth' import type { ChildProcess } from 'node:child_process' import { access, mkdir, readdir, rm } from 'node:fs/promises' -import { gitExecFileAsync, gitSpawn, nonInteractiveGitEnv } from '../git/runner' +import { + awaitWindowsHostGitEnvironmentReady, + gitExecFileAsync, + gitSpawnAfterWindowsEnvironmentReady, + nonInteractiveGitEnv +} from '../git/runner' import { isAbsolute, join, posix } from 'node:path' import { cleanupClaimedCloneTarget, @@ -283,6 +288,9 @@ async function addLocalRepoFromPath( kind: 'git' | 'folder' = 'git' ): Promise<{ repo: Repo; alreadyExisted: boolean } | { error: string }> { const repoKind = kind === 'folder' ? 'folder' : 'git' + if (repoKind === 'git') { + await awaitWindowsHostGitEnvironmentReady({ cwd: path }) + } if (repoKind === 'git' && !isGitRepo(path)) { return { error: `Not a valid git repository: ${path}` } } @@ -762,6 +770,7 @@ export function setRepoRemoteClientNotifier(notifier: RepoRemoteClientNotifier): // Why: module-scoped so the abort handle survives macOS window re-creation, when registerRepoHandlers re-runs. let activeClone: ActiveCloneMetadata | null = null +const pendingLocalCloneControllers = new Set() let activeRemoteClone: ActiveRemoteCloneMetadata | null = null let nextCloneGeneration = 1 const latestCloneGenerationByPath = new Map() @@ -1169,6 +1178,10 @@ async function scanNestedReposForIpc(args: { }): Promise { validateNestedRepoScanRoot(args.path, args.connectionId) if (!args.connectionId) { + await awaitWindowsHostGitEnvironmentReady({ + cwd: args.path, + ...(args.signal ? { signal: args.signal } : {}) + }) return scanNestedRepos({ path: args.path, options: args.options, @@ -1702,10 +1715,16 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v continue } importRepoPath = await importTargetResolver.resolveSsh(repoPath, gitProvider) - } else if (!isGitRepo(repoPath)) { - results.push({ path: repoPath, status: 'failed', error: 'Not a valid git repository' }) - continue } else { + await awaitWindowsHostGitEnvironmentReady({ cwd: repoPath }) + if (!isGitRepo(repoPath)) { + results.push({ + path: repoPath, + status: 'failed', + error: 'Not a valid git repository' + }) + continue + } importRepoPath = await importTargetResolver.resolveLocal(repoPath) } const normalizedImportRepoPath = normalizeRuntimePathForComparison(importRepoPath) @@ -2325,6 +2344,10 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v }) ipcMain.handle('repos:cloneAbort', async () => { + for (const controller of pendingLocalCloneControllers) { + controller.abort() + } + pendingLocalCloneControllers.clear() if (activeClone) { const clone = activeClone clone.abortRequested = true @@ -2361,24 +2384,30 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v // Why: spawn (not execFile) avoids the maxBuffer limit — clone progress on stderr can exceed Node's 1 MB default. // Why: --progress forces git to emit progress even when stderr isn't a TTY. const cloneMetadataRef: { current: ActiveCloneMetadata | null } = { current: null } - await new Promise((resolve, reject) => { + let proc: Awaited> + const pendingController = new AbortController() + pendingLocalCloneControllers.add(pendingController) + try { // Why: use the parent destination as cwd so the runner detects a WSL path and routes through wsl.exe. // Why: '--' isolates the URL so a malicious URL can't be read as git flags (command injection). - let proc: ReturnType - try { - proc = gitSpawn(['clone', '--progress', '--', args.url, clonePath], { + proc = await gitSpawnAfterWindowsEnvironmentReady( + ['clone', '--progress', '--', args.url, clonePath], + { cwd: args.destination, // Why: without this, an auth-needing clone pops Git Credential Manager's OAuth window on Windows, unclosable in a restricted env (issue #7652). env: nonInteractiveGitEnv(), + signal: pendingController.signal, stdio: ['ignore', 'ignore', 'pipe'] - }) - } catch (err) { - void cleanupClaimedCloneTarget(clonePath, claimedTarget).finally(() => { - const message = err instanceof Error ? err.message : String(err) - reject(new Error(`Clone failed: ${message}`)) - }) - return - } + } + ) + } catch (err) { + await cleanupClaimedCloneTarget(clonePath, claimedTarget) + const message = err instanceof Error ? err.message : String(err) + throw new Error(`Clone failed: ${message}`) + } finally { + pendingLocalCloneControllers.delete(pendingController) + } + await new Promise((resolve, reject) => { const generation = nextCloneGeneration++ latestCloneGenerationByPath.set(clonePathKey, generation) const metadata: ActiveCloneMetadata = { diff --git a/src/main/plugins/plugin-git-repository.ts b/src/main/plugins/plugin-git-repository.ts index e8870f9e7fe..ac786273fc1 100644 --- a/src/main/plugins/plugin-git-repository.ts +++ b/src/main/plugins/plugin-git-repository.ts @@ -1,26 +1,17 @@ -import { execFile } from 'node:child_process' -import { promisify } from 'node:util' import { isAllowedPluginGitUrl, PLUGIN_COMMIT_PATTERN } from '../../shared/plugins/plugin-install-lockfile' +import { gitExecFileAsync } from '../git/runner' -const execFileAsync = promisify(execFile) const PLUGIN_GIT_TIMEOUT_MS = 120_000 /** Runs system Git with argv-only invocation so credential helpers and SSH * remotes work without exposing an executable remote-helper surface. */ export async function runPluginGit(args: string[], cwd: string): Promise { - const { stdout } = await execFileAsync('git', args, { + const { stdout } = await gitExecFileAsync(args, { cwd, - timeout: PLUGIN_GIT_TIMEOUT_MS, - windowsHide: true, - env: { - ...process.env, - // Existing non-interactive helpers and SSH agents still work, but a - // background marketplace refresh can never hang on a terminal prompt. - GIT_TERMINAL_PROMPT: '0' - } + timeout: PLUGIN_GIT_TIMEOUT_MS }) return stdout.trim() } diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index 0c1444c0294..022b53ac948 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -79,7 +79,7 @@ import type { import { prepareLocalCommitMessageAgentEnv } from '../text-generation/commit-message-agent-environment' import { getPullRequestDraftContext } from '../text-generation/pull-request-context' import { normalizeRuntimeRelativePath } from './runtime-relative-paths' -import { gitExecFileAsync } from '../git/runner' +import { awaitWindowsHostGitEnvironmentReady, gitExecFileAsync } from '../git/runner' import type { GitRuntimeOptions } from '../git/git-runtime-options' import { resolveHostedReviewBodyForGeneration } from '../source-control/pull-request-template' import { @@ -986,6 +986,7 @@ export class RuntimeGitCommands { } return provider.getRemoteFileUrl(target.worktree.path, normalizedRelativePath, line) } + await awaitWindowsHostGitEnvironmentReady({ cwd: target.worktree.path }) return getRemoteFileUrl(target.worktree.path, normalizedRelativePath, line) } @@ -1001,6 +1002,7 @@ export class RuntimeGitCommands { } return provider.getRemoteCommitUrl(target.worktree.path, sha) } + await awaitWindowsHostGitEnvironmentReady({ cwd: target.worktree.path }) return getRemoteCommitUrl(target.worktree.path, sha) } } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 6545486a3a2..aa40c7ee64b 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -7761,7 +7761,7 @@ describe('OrcaRuntimeService', () => { it('keeps project clone setup on the cloned host-qualified repo', async () => { const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-project-clone-')) const clonePath = join(destination, 'orca') - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const repos: Record[] = [] getRepoUpstreamMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' }) const runtimeStore = { @@ -7786,7 +7786,7 @@ describe('OrcaRuntimeService', () => { spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => { + setImmediate(() => { mkdirSync(clonePath, { recursive: true }) execFileSync('git', ['init'], { cwd: clonePath, stdio: 'ignore' }) proc.emit('close', 0, null) @@ -7828,14 +7828,16 @@ describe('OrcaRuntimeService', () => { const existingFolder = join(destination, 'orca') mkdirSync(existingFolder, { recursive: true }) execFileSync('git', ['init'], { cwd: existingFolder, stdio: 'ignore' }) - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn').mockImplementation(() => { - // Why: unreachable while the guard holds; stubbed so a regression records the call - // instead of shelling out to a real network clone. - const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } - proc.stderr = new EventEmitter() - queueMicrotask(() => proc.emit('close', 1, null)) - return proc as never - }) + const spawnSpy = vi + .spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') + .mockImplementation(() => { + // Why: unreachable while the guard holds; stubbed so a regression records the call + // instead of shelling out to a real network clone. + const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } + proc.stderr = new EventEmitter() + setImmediate(() => proc.emit('close', 1, null)) + return proc as never + }) const repos: Record[] = [] const runtimeStore = { ...store, @@ -7884,7 +7886,7 @@ describe('OrcaRuntimeService', () => { it('adopts public clone repos into host-qualified project setup', async () => { const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-project-clone-')) const clonePath = join(destination, 'orca') - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const repos: Record[] = [] getRepoUpstreamMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' }) const runtimeStore = { @@ -7909,7 +7911,7 @@ describe('OrcaRuntimeService', () => { spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => { + setImmediate(() => { mkdirSync(clonePath, { recursive: true }) execFileSync('git', ['init'], { cwd: clonePath, stdio: 'ignore' }) proc.emit('close', 0, null) @@ -7954,7 +7956,7 @@ describe('OrcaRuntimeService', () => { it('keeps project clone repos split by runtime host on the same clone path', async () => { const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-project-clone-')) const clonePath = join(destination, 'orca') - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const repos: Record[] = [ { id: 'repo-host-a', @@ -7989,7 +7991,7 @@ describe('OrcaRuntimeService', () => { spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => { + setImmediate(() => { mkdirSync(clonePath, { recursive: true }) execFileSync('git', ['init'], { cwd: clonePath, stdio: 'ignore' }) proc.emit('close', 0, null) @@ -8126,7 +8128,7 @@ describe('OrcaRuntimeService', () => { }) it('defaults runtime cloneRepo badgeColor to DEFAULT_REPO_BADGE_COLOR', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const added: Record[] = [] const colorStore = { ...store, @@ -8139,7 +8141,7 @@ describe('OrcaRuntimeService', () => { spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => proc.emit('close', 0, null)) + setImmediate(() => proc.emit('close', 0, null)) return proc as never }) const runtime = new OrcaRuntimeService(colorStore as never) @@ -8162,7 +8164,7 @@ describe('OrcaRuntimeService', () => { }) it('drops a same-path negative submodule cache before runtime cloneRepo', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-reclone-')) const clonePath = join(destination, 'reclone') const added: Record[] = [] @@ -8175,7 +8177,7 @@ describe('OrcaRuntimeService', () => { spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => { + setImmediate(() => { void mkdir(clonePath, { recursive: true }) .then(() => writeFile(join(clonePath, '.gitmodules'), '[submodule "lib"]\n\tpath = vendor/lib\n') @@ -8199,11 +8201,11 @@ describe('OrcaRuntimeService', () => { }) it('preserves existing badgeColor on runtime cloneRepo folder->git dedupe upgrade', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - queueMicrotask(() => proc.emit('close', 0, null)) + setImmediate(() => proc.emit('close', 0, null)) return proc as never }) const existing = { @@ -8297,7 +8299,7 @@ describe('OrcaRuntimeService', () => { }) it('rejects runtime cloneRepo dot-segment URLs before spawning git', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const runtime = createRuntime() const tempRoot = await mkdtemp(join(tmpdir(), 'orca-runtime-clone-')) const destination = join(tempRoot, 'destination') @@ -8314,7 +8316,7 @@ describe('OrcaRuntimeService', () => { }) it('rejects runtime cloneRepo parent-segment URLs before spawning git', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const runtime = createRuntime() const tempRoot = await mkdtemp(join(tmpdir(), 'orca-runtime-clone-')) const destination = join(tempRoot, 'destination') @@ -8331,10 +8333,10 @@ describe('OrcaRuntimeService', () => { }) it('removes an owned runtime clone target when git exits unsuccessfully', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - spawnSpy.mockReturnValue(proc as never) + spawnSpy.mockResolvedValue(proc as never) const runtime = createRuntime() const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-clone-')) const clonePath = join(destination, 'repo-badge-color') @@ -8358,10 +8360,10 @@ describe('OrcaRuntimeService', () => { }) it('preserves an existing runtime clone target when git exits unsuccessfully', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - spawnSpy.mockReturnValue(proc as never) + spawnSpy.mockResolvedValue(proc as never) const runtime = createRuntime() const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-clone-')) const clonePath = join(destination, 'repo-badge-color') @@ -8385,10 +8387,10 @@ describe('OrcaRuntimeService', () => { }) it('skips runtime clone failure cleanup when the owned target is replaced', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - spawnSpy.mockReturnValue(proc as never) + spawnSpy.mockResolvedValue(proc as never) const runtime = createRuntime() const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-clone-')) const clonePath = join(destination, 'repo-badge-color') @@ -8414,10 +8416,10 @@ describe('OrcaRuntimeService', () => { }) it('serializes concurrent runtime clones for the same target', async () => { - const spawnSpy = vi.spyOn(gitRunner, 'gitSpawn') + const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') const firstProc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } firstProc.stderr = new EventEmitter() - spawnSpy.mockReturnValueOnce(firstProc as never) + spawnSpy.mockResolvedValueOnce(firstProc as never) const added: Record[] = [] const colorStore = { ...store, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 6c0f90c6f6b..691486d6dce 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -94,7 +94,12 @@ import { AGENT_PROMPT_SUBMIT_DELAY_MS, buildAgentPromptPasteBytes } from '../../shared/agent-prompt-injection' -import { gitExecFileAsync, gitSpawn, nonInteractiveGitEnv } from '../git/runner' +import { + awaitWindowsHostGitEnvironmentReady, + gitExecFileAsync, + gitSpawnAfterWindowsEnvironmentReady, + nonInteractiveGitEnv +} from '../git/runner' import { runWithGitReadCacheInvalidation } from '../git/status' import { cleanupClaimedCloneTarget, @@ -18479,6 +18484,7 @@ export class OrcaRuntimeService { if (!isAbsolute(path)) { throw new Error('Project path must be an absolute path') } + await awaitWindowsHostGitEnvironmentReady({ cwd: path }) return scanNestedRepos({ path, options: { timeoutMs: 15_000 } }) } @@ -18527,6 +18533,7 @@ export class OrcaRuntimeService { projectPaths: string[] mode: ProjectGroupImportMode }): Promise { + await awaitWindowsHostGitEnvironmentReady({ cwd: args.parentPath }) if (!this.store?.createProjectGroup || !this.store?.moveProjectToGroup) { throw new Error('runtime_unavailable') } @@ -18554,6 +18561,7 @@ export class OrcaRuntimeService { const importTargetResolver = createNestedRepoImportTargetResolver() for (const [projectGroupOrder, repoPath] of selection.selectedPaths.entries()) { try { + await awaitWindowsHostGitEnvironmentReady({ cwd: repoPath }) if (!isGitRepo(repoPath)) { results.push({ path: repoPath, status: 'failed', error: 'Not a valid git repository' }) continue @@ -18681,6 +18689,9 @@ export class OrcaRuntimeService { // server-side repo paths to be explicit so `orca serve` cwd is irrelevant. throw new Error('Project path must be an absolute path') } + if (kind === 'git') { + await awaitWindowsHostGitEnvironmentReady({ cwd: path }) + } if (kind === 'git' && !isGitRepo(path)) { throw new Error(`Not a valid git repository: ${path}`) } @@ -18927,10 +18938,11 @@ export class OrcaRuntimeService { await mkdir(trimmedDestination, { recursive: true }) const claimedTarget = await claimCloneTarget(clonePath) - await new Promise((resolve, reject) => { - let proc: ReturnType - try { - proc = gitSpawn(['clone', '--progress', '--', trimmedUrl, clonePath], { + let proc: Awaited> + try { + proc = await gitSpawnAfterWindowsEnvironmentReady( + ['clone', '--progress', '--', trimmedUrl, clonePath], + { cwd: trimmedDestination, // Why: without the non-interactive guard, a clone that needs GitHub // auth makes Git Credential Manager pop its "Connect to GitHub" OAuth @@ -18939,14 +18951,14 @@ export class OrcaRuntimeService { // (issue #7652). Fail fast with a clear error instead. env: nonInteractiveGitEnv(), stdio: ['ignore', 'ignore', 'pipe'] - }) - } catch (err) { - void cleanupClaimedCloneTarget(clonePath, claimedTarget).finally(() => { - const message = err instanceof Error ? err.message : String(err) - reject(new Error(`Clone failed: ${message}`)) - }) - return - } + } + ) + } catch (err) { + await cleanupClaimedCloneTarget(clonePath, claimedTarget) + const message = err instanceof Error ? err.message : String(err) + throw new Error(`Clone failed: ${message}`) + } + await new Promise((resolve, reject) => { let stderrTail = '' let settled = false proc.stderr?.on('data', (chunk: Buffer) => { diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index 376e6a48814..3e40b5e5f57 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -26,15 +26,20 @@ describe('startup ordering', () => { expect(attachBlock).toContain( 'awaitLocalPtyProviderStartup: () => localPtyProviderStartupReady' ) - expect(source).toContain('firstWindowStartupServicesReady = startupServices.firstWindowReady') - expect(source).toContain('localPtyStartupReady = startupServices.localPtyReady') + expect(source).toContain( + 'firstWindowStartupServicesReady = services.then((value) => value.firstWindowReady)' + ) + expect(source).toContain('localPtyStartupReady = services.then((value) => value.localPtyReady)') - const windowIndex = desktopStartup.indexOf('Promise.resolve(openMainWindow())') + const windowIndex = desktopStartup.indexOf('Promise.resolve(desktopWindow ?? openMainWindow())') const rpcStartIndex = desktopStartup.indexOf('desktopRuntimeRpc.start()') const legacyRpcStartIndex = desktopStartup.indexOf('runtimeRpc.start()') expect(windowIndex).toBeGreaterThanOrEqual(0) expect(Math.max(rpcStartIndex, legacyRpcStartIndex)).toBeGreaterThanOrEqual(0) + expect(desktopStartup).toMatch( + /shellPathReady\s*\.then\(\(\) => (?:desktopRuntimeRpc|runtimeRpc)\.start\(\)\)/ + ) expect(desktopStartup).toContain('recordRuntimeRpcStartFailure(') // Why: `void`, not `await` — awaiting the dialog would park the rest of startup behind a modal. expect(desktopStartup).toMatch(/void showRuntimeRpcStartupFailureDialog\(\s*win,/) @@ -60,7 +65,7 @@ describe('startup ordering', () => { it('bounds WSL reconciliation before serve RPC while leaving desktop startup independent', () => { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') const barrierStart = source.indexOf("ipcMain.handle('app:awaitFirstWindowStartupServices'") - const barrierEnd = source.indexOf("ipcMain.handle(\n 'app:startupDiagnostic'", barrierStart) + const barrierEnd = source.indexOf("'app:startupDiagnostic'", barrierStart) const barrier = source.slice(barrierStart, barrierEnd) const reconciliationStart = source.indexOf( 'managedWslCliReconciliationReady = reconcileManagedWslCliRegistrations(' @@ -68,22 +73,40 @@ describe('startup ordering', () => { const serveStart = source.indexOf('if (serveOptions) {', reconciliationStart) const serveReady = source.indexOf('await printServeReady(serveOptions)', serveStart) const serveEnd = source.indexOf('return', serveReady) - const desktopWindowStart = source.indexOf('Promise.resolve(openMainWindow())') + const desktopWindowStart = source.indexOf( + 'const desktopStartup = startWindowsDesktopBeforeShellPathReady(' + ) + const desktopWindowJoin = source.indexOf( + 'Promise.resolve(desktopWindow ?? openMainWindow())', + serveEnd + ) const serveStartup = source.slice(serveStart, serveEnd) - const desktopStartup = source.slice(serveEnd, desktopWindowStart) + const desktopStartup = source.slice(reconciliationStart, serveStart) + expect(barrierStart).toBeGreaterThanOrEqual(0) + expect(barrierEnd).toBeGreaterThan(barrierStart) expect(reconciliationStart).toBeGreaterThanOrEqual(0) expect(serveStart).toBeGreaterThan(reconciliationStart) expect(serveEnd).toBeGreaterThan(serveStart) // Why: bound against serveEnd, not reconciliationStart — an earlier openMainWindow() call // would steal this anchor, collapse desktopStartup to '', and pass the negative check below. - expect(desktopWindowStart).toBeGreaterThan(serveEnd) + expect(desktopWindowStart).toBeGreaterThan(reconciliationStart) + expect(desktopWindowStart).toBeLessThan(serveStart) + expect(desktopWindowJoin).toBeGreaterThan(serveEnd) expect(serveStartup).toContain('await managedWslCliStartupBarrierReady') expect(serveStartup).not.toContain('await managedWslCliReconciliationReady') expect(serveStartup.indexOf('await managedWslCliStartupBarrierReady')).toBeLessThan( serveStartup.indexOf('await runtimeRpc.start()') ) expect(desktopStartup).not.toContain('await managedWslCliReconciliationReady') + expect(desktopStartup).toContain( + "process.platform === 'win32' && app.isPackaged && !serveOptions" + ) + expect(desktopStartup).toContain( + 'openWindow: () => openMainWindow({ revealOnDidFinishLoad: true })' + ) + expect(desktopStartup).toContain('shellPathReady,') + expect(desktopStartup).toContain('startServices: startTerminalRuntimeStartupServices') expect(barrier).toContain('managedWslCliStartupBarrierReady') expect(barrier).not.toContain('managedWslCliReconciliationReady') expect(barrier).toContain("ipcMain.handle('app:recoverLegacyWorkerTerminalsForRendererStartup'") @@ -110,7 +133,10 @@ describe('startup ordering', () => { inventoryIndex ) const serveIndex = source.indexOf('if (serveOptions) {', reconciliationIndex) - const desktopIndex = source.indexOf('Promise.resolve(openMainWindow())', serveIndex) + const desktopIndex = source.indexOf( + 'Promise.resolve(desktopWindow ?? openMainWindow())', + serveIndex + ) expect(daemonInitIndex).toBeGreaterThanOrEqual(0) expect(retainedPaneGateIndex).toBeGreaterThan(daemonInitIndex) diff --git a/src/main/startup/serve-desktop-activation-wiring.test.ts b/src/main/startup/serve-desktop-activation-wiring.test.ts index d1cb5bfd2fb..239a7618567 100644 --- a/src/main/startup/serve-desktop-activation-wiring.test.ts +++ b/src/main/startup/serve-desktop-activation-wiring.test.ts @@ -16,7 +16,7 @@ describe('serve desktop activation wiring', () => { it('settles the persistent provider before headless PTY registration', () => { const appReadyIndex = source.indexOf('app.whenReady().then(async () => {') const startupIndex = source.indexOf( - '\n startTerminalRuntimeStartupServices()\n', + 'bindTerminalRuntimeStartupServices(Promise.resolve(startTerminalRuntimeStartupServices()))', appReadyIndex ) const serveIndex = source.indexOf('if (serveOptions) {', appReadyIndex) diff --git a/src/main/startup/windows-desktop-shell-path-startup.test.ts b/src/main/startup/windows-desktop-shell-path-startup.test.ts new file mode 100644 index 00000000000..3778e0b2688 --- /dev/null +++ b/src/main/startup/windows-desktop-shell-path-startup.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it, vi } from 'vitest' +import { startWindowsDesktopBeforeShellPathReady } from './windows-desktop-shell-path-startup' + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +describe('Windows desktop shell PATH startup', () => { + it('opens the first window while shell PATH hydration remains unresolved', async () => { + const shellPath = deferred() + const window = { visible: true } + const openWindow = vi.fn(() => window) + const startServices = vi.fn(() => ({ + firstWindowReady: Promise.resolve(), + localPtyReady: Promise.resolve(), + localPtyProviderReady: Promise.resolve() + })) + + const startup = startWindowsDesktopBeforeShellPathReady({ + openWindow, + shellPathReady: shellPath.promise, + startServices + }) + + expect(startup.window).toBe(window) + expect(openWindow).toHaveBeenCalledOnce() + expect(startServices).not.toHaveBeenCalled() + + shellPath.resolve() + await startup.services + expect(startServices).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/startup/windows-desktop-shell-path-startup.ts b/src/main/startup/windows-desktop-shell-path-startup.ts new file mode 100644 index 00000000000..3933a79b64b --- /dev/null +++ b/src/main/startup/windows-desktop-shell-path-startup.ts @@ -0,0 +1,18 @@ +export type WindowsDesktopStartupServices = { + firstWindowReady: Promise + localPtyReady: Promise + localPtyProviderReady: Promise +} + +type WindowsDesktopShellPathStartupOptions = { + openWindow: () => TWindow + shellPathReady: Promise + startServices: () => WindowsDesktopStartupServices +} + +export function startWindowsDesktopBeforeShellPathReady( + options: WindowsDesktopShellPathStartupOptions +): { window: TWindow; services: Promise } { + const services = options.shellPathReady.then(options.startServices) + return { window: options.openWindow(), services } +} diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index b6d1ff70035..3f02ba389d2 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -157,6 +157,9 @@ describe('createMainWindow', () => { on: vi.fn((event, handler) => { windowHandlers[event] = handler }), + once: vi.fn((event, handler) => { + windowHandlers[event] = handler + }), setZoomLevel: vi.fn(), setBackgroundThrottling: vi.fn(), invalidate: vi.fn(), @@ -699,6 +702,9 @@ describe('createMainWindow', () => { on: vi.fn((event, handler) => { windowHandlers[event] = handler }), + once: vi.fn((event, handler) => { + windowHandlers[event] = handler + }), setZoomLevel: vi.fn(), setBackgroundThrottling: vi.fn(), invalidate: vi.fn(), @@ -3431,6 +3437,9 @@ describe('createMainWindow', () => { on: vi.fn((event, handler) => { windowHandlers[event] = handler }), + once: vi.fn((event, handler) => { + windowHandlers[event] = handler + }), setZoomLevel: vi.fn(), setBackgroundThrottling: vi.fn(), invalidate: vi.fn(), @@ -3490,6 +3499,21 @@ describe('createMainWindow', () => { expect(browserWindowInstance.show).toHaveBeenCalledTimes(1) }) + it('can reveal the startup window after renderer load before ready-to-show', () => { + const { browserWindowInstance, windowHandlers } = createStartupRevealWindowFixture() + + createMainWindow(null, { revealOnDidFinishLoad: true }) + const revealAfterLoad = browserWindowInstance.webContents.on.mock.calls.find( + ([event]) => event === 'did-finish-load' + )?.[1] + expect(revealAfterLoad).toBeTypeOf('function') + revealAfterLoad?.() + + expect(browserWindowInstance.show).toHaveBeenCalledTimes(1) + windowHandlers['ready-to-show']() + expect(browserWindowInstance.show).toHaveBeenCalledTimes(1) + }) + it('reveals the startup window on Windows when ready-to-show never fires', () => { vi.useFakeTimers() const { browserWindowInstance } = createStartupRevealWindowFixture() diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 17bcd061e81..e46d84855f3 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -206,6 +206,8 @@ type CreateMainWindowOptions = { }) => void /** Defer renderer load until IPC handlers are registered, or eager renderer calls race into missing channels. */ deferLoad?: boolean + /** Reveal after load instead of first paint when startup must show the shell before slower renderer work. */ + revealOnDidFinishLoad?: boolean title?: string getKeybindings?: () => KeybindingOverrides | undefined onBeforeReload?: (options: { ignoreCache: boolean; webContentsId: number }) => void @@ -377,6 +379,9 @@ export function createMainWindow( mainWindow.show() } mainWindow.on('ready-to-show', revealInitialWindow) + if (opts?.revealOnDidFinishLoad === true) { + mainWindow.webContents.on('did-finish-load', revealInitialWindow) + } // Why: persist window bounds to restore last position/size; debounce to avoid hammering persistence during resize drags. let boundsTimer: ReturnType | null = null diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 5f9b6567270..84b6bb9e155 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -963,6 +963,10 @@ function App(): React.JSX.Element { // Why: disconnected SSH repos hydrate from local metadata; only runtime-owned repos use placeholders. parseExecutionHostId(getRepoExecutionHostId(repo))?.kind !== 'runtime' ) + // Why: worktree refresh can spawn host Git; wait for main's shell-PATH generation fence first. + await timeRendererStartupStep('first-window-services-await', () => + window.api.app.awaitFirstWindowStartupServices() + ) await timeRendererStartupStep('fetch-hydration-worktrees', () => mapWithConcurrency(hydrationRepos, WORKTREE_REFRESH_CONCURRENCY, (repo) => actions.fetchWorktrees(repo.id, { executionHostId: getRepoExecutionHostId(repo) }) @@ -1095,10 +1099,7 @@ function App(): React.JSX.Element { logRendererStartupDiagnostic('ssh-reconnect-skipped', { connectionIds: 0 }) } - // Why: main overlaps daemon/hook startup with hydration, but restored terminals need those services ready before they spawn/reconnect PTYs. - await timeRendererStartupStep('first-window-services-await', () => - window.api.app.awaitFirstWindowStartupServices() - ) + // first-window-services-await already fenced worktree hydration; terminal recovery reuses that ready state. await timeRendererStartupStep('recover-legacy-worker-terminals-pre-reconnect', () => window.api.app.recoverLegacyWorkerTerminalsForRendererStartup() ) diff --git a/src/renderer/src/app-startup-routing.test.ts b/src/renderer/src/app-startup-routing.test.ts index 80bb0037eac..9a64224f072 100644 --- a/src/renderer/src/app-startup-routing.test.ts +++ b/src/renderer/src/app-startup-routing.test.ts @@ -36,6 +36,10 @@ describe('renderer startup runtime routing', () => { const hydrationWorktreesIndex = source.indexOf( "timeRendererStartupStep('fetch-hydration-worktrees'" ) + const servicesIndex = source.indexOf( + "timeRendererStartupStep('first-window-services-await'", + sessionIndex + ) const fullWorktreesIndex = source.indexOf('await actions.fetchAllWorktrees()') const lineageIndex = startupBlock.indexOf('actions.fetchWorktreeLineage()') @@ -53,7 +57,8 @@ describe('renderer startup runtime routing', () => { expect(localReposIndex).toBeLessThan(localGroupsIndex) expect(localGroupsIndex).toBeLessThan(localFoldersIndex) expect(localReposIndex).toBeLessThan(sessionIndex) - expect(sessionIndex).toBeLessThan(hydrationWorktreesIndex) + expect(sessionIndex).toBeLessThan(servicesIndex) + expect(servicesIndex).toBeLessThan(hydrationWorktreesIndex) const hydrationWorktreeBlock = source.slice( hydrationWorktreesIndex, source.indexOf('await keybindingsPromise')