mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
Merge remote-tracking branch 'origin/nwparker/worktree-create-technical' into nwparker/worktree-create-technical
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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')
|
||||
},
|
||||
|
||||
@@ -144,7 +144,6 @@
|
||||
"bench:agent-inspection-cadence": "node config/scripts/agent-inspection-cadence-batching-benchmark.mjs",
|
||||
"bench:renderer-quadratic-scans": "node config/scripts/renderer-quadratic-scan-benchmark.mjs",
|
||||
"bench:session-write-hot-path": "node config/scripts/session-write-hot-path-benchmark.mjs",
|
||||
"bench:terminal-partial-escape-tail": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON config/scripts/terminal-partial-escape-tail-benchmark.mjs",
|
||||
"bench:terminal-partial-escape-tail": "node config/scripts/terminal-partial-escape-tail-benchmark.mjs",
|
||||
"bench:worktree-refresh-churn": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON config/scripts/worktree-refresh-churn-benchmark.mjs",
|
||||
"bench:multi-workspace-typing": "pnpm run ensure:electron-runtime && node config/scripts/run-multi-workspace-typing-bench.mjs",
|
||||
|
||||
@@ -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
|
||||
}))
|
||||
|
||||
|
||||
@@ -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<AgentHookCommandResult> {
|
||||
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<string, CommandHandler> = {
|
||||
})
|
||||
},
|
||||
'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(),
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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[]
|
||||
|
||||
@@ -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])
|
||||
|
||||
@@ -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([])
|
||||
})
|
||||
})
|
||||
@@ -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'] : []
|
||||
}
|
||||
@@ -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.
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -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<void> {
|
||||
for (const pid of readDaemonPidFiles(userDataDir)) {
|
||||
await forceKillPidTree(pid)
|
||||
}
|
||||
cleanupE2ECrashpad(userDataDir)
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
@@ -354,6 +355,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,24 +367,29 @@ 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}`)
|
||||
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 }
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user