From 9d887181700ac3f71c63487dd54fcc47c06bd39e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:53:41 -0700 Subject: [PATCH 1/7] perf: preserve user Git checkout worker settings --- docs/reference/git-compatibility.md | 11 ----- src/main/git/add-sparse-worktree.test.ts | 2 +- .../git/worktree-add-creation-config.test.ts | 37 ++-------------- .../worktree-add-local-base-refresh.test.ts | 5 --- ...worktree-add-local-base-suggestion.test.ts | 1 - src/main/git/worktree-add.ts | 8 +--- src/main/git/worktree-create-preparation.ts | 17 +------- src/shared/git-binary-compatibility.test.ts | 43 ------------------- src/shared/worktree-checkout-config.test.ts | 12 ------ src/shared/worktree-checkout-config.ts | 7 --- 10 files changed, 7 insertions(+), 136 deletions(-) delete mode 100644 src/shared/worktree-checkout-config.test.ts delete mode 100644 src/shared/worktree-checkout-config.ts diff --git a/docs/reference/git-compatibility.md b/docs/reference/git-compatibility.md index bc75543ac0f..1e19860385e 100644 --- a/docs/reference/git-compatibility.md +++ b/docs/reference/git-compatibility.md @@ -53,17 +53,6 @@ in one record and pick at parse time. | --------------- | ----------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | | `%(decorate:…)` | Git 2.43 separates commit decorations with `\x1f`, so ref names containing commas survive | The same record also carries `%D` (Git 2.10); an unexpanded `%(decorate` placeholder selects it, at the cost of comma-splitting | -### Parallel checkout configuration - -Native macOS worktree creation and prepared-checkout materialization pass the -command-local setting `-c checkout.workers=4`. Parallel checkout arrived in Git -2.32; Git 2.25–2.31 accepts and ignores this unknown configuration key, preserving -serial checkout without a rejected command or retry. No capability-cache entry is -needed for this setting. The real-binary contract checks the worker boundary using -Trace2 and verifies checked-out content and clean status. Windows, WSL, Linux, and -SSH keep their existing checkout settings pending host measurements. The setting -is never persisted to repository or user configuration. - ## Why Not `simple-git` `simple-git` is a process wrapper around the installed Git binary. Its custom diff --git a/src/main/git/add-sparse-worktree.test.ts b/src/main/git/add-sparse-worktree.test.ts index c31b8de85cf..2541b822438 100644 --- a/src/main/git/add-sparse-worktree.test.ts +++ b/src/main/git/add-sparse-worktree.test.ts @@ -167,7 +167,7 @@ branch refs/heads/main const calls = getGitCalls() expect(calls).toEqual( expect.arrayContaining([ - 'git -c checkout.workers=4 worktree add --no-checkout --no-track -b feature/test /repo-feature', + 'git worktree add --no-checkout --no-track -b feature/test /repo-feature', 'git config --get push.autoSetupRemote', 'git config --local push.autoSetupRemote true', 'git sparse-checkout init --cone', diff --git a/src/main/git/worktree-add-creation-config.test.ts b/src/main/git/worktree-add-creation-config.test.ts index eaa76f37083..ff44ca7ac6d 100644 --- a/src/main/git/worktree-add-creation-config.test.ts +++ b/src/main/git/worktree-add-creation-config.test.ts @@ -68,8 +68,6 @@ describe('addWorktree', () => { [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], [ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -104,7 +102,7 @@ describe('addWorktree', () => { expect(gitExecFileAsyncMock.mock.calls).toEqual([ [ - ['-c', 'checkout.workers=4', 'worktree', 'add', '/repo-feature', 'feature/test'], + ['worktree', 'add', '/repo-feature', 'feature/test'], { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ] ]) @@ -184,13 +182,7 @@ describe('addWorktree', () => { }) expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - [ - ...(platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), - 'worktree', - 'add', - '/repo-feature', - 'feature/test' - ], + ['worktree', 'add', '/repo-feature', 'feature/test'], { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ) } @@ -235,16 +227,7 @@ describe('addWorktree', () => { expect(gitExecFileAsyncMock.mock.calls).toEqual([ [ - [ - '-c', - 'checkout.workers=4', - 'worktree', - 'add', - '--no-track', - '-b', - 'feature/no-base', - '/repo-feature' - ], + ['worktree', 'add', '--no-track', '-b', 'feature/no-base', '/repo-feature'], { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [['config', '--get', 'push.autoSetupRemote'], { cwd: '/repo-feature' }] @@ -319,8 +302,6 @@ describe('addWorktree', () => { [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], [ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -359,8 +340,6 @@ describe('addWorktree', () => { [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], [ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -400,8 +379,6 @@ describe('addWorktree', () => { [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], [ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -442,8 +419,6 @@ describe('addWorktree', () => { [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], [ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -475,8 +450,6 @@ describe('addWorktree', () => { ]) expect(gitExecFileAsyncMock.mock.calls[1]).toEqual([ [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -503,8 +476,6 @@ describe('addWorktree', () => { ['rev-parse', '--verify', '--quiet', 'refs/remotes/release/main^{commit}'], ['rev-parse', '--verify', '--quiet', 'refs/heads/release/main^{commit}'], [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', @@ -546,8 +517,6 @@ describe('addWorktree', () => { ['rev-parse', '--verify', '--quiet', 'refs/remotes/release/main^{commit}'], ['rev-parse', '--verify', '--quiet', 'refs/heads/release/main^{commit}'], [ - '-c', - 'checkout.workers=4', 'worktree', 'add', '--no-track', diff --git a/src/main/git/worktree-add-local-base-refresh.test.ts b/src/main/git/worktree-add-local-base-refresh.test.ts index a684594291b..1ba2e6c16b8 100644 --- a/src/main/git/worktree-add-local-base-refresh.test.ts +++ b/src/main/git/worktree-add-local-base-refresh.test.ts @@ -82,7 +82,6 @@ describe('addWorktree', () => { [['reset', '--hard', 'remote-main'], { cwd: '/repo' }], [ [ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', @@ -320,7 +319,6 @@ describe('addWorktree', () => { 'refs/remotes/origin/main^{commit}' ]) expect(gitExecFileAsyncMock.mock.calls[7]?.[0]).toEqual([ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', @@ -380,7 +378,6 @@ describe('addWorktree', () => { ], [ [ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', @@ -437,7 +434,6 @@ describe('addWorktree', () => { expect(result.localBaseRefRefresh).toBeUndefined() expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toContainEqual([ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', @@ -559,7 +555,6 @@ describe('addWorktree', () => { ], [ [ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', diff --git a/src/main/git/worktree-add-local-base-suggestion.test.ts b/src/main/git/worktree-add-local-base-suggestion.test.ts index b40d65d8955..3b449c339f6 100644 --- a/src/main/git/worktree-add-local-base-suggestion.test.ts +++ b/src/main/git/worktree-add-local-base-suggestion.test.ts @@ -95,7 +95,6 @@ describe('addWorktree', () => { ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], ['rev-list', '--left-right', '--count', 'refs/heads/main...refs/remotes/origin/main'], [ - ...(process.platform === 'darwin' ? ['-c', 'checkout.workers=4'] : []), 'worktree', 'add', '--no-track', diff --git a/src/main/git/worktree-add.ts b/src/main/git/worktree-add.ts index f36f2501787..380f3a3cc34 100644 --- a/src/main/git/worktree-add.ts +++ b/src/main/git/worktree-add.ts @@ -20,7 +20,6 @@ import type { } from './worktree-operation-options' import { gitExecOptions, resolveWorktreeAddTimeoutMs } from './worktree-operation-options' import { bumpWorktreeScanGeneration } from './worktree-scan-cache' -import { worktreeCheckoutGitArgs } from '../../shared/worktree-checkout-config' export type WorktreeAddBaseContext = AddWorktreeResult & { effectiveBase: string @@ -188,12 +187,7 @@ async function performAddWorktree( let localBaseRefRefresh: LocalBaseRefRefreshResult | undefined let localBaseRefUpdateSuggestion: LocalBaseRefUpdateSuggestion | undefined // Why: enable long paths for this Windows checkout without changing user Git config. - const args = [ - ...windowsLongPathGitArgs(repoPath), - ...worktreeCheckoutGitArgs(options), - 'worktree', - 'add' - ] + const args = [...windowsLongPathGitArgs(repoPath), 'worktree', 'add'] let effectiveBase: string | undefined if (noCheckout) { args.push('--no-checkout') diff --git a/src/main/git/worktree-create-preparation.ts b/src/main/git/worktree-create-preparation.ts index b42771353fa..78713957690 100644 --- a/src/main/git/worktree-create-preparation.ts +++ b/src/main/git/worktree-create-preparation.ts @@ -14,7 +14,6 @@ import { withRepoRefMaintenancePaused } from './local-repo-ref-maintenance' import { gitExecFileAsync } from './runner' import { runWithGitReadCacheInvalidation } from './status' import { invalidateWslLinkedWorktreeGitRouting } from './wsl-linked-worktree-git-routing' -import { worktreeCheckoutGitArgs } from '../../shared/worktree-checkout-config' function gitExecOptions( cwd: string, @@ -93,13 +92,7 @@ export async function prepareWorktreeCreateCheckout( invalidateWslLinkedWorktreeGitRouting(worktreePath) // Why: reset materializes files without running user post-checkout hooks before submit. await gitExecFileAsync( - [ - ...windowsLongPathGitArgs(worktreePath), - ...worktreeCheckoutGitArgs(options), - 'reset', - '--hard', - effectiveBase - ], + [...windowsLongPathGitArgs(worktreePath), 'reset', '--hard', effectiveBase], { ...gitExecOptions(worktreePath, options), timeout: resolveWorktreeAddTimeoutMs() } ) await gitExecFileAsync( @@ -236,13 +229,7 @@ export async function finalizePreparedWorktree( const preparedHeadOutput = preparedResult.value.stdout if (preparedHeadOutput.trim() !== targetHead) { await gitExecFileAsync( - [ - ...windowsLongPathGitArgs(preparedPath), - ...worktreeCheckoutGitArgs(options), - 'reset', - '--hard', - targetHead - ], + [...windowsLongPathGitArgs(preparedPath), 'reset', '--hard', targetHead], gitExecOptions(preparedPath, finalizeGitOptions) ) } diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index e7ae0bc8797..fb8161b9f90 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -23,7 +23,6 @@ import { gitlabMergeRequestHeadLocalRef, reviewHeadRemoteRefComponent } from './review-head-tracking-ref' -import { worktreeCheckoutGitArgs } from './worktree-checkout-config' const execFileAsync = promisify(execFile) const image = process.env.ORCA_GIT_COMPAT_IMAGE @@ -112,48 +111,6 @@ describeBinaryCompatibility('real Git binary compatibility', () => { } }) - it('materializes parallel checkouts with a serial fallback before Git 2.32', async () => { - await runGit(['worktree', 'add', '--detach', 'parallel-source', 'HEAD']) - await Promise.all( - Array.from({ length: 16 }, (_, i) => - writeFile(join(repoPath, 'parallel-source', `parallel-${i}.txt`), `file ${i}\n`) - ) - ) - await runGit(['-C', 'parallel-source', 'add', '.']) - await runGit(['-C', 'parallel-source', 'commit', '-qm', 'parallel fixture']) - const head = (await runGit(['-C', 'parallel-source', 'rev-parse', 'HEAD'])).stdout.trim() - await runGit(['worktree', 'add', '--detach', '--no-checkout', 'parallel-wt', head]) - try { - const result = await runGit( - [ - '-C', - 'parallel-wt', - ...worktreeCheckoutGitArgs({}, 'darwin'), - '-c', - 'checkout.thresholdForParallelism=0', - 'reset', - '--hard', - 'HEAD' - ], - { GIT_TRACE2_EVENT: '1' } - ) - expect(result.stderr.includes('checkout--worker')).toBe(supports(2, 32)) - expect( - (await readFile(join(repoPath, 'parallel-wt', 'tracked.txt'), 'utf8')).replaceAll( - '\r\n', - '\n' - ) - ).toBe('compatibility\n') - expect((await runGit(['-C', 'parallel-wt', 'status', '--porcelain'])).stdout).toBe('') - await expect( - runGit(['config', '--local', '--get', 'checkout.workers']) - ).rejects.toMatchObject({ code: 1 }) - } finally { - await runGit(['worktree', 'remove', '--force', 'parallel-wt']) - await runGit(['worktree', 'remove', '--force', 'parallel-source']) - } - }) - it('quietly distinguishes present and absent branch refs', async () => { const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() await runGit(['branch', 'quiet-probe-present', head]) diff --git a/src/shared/worktree-checkout-config.test.ts b/src/shared/worktree-checkout-config.test.ts deleted file mode 100644 index 01cd98eee39..00000000000 --- a/src/shared/worktree-checkout-config.test.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { worktreeCheckoutGitArgs } from './worktree-checkout-config' - -describe('worktree checkout concurrency', () => { - it('bounds native Mac checkout workers without changing other execution hosts', () => { - expect(worktreeCheckoutGitArgs({}, 'darwin')).toEqual(['-c', 'checkout.workers=4']) - expect(worktreeCheckoutGitArgs({ wslDistro: 'Ubuntu' }, 'darwin')).toEqual([]) - expect(worktreeCheckoutGitArgs({}, 'win32')).toEqual([]) - expect(worktreeCheckoutGitArgs({ wslDistro: 'Ubuntu' }, 'win32')).toEqual([]) - expect(worktreeCheckoutGitArgs({}, 'linux')).toEqual([]) - }) -}) diff --git a/src/shared/worktree-checkout-config.ts b/src/shared/worktree-checkout-config.ts deleted file mode 100644 index ce705f80241..00000000000 --- a/src/shared/worktree-checkout-config.ts +++ /dev/null @@ -1,7 +0,0 @@ -export function worktreeCheckoutGitArgs( - options: { wslDistro?: string } = {}, - platform: NodeJS.Platform = process.platform -): string[] { - // Four workers halved native Mac checkout time; Git before 2.32 ignores this config. - return platform === 'darwin' && !options.wslDistro ? ['-c', 'checkout.workers=4'] : [] -} From 5e2e2e1951a4d1682ea70b8d3e49f96cb4c275c7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:13:12 -0700 Subject: [PATCH 2/7] perf(git): skip malformed remote base probes --- src/main/runtime/fetch-remote-cache.test.ts | 13 +++++++++++-- src/main/runtime/runtime-remote-fetch-controller.ts | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index 11cd2e0260d..5f0b8e249d6 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -176,8 +176,17 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { expect(caches.fetchLastCompletedAt.has('/repo/cache-0::origin')).toBe(false) }) - it.each(['main', 'a'.repeat(40), 'refs/remotes/main', ''])( - 'does not launch Git for a base without a remote/branch separator: %s', + it.each([ + 'main', + 'a'.repeat(40), + 'refs/remotes/main', + '', + 'origin/', + '/main', + 'refs/remotes/origin/', + 'refs/remotes//main' + ])( + 'does not launch Git for a base without both remote and branch components: %s', async (base) => { const runtime = new OrcaRuntimeService(null) await expect(runtime.resolveRemoteTrackingBase('/repo/e', base)).resolves.toBeNull() diff --git a/src/main/runtime/runtime-remote-fetch-controller.ts b/src/main/runtime/runtime-remote-fetch-controller.ts index f40f25f6c82..dbcc240525b 100644 --- a/src/main/runtime/runtime-remote-fetch-controller.ts +++ b/src/main/runtime/runtime-remote-fetch-controller.ts @@ -231,7 +231,7 @@ export class RuntimeRemoteFetchController { ? baseBranch.slice(remoteRefPrefix.length) : baseBranch // A remote-tracking base needs both a configured remote and a branch component. - if (!shortBaseBranch.includes('/')) { + if (shortBaseBranch.indexOf('/') <= 0 || shortBaseBranch.endsWith('/')) { return null } let remotes: string[] From 9a84a5788288d3d4faa86a57d1753a4855e71d43 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:50:42 -0700 Subject: [PATCH 3/7] perf(cli): avoid loading other agent hooks for Codex preflight --- src/cli/handlers/agent-hooks.test.ts | 5 ++++- src/cli/handlers/agent-hooks.ts | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cli/handlers/agent-hooks.test.ts b/src/cli/handlers/agent-hooks.test.ts index 4fcc186b0d8..279a8900bec 100644 --- a/src/cli/handlers/agent-hooks.test.ts +++ b/src/cli/handlers/agent-hooks.test.ts @@ -56,7 +56,10 @@ vi.mock('../runtime-client', () => { vi.mock('../../main/agent-hooks/managed-agent-hook-controls', () => ({ applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock, - getManagedAgentHookStatuses: getManagedAgentHookStatusesMock, + getManagedAgentHookStatuses: getManagedAgentHookStatusesMock +})) + +vi.mock('../../main/codex/managed-home-shell-preflight', () => ({ prepareManagedCodexHomeBeforeShellLaunch: prepareManagedCodexHomeBeforeShellLaunchMock })) diff --git a/src/cli/handlers/agent-hooks.ts b/src/cli/handlers/agent-hooks.ts index bf44211b4a7..4fcfe64f9b7 100644 --- a/src/cli/handlers/agent-hooks.ts +++ b/src/cli/handlers/agent-hooks.ts @@ -15,11 +15,7 @@ import { getDefaultPersistedState } from '../../shared/constants' import { normalizeDisabledTuiAgents } from '../../shared/tui-agent-selection' import type { GlobalSettings } from '../../shared/global-settings-types' import type { PersistedState } from '../../shared/persisted-state-types' -import { - applyAgentStatusHooksEnabled, - getManagedAgentHookStatuses, - prepareManagedCodexHomeBeforeShellLaunch -} from '../../main/agent-hooks/managed-agent-hook-controls' +import { prepareManagedCodexHomeBeforeShellLaunch } from '../../main/codex/managed-home-shell-preflight' type AgentHookCommandResult = { enabled: boolean @@ -194,6 +190,8 @@ async function setAgentHooksEnabled( client: RuntimeClient, enabled: boolean ): Promise { + const { applyAgentStatusHooksEnabled, getManagedAgentHookStatuses } = + await import('../../main/agent-hooks/managed-agent-hook-controls.js') const updatedRuntime = await updateRunningRuntime(client, enabled) const offlineUpdate = updatedRuntime ? null : updateEnabledOnDisk(enabled) const settingsPath = offlineUpdate?.settingsPath ?? getDataPath() @@ -234,6 +232,8 @@ export const AGENT_HOOK_HANDLERS: Record = { }) }, 'agent hooks status': async ({ json }) => { + const { getManagedAgentHookStatuses } = + await import('../../main/agent-hooks/managed-agent-hook-controls.js') const result: AgentHookCommandResult = { enabled: readHookSettingsFromDisk().agentStatusHooksEnabled, settingsPath: getDataPath(), From a9f00a034de169a6b44303e8507c294bcdd3e2de Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:59:11 -0700 Subject: [PATCH 4/7] fix(build): retain Codex preflight entry for packaged CLI --- electron.vite.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 4ed4641cde1..90dc637c204 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -253,6 +253,9 @@ export const electronViteConfig: UserConfig = { 'agent-hooks/managed-agent-hook-controls': resolve( 'src/main/agent-hooks/managed-agent-hook-controls.ts' ), + 'codex/managed-home-shell-preflight': resolve( + 'src/main/codex/managed-home-shell-preflight.ts' + ), // Why: account import mutates the user's macOS Keychain from the CLI. 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts') }, From 148ee486d1f7b3b3d368c1f3db5e4b1a9733871e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:28:12 -0700 Subject: [PATCH 5/7] test(ssh): wait for replacement PTY before lease recovery input --- tests/e2e/ssh-docker-transport-drop-recovery.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts index e18336d12c5..4d0e91eae0e 100644 --- a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts +++ b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts @@ -354,6 +354,7 @@ test.describe('SSH transport drop recovery', () => { const generations: string[][] = [] for (let generation = 1; generation <= 5; generation++) { + const previousPtyId = await waitForActivePanePtyId(orcaPage, 60_000) expect( killDockerSshRelayDaemon(target), 'no relay process was found to kill' @@ -365,8 +366,13 @@ test.describe('SSH transport drop recovery', () => { }) .toBe('connected') await waitForActiveTerminalManager(orcaPage, 120_000) - // The pane must be usable again before the count is meaningful: recovery is what mints the - // successor lease that retires the generation before it. + // Transport status can still be connected while the pane retains its old binding. + await expect + .poll(() => waitForActivePanePtyId(orcaPage, 60_000).catch(() => previousPtyId), { + timeout: 120_000, + message: `pane kept its old PTY binding after relay kill ${generation}` + }) + .not.toBe(previousPtyId) const ptyId = await waitForActivePanePtyId(orcaPage, 120_000) const marker = `LEASE_GEN_${generation}_${Date.now()}` await execInTerminal(orcaPage, ptyId, `printf '%s\\n' ${marker}`) From 80c93771548075b2c169f0cc0196543bc1430d8d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:44:24 -0700 Subject: [PATCH 6/7] test(ssh): verify recovered shell execution and lease ownership --- .../ssh-docker-transport-drop-recovery.spec.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts index 4d0e91eae0e..c7208f8e3cd 100644 --- a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts +++ b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts @@ -4,6 +4,7 @@ import type { ElectronApplication, Page } from '@playwright/test' import { test, expect } from './helpers/orca-app' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import { sshRemotePtyLeaseAllowsReattach, type SshRemotePtyLease } from '../../src/shared/ssh-types' +import { toRelaySshPtyId } from '../../src/shared/ssh-pty-id' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { execInTerminal, @@ -374,21 +375,21 @@ test.describe('SSH transport drop recovery', () => { }) .not.toBe(previousPtyId) const ptyId = await waitForActivePanePtyId(orcaPage, 120_000) - const marker = `LEASE_GEN_${generation}_${Date.now()}` - await execInTerminal(orcaPage, ptyId, `printf '%s\\n' ${marker}`) + const markerSuffix = `${generation}_${Date.now()}` + const marker = `LEASE_GEN_${markerSuffix}` + await execInTerminal(orcaPage, ptyId, `printf 'LEASE_GEN_%s\\n' ${markerSuffix}`) await waitForTerminalOutput(orcaPage, marker, 60_000) try { await expect - .poll(() => readReattachablePtyIds(userDataDir, remote.targetId).length, { + .poll(() => readReattachablePtyIds(userDataDir, remote.targetId), { timeout: 60_000 }) - .toBe(1) + .toEqual([toRelaySshPtyId(remote.targetId, ptyId)]) } catch (error) { - // Why re-thrown with the rows: the count alone cannot say WHICH predecessor stayed - // reattachable, and the user-data dir is torn down before the report is read. + // Preserve lease ownership diagnostics before the user-data directory is removed. throw new Error( - `reattachable lease count never settled at 1 in generation ${generation}; leases: ${describeSshLeases(userDataDir, remote.targetId)}`, + `reattachable leases never settled at the active PTY ${ptyId} in generation ${generation}; leases: ${describeSshLeases(userDataDir, remote.targetId)}`, { cause: error } ) } From 0575b264b1d2db91644dd0f490ac4edbfc06c217 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:59:20 -0700 Subject: [PATCH 7/7] test(electron): reap isolated macOS crash reporters on teardown --- .../e2e/helpers/electron-crashpad-cleanup.ts | 46 +++++++++++++++++++ .../electron-crashpad-cleanup.unit.test.ts | 45 ++++++++++++++++++ .../e2e/helpers/electron-process-shutdown.ts | 2 + 3 files changed, 93 insertions(+) create mode 100644 tests/e2e/helpers/electron-crashpad-cleanup.ts create mode 100644 tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts diff --git a/tests/e2e/helpers/electron-crashpad-cleanup.ts b/tests/e2e/helpers/electron-crashpad-cleanup.ts new file mode 100644 index 00000000000..8ec1a98b26b --- /dev/null +++ b/tests/e2e/helpers/electron-crashpad-cleanup.ts @@ -0,0 +1,46 @@ +import { execFileSync } from 'node:child_process' +import path from 'node:path' + +function ownsCrashpad(command: string, userDataDir: string): boolean { + return ( + command.includes('/chrome_crashpad_handler ') && + command.includes(` --database=${path.join(userDataDir, 'Crashpad')} `) + ) +} + +export function cleanupE2ECrashpad(userDataDir: string): void { + if (process.platform !== 'darwin') { + return + } + + // macOS reparents Crashpad before app exit; its inherited stderr can keep Playwright open. + try { + const table = execFileSync('ps', ['-axo', 'pid=,command='], { + encoding: 'utf8', + timeout: 5_000 + }) + for (const row of table.split('\n')) { + const match = row.match(/^\s*(\d+)\s+(.+)$/) + if (!match || !ownsCrashpad(match[2], userDataDir)) { + continue + } + const pid = Number(match[1]) + if (!Number.isSafeInteger(pid) || pid <= 1) { + continue + } + try { + const command = execFileSync('ps', ['-p', String(pid), '-o', 'command='], { + encoding: 'utf8', + timeout: 5_000 + }) + if (ownsCrashpad(command, userDataDir)) { + process.kill(pid, 'SIGTERM') + } + } catch { + // The test-owned reporter may already have exited. + } + } + } catch { + // Cleanup remains best-effort when process enumeration is unavailable. + } +} diff --git a/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts b/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts new file mode 100644 index 00000000000..cdf552e2548 --- /dev/null +++ b/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { execFileSync } from 'node:child_process' +import path from 'node:path' +import { cleanupE2ECrashpad } from './electron-crashpad-cleanup' + +vi.mock('node:child_process', () => ({ execFileSync: vi.fn() })) + +const profile = '/tmp/test profile' +const database = path.join(profile, 'Crashpad') +const reporter = `/Electron Framework/Helpers/chrome_crashpad_handler --database=${database} --annotation=prod=Electron` + +afterEach(() => vi.restoreAllMocks()) + +describe('test-owned macOS Crashpad cleanup', () => { + it('terminates only the reporter for the exact temporary profile after rechecking ownership', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const kill = vi.spyOn(process, 'kill').mockReturnValue(true) + vi.mocked(execFileSync) + .mockReturnValueOnce( + `111 ${reporter}\n222 ${reporter.replace('Crashpad ', 'Crashpad-old ')}\n333 ${reporter.replace('test profile', 'another profile')}\n444 /bin/echo --database=${database} \n` + ) + .mockReturnValueOnce(reporter) + cleanupE2ECrashpad(profile) + expect(kill).toHaveBeenCalledExactlyOnceWith(111, 'SIGTERM') + expect(execFileSync).toHaveBeenLastCalledWith('ps', ['-p', '111', '-o', 'command='], { + encoding: 'utf8', + timeout: 5_000 + }) + }) + + it('does not signal a PID whose ownership changed after enumeration', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const kill = vi.spyOn(process, 'kill').mockReturnValue(true) + vi.mocked(execFileSync).mockReturnValueOnce(`111 ${reporter}`).mockReturnValueOnce('/bin/sh') + cleanupE2ECrashpad(profile) + expect(kill).not.toHaveBeenCalled() + }) + + it.each(['win32', 'linux'] as const)('does not enumerate processes on %s', (platform) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + vi.mocked(execFileSync).mockClear() + cleanupE2ECrashpad(profile) + expect(execFileSync).not.toHaveBeenCalled() + }) +}) diff --git a/tests/e2e/helpers/electron-process-shutdown.ts b/tests/e2e/helpers/electron-process-shutdown.ts index 5180575f1a6..7a63aaa9c31 100644 --- a/tests/e2e/helpers/electron-process-shutdown.ts +++ b/tests/e2e/helpers/electron-process-shutdown.ts @@ -2,6 +2,7 @@ import type { ChildProcess } from 'node:child_process' import { execFileSync } from 'node:child_process' import { existsSync, readFileSync, readdirSync } from 'node:fs' import path from 'node:path' +import { cleanupE2ECrashpad } from './electron-crashpad-cleanup' import type { ElectronApplication } from '@stablyai/playwright-test' const GRACEFUL_CLOSE_TIMEOUT_MS = 10_000 @@ -221,4 +222,5 @@ export async function cleanupE2EDaemons(userDataDir: string): Promise { for (const pid of readDaemonPidFiles(userDataDir)) { await forceKillPidTree(pid) } + cleanupE2ECrashpad(userDataDir) }