diff --git a/.github/workflows/computer-e2e.yml b/.github/workflows/computer-e2e.yml index 29996c70361..e6b16fab7a7 100644 --- a/.github/workflows/computer-e2e.yml +++ b/.github/workflows/computer-e2e.yml @@ -9,6 +9,12 @@ on: - 'config/scripts/build-windows-cli-launcher.mjs' - 'config/scripts/build-windows-cli-launcher.test.mjs' - 'config/scripts/computer-e2e-workflow.test.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-benchmark.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-metrics.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-processes.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs' - 'config/scripts/computer-use-modifier-safety.test.mjs' - 'config/scripts/computer-use-skill-guidance.test.mjs' - 'config/scripts/computer-use-smoke.mjs' @@ -59,6 +65,8 @@ jobs: runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 + with: + persist-credentials: false - uses: actions/setup-node@v6 with: node-version-file: package.json @@ -85,6 +93,8 @@ jobs: config/scripts/build-windows-cli-launcher.test.mjs src/main/ssh/ssh-remote-cli-launcher.test.ts config/scripts/computer-e2e-workflow.test.mjs + config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs + config/scripts/macos-computer-helper-owner-loss-processes.test.mjs config/scripts/computer-use-modifier-safety.test.mjs config/scripts/computer-use-skill-guidance.test.mjs config/scripts/computer-use-smoke.test.mjs @@ -140,6 +150,32 @@ jobs: ORCA_COMPUTER_E2E: '1' run: pnpm test:e2e:computer --reporter=verbose tests/e2e/computer-windows.e2e.ts + mac-native-owner-smoke: + if: github.event_name == 'pull_request' + runs-on: macos-15 + permissions: + contents: read + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - uses: actions/setup-node@v6 + with: + node-version-file: package.json + - uses: pnpm/action-setup@v6 + with: + run_install: false + - run: pnpm install --frozen-lockfile + - name: Owner-loss benchmark process cleanup + run: >- + pnpm vitest run + config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs + config/scripts/macos-computer-helper-owner-loss-processes.test.mjs + - name: Authenticated helper owner-loss smoke + run: pnpm bench:macos-computer-helper-owner-loss --expect reaped --trials 1 + - name: Swift tests and signed universal helper verification + run: pnpm verify:computer-native + mac: # macOS Accessibility and Screen Recording require user-granted TCC entries. # Keep this on manual/scheduled permission-bearing runners instead of PR CI. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 2f0e19980a6..261c8d18adb 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -273,6 +273,9 @@ jobs: - name: Smoke packaged CLI run: node config/scripts/smoke-packaged-cli.mjs --app-dir=dist/linux-unpacked + - name: Smoke packaged hang watchdog worker + run: xvfb-run --auto-servernum node config/scripts/smoke-packaged-hang-watchdog-worker.mjs --app-dir=dist/linux-unpacked + package_windows: name: package (windows) runs-on: windows-2022 diff --git a/build-plugins/plain-node-entry-guard.ts b/build-plugins/plain-node-entry-guard.ts index 6638cca97c5..a7ad171a402 100644 --- a/build-plugins/plain-node-entry-guard.ts +++ b/build-plugins/plain-node-entry-guard.ts @@ -21,7 +21,6 @@ type OutputChunk = Rollup.OutputChunk const PLAIN_NODE_ENTRY_NAMES = [ 'daemon-entry', 'parcel-watcher-process-entry', - 'main-thread-hang-watchdog-entry', 'computer-sidecar', 'agent-hooks/managed-agent-hook-controls', 'codex/codex-app-server-grant-entry' diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index e7a80704c14..da0f690ad80 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -154,7 +154,6 @@ module.exports = { 'out/main/plugin-host-entry.js', 'out/main/computer-sidecar.js', 'out/main/parcel-watcher-process-entry.js', - 'out/main/main-thread-hang-watchdog-entry.js', 'out/main/chunks/**', 'resources/**', 'node_modules/ws/**', diff --git a/config/scripts/computer-e2e-workflow.test.mjs b/config/scripts/computer-e2e-workflow.test.mjs index d6db7c5bbe9..a4230668bc7 100644 --- a/config/scripts/computer-e2e-workflow.test.mjs +++ b/config/scripts/computer-e2e-workflow.test.mjs @@ -47,6 +47,7 @@ describe('computer-use e2e workflow', () => { expect(triggerPaths).toEqual( expect.arrayContaining([ 'config/scripts/computer-e2e-workflow.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', 'config/scripts/computer-use-modifier-safety.test.mjs', 'config/scripts/computer-use-skill-guidance.test.mjs', 'config/scripts/computer-use-smoke.mjs', @@ -70,9 +71,14 @@ describe('computer-use e2e workflow', () => { const nativeSmokeRuns = workflow.jobs['native-smoke'].steps .map((step) => step.run) .filter((run) => typeof run === 'string') + const checkout = workflow.jobs['native-smoke'].steps.find( + (step) => step.uses === 'actions/checkout@v6' + ) const regressionRun = nativeSmokeRuns.find((run) => run.includes('pnpm vitest run')) const expectedRegressionFiles = [ 'config/scripts/computer-e2e-workflow.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs', 'config/scripts/computer-use-modifier-safety.test.mjs', 'config/scripts/computer-use-skill-guidance.test.mjs', 'config/scripts/computer-use-smoke.test.mjs', @@ -106,12 +112,68 @@ describe('computer-use e2e workflow', () => { 'src/shared/remote-runtime-client.test.ts' ] + expect(checkout.with['persist-credentials']).toBe(false) expect(regressionRun).toBeTruthy() for (const file of expectedRegressionFiles) { expect(regressionRun).toContain(file) } }) + it('builds and tests the macOS helper on pull requests without TCC e2e', () => { + const workflow = parse( + readFileSync(join(projectDir, '.github/workflows/computer-e2e.yml'), 'utf8') + ) + const job = workflow.jobs['mac-native-owner-smoke'] + const runs = job.steps.map((step) => step.run).filter((run) => typeof run === 'string') + const checkout = job.steps.find((step) => step.uses === 'actions/checkout@v6') + + expect(job.if).toBe("github.event_name == 'pull_request'") + expect(job['runs-on']).toBe('macos-15') + expect(checkout.with['persist-credentials']).toBe(false) + expect(runs).toContain('pnpm bench:macos-computer-helper-owner-loss --expect reaped --trials 1') + const cleanupRun = runs.find((run) => + run.includes('config/scripts/macos-computer-helper-owner-loss-processes.test.mjs') + ) + expect(cleanupRun).toContain( + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs' + ) + expect(runs).toContain('pnpm verify:computer-native') + expect(runs.join('\n')).not.toContain('test:e2e:computer') + expect(workflow.on.pull_request.paths).toEqual( + expect.arrayContaining([ + 'config/scripts/macos-computer-helper-owner-loss-benchmark.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-metrics.mjs', + 'config/scripts/macos-computer-helper-owner-loss-processes.mjs', + 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs' + ]) + ) + }) + + it('runs deterministic macOS owner-loss benchmark cleanup coverage', () => { + const benchmark = readFileSync( + join(projectDir, 'config/scripts/macos-computer-helper-owner-loss-benchmark.mjs'), + 'utf8' + ) + const cleanup = readFileSync( + join(projectDir, 'config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs'), + 'utf8' + ) + + expect(benchmark).toContain('spawnBenchmarkProcess(executable, [launcherDir]') + expect(benchmark).toContain("stdio: ['ignore', stdoutDescriptor, stderrDescriptor]") + expect(benchmark).toContain('cleanupOwnerLossTrial({') + const parseIndex = benchmark.indexOf('parseBenchmarkTrialResult(serializedResult)') + const cleanupIndex = benchmark.indexOf('cleanupOwnerLossTrial({') + expect(parseIndex).toBeGreaterThanOrEqual(0) + expect(cleanupIndex).toBeGreaterThanOrEqual(0) + expect(parseIndex).toBeLessThan(cleanupIndex) + expect(benchmark).toContain('trialCleanupSha256: artifactSha256(trialCleanupPath)') + expect(cleanup).toContain('killRecordedAndMatchingProcesses(options.recordPath') + expect(cleanup).toContain("signalValidatedProcessGroup(options.pid, options.marker, 'SIGKILL'") + }) + it('boots the built daemon under plain Node in the PR native-smoke job after the main build', () => { const workflow = parse( readFileSync(join(projectDir, '.github/workflows/computer-e2e.yml'), 'utf8') diff --git a/config/scripts/electron-builder-config.test.mjs b/config/scripts/electron-builder-config.test.mjs index 7d761c70af7..1dd0cf78ff3 100644 --- a/config/scripts/electron-builder-config.test.mjs +++ b/config/scripts/electron-builder-config.test.mjs @@ -171,11 +171,9 @@ describe('electron-builder config', () => { ) }) - // Why: the watchdog only arms in packaged builds, and its ELECTRON_RUN_AS_NODE - // fork resolves the entry from app.asar.unpacked — inside the asar it never runs. - it('unpacks the forked main-thread hang-watchdog entry', () => { - expect(electronBuilderConfig.asarUnpack).toEqual( - expect.arrayContaining(['out/main/main-thread-hang-watchdog-entry.js']) + it('keeps the worker-thread hang watchdog inside app.asar', () => { + expect(electronBuilderConfig.asarUnpack).not.toContain( + 'out/main/main-thread-hang-watchdog-entry.js' ) }) diff --git a/config/scripts/hang-watchdog-memory-benchmark.mjs b/config/scripts/hang-watchdog-memory-benchmark.mjs new file mode 100644 index 00000000000..49a2dd63136 --- /dev/null +++ b/config/scripts/hang-watchdog-memory-benchmark.mjs @@ -0,0 +1,443 @@ +#!/usr/bin/env node +import { execFileSync, fork, spawnSync } from 'node:child_process' +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { Worker } from 'node:worker_threads' +import { + childRssBytes, + median, + physicalFootprintBytes, + sampleMemory, + sampleProductionPerformance +} from './hang-watchdog-process-metrics.mjs' + +const INTERNAL_ENV = 'ORCA_HANG_WATCHDOG_BENCH_INTERNAL' +const BOUNDARY_ENV = 'ORCA_HANG_WATCHDOG_BENCH_BOUNDARY' +const RESULT_PREFIX = 'ORCA_HANG_WATCHDOG_BENCH_RESULT=' +const DEFAULT_TRIALS = 7 +const SETTLE_MS = 2_000 +const SAMPLE_COUNT = 5 +const SAMPLE_INTERVAL_MS = 200 +const VERIFY_TIMEOUT_MS = 500 +const VERIFY_CHECK_INTERVAL_MS = 50 +const VERIFY_BLOCK_MS = 1_200 +const PRODUCTION_HEARTBEAT_INTERVAL_MS = 2_000 +const PRODUCTION_TIMEOUT_MS = 45_000 +const PRODUCTION_CHECK_INTERVAL_MS = 5_000 +const PRODUCTION_SAMPLE_MS = 30_000 +const MAX_LAUNCH_ATTEMPTS = 3 +const MIB = 1024 * 1024 +const scriptPath = import.meta.filename +const repoRoot = path.resolve(import.meta.dirname, '..', '..') +const entryPath = path.join(repoRoot, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +function forceGc() { + if (typeof global.gc !== 'function') { + throw new Error('Electron did not expose GC; keep --js-flags=--expose-gc in the harness') + } + global.gc() + global.gc() +} + +function blockMainThread(ms) { + const startedAt = Date.now() + while (Date.now() - startedAt < ms) { + // Intentional synchronous stall. + } + return { startedAt, endedAt: Date.now() } +} + +function readMarker(markerPath) { + try { + return JSON.parse(readFileSync(markerPath, 'utf8')) + } catch { + return null + } +} + +async function verifyBlockedMainDetection(markerPath, sendHeartbeat) { + const block = blockMainThread(VERIFY_BLOCK_MS) + const detected = readMarker(markerPath) + sendHeartbeat() + const deadline = Date.now() + VERIFY_TIMEOUT_MS + let resolved + do { + resolved = readMarker(markerPath) + if (resolved?.selfRecovered === true) { + break + } + await sleep(VERIFY_CHECK_INTERVAL_MS) + } while (Date.now() < deadline) + const verified = + detected?.detectedAt >= block.startedAt && + detected.detectedAt <= block.endedAt && + detected.selfRecovered === false && + resolved?.selfRecovered === true + if (!verified) { + throw new Error( + `Built watchdog failed blocked-main verification: ${JSON.stringify({ detected, resolved })}` + ) + } + return true +} + +function startChild(markerPath, timeoutMs, checkIntervalMs) { + const startedAt = process.hrtime.bigint() + const child = fork(entryPath, [], { + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + env: { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + ORCA_HANG_WATCHDOG_PARENT_PID: String(process.pid), + ORCA_HANG_WATCHDOG_MARKER_PATH: markerPath, + ORCA_HANG_WATCHDOG_TIMEOUT_MS: String(timeoutMs), + ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS: String(checkIntervalMs) + } + }) + const startupMs = Number(process.hrtime.bigint() - startedAt) / 1e6 + return { + pids: [process.pid, child.pid], + startupMs, + sendHeartbeat: () => child.send?.({ type: 'heartbeat' }), + shutdown: async () => { + if (child.exitCode !== null) { + return + } + const exitPromise = new Promise((resolve) => child.once('exit', resolve)) + child.send?.({ type: 'shutdown' }) + if (child.connected) { + child.disconnect() + } + await exitPromise + } + } +} + +function startWorker(markerPath, timeoutMs, checkIntervalMs) { + const startedAt = process.hrtime.bigint() + const worker = new Worker(entryPath, { + workerData: { + parentPid: process.pid, + markerPath, + timeoutMs, + checkIntervalMs + } + }) + const startupMs = Number(process.hrtime.bigint() - startedAt) / 1e6 + return { + pids: [process.pid], + startupMs, + sendHeartbeat: () => worker.postMessage({ type: 'heartbeat' }), + shutdown: async () => { + if (worker.threadId === -1) { + return + } + const exitPromise = new Promise((resolve) => worker.once('exit', resolve)) + worker.postMessage({ type: 'shutdown' }) + await exitPromise + } + } +} + +async function verifyBoundary(markerPath, startBoundary) { + const boundary = startBoundary(markerPath, VERIFY_TIMEOUT_MS, VERIFY_CHECK_INTERVAL_MS) + const heartbeat = setInterval(boundary.sendHeartbeat, 100) + try { + await sleep(SETTLE_MS) + return await verifyBlockedMainDetection(markerPath, boundary.sendHeartbeat) + } finally { + clearInterval(heartbeat) + await boundary.shutdown() + } +} + +async function measureChild(markerPath) { + forceGc() + await sleep(SETTLE_MS) + forceGc() + const before = await sampleMemory( + () => process.memoryUsage().rss, + () => physicalFootprintBytes([process.pid]), + { sampleCount: SAMPLE_COUNT, sampleIntervalMs: SAMPLE_INTERVAL_MS, sleep } + ) + const child = startChild(markerPath, PRODUCTION_TIMEOUT_MS, PRODUCTION_CHECK_INTERVAL_MS) + let measurements + try { + await sleep(SETTLE_MS) + forceGc() + const childRss = await sampleMemory( + () => childRssBytes(child.pids[1]), + () => physicalFootprintBytes([child.pids[1]]), + { sampleCount: SAMPLE_COUNT, sampleIntervalMs: SAMPLE_INTERVAL_MS, sleep } + ) + const total = await sampleMemory( + () => process.memoryUsage().rss + childRssBytes(child.pids[1]), + () => physicalFootprintBytes(child.pids), + { sampleCount: SAMPLE_COUNT, sampleIntervalMs: SAMPLE_INTERVAL_MS, sleep } + ) + const performance = await sampleProductionPerformance(child, { + heartbeatIntervalMs: PRODUCTION_HEARTBEAT_INTERVAL_MS, + sampleMs: PRODUCTION_SAMPLE_MS, + sleep + }) + measurements = { + rssBytes: childRss.rssBytes, + summedProcessRssDeltaBytes: Math.max(0, total.rssBytes - before.rssBytes), + physicalFootprintDeltaBytes: Math.max( + 0, + total.physicalFootprintBytes - before.physicalFootprintBytes + ), + startupMs: child.startupMs, + ...performance + } + } finally { + await child.shutdown() + } + rmSync(markerPath, { force: true }) + return { + ...measurements, + blockedMainThreadVerified: await verifyBoundary(markerPath, startChild) + } +} + +async function measureWorker(markerPath) { + forceGc() + await sleep(SETTLE_MS) + forceGc() + const before = await sampleMemory( + () => process.memoryUsage().rss, + () => physicalFootprintBytes([process.pid]), + { sampleCount: SAMPLE_COUNT, sampleIntervalMs: SAMPLE_INTERVAL_MS, sleep } + ) + const worker = startWorker(markerPath, PRODUCTION_TIMEOUT_MS, PRODUCTION_CHECK_INTERVAL_MS) + let measurements + try { + await sleep(SETTLE_MS) + forceGc() + const after = await sampleMemory( + () => process.memoryUsage().rss, + () => physicalFootprintBytes([process.pid]), + { sampleCount: SAMPLE_COUNT, sampleIntervalMs: SAMPLE_INTERVAL_MS, sleep } + ) + const performance = await sampleProductionPerformance(worker, { + heartbeatIntervalMs: PRODUCTION_HEARTBEAT_INTERVAL_MS, + sampleMs: PRODUCTION_SAMPLE_MS, + sleep + }) + const rssBytes = Math.max(0, after.rssBytes - before.rssBytes) + measurements = { + rssBytes, + summedProcessRssDeltaBytes: rssBytes, + physicalFootprintDeltaBytes: Math.max( + 0, + after.physicalFootprintBytes - before.physicalFootprintBytes + ), + startupMs: worker.startupMs, + ...performance + } + } finally { + await worker.shutdown() + } + rmSync(markerPath, { force: true }) + return { + ...measurements, + blockedMainThreadVerified: await verifyBoundary(markerPath, startWorker) + } +} + +async function runInternal() { + if (process.platform !== 'darwin') { + throw new Error('The production watchdog is macOS-only; run this benchmark on macOS') + } + const { app } = await import('electron') + const boundary = process.env[BOUNDARY_ENV] + const profileDir = mkdtempSync(path.join(tmpdir(), 'orca-watchdog-bench-')) + app.setPath('userData', profileDir) + try { + await app.whenReady() + const markerPath = path.join(profileDir, 'main-thread-hang.json') + const result = + boundary === 'child' + ? await measureChild(markerPath) + : boundary === 'worker' + ? await measureWorker(markerPath) + : (() => { + throw new Error(`Unsupported boundary: ${boundary}`) + })() + process.stdout.write(`${RESULT_PREFIX}${JSON.stringify(result)}\n`) + } finally { + app.quit() + rmSync(profileDir, { recursive: true, force: true }) + } +} + +function parseArgs(argv) { + const options = { boundary: '', trials: DEFAULT_TRIALS, output: '' } + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index] + const value = argv[index + 1] + if (arg === '--boundary' || arg === '--trials' || arg === '--output') { + if (!value) { + throw new Error(`Missing value for ${arg}`) + } + options[arg.slice(2)] = arg === '--trials' ? Number(value) : value + index += 1 + } else { + throw new Error(`Unknown argument: ${arg}`) + } + } + if (!['child', 'worker'].includes(options.boundary)) { + throw new Error('--boundary must be child or worker') + } + if (!Number.isInteger(options.trials) || options.trials < 1) { + throw new Error('--trials must be a positive integer') + } + return options +} + +function electronPath() { + const requirePath = import.meta.resolve('electron') + const electronModulePath = fileURLToPath(requirePath) + return execFileSync( + process.execPath, + ['-e', `process.stdout.write(require(${JSON.stringify(electronModulePath)}))`], + { + encoding: 'utf8' + } + ) +} + +function runTrial(executable, boundary) { + for (let attempt = 1; attempt <= MAX_LAUNCH_ATTEMPTS; attempt += 1) { + const env = { ...process.env, [INTERNAL_ENV]: '1', [BOUNDARY_ENV]: boundary } + delete env.ELECTRON_RUN_AS_NODE + const launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-watchdog-bench-launcher-')) + writeFileSync( + path.join(launcherDir, 'package.json'), + JSON.stringify({ name: 'orca-watchdog-benchmark', main: 'main.cjs' }) + ) + writeFileSync( + path.join(launcherDir, 'main.cjs'), + `import(${JSON.stringify(pathToFileURL(scriptPath).href)}).catch((error) => { + console.error(error) + process.exitCode = 1 +})\n` + ) + let result + try { + result = spawnSync(executable, ['--js-flags=--expose-gc', launcherDir], { + cwd: repoRoot, + env, + encoding: 'utf8', + timeout: 90_000 + }) + } finally { + rmSync(launcherDir, { recursive: true, force: true }) + } + if (result.status !== 0) { + throw new Error( + `Electron trial failed (${result.error?.message ?? result.signal ?? result.status}):\n` + + `${result.stderr || result.stdout}` + ) + } + const line = result.stdout.split('\n').find((candidate) => candidate.startsWith(RESULT_PREFIX)) + if (line) { + return { ...JSON.parse(line.slice(RESULT_PREFIX.length)), launchAttempts: attempt } + } + if (attempt === MAX_LAUNCH_ATTEMPTS || result.stderr || result.stdout) { + throw new Error(`Electron trial did not report a result (status ${result.status})`) + } + } + throw new Error('Electron trial exhausted launcher attempts') +} + +function runBenchmark() { + if (process.platform !== 'darwin') { + throw new Error('The production watchdog is macOS-only; run this benchmark on macOS') + } + if (!existsSync(entryPath)) { + throw new Error(`Missing ${entryPath}; run pnpm exec electron-vite build first`) + } + const options = parseArgs(process.argv.slice(2)) + const builtEntry = readFileSync(entryPath, 'utf8') + const hasChildContract = builtEntry.includes('ORCA_HANG_WATCHDOG_PARENT_PID') + const hasWorkerContract = builtEntry.includes('workerData') && builtEntry.includes('parentPort') + if ( + (options.boundary === 'child' && !hasChildContract) || + (options.boundary === 'worker' && !hasWorkerContract) + ) { + throw new Error( + `Built watchdog does not implement the requested ${options.boundary} boundary; rebuild the matching revision` + ) + } + const executable = electronPath() + const results = Array.from({ length: options.trials }, () => + runTrial(executable, options.boundary) + ) + const rssBytes = results.map((result) => result.rssBytes) + const summedProcessRssDeltaBytes = results.map((result) => result.summedProcessRssDeltaBytes) + const physicalFootprintDeltaBytes = results.map((result) => result.physicalFootprintDeltaBytes) + const startupMs = results.map((result) => result.startupMs) + const cpuMs = results.map((result) => result.cpuMs) + const eventLoopDelayP95Ms = results.map((result) => result.eventLoopDelayP95Ms) + const eventLoopDelayP99Ms = results.map((result) => result.eventLoopDelayP99Ms) + const eventLoopDelayMaxMs = results.map((result) => result.eventLoopDelayMaxMs) + const report = { + benchmark: 'hang-watchdog-memory', + boundary: options.boundary, + revision: execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: repoRoot, + encoding: 'utf8' + }).trim(), + electron: execFileSync(executable, ['-e', 'process.stdout.write(process.versions.electron)'], { + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + encoding: 'utf8' + }).trim(), + settleMs: SETTLE_MS, + samplesPerTrial: SAMPLE_COUNT, + productionHeartbeatIntervalMs: PRODUCTION_HEARTBEAT_INTERVAL_MS, + productionCheckIntervalMs: PRODUCTION_CHECK_INTERVAL_MS, + productionSampleMs: PRODUCTION_SAMPLE_MS, + trials: options.trials, + rssMiB: rssBytes.map((value) => Number((value / MIB).toFixed(2))), + medianRssMiB: Number((median(rssBytes) / MIB).toFixed(2)), + summedProcessRssDeltaMiB: summedProcessRssDeltaBytes.map((value) => + Number((value / MIB).toFixed(2)) + ), + medianSummedProcessRssDeltaMiB: Number((median(summedProcessRssDeltaBytes) / MIB).toFixed(2)), + physicalFootprintDeltaMiB: physicalFootprintDeltaBytes.map((value) => + Number((value / MIB).toFixed(2)) + ), + medianPhysicalFootprintDeltaMiB: Number((median(physicalFootprintDeltaBytes) / MIB).toFixed(2)), + startupMs: startupMs.map((value) => Number(value.toFixed(3))), + medianStartupMs: Number(median(startupMs).toFixed(3)), + cpuMs: cpuMs.map((value) => Number(value.toFixed(2))), + medianCpuMs: Number(median(cpuMs).toFixed(2)), + eventLoopDelayP95Ms: eventLoopDelayP95Ms.map((value) => Number(value.toFixed(3))), + medianEventLoopDelayP95Ms: Number(median(eventLoopDelayP95Ms).toFixed(3)), + eventLoopDelayP99Ms: eventLoopDelayP99Ms.map((value) => Number(value.toFixed(3))), + medianEventLoopDelayP99Ms: Number(median(eventLoopDelayP99Ms).toFixed(3)), + eventLoopDelayMaxMs: eventLoopDelayMaxMs.map((value) => Number(value.toFixed(3))), + medianEventLoopDelayMaxMs: Number(median(eventLoopDelayMaxMs).toFixed(3)), + heartbeatCounts: results.map((result) => result.heartbeatCount), + launchAttempts: results.map((result) => result.launchAttempts), + blockedMainThreadVerified: results.every((result) => result.blockedMainThreadVerified) + } + const serialized = `${JSON.stringify(report, null, 2)}\n` + process.stdout.write(serialized) + if (options.output) { + writeFileSync(path.resolve(options.output), serialized) + } +} + +if (process.env[INTERNAL_ENV] === '1') { + await runInternal() +} else { + runBenchmark() +} diff --git a/config/scripts/hang-watchdog-process-metrics.mjs b/config/scripts/hang-watchdog-process-metrics.mjs new file mode 100644 index 00000000000..7e5ee4de53f --- /dev/null +++ b/config/scripts/hang-watchdog-process-metrics.mjs @@ -0,0 +1,108 @@ +import { execFileSync } from 'node:child_process' +import { monitorEventLoopDelay } from 'node:perf_hooks' + +export function median(values) { + const sorted = [...values].sort((left, right) => left - right) + const middle = Math.floor(sorted.length / 2) + return sorted.length % 2 === 0 ? (sorted[middle - 1] + sorted[middle]) / 2 : sorted[middle] +} + +export async function sampleMemory(readRss, readPhysicalFootprint, options) { + const rssSamples = [] + const physicalFootprintSamples = [] + for (let index = 0; index < options.sampleCount; index += 1) { + rssSamples.push(readRss()) + physicalFootprintSamples.push(readPhysicalFootprint()) + await options.sleep(options.sampleIntervalMs) + } + return { + rssBytes: median(rssSamples), + physicalFootprintBytes: median(physicalFootprintSamples) + } +} + +export function childRssBytes(pid) { + const raw = execFileSync('ps', ['-o', 'rss=', '-p', String(pid)], { + encoding: 'utf8' + }).trim() + const rssKiB = Number(raw) + if (!Number.isFinite(rssKiB) || rssKiB <= 0) { + throw new Error(`Could not read watchdog child RSS for PID ${pid}`) + } + return rssKiB * 1024 +} + +export function parsePhysicalFootprintBytes(output, processCount) { + const match = + processCount > 1 + ? output.match(/^Summary Footprint:\s+(\d+) B$/m) + : output.match(/^[^\s].*\sFootprint:\s+(\d+) B/m) + const bytes = Number(match?.[1]) + return Number.isFinite(bytes) && bytes > 0 ? bytes : null +} + +export function physicalFootprintBytes(pids) { + const pidArgs = pids.flatMap((pid) => ['--pid', String(pid)]) + const output = execFileSync( + '/usr/bin/footprint', + [...pidArgs, '--format', 'bytes', '--noCategories'], + { encoding: 'utf8' } + ) + const bytes = parsePhysicalFootprintBytes(output, pids.length) + if (bytes === null) { + throw new Error(`Could not read physical footprint for PIDs ${pids.join(', ')}`) + } + return bytes +} + +export function parseProcessCpuTimeMs(raw) { + if (!raw.trim()) { + return null + } + const parts = raw.split(':').map(Number) + if (!parts.length || parts.some((part) => !Number.isFinite(part))) { + return null + } + const seconds = parts.reduce((total, part) => total * 60 + part, 0) + const milliseconds = seconds * 1_000 + return milliseconds >= 0 ? milliseconds : null +} + +function processCpuTimeMs(pid) { + const raw = execFileSync('ps', ['-o', 'time=', '-p', String(pid)], { + encoding: 'utf8' + }).trim() + const milliseconds = parseProcessCpuTimeMs(raw) + if (milliseconds === null) { + throw new Error(`Could not read CPU time for PID ${pid}`) + } + return milliseconds +} + +function combinedCpuTimeMs(pids) { + return pids.reduce((total, pid) => total + processCpuTimeMs(pid), 0) +} + +export async function sampleProductionPerformance(boundary, options) { + const loopDelay = monitorEventLoopDelay({ resolution: 10 }) + let heartbeatCount = 0 + const heartbeat = setInterval(() => { + heartbeatCount += 1 + boundary.sendHeartbeat() + }, options.heartbeatIntervalMs) + const cpuBefore = combinedCpuTimeMs(boundary.pids) + loopDelay.enable() + try { + await options.sleep(options.sampleMs) + } finally { + loopDelay.disable() + clearInterval(heartbeat) + } + return { + cpuMs: Math.max(0, combinedCpuTimeMs(boundary.pids) - cpuBefore), + heartbeatCount, + eventLoopDelayP95Ms: loopDelay.percentile(95) / 1e6, + eventLoopDelayP99Ms: loopDelay.percentile(99) / 1e6, + eventLoopDelayMaxMs: loopDelay.max / 1e6 + } +} diff --git a/config/scripts/hang-watchdog-process-metrics.test.mjs b/config/scripts/hang-watchdog-process-metrics.test.mjs new file mode 100644 index 00000000000..2d54e691d00 --- /dev/null +++ b/config/scripts/hang-watchdog-process-metrics.test.mjs @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { + parsePhysicalFootprintBytes, + parseProcessCpuTimeMs +} from './hang-watchdog-process-metrics.mjs' + +describe('hang watchdog process metrics', () => { + it('uses the de-duplicated summary for multiple processes', () => { + const output = ` +Electron [101]: 64-bit Footprint: 5000000 B (16384 bytes per page) + phys_footprint: 5100000 B +Electron Helper [102]: 64-bit Footprint: 2000000 B (16384 bytes per page) + phys_footprint: 2100000 B +Summary Footprint: 6259264 B +` + expect(parsePhysicalFootprintBytes(output, 2)).toBe(6_259_264) + }) + + it('uses the process footprint rather than auxiliary accounting for one process', () => { + const output = ` +Electron [101]: 64-bit Footprint: 5000000 B (16384 bytes per page) + phys_footprint: 5100000 B +` + expect(parsePhysicalFootprintBytes(output, 1)).toBe(5_000_000) + }) + + it('rejects missing or zero footprint summaries', () => { + expect(parsePhysicalFootprintBytes('phys_footprint: 100 B', 2)).toBeNull() + expect(parsePhysicalFootprintBytes('Summary Footprint: 0 B', 2)).toBeNull() + }) + + it.each([ + ['0:00.04', 40], + ['1:02.50', 62_500], + ['2:01:02.50', 7_262_500] + ])('parses ps CPU time %s', (value, expected) => { + expect(parseProcessCpuTimeMs(value)).toBe(expected) + }) + + it('rejects invalid CPU times', () => { + expect(parseProcessCpuTimeMs('')).toBeNull() + expect(parseProcessCpuTimeMs('not-a-time')).toBeNull() + expect(parseProcessCpuTimeMs('-1:00')).toBeNull() + }) +}) diff --git a/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs b/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs new file mode 100644 index 00000000000..8724cf8faf4 --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs @@ -0,0 +1,614 @@ +#!/usr/bin/env node +import { createHash } from 'node:crypto' +import { execFileSync, fork } from 'node:child_process' +import { existsSync, mkdtempSync, openSync, readFileSync, writeFileSync } from 'node:fs' +import net from 'node:net' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { + median, + percentile, + processSnapshot, + sampleProcess +} from './macos-computer-helper-owner-loss-metrics.mjs' +import { + benchmarkTrialNeedsCleanup, + parseBenchmarkTrialResult, + processIdentityIsCurrent, + signalProcessIdentity, + spawnBenchmarkProcess, + throwBenchmarkTrialFailures, + writeProcessRecord +} from './macos-computer-helper-owner-loss-processes.mjs' +import { cleanupOwnerLossTrial } from './macos-computer-helper-owner-loss-trial-cleanup.mjs' + +const INTERNAL_ENV = 'ORCA_COMPUTER_HELPER_OWNER_BENCH_INTERNAL' +const EXPECTATION_ENV = 'ORCA_COMPUTER_HELPER_OWNER_BENCH_EXPECTATION' +const HELPER_RECORD_PATH_ENV = 'ORCA_COMPUTER_HELPER_OWNER_BENCH_HELPER_RECORD_PATH' +const RESULT_PATH_ENV = 'ORCA_COMPUTER_HELPER_OWNER_BENCH_RESULT_PATH' +const ACTIVE_REQUEST_COUNT = 100_000 +const DEFAULT_TRIALS = 3 +const OWNER_HOLD_MS = 31_000 +const PROCESS_EXIT_TIMEOUT_MS = 5_000 +const RETAIN_PROOF_MS = 3_000 +const TRIAL_TIMEOUT_MS = OWNER_HOLD_MS + 4 * PROCESS_EXIT_TIMEOUT_MS + 120_000 +const MIB = 1024 * 1024 +const scriptPath = import.meta.filename +const repoRoot = path.resolve(import.meta.dirname, '..', '..') +const metricsPath = path.join(import.meta.dirname, 'macos-computer-helper-owner-loss-metrics.mjs') +const processCleanupPath = path.join( + import.meta.dirname, + 'macos-computer-helper-owner-loss-processes.mjs' +) +const trialCleanupPath = path.join( + import.meta.dirname, + 'macos-computer-helper-owner-loss-trial-cleanup.mjs' +) +const sidecarPath = path.join(repoRoot, 'out', 'main', 'computer-sidecar.js') +const helperAppPath = path.join( + repoRoot, + 'native', + 'computer-use-macos', + '.build', + 'release', + 'Orca Computer Use.app' +) +const helperPath = path.join(helperAppPath, 'Contents', 'MacOS', 'orca-computer-use-macos') + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +function isProcessAlive(pid) { + try { + process.kill(pid, 0) + return true + } catch { + return false + } +} + +async function waitForProcessExit(identity, timeoutMs) { + const startedAt = performance.now() + while (performance.now() - startedAt < timeoutMs) { + if (!processIdentityIsCurrent(identity)) { + return performance.now() - startedAt + } + await sleep(50) + } + return null +} + +async function stopProcess(identity) { + if (!identity || !processIdentityIsCurrent(identity)) { + return + } + signalProcessIdentity(identity, helperPath, 'SIGTERM') + if ((await waitForProcessExit(identity, 2_000)) !== null) { + return + } + signalProcessIdentity(identity, helperPath, 'SIGKILL') + await waitForProcessExit(identity, 2_000) +} + +function startSidecar() { + const errors = [] + const child = fork(sidecarPath, [], { + stdio: ['ignore', 'pipe', 'pipe', 'ipc'], + env: { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + ORCA_COMPUTER_SIDECAR: '1', + ORCA_COMPUTER_MACOS_HELPER_APP_PATH: helperAppPath + } + }) + child.on('error', (error) => errors.push(error.stack ?? error.message)) + child.stderr?.on('data', (chunk) => errors.push(String(chunk))) + return { child, errors } +} + +function requestSidecar(sidecar, id, method) { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + cleanup() + reject(new Error(`Sidecar ${method} request timed out: ${sidecar.errors.join('')}`)) + }, 10_000) + const onMessage = (message) => { + if (message?.id !== id) { + return + } + cleanup() + if (message.ok) { + resolve(message.result) + } else { + reject(new Error(`Sidecar ${method} failed: ${JSON.stringify(message.error)}`)) + } + } + const onExit = (code, signal) => { + cleanup() + reject( + new Error( + `Sidecar exited during ${method}: ${JSON.stringify({ code, signal, stderr: sidecar.errors.join('') })}` + ) + ) + } + const onError = (error) => { + cleanup() + reject(new Error(`Sidecar ${method} process error: ${error.message}`)) + } + const cleanup = () => { + clearTimeout(timeout) + sidecar.child.off('message', onMessage) + sidecar.child.off('exit', onExit) + sidecar.child.off('error', onError) + } + sidecar.child.on('message', onMessage) + sidecar.child.once('exit', onExit) + sidecar.child.once('error', onError) + try { + sidecar.child.send({ id, method, params: {} }) + } catch (error) { + onError(error) + } + }) +} + +async function waitForHelper(sidecarPid) { + const deadline = Date.now() + 10_000 + while (Date.now() < deadline) { + const output = execFileSync('ps', ['-axo', 'pid=,ppid=,pgid=,command='], { + encoding: 'utf8', + maxBuffer: 20 * 1024 * 1024 + }) + for (const line of output.split('\n')) { + const match = line.trim().match(/^(\d+)\s+(\d+)\s+(\d+)\s+(.+)$/) + if ( + match && + Number(match[2]) === sidecarPid && + Number(match[3]) === Number(match[1]) && + match[4].includes(helperPath) && + match[4].includes(' --agent ') + ) { + return { pid: Number(match[1]), pgid: Number(match[3]), command: match[4] } + } + } + await sleep(50) + } + throw new Error(`Could not find helper owned by sidecar ${sidecarPid}`) +} + +function socketPathFromCommand(command) { + const match = command.match(/ --agent (.+?) --token-file /) + if (!match) { + throw new Error(`Could not read helper socket path from command: ${command}`) + } + return match[1] +} + +function connectInvalidPeer(socketPath) { + return new Promise((resolve, reject) => { + const socket = net.createConnection(socketPath) + let accepted = false + const timeout = setTimeout(() => { + socket.destroy() + reject(new Error('Invalid-peer connection timed out')) + }, 5_000) + let buffer = '' + const cleanup = () => { + clearTimeout(timeout) + socket.off('error', onError) + socket.off('data', onData) + } + const onError = (error) => { + if (accepted) { + return + } + cleanup() + reject(error) + } + const onData = (chunk) => { + buffer += chunk + const newline = buffer.indexOf('\n') + if (newline < 0) { + return + } + let response + try { + response = JSON.parse(buffer.slice(0, newline)) + } catch (error) { + cleanup() + socket.destroy() + reject(error) + return + } + if (response.ok !== false || response.error?.code !== 'permission_denied') { + cleanup() + socket.destroy() + reject(new Error(`Invalid peer was not rejected: ${JSON.stringify(response)}`)) + return + } + clearTimeout(timeout) + socket.off('data', onData) + accepted = true + resolve(socket) + } + socket.setEncoding('utf8') + socket.on('error', onError) + socket.on('data', onData) + socket.once('connect', () => { + socket.write( + `${JSON.stringify({ id: 991, method: 'handshake', params: {}, token: 'invalid' })}\n` + ) + }) + }) +} + +async function waitForChildExit(child, timeoutMs) { + if (child.exitCode !== null || child.signalCode !== null) { + return true + } + return await Promise.race([ + new Promise((resolve) => child.once('exit', () => resolve(true))), + sleep(timeoutMs).then(() => false) + ]) +} + +async function startAuthenticatedSession() { + const sidecar = startSidecar() + let helper + try { + const capabilitiesResult = requestSidecar(sidecar, 1, 'capabilities').then( + (capabilities) => ({ capabilities }), + (error) => ({ error }) + ) + helper = await waitForHelper(sidecar.child.pid) + const helperRecordPath = process.env[HELPER_RECORD_PATH_ENV] + if (!helperRecordPath) { + throw new Error('Missing helper process record path') + } + writeProcessRecord(helperRecordPath, helper) + const { capabilities, error } = await capabilitiesResult + if (error) { + throw error + } + if (capabilities?.protocolVersion !== 1) { + throw new Error(`Unexpected helper handshake: ${JSON.stringify(capabilities)}`) + } + return { authenticated: capabilities.protocolVersion === 1, sidecar, helper } + } catch (error) { + sidecar.child.kill('SIGKILL') + await stopProcess(helper) + throw error + } +} + +async function exerciseActiveRequests(sidecar) { + const latencies = [] + const startedAt = performance.now() + for (let index = 0; index < ACTIVE_REQUEST_COUNT; index += 1) { + const requestStartedAt = performance.now() + const result = await requestSidecar(sidecar, 10_000 + index, 'listApps') + if (!Array.isArray(result?.apps)) { + throw new Error(`Unexpected listApps response: ${JSON.stringify(result)}`) + } + latencies.push(performance.now() - requestStartedAt) + } + const totalMs = performance.now() - startedAt + return { + totalMs, + requestsPerSecond: (ACTIVE_REQUEST_COUNT * 1_000) / totalMs, + medianLatencyMs: median(latencies), + p95LatencyMs: percentile(latencies, 0.95), + maxLatencyMs: Math.max(...latencies) + } +} + +async function verifyGracefulClose() { + const { sidecar, helper } = await startAuthenticatedSession() + try { + const startedAt = performance.now() + sidecar.child.disconnect() + if (!(await waitForChildExit(sidecar.child, PROCESS_EXIT_TIMEOUT_MS))) { + throw new Error('Sidecar did not exit after graceful IPC close') + } + const helperExitMs = await waitForProcessExit(helper, PROCESS_EXIT_TIMEOUT_MS) + if (helperExitMs === null) { + throw new Error('Helper did not exit after graceful owner close') + } + return Math.round(performance.now() - startedAt) + } finally { + sidecar.child.kill('SIGKILL') + await stopProcess(helper) + } +} + +async function runInternalTrial(expectation) { + let sidecar + let helper + let invalidPeer + let invalidPeerRejected = false + try { + const session = await startAuthenticatedSession() + sidecar = session.sidecar + helper = session.helper + const authenticatedAt = performance.now() + const initial = await sampleProcess(helper.pid) + const activeRequests = await exerciseActiveRequests(sidecar) + const remainingHoldMs = Math.max(0, OWNER_HOLD_MS - (performance.now() - authenticatedAt)) + await sleep(remainingHoldMs) + const connected = await sampleProcess(helper.pid) + const invalidSocketPath = socketPathFromCommand(helper.command) + invalidPeer = await connectInvalidPeer(invalidSocketPath) + invalidPeerRejected = true + const survivedClaimDeadline = + performance.now() - authenticatedAt >= OWNER_HOLD_MS && isProcessAlive(helper.pid) + + sidecar.child.kill('SIGKILL') + await waitForChildExit(sidecar.child, PROCESS_EXIT_TIMEOUT_MS) + const abruptExitMs = await waitForProcessExit( + helper, + expectation === 'reaped' ? PROCESS_EXIT_TIMEOUT_MS : RETAIN_PROOF_MS + ) + const helperExitedAfterAbruptLoss = abruptExitMs !== null + if (expectation === 'reaped' && !helperExitedAfterAbruptLoss) { + throw new Error('Expected helper to exit after abrupt authenticated owner loss') + } + if (expectation === 'retained' && helperExitedAfterAbruptLoss) { + throw new Error('Expected baseline helper to remain after abrupt owner loss') + } + const postLossRssBytes = helperExitedAfterAbruptLoss ? 0 : processSnapshot(helper.pid).rssBytes + await stopProcess(helper) + helper = null + invalidPeer.destroy() + invalidPeer = null + + const gracefulExitMs = await verifyGracefulClose() + return { + authenticated: session.authenticated, + survivedClaimDeadline, + invalidPeerRejectedAndDidNotRetain: invalidPeerRejected && helperExitedAfterAbruptLoss, + connectedRssBytes: connected.rssBytes, + connectedCpuMilliseconds: Math.max( + 0, + Math.round((connected.cpuTimeSeconds - initial.cpuTimeSeconds) * 1_000) + ), + activeRequests, + cpuSampleMs: Math.round(performance.now() - authenticatedAt), + helperExitedAfterAbruptLoss, + abruptExitMs: abruptExitMs === null ? null : Math.round(abruptExitMs), + postLossRssBytes, + gracefulExitMs + } + } finally { + invalidPeer?.destroy() + sidecar?.child.kill('SIGKILL') + await stopProcess(helper) + } +} + +function parseArgs(argv) { + const options = { expect: '', trials: DEFAULT_TRIALS, output: '' } + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index] + const value = argv[index + 1] + if (arg === '--expect' || arg === '--trials' || arg === '--output') { + if (!value) { + throw new Error(`Missing value for ${arg}`) + } + options[arg.slice(2)] = arg === '--trials' ? Number(value) : value + index += 1 + } else { + throw new Error(`Unknown argument: ${arg}`) + } + } + if (!['retained', 'reaped'].includes(options.expect)) { + throw new Error('--expect must be retained or reaped') + } + if (!Number.isInteger(options.trials) || options.trials < 1) { + throw new Error('--trials must be a positive integer') + } + return options +} + +function electronPath() { + const electronModulePath = fileURLToPath(import.meta.resolve('electron')) + return execFileSync( + process.execPath, + ['-e', `process.stdout.write(require(${JSON.stringify(electronModulePath)}))`], + { encoding: 'utf8' } + ) +} + +function buildArtifacts() { + execFileSync('pnpm', ['exec', 'electron-vite', 'build'], { + cwd: repoRoot, + stdio: 'inherit' + }) + execFileSync('pnpm', ['build:computer-macos'], { + cwd: repoRoot, + stdio: 'inherit' + }) +} + +function artifactSha256(artifactPath) { + return createHash('sha256').update(readFileSync(artifactPath)).digest('hex') +} + +function runTrial(executable, expectation) { + let launcherDir + let trialTempDir + let helperRecordPath + let resultPath + let stderrPath + let stdoutPath + let result + let serializedResult + let parsedResult + let parsedResultAvailable = false + let trialError + let cleanupError + let stderrDescriptor + let stdoutDescriptor + let trialOutput = '' + try { + launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-helper-owner-bench-launcher-')) + trialTempDir = mkdtempSync(path.join(path.sep, 'tmp', 'orca-owner-bench-')) + helperRecordPath = path.join(launcherDir, 'helper.json') + resultPath = path.join(launcherDir, 'result.json') + stderrPath = path.join(launcherDir, 'stderr.log') + stdoutPath = path.join(launcherDir, 'stdout.log') + writeFileSync( + path.join(launcherDir, 'package.json'), + JSON.stringify({ name: 'orca-helper-owner-benchmark', main: 'main.cjs' }) + ) + writeFileSync( + path.join(launcherDir, 'main.cjs'), + `import(${JSON.stringify(pathToFileURL(scriptPath).href)}).catch((error) => { + console.error(error) + process.exitCode = 1 +})\n` + ) + const env = { + ...process.env, + TMPDIR: trialTempDir, + [INTERNAL_ENV]: '1', + [EXPECTATION_ENV]: expectation, + [HELPER_RECORD_PATH_ENV]: helperRecordPath, + [RESULT_PATH_ENV]: resultPath + } + delete env.ELECTRON_RUN_AS_NODE + stderrDescriptor = openSync(stderrPath, 'w') + stdoutDescriptor = openSync(stdoutPath, 'w') + result = spawnBenchmarkProcess(executable, [launcherDir], { + cwd: repoRoot, + env, + stdio: ['ignore', stdoutDescriptor, stderrDescriptor], + timeout: TRIAL_TIMEOUT_MS + }) + if (result.status === 0 && existsSync(resultPath)) { + serializedResult = readFileSync(resultPath, 'utf8') + parsedResult = parseBenchmarkTrialResult(serializedResult) + parsedResultAvailable = true + } + } catch (error) { + trialError = error + } finally { + const failedTrial = benchmarkTrialNeedsCleanup(result, parsedResultAvailable) + const trialMarker = trialTempDir ? `TMPDIR=${trialTempDir}` : undefined + const cleanup = cleanupOwnerLossTrial({ + failed: failedTrial, + pid: result?.pid, + marker: trialMarker, + recordPath: helperRecordPath, + helperPath, + tempDir: trialTempDir, + stderrDescriptor, + stdoutDescriptor, + outputPaths: [stderrPath, stdoutPath], + launcherDir + }) + cleanupError = cleanup.error + trialOutput = cleanup.output + } + if (!trialError && result?.status !== 0) { + trialError = new Error( + `Electron trial failed (${result.error?.message ?? result.signal ?? result.status}):\n${trialOutput}` + ) + } + if (!trialError && !serializedResult) { + trialError = new Error(`Electron trial did not write a result:\n${trialOutput}`) + } + throwBenchmarkTrialFailures(trialError, cleanupError) + return parsedResult +} + +function runBenchmark() { + if (process.platform !== 'darwin') { + throw new Error('The computer-use helper owner benchmark is macOS-only') + } + const options = parseArgs(process.argv.slice(2)) + const dirty = execFileSync('git', ['status', '--porcelain'], { + cwd: repoRoot, + encoding: 'utf8' + }).trim() + if (dirty) { + throw new Error('Commit or stash changes before running the provenance-bound benchmark') + } + buildArtifacts() + if (!existsSync(sidecarPath) || !existsSync(helperPath)) { + throw new Error('Fresh production sidecar/helper build did not produce the expected artifacts') + } + const executable = electronPath() + const results = Array.from({ length: options.trials }, () => runTrial(executable, options.expect)) + const rssBytes = results.map((result) => result.connectedRssBytes) + const cpuMilliseconds = results.map((result) => result.connectedCpuMilliseconds) + const activeRequestTotals = results.map((result) => result.activeRequests.totalMs) + const activeRequestRates = results.map((result) => result.activeRequests.requestsPerSecond) + const activeRequestMedians = results.map((result) => result.activeRequests.medianLatencyMs) + const activeRequestP95s = results.map((result) => result.activeRequests.p95LatencyMs) + const activeRequestMaxes = results.map((result) => result.activeRequests.maxLatencyMs) + const postLossRssBytes = results.map((result) => result.postLossRssBytes) + const report = { + benchmark: 'macos-computer-helper-authenticated-owner-loss', + revision: execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: repoRoot, + encoding: 'utf8' + }).trim(), + artifacts: { + sidecarSha256: artifactSha256(sidecarPath), + helperSha256: artifactSha256(helperPath) + }, + sources: { + benchmarkSha256: artifactSha256(scriptPath), + metricsSha256: artifactSha256(metricsPath), + processCleanupSha256: artifactSha256(processCleanupPath), + trialCleanupSha256: artifactSha256(trialCleanupPath) + }, + expectation: options.expect, + trials: options.trials, + ownerHoldMs: OWNER_HOLD_MS, + activeRequestCount: ACTIVE_REQUEST_COUNT, + authenticated: results.every((result) => result.authenticated), + survivedClaimDeadline: results.every((result) => result.survivedClaimDeadline), + invalidPeerRejectedAndDidNotRetain: results.every( + (result) => result.invalidPeerRejectedAndDidNotRetain + ), + connectedRssMiB: rssBytes.map((value) => Number((value / MIB).toFixed(2))), + medianConnectedRssMiB: Number((median(rssBytes) / MIB).toFixed(2)), + connectedCpuMilliseconds: cpuMilliseconds, + medianConnectedCpuMilliseconds: median(cpuMilliseconds), + activeRequestTotalMs: activeRequestTotals.map((value) => Number(value.toFixed(3))), + medianActiveRequestTotalMs: Number(median(activeRequestTotals).toFixed(3)), + activeRequestsPerSecond: activeRequestRates.map((value) => Number(value.toFixed(2))), + medianActiveRequestsPerSecond: Number(median(activeRequestRates).toFixed(2)), + activeRequestMedianLatencyMs: activeRequestMedians.map((value) => Number(value.toFixed(3))), + medianActiveRequestMedianLatencyMs: Number(median(activeRequestMedians).toFixed(3)), + activeRequestP95LatencyMs: activeRequestP95s.map((value) => Number(value.toFixed(3))), + medianActiveRequestP95LatencyMs: Number(median(activeRequestP95s).toFixed(3)), + activeRequestMaxLatencyMs: activeRequestMaxes.map((value) => Number(value.toFixed(3))), + helperExitedAfterAbruptLoss: results.map((result) => result.helperExitedAfterAbruptLoss), + abruptExitMs: results.map((result) => result.abruptExitMs), + postLossRssMiB: postLossRssBytes.map((value) => Number((value / MIB).toFixed(2))), + medianPostLossRssMiB: Number((median(postLossRssBytes) / MIB).toFixed(2)), + gracefulExitMs: results.map((result) => result.gracefulExitMs) + } + const serialized = `${JSON.stringify(report, null, 2)}\n` + process.stdout.write(serialized) + if (options.output) { + writeFileSync(path.resolve(options.output), serialized) + } +} + +if (process.env[INTERNAL_ENV] === '1') { + const { app } = await import('electron') + await app.whenReady() + try { + const result = await runInternalTrial(process.env[EXPECTATION_ENV]) + writeFileSync(process.env[RESULT_PATH_ENV], JSON.stringify(result)) + } finally { + app.quit() + } +} else { + runBenchmark() +} diff --git a/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs b/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs new file mode 100644 index 00000000000..512a663cddb --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import { signalValidatedProcessGroup } from './macos-computer-helper-owner-loss-processes.mjs' + +describe('macOS helper owner-loss benchmark group recovery', () => { + it('retains uncertain stop state across cleanup stage failures', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] + const groupState = { stopped: false } + let scanCount = 0 + let continueAttempts = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount >= 3) { + throw new Error( + scanCount === 3 ? 'post-stop inspection failed' : 'final inspection failed' + ) + } + return members + }, + signalProcess: (pid, signal) => { + if (pid === -41 && signal === 'SIGCONT') { + continueAttempts += 1 + if (continueAttempts === 1) { + throw new Error('first compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(groupState.stopped).toBe(true) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('final inspection failed') + expect(continueAttempts).toBe(2) + expect(groupState.stopped).toBe(false) + }) + + it('retains an uncertain anchor stop across cleanup stage failures', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] + const groupState = { stopped: false, anchorPid: null } + let scanCount = 0 + let continueAttempts = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount >= 2) { + throw new Error( + scanCount === 2 ? 'post-anchor inspection failed' : 'final inspection failed' + ) + } + return members + }, + signalProcess: (pid, signal) => { + if (pid === 41 && signal === 'SIGCONT') { + continueAttempts += 1 + if (continueAttempts === 1) { + throw new Error('first anchor compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(groupState.anchorPid).toBe(41) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('final inspection failed') + expect(continueAttempts).toBe(2) + expect(groupState.anchorPid).toBeNull() + }) + + it('recovers a retained anchor before selecting a new one', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const firstAnchor = { pid: 41, pgid: 41, command: `/launcher ${marker}` } + const finalAnchor = { pid: 42, pgid: 41, command: `/child ${marker}` } + const groupState = { stopped: false, anchorPid: null } + let scanCount = 0 + let firstAnchorContinues = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount === 2) { + throw new Error('post-anchor inspection failed') + } + return scanCount === 1 ? [firstAnchor] : [finalAnchor] + }, + signalProcess: (pid, signal) => { + if (pid === 41 && signal === 'SIGCONT') { + firstAnchorContinues += 1 + if (firstAnchorContinues === 1) { + throw new Error('first anchor compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations)).toBe(true) + expect(firstAnchorContinues).toBe(2) + expect(groupState).toEqual({ stopped: false, anchorPid: null }) + }) + + it('preserves group authority errors when compensation fails', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const ownershipError = 'Benchmark process group no longer belongs to this trial' + let thrown + + try { + signalValidatedProcessGroup( + 41, + marker, + 'SIGKILL', + { stopped: true }, + { + processIdentities: () => [{ pid: 41, pgid: 41, command: '/unrelated' }], + signalProcess: () => { + throw new Error('resume denied') + } + } + ) + } catch (error) { + thrown = error + } + expect(thrown).toBeInstanceOf(AggregateError) + expect(thrown.errors.map((error) => error.message)).toEqual([ownershipError, 'resume denied']) + }) +}) diff --git a/config/scripts/macos-computer-helper-owner-loss-metrics.mjs b/config/scripts/macos-computer-helper-owner-loss-metrics.mjs new file mode 100644 index 00000000000..b65a062ad34 --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-metrics.mjs @@ -0,0 +1,62 @@ +import { execFileSync } from 'node:child_process' + +const SAMPLE_COUNT = 5 +const SAMPLE_INTERVAL_MS = 200 + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +export function median(values) { + const sorted = [...values].sort((left, right) => left - right) + const middle = Math.floor(sorted.length / 2) + return sorted.length % 2 === 0 ? (sorted[middle - 1] + sorted[middle]) / 2 : sorted[middle] +} + +export function percentile(values, fraction) { + const sorted = [...values].sort((left, right) => left - right) + return sorted[Math.max(0, Math.ceil(sorted.length * fraction) - 1)] +} + +function parseCpuTimeSeconds(value) { + const [dayOrTime, clock] = value.includes('-') ? value.split('-', 2) : [null, value] + const days = dayOrTime === null ? 0 : Number(dayOrTime) + const parts = clock.split(':').map(Number) + if (!Number.isFinite(days) || parts.some((part) => !Number.isFinite(part))) { + throw new Error(`Invalid process CPU time: ${value}`) + } + const seconds = parts.pop() ?? 0 + const minutes = parts.pop() ?? 0 + const hours = parts.pop() ?? 0 + return days * 86_400 + hours * 3_600 + minutes * 60 + seconds +} + +export function processSnapshot(pid) { + const raw = execFileSync( + 'ps', + ['-o', 'rss=', '-o', 'time=', '-o', 'command=', '-p', String(pid)], + { encoding: 'utf8' } + ).trim() + const match = raw.match(/^(\d+)\s+(\S+)\s+(.+)$/) + if (!match) { + throw new Error(`Could not inspect process ${pid}: ${raw}`) + } + return { + rssBytes: Number(match[1]) * 1024, + cpuTimeSeconds: parseCpuTimeSeconds(match[2]), + command: match[3] + } +} + +export async function sampleProcess(pid) { + const samples = [] + for (let index = 0; index < SAMPLE_COUNT; index += 1) { + samples.push(processSnapshot(pid)) + await sleep(SAMPLE_INTERVAL_MS) + } + return { + rssBytes: median(samples.map((sample) => sample.rssBytes)), + cpuTimeSeconds: samples.at(-1).cpuTimeSeconds, + command: samples.at(-1).command + } +} diff --git a/config/scripts/macos-computer-helper-owner-loss-processes.mjs b/config/scripts/macos-computer-helper-owner-loss-processes.mjs new file mode 100644 index 00000000000..952d726d28d --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-processes.mjs @@ -0,0 +1,418 @@ +import { execFileSync, spawnSync } from 'node:child_process' +import { existsSync, readFileSync, renameSync, writeFileSync } from 'node:fs' + +const PROCESS_EXIT_TIMEOUT_MS = 2_000 +const PROCESS_POLL_MS = 25 +const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)) + +const processIdentityOperations = { + executePs: execFileSync, + signalProcess: process.kill.bind(process) +} + +export function processIdentity(pid, operations = processIdentityOperations) { + if (!Number.isInteger(pid) || pid <= 0) { + return null + } + try { + const output = operations + .executePs('ps', ['-p', String(pid), '-o', 'pid=,pgid=,command='], { + encoding: 'utf8' + }) + .trim() + const match = output.match(/^(\d+)\s+(\d+)\s+(.+)$/) + if (!match) { + throw new Error(`Could not parse process identity for ${pid}`) + } + return { pid: Number(match[1]), pgid: Number(match[2]), command: match[3] } + } catch (error) { + try { + operations.signalProcess(pid, 0) + } catch (lookupError) { + if (lookupError?.code === 'ESRCH') { + return null + } + } + throw error + } +} + +function matchingDetachedProcesses(identities, expectedCommandFragments) { + return identities.filter( + (identity) => + identity.pgid === identity.pid && + expectedCommandFragments.every((fragment) => identity.command.includes(fragment)) + ) +} + +const matchingProcessOperations = { + processIdentities, + signalProcessIdentity, + waitForIdentityExit +} + +export function killProcessMatchingCommand( + expectedCommandFragments, + operations = matchingProcessOperations +) { + const matches = matchingDetachedProcesses( + operations.processIdentities(), + expectedCommandFragments + ) + if (matches.length === 0) { + return false + } + const errors = [] + for (const match of matches) { + try { + if (operations.signalProcessIdentity(match, expectedCommandFragments[0], 'SIGKILL')) { + operations.waitForIdentityExit(match) + } + } catch (error) { + errors.push(error) + } + } + try { + const remaining = matchingDetachedProcesses( + operations.processIdentities(), + expectedCommandFragments + ) + if (remaining.length > 0) { + errors.push( + new Error( + `Benchmark helper cleanup left matching processes: ${remaining + .map((identity) => identity.pid) + .join(', ')}` + ) + ) + } + } catch (error) { + errors.push(error) + } + if (errors.length === 1) { + throw errors[0] + } + if (errors.length > 1) { + throw new AggregateError(errors, 'Benchmark exact-command cleanup failed') + } + return true +} + +function sleepSync(milliseconds) { + Atomics.wait(sleepBuffer, 0, 0, milliseconds) +} + +function validateDetachedIdentity(identity, expectedCommandFragment) { + if ( + !Number.isInteger(identity?.pid) || + identity.pid <= 0 || + identity.pgid !== identity.pid || + typeof identity.command !== 'string' || + !identity.command.includes(expectedCommandFragment) + ) { + throw new Error('Recorded benchmark helper identity is invalid') + } +} + +function sameIdentity(left, right) { + return left?.pid === right?.pid && left?.pgid === right?.pgid && left?.command === right?.command +} + +function processIdentities(includeEnvironment = false) { + const args = includeEnvironment + ? ['eww', '-axo', 'pid=,pgid=,command='] + : ['-axo', 'pid=,pgid=,command='] + const output = execFileSync('ps', args, { + encoding: 'utf8', + maxBuffer: 20 * 1024 * 1024 + }) + return output + .split('\n') + .map((line) => line.trim().match(/^(\d+)\s+(\d+)\s+(.+)$/)) + .filter(Boolean) + .map((match) => ({ + pid: Number(match[1]), + pgid: Number(match[2]), + command: match[3] + })) +} + +function waitForIdentityExit(identity) { + const deadline = Date.now() + PROCESS_EXIT_TIMEOUT_MS + while (Date.now() < deadline) { + if (!processIdentityIsCurrent(identity)) { + return true + } + sleepSync(PROCESS_POLL_MS) + } + throw new Error(`Recorded benchmark helper ${identity.pid} did not exit`) +} + +export function spawnBenchmarkProcess(executable, args, options) { + return spawnSync(executable, args, { + ...options, + detached: true, + killSignal: 'SIGKILL' + }) +} + +export function runBenchmarkCleanupStages(stages) { + const errors = [] + for (const stage of stages) { + try { + stage() + } catch (error) { + errors.push(error) + } + } + if (errors.length === 1) { + throw errors[0] + } + if (errors.length > 1) { + throw new AggregateError(errors, 'Benchmark trial cleanup failed') + } +} + +export function throwBenchmarkTrialFailures(trialError, cleanupError) { + if (trialError && cleanupError) { + throw new AggregateError([trialError, cleanupError], 'Electron trial and cleanup failed') + } + if (trialError) { + throw trialError + } + if (cleanupError) { + throw cleanupError + } +} + +export function parseBenchmarkTrialResult(serializedResult) { + return JSON.parse(serializedResult) +} + +export function benchmarkTrialNeedsCleanup(spawnResult, parsedResultAvailable) { + return spawnResult?.status !== 0 || !parsedResultAvailable +} + +const processGroupSignalOperations = { + processIdentities, + signalProcess: process.kill.bind(process) +} + +function compensateStoppedGroup(pgid, groupState, operations) { + const errors = [] + const targets = [ + groupState.stopped ? [-pgid, 'stopped'] : null, + groupState.anchorPid ? [groupState.anchorPid, 'anchorPid'] : null + ].filter(Boolean) + for (const [pid, stateKey] of targets) { + try { + operations.signalProcess(pid, 'SIGCONT') + groupState[stateKey] = stateKey === 'stopped' ? false : null + } catch (error) { + if (error?.code === 'ESRCH') { + groupState[stateKey] = stateKey === 'stopped' ? false : null + } else { + errors.push(error) + } + } + } + return errors +} + +export function signalValidatedProcessGroup( + pgid, + environmentFragment, + signal, + groupState = { stopped: false, anchorPid: null }, + operations = processGroupSignalOperations +) { + if (!Number.isInteger(pgid) || pgid <= 0) { + return false + } + let members + try { + members = operations.processIdentities(true).filter((identity) => identity.pgid === pgid) + } catch (error) { + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError( + [error, ...recoveryErrors], + 'Benchmark process group recovery failed before validation' + ) + } + throw error + } + if (members.length === 0) { + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError(recoveryErrors, 'Benchmark missing process group recovery failed') + } + return false + } + if (members.some((identity) => !identity.command.includes(environmentFragment))) { + const ownershipError = new Error('Benchmark process group no longer belongs to this trial') + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError( + [ownershipError, ...recoveryErrors], + 'Benchmark process group authority recovery failed' + ) + } + throw ownershipError + } + if (groupState.anchorPid) { + try { + operations.signalProcess(groupState.anchorPid, 'SIGCONT') + groupState.anchorPid = null + } catch (error) { + if (error?.code === 'ESRCH') { + groupState.anchorPid = null + } else { + throw new AggregateError([error], 'Benchmark pending anchor recovery failed') + } + } + } + const anchor = members[0] + try { + operations.signalProcess(anchor.pid, 'SIGSTOP') + groupState.anchorPid = anchor.pid + const stoppedAnchor = operations + .processIdentities(true) + .find((identity) => identity.pid === anchor.pid) + if (!sameIdentity(stoppedAnchor, anchor)) { + throw new Error('Benchmark process group anchor changed before signaling') + } + operations.signalProcess(-pgid, 'SIGSTOP') + groupState.stopped = true + groupState.anchorPid = null + const stoppedMembers = operations + .processIdentities(true) + .filter((identity) => identity.pgid === pgid) + if ( + stoppedMembers.length === 0 || + stoppedMembers.some((identity) => !identity.command.includes(environmentFragment)) + ) { + throw new Error('Benchmark process group changed before signaling') + } + if (signal !== 'SIGSTOP') { + operations.signalProcess(-pgid, signal) + if (signal !== 'SIGKILL') { + operations.signalProcess(-pgid, 'SIGCONT') + } + groupState.stopped = false + groupState.anchorPid = null + } + return true + } catch (error) { + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError( + [error, ...recoveryErrors], + 'Benchmark process group signal recovery failed' + ) + } + if (error.code === 'ESRCH') { + return false + } + throw error + } +} + +export function writeProcessRecord(recordPath, processIdentity) { + const temporaryPath = `${recordPath}.${process.pid}.tmp` + writeFileSync(temporaryPath, JSON.stringify(processIdentity)) + renameSync(temporaryPath, recordPath) +} + +export function processIdentityIsCurrent(identity) { + return sameIdentity(processIdentity(identity?.pid), identity) +} + +const processSignalOperations = { + processIdentity, + signalProcess: process.kill.bind(process) +} + +export function signalProcessIdentity( + identity, + expectedCommandFragment, + signal, + operations = processSignalOperations +) { + validateDetachedIdentity(identity, expectedCommandFragment) + const currentIdentity = operations.processIdentity(identity.pid) + if (!currentIdentity) { + return false + } + if (!sameIdentity(currentIdentity, identity)) { + throw new Error('Recorded benchmark helper PID now belongs to another process') + } + let stopped = false + try { + operations.signalProcess(identity.pid, 'SIGSTOP') + stopped = true + const stoppedIdentity = operations.processIdentity(identity.pid) + if (!sameIdentity(stoppedIdentity, identity)) { + throw new Error('Recorded benchmark helper PID changed before signaling') + } + operations.signalProcess(-identity.pgid, signal) + if (signal !== 'SIGKILL') { + operations.signalProcess(-identity.pgid, 'SIGCONT') + } + stopped = false + return true + } catch (error) { + let resumeError + if (stopped) { + try { + operations.signalProcess(identity.pid, 'SIGCONT') + } catch (caught) { + if (caught?.code !== 'ESRCH') { + resumeError = caught + } + } + } + if (resumeError) { + throw new AggregateError([error, resumeError], 'Benchmark helper signal recovery failed') + } + if (error.code === 'ESRCH') { + return false + } + throw error + } +} + +export function killRecordedProcess(recordPath, expectedCommandFragment) { + if (!existsSync(recordPath)) { + return false + } + const record = JSON.parse(readFileSync(recordPath, 'utf8')) + if (!signalProcessIdentity(record, expectedCommandFragment, 'SIGKILL')) { + return false + } + return waitForIdentityExit(record) +} + +export function killRecordedAndMatchingProcesses( + recordPath, + recordedCommandFragment, + matchingCommandFragments +) { + const errors = [] + try { + killRecordedProcess(recordPath, recordedCommandFragment) + } catch (error) { + errors.push(error) + } + try { + killProcessMatchingCommand(matchingCommandFragments) + } catch (error) { + errors.push(error) + } + if (errors.length === 1) { + throw errors[0] + } + if (errors.length > 1) { + throw new AggregateError(errors, 'Benchmark helper cleanup failed') + } +} diff --git a/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs b/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs new file mode 100644 index 00000000000..764eb73009f --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs @@ -0,0 +1,558 @@ +import { execFileSync, spawn } from 'node:child_process' +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { + benchmarkTrialNeedsCleanup, + killProcessMatchingCommand, + killRecordedAndMatchingProcesses, + killRecordedProcess, + parseBenchmarkTrialResult, + processIdentity, + runBenchmarkCleanupStages, + signalProcessIdentity, + signalValidatedProcessGroup, + spawnBenchmarkProcess, + throwBenchmarkTrialFailures, + writeProcessRecord +} from './macos-computer-helper-owner-loss-processes.mjs' +import { cleanupOwnerLossTrial } from './macos-computer-helper-owner-loss-trial-cleanup.mjs' + +const describeMacOS = process.platform === 'darwin' ? describe : describe.skip +const spawnedPids = new Set() +const temporaryDirectories = new Set() + +afterEach(() => { + for (const pid of spawnedPids) { + try { + process.kill(pid, 'SIGKILL') + } catch {} + } + spawnedPids.clear() + for (const temporaryDirectory of temporaryDirectories) { + rmSync(temporaryDirectory, { recursive: true, force: true }) + } + temporaryDirectories.clear() +}) + +describeMacOS('macOS helper owner-loss benchmark process cleanup', () => { + it('enforces a hard timeout when the trial ignores SIGTERM', () => { + const startedAt = Date.now() + const result = spawnBenchmarkProcess( + process.execPath, + ['-e', "process.on('SIGTERM', () => {}); setInterval(() => {}, 1_000)"], + { stdio: 'ignore', timeout: 100 } + ) + expect(result.error?.code).toBe('ETIMEDOUT') + expect(result.signal).toBe('SIGKILL') + expect(Date.now() - startedAt).toBeLessThan(2_000) + expect(() => process.kill(result.pid, 0)).toThrow() + }) + + it('runs every cleanup stage before aggregating errors', () => { + const completed = [] + let thrown + + try { + runBenchmarkCleanupStages([ + () => { + completed.push(1) + throw new Error('first failure') + }, + () => { + completed.push(2) + }, + () => { + completed.push(3) + throw new Error('last failure') + } + ]) + } catch (error) { + thrown = error + } + expect(completed).toEqual([1, 2, 3]) + expect(thrown).toBeInstanceOf(AggregateError) + expect(thrown.errors.map((error) => error.message)).toEqual(['first failure', 'last failure']) + }) + + it('preserves malformed-result and cleanup failures', () => { + let trialError + try { + parseBenchmarkTrialResult('{malformed') + } catch (error) { + trialError = error + } + const cleanupError = new Error('cleanup failed') + let thrown + + try { + throwBenchmarkTrialFailures(trialError, cleanupError) + } catch (error) { + thrown = error + } + expect(trialError).toBeInstanceOf(SyntaxError) + expect(thrown).toBeInstanceOf(AggregateError) + expect(thrown.errors).toEqual([trialError, cleanupError]) + }) + + it('cleans up a status-zero trial whose result could not be parsed', () => { + expect(benchmarkTrialNeedsCleanup(undefined, false)).toBe(true) + expect(benchmarkTrialNeedsCleanup({ status: 0 }, false)).toBe(true) + expect(benchmarkTrialNeedsCleanup({ status: 0 }, true)).toBe(false) + expect(benchmarkTrialNeedsCleanup({ status: 1 }, true)).toBe(true) + }) + + it('removes a launcher directory after partial trial setup', () => { + const launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-owner-partial-setup-test-')) + temporaryDirectories.add(launcherDir) + + const cleanup = cleanupOwnerLossTrial({ + failed: true, + launcherDir, + outputPaths: [] + }) + + expect(cleanup.error).toBeUndefined() + expect(existsSync(launcherDir)).toBe(false) + temporaryDirectories.delete(launcherDir) + }) + + it('kills a timed-out trial group only after validating its environment', async () => { + const temporaryDirectory = mkdtempSync(path.join(tmpdir(), 'orca-owner-benchmark-group-test-')) + temporaryDirectories.add(temporaryDirectory) + const childPidPath = path.join(temporaryDirectory, 'child.pid') + const environmentName = `ORCA_OWNER_GROUP_${process.pid}` + const environmentValue = `${Date.now()}` + const fixture = ` + const { spawn } = require('node:child_process') + const { writeFileSync } = require('node:fs') + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + stdio: 'ignore' + }) + writeFileSync(${JSON.stringify(childPidPath)}, String(child.pid)) + setInterval(() => {}, 1000) + ` + const result = spawnBenchmarkProcess(process.execPath, ['-e', fixture], { + env: { ...process.env, [environmentName]: environmentValue }, + stdio: 'ignore', + timeout: 100 + }) + const childPid = Number(readFileSync(childPidPath, 'utf8')) + spawnedPids.add(childPid) + const environmentFragment = `${environmentName}=${environmentValue}` + const groupState = { stopped: false } + + expect(() => + signalValidatedProcessGroup(result.pid, `${environmentName}=wrong`, 'SIGSTOP') + ).toThrow('Benchmark process group no longer belongs to this trial') + expect(() => process.kill(childPid, 0)).not.toThrow() + expect( + signalValidatedProcessGroup(result.pid, environmentFragment, 'SIGSTOP', groupState) + ).toBe(true) + expect( + signalValidatedProcessGroup(result.pid, environmentFragment, 'SIGKILL', groupState) + ).toBe(true) + await expect + .poll(() => { + try { + process.kill(childPid, 0) + return true + } catch { + return false + } + }) + .toBe(false) + + spawnedPids.delete(childPid) + }) + + it('resumes the group after post-stop revalidation fails', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [ + { pid: 41, pgid: 41, command: `/launcher ${marker}` }, + { pid: 42, pgid: 41, command: `/child ${marker}` } + ] + const signals = [] + let scanCount = 0 + + expect(() => + signalValidatedProcessGroup( + 41, + marker, + 'SIGKILL', + { stopped: false }, + { + processIdentities: () => { + scanCount += 1 + if (scanCount === 3) { + throw new Error('transient group inspection failure') + } + return members + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + } + ) + ).toThrow('transient group inspection failure') + expect(signals).toEqual([ + [41, 'SIGSTOP'], + [-41, 'SIGSTOP'], + [-41, 'SIGCONT'] + ]) + }) + + it('compensates a possible stop after group anchor replacement', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const anchor = { pid: 41, pgid: 41, command: `/launcher ${marker}` } + const replacement = { pid: 41, pgid: 99, command: '/unrelated' } + const signals = [] + let scanCount = 0 + + expect(() => + signalValidatedProcessGroup( + 41, + marker, + 'SIGKILL', + { stopped: false }, + { + processIdentities: () => { + scanCount += 1 + return scanCount === 1 ? [anchor] : [replacement] + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + } + ) + ).toThrow('Benchmark process group anchor changed before signaling') + expect(signals).toEqual([ + [41, 'SIGSTOP'], + [41, 'SIGCONT'] + ]) + }) + + it('resumes a previously frozen group when final inspection fails', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] + const signals = [] + let scanCount = 0 + const groupState = { stopped: false } + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount === 4) { + throw new Error('transient final inspection failure') + } + return members + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + } + + expect(signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations)).toBe(true) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('transient final inspection failure') + expect(signals).toEqual([ + [41, 'SIGSTOP'], + [-41, 'SIGSTOP'], + [-41, 'SIGCONT'] + ]) + }) + + it('resumes a previously frozen group when final anchor stop fails', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [ + { pid: 41, pgid: 41, command: `/launcher ${marker}` }, + { pid: 42, pgid: 41, command: `/child ${marker}` } + ] + const signals = [] + let finalCall = false + const groupState = { stopped: false } + const missingProcessError = Object.assign(new Error('anchor exited'), { code: 'ESRCH' }) + const operations = { + processIdentities: () => members, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + if (finalCall && pid === 41 && signal === 'SIGSTOP') { + throw missingProcessError + } + } + } + + expect(signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations)).toBe(true) + finalCall = true + expect(signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations)).toBe(false) + expect(signals.at(-1)).toEqual([-41, 'SIGCONT']) + }) + + it('resumes a previously frozen group after final anchor replacement', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const anchor = { pid: 41, pgid: 41, command: `/launcher ${marker}` } + const child = { pid: 42, pgid: 41, command: `/child ${marker}` } + const replacement = { pid: 41, pgid: 99, command: '/unrelated' } + const signals = [] + let scanCount = 0 + const groupState = { stopped: false } + const operations = { + processIdentities: () => { + scanCount += 1 + return scanCount === 5 ? [replacement, child] : [anchor, child] + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + } + + expect(signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations)).toBe(true) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('Benchmark process group anchor changed before signaling') + expect(signals.slice(-2)).toEqual([ + [-41, 'SIGCONT'], + [41, 'SIGCONT'] + ]) + }) + + it('kills a recorded helper in a separate process group', async () => { + const temporaryDirectory = mkdtempSync( + path.join(tmpdir(), 'orca-owner-benchmark-cleanup-test-') + ) + temporaryDirectories.add(temporaryDirectory) + const recordPath = path.join(temporaryDirectory, 'helper.json') + const marker = `orca-owner-cleanup-${process.pid}-${Date.now()}` + const helper = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1_000)', marker], { + detached: true, + stdio: 'ignore' + }) + spawnedPids.add(helper.pid) + helper.unref() + const exited = new Promise((resolve) => helper.once('exit', resolve)) + const command = execFileSync('ps', ['-p', String(helper.pid), '-o', 'command='], { + encoding: 'utf8' + }).trim() + const processGroup = Number( + execFileSync('ps', ['-p', String(helper.pid), '-o', 'pgid='], { + encoding: 'utf8' + }).trim() + ) + writeProcessRecord(recordPath, { pid: helper.pid, pgid: processGroup, command }) + + expect(processGroup).toBe(helper.pid) + expect(killRecordedProcess(recordPath, marker)).toBe(true) + await exited + expect(() => process.kill(helper.pid, 0)).toThrow() + + spawnedPids.delete(helper.pid) + }) + + it('kills every unrecorded helper using its unique trial command', async () => { + const marker = `orca-owner-unrecorded-${process.pid}-${Date.now()}` + const helpers = Array.from({ length: 2 }, () => + spawn(process.execPath, ['-e', 'setInterval(() => {}, 1_000)', marker], { + detached: true, + stdio: 'ignore' + }) + ) + for (const helper of helpers) { + spawnedPids.add(helper.pid) + helper.unref() + } + const exited = Promise.all( + helpers.map((helper) => new Promise((resolve) => helper.once('exit', resolve))) + ) + + expect(killProcessMatchingCommand([process.execPath, marker])).toBe(true) + await exited + for (const helper of helpers) { + expect(() => process.kill(helper.pid, 0)).toThrow() + spawnedPids.delete(helper.pid) + } + }) + + it('continues exact-match cleanup after an earlier match fails', () => { + const marker = `orca-owner-multiple-${process.pid}-${Date.now()}` + const matches = [ + { pid: 41, pgid: 41, command: `/helper ${marker}` }, + { pid: 42, pgid: 42, command: `/helper ${marker}` } + ] + const attempted = [] + let scanCount = 0 + + expect(() => + killProcessMatchingCommand(['/helper', marker], { + processIdentities: () => { + scanCount += 1 + return scanCount === 1 ? matches : [matches[0]] + }, + signalProcessIdentity: (identity) => { + attempted.push(identity.pid) + if (identity.pid === matches[0].pid) { + throw new Error('identity changed') + } + return true + }, + waitForIdentityExit: () => {} + }) + ).toThrow('Benchmark exact-command cleanup failed') + expect(attempted).toEqual([41, 42]) + }) + + it('does not treat an identity query failure as process exit', () => { + const queryError = new Error('transient ps failure') + + expect(() => + processIdentity(41, { + executePs: () => { + throw queryError + }, + signalProcess: () => {} + }) + ).toThrow(queryError) + }) + + it('resumes a helper when post-stop identity inspection fails', () => { + const identity = { pid: 41, pgid: 41, command: '/helper marker' } + const signals = [] + let inspectionCount = 0 + + expect(() => + signalProcessIdentity(identity, 'marker', 'SIGKILL', { + processIdentity: () => { + inspectionCount += 1 + if (inspectionCount === 2) { + throw new Error('transient ps failure after stop') + } + return identity + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + }) + ).toThrow('transient ps failure after stop') + expect(signals).toEqual([ + [41, 'SIGSTOP'], + [41, 'SIGCONT'] + ]) + }) + + it('compensates a possible stop after helper PID replacement', () => { + const identity = { pid: 41, pgid: 41, command: '/helper marker' } + const replacement = { pid: 41, pgid: 41, command: '/unrelated' } + const signals = [] + let inspectionCount = 0 + + expect(() => + signalProcessIdentity(identity, 'marker', 'SIGKILL', { + processIdentity: () => { + inspectionCount += 1 + return inspectionCount === 1 ? identity : replacement + }, + signalProcess: (pid, signal) => { + signals.push([pid, signal]) + } + }) + ).toThrow('Recorded benchmark helper PID changed before signaling') + expect(signals).toEqual([ + [41, 'SIGSTOP'], + [41, 'SIGCONT'] + ]) + }) + + it('preserves helper identity errors when compensation fails', () => { + const identity = { pid: 41, pgid: 41, command: '/helper marker' } + const replacement = { pid: 41, pgid: 41, command: '/unrelated' } + let inspectionCount = 0 + let thrown + + try { + signalProcessIdentity(identity, 'marker', 'SIGKILL', { + processIdentity: () => { + inspectionCount += 1 + return inspectionCount === 1 ? identity : replacement + }, + signalProcess: (_pid, signal) => { + if (signal === 'SIGCONT') { + throw new Error('resume denied') + } + } + }) + } catch (error) { + thrown = error + } + expect(thrown).toBeInstanceOf(AggregateError) + expect(thrown.errors.map((error) => error.message)).toEqual([ + 'Recorded benchmark helper PID changed before signaling', + 'resume denied' + ]) + }) + + it('treats a missing PID as process exit after an identity query failure', () => { + const missingProcessError = Object.assign(new Error('missing process'), { code: 'ESRCH' }) + + expect( + processIdentity(41, { + executePs: () => { + throw new Error('ps found no process') + }, + signalProcess: () => { + throw missingProcessError + } + }) + ).toBeNull() + }) + + it('runs unique-command cleanup after an invalid process record', async () => { + const temporaryDirectory = mkdtempSync( + path.join(tmpdir(), 'orca-owner-benchmark-fallback-test-') + ) + temporaryDirectories.add(temporaryDirectory) + const recordPath = path.join(temporaryDirectory, 'helper.json') + const marker = `orca-owner-invalid-record-${process.pid}-${Date.now()}` + const helper = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1_000)', marker], { + detached: true, + stdio: 'ignore' + }) + spawnedPids.add(helper.pid) + helper.unref() + const exited = new Promise((resolve) => helper.once('exit', resolve)) + const command = execFileSync('ps', ['-p', String(helper.pid), '-o', 'command='], { + encoding: 'utf8' + }).trim() + writeProcessRecord(recordPath, { pid: helper.pid, pgid: helper.pid - 1, command }) + + expect(() => + killRecordedAndMatchingProcesses(recordPath, marker, [process.execPath, marker]) + ).toThrow('Recorded benchmark helper identity is invalid') + await exited + expect(() => process.kill(helper.pid, 0)).toThrow() + + spawnedPids.delete(helper.pid) + }) + + it('rejects a record that is not a detached process-group identity', () => { + const temporaryDirectory = mkdtempSync( + path.join(tmpdir(), 'orca-owner-benchmark-identity-test-') + ) + temporaryDirectories.add(temporaryDirectory) + const recordPath = path.join(temporaryDirectory, 'helper.json') + const marker = `orca-owner-invalid-identity-${process.pid}-${Date.now()}` + const helper = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1_000)', marker], { + stdio: 'ignore' + }) + spawnedPids.add(helper.pid) + helper.unref() + const command = execFileSync('ps', ['-p', String(helper.pid), '-o', 'command='], { + encoding: 'utf8' + }).trim() + writeProcessRecord(recordPath, { pid: helper.pid, pgid: helper.pid - 1, command }) + + expect(() => killRecordedProcess(recordPath, marker)).toThrow( + 'Recorded benchmark helper identity is invalid' + ) + expect(() => process.kill(helper.pid, 0)).not.toThrow() + }) +}) diff --git a/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs b/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs new file mode 100644 index 00000000000..c3fb4aaf384 --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs @@ -0,0 +1,63 @@ +import { closeSync, existsSync, readFileSync, rmSync } from 'node:fs' +import { + killRecordedAndMatchingProcesses, + runBenchmarkCleanupStages, + signalValidatedProcessGroup +} from './macos-computer-helper-owner-loss-processes.mjs' + +export function cleanupOwnerLossTrial(options) { + const groupState = { stopped: false, anchorPid: null } + let error + let output = '' + try { + runBenchmarkCleanupStages([ + () => { + if (options.failed && Number.isInteger(options.pid) && options.marker) { + signalValidatedProcessGroup(options.pid, options.marker, 'SIGSTOP', groupState) + } + }, + () => { + if (options.failed && options.recordPath && options.tempDir) { + killRecordedAndMatchingProcesses(options.recordPath, options.helperPath, [ + options.helperPath, + options.tempDir + ]) + } + }, + () => { + if (options.failed && Number.isInteger(options.pid) && options.marker) { + signalValidatedProcessGroup(options.pid, options.marker, 'SIGKILL', groupState) + } + }, + () => { + if (options.stderrDescriptor !== undefined) { + closeSync(options.stderrDescriptor) + } + }, + () => { + if (options.stdoutDescriptor !== undefined) { + closeSync(options.stdoutDescriptor) + } + }, + () => { + output = (options.outputPaths ?? []) + .filter((outputPath) => outputPath && existsSync(outputPath)) + .map((outputPath) => readFileSync(outputPath, 'utf8')) + .join('') + }, + () => { + if (options.launcherDir) { + rmSync(options.launcherDir, { recursive: true, force: true }) + } + }, + () => { + if (options.tempDir) { + rmSync(options.tempDir, { recursive: true, force: true }) + } + } + ]) + } catch (caught) { + error = caught + } + return { error, output } +} diff --git a/config/scripts/packaged-hang-watchdog-worker-contract.test.mjs b/config/scripts/packaged-hang-watchdog-worker-contract.test.mjs new file mode 100644 index 00000000000..1ae558c7691 --- /dev/null +++ b/config/scripts/packaged-hang-watchdog-worker-contract.test.mjs @@ -0,0 +1,35 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' +import { describe, expect, it } from 'vitest' + +describe('packaged hang watchdog worker contract', () => { + it('boots the worker from app.asar in PR checks', () => { + const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) + const smokeSource = readFileSync( + 'config/scripts/smoke-packaged-hang-watchdog-worker.mjs', + 'utf8' + ) + const smokeStep = workflow.jobs.package.steps.find( + (step) => step.name === 'Smoke packaged hang watchdog worker' + ) + + expect(smokeStep.run).toBe( + 'xvfb-run --auto-servernum node config/scripts/smoke-packaged-hang-watchdog-worker.mjs --app-dir=dist/linux-unpacked' + ) + expect(smokeSource).toContain( + "process.platform === 'linux' ? ['--no-sandbox', launcherDir] : [launcherDir]" + ) + }) + + // Why: Electron ignores process.exitCode, so the gate needs app.exit plus a stdout assertion. + it('fails the smoke when the packaged worker never reports success', () => { + const smokeSource = readFileSync( + 'config/scripts/smoke-packaged-hang-watchdog-worker.mjs', + 'utf8' + ) + + expect(smokeSource).toContain('app.exit(1)') + expect(smokeSource).not.toContain('app.quit()') + expect(smokeSource).toContain('if (!result.stdout.includes(SUCCESS_LINE))') + }) +}) diff --git a/config/scripts/smoke-packaged-hang-watchdog-worker.mjs b/config/scripts/smoke-packaged-hang-watchdog-worker.mjs new file mode 100644 index 00000000000..ae9b340785a --- /dev/null +++ b/config/scripts/smoke-packaged-hang-watchdog-worker.mjs @@ -0,0 +1,179 @@ +import { spawnSync } from 'node:child_process' +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { pathToFileURL } from 'node:url' +import { Worker } from 'node:worker_threads' + +const INTERNAL_ENV = 'ORCA_PACKAGED_WATCHDOG_SMOKE_INTERNAL' +const ASAR_ENV = 'ORCA_PACKAGED_WATCHDOG_SMOKE_ASAR' +const TIMEOUT_MS = 100 +const CHECK_INTERVAL_MS = 20 +const POLL_TIMEOUT_MS = 5_000 +const SUCCESS_LINE = '[packaged-watchdog-smoke] app.asar worker detected and recovered a stall' + +function sleep(ms) { + return new Promise((resolveSleep) => setTimeout(resolveSleep, ms)) +} + +function readAppDirArg(argv) { + const explicit = argv.find((arg) => arg.startsWith('--app-dir=')) + if (explicit) { + return explicit.slice('--app-dir='.length) + } + if (process.platform === 'darwin') { + return 'dist/mac-arm64/Orca.app' + } + if (process.platform === 'win32') { + return 'dist/win-unpacked' + } + return 'dist/linux-unpacked' +} + +function getResourcesDir(appDir) { + return process.platform === 'darwin' || appDir.endsWith('.app') + ? join(appDir, 'Contents', 'Resources') + : join(appDir, 'resources') +} + +function readMarker(markerPath) { + try { + return JSON.parse(readFileSync(markerPath, 'utf8')) + } catch { + return null + } +} + +async function waitForMarker(markerPath, predicate, workerError) { + const deadline = Date.now() + POLL_TIMEOUT_MS + while (Date.now() < deadline) { + if (workerError.current) { + throw workerError.current + } + const marker = readMarker(markerPath) + if (predicate(marker)) { + return marker + } + await sleep(CHECK_INTERVAL_MS) + } + throw new Error(`Timed out waiting for packaged watchdog marker at ${markerPath}`) +} + +async function waitForExit(worker, workerError) { + const exitCode = await Promise.race([ + new Promise((resolveExit) => worker.once('exit', resolveExit)), + sleep(POLL_TIMEOUT_MS).then(() => { + throw new Error('Timed out waiting for packaged watchdog worker to exit') + }) + ]) + if (workerError.current) { + throw workerError.current + } + if (exitCode !== 0) { + throw new Error(`Packaged watchdog worker exited with code ${exitCode}`) + } +} + +async function runInternal() { + const appAsar = process.env[ASAR_ENV] + if (!appAsar) { + throw new Error(`Missing ${ASAR_ENV}`) + } + const { app } = await import('electron') + const tempRoot = mkdtempSync(join(tmpdir(), 'orca-packaged-watchdog-smoke-')) + const markerPath = join(tempRoot, 'main-thread-hang.json') + const entryPath = join(appAsar, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + let worker + try { + await app.whenReady() + if (!existsSync(entryPath)) { + throw new Error(`Packaged watchdog entry is missing from app.asar: ${entryPath}`) + } + const workerError = { current: null } + worker = new Worker(entryPath, { + workerData: { + parentPid: process.pid, + markerPath, + timeoutMs: TIMEOUT_MS, + checkIntervalMs: CHECK_INTERVAL_MS + } + }) + worker.once('error', (error) => { + workerError.current = error + }) + await waitForMarker(markerPath, (marker) => marker?.selfRecovered === false, workerError) + worker.postMessage({ type: 'heartbeat' }) + await waitForMarker(markerPath, (marker) => marker?.selfRecovered === true, workerError) + worker.postMessage({ type: 'shutdown' }) + await waitForExit(worker, workerError) + worker = undefined + console.log(SUCCESS_LINE) + } finally { + await worker?.terminate() + rmSync(tempRoot, { recursive: true, force: true }) + } +} + +function runSmoke() { + const appDir = resolve(readAppDirArg(process.argv.slice(2))) + const appAsar = join(getResourcesDir(appDir), 'app.asar') + if (!existsSync(appAsar)) { + throw new Error(`Packaged app archive is missing: ${appAsar}`) + } + const require = createRequire(import.meta.url) + const executable = require('electron') + const launcherDir = mkdtempSync(join(tmpdir(), 'orca-packaged-watchdog-launcher-')) + const launcherPath = join(launcherDir, 'main.cjs') + writeFileSync( + join(launcherDir, 'package.json'), + JSON.stringify({ name: 'orca-packaged-watchdog-smoke', main: 'main.cjs' }) + ) + writeFileSync( + launcherPath, + `import(${JSON.stringify(pathToFileURL(import.meta.filename).href)}).catch((error) => { + console.error(error) + process.exitCode = 1 +})\n` + ) + const env = { ...process.env, [INTERNAL_ENV]: '1', [ASAR_ENV]: appAsar, NODE_PATH: '' } + delete env.ELECTRON_RUN_AS_NODE + try { + const electronArgs = + process.platform === 'linux' ? ['--no-sandbox', launcherDir] : [launcherDir] + const result = spawnSync(executable, electronArgs, { + env, + encoding: 'utf8', + timeout: 15_000 + }) + if (result.status !== 0) { + throw new Error( + `Packaged watchdog smoke failed (${result.error?.message ?? result.signal ?? result.status}):\n` + + `${result.stderr || result.stdout}` + ) + } + // Why: Electron discards process.exitCode, so status 0 alone can't prove the worker ran. + if (!result.stdout.includes(SUCCESS_LINE)) { + throw new Error( + `Packaged watchdog smoke did not report success:\n${result.stderr || result.stdout}` + ) + } + process.stdout.write(result.stdout) + } finally { + rmSync(launcherDir, { recursive: true, force: true }) + } +} + +if (process.env[INTERNAL_ENV] === '1') { + // Why: a graceful quit exits 0 regardless of process.exitCode; only app.exit propagates failure. + const { app } = await import('electron') + try { + await runInternal() + app.exit(0) + } catch (error) { + console.error(error) + app.exit(1) + } +} else { + runSmoke() +} diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 4a4cd6c8eba..957e3f60b38 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -243,9 +243,8 @@ export const electronViteConfig: UserConfig = { // Why: forked with ELECTRON_RUN_AS_NODE so @parcel/watcher faults // can't take down the main process (issue #7547). 'parcel-watcher-process-entry': resolve('src/main/ipc/parcel-watcher-process-entry.ts'), - // Why: forked with ELECTRON_RUN_AS_NODE so it survives a deadlocked - // main thread (macOS 26 AppKit scene-update deadlock) and can record - // the stall for the next launch to report. + // Why: a worker thread survives the macOS 26 AppKit main-thread deadlock + // without paying for another Electron process. 'main-thread-hang-watchdog-entry': resolve( 'src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts' ), diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift index 8edeb06131b..50ecae7d024 100644 --- a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift @@ -2579,9 +2579,12 @@ private enum KeyMap { } private final class AgentRuntime: NSObject, NSApplicationDelegate { + private static let unclaimedSessionDeadline: TimeInterval = 30 + private let socketPath: String private let token: String? private var listener: SocketListener? + private var unclaimedSessionTimeout: DispatchWorkItem? init(socketPath: String, token: String?) { self.socketPath = socketPath @@ -2590,9 +2593,29 @@ private final class AgentRuntime: NSObject, NSApplicationDelegate { func applicationDidFinishLaunching(_ notification: Notification) { do { - let listener = try SocketListener(socketPath: socketPath, token: token) + let timeout = DispatchWorkItem { + fputs("computer-use agent received no authenticated session before its deadline\n", stderr) + NSApp.terminate(nil) + } + unclaimedSessionTimeout = timeout + let listener = try SocketListener( + socketPath: socketPath, + token: token, + onSessionClaimed: { + timeout.cancel() + }, + onSessionClosed: { + DispatchQueue.main.async { + NSApp.terminate(nil) + } + } + ) self.listener = listener listener.start() + DispatchQueue.main.asyncAfter( + deadline: .now() + Self.unclaimedSessionDeadline, + execute: timeout + ) } catch { fputs("failed to start computer-use socket: \(error)\n", stderr) NSApp.terminate(nil) @@ -2600,6 +2623,8 @@ private final class AgentRuntime: NSObject, NSApplicationDelegate { } func applicationWillTerminate(_ notification: Notification) { + unclaimedSessionTimeout?.cancel() + unclaimedSessionTimeout = nil listener?.stop() } } @@ -3505,14 +3530,26 @@ private final class ButtonTarget: NSObject { private final class SocketListener: @unchecked Sendable { private let socketPath: String private let token: String? + private let onSessionClaimed: () -> Void + private let onSessionClosed: () -> Void private let provider = Provider() private let providerLock = NSLock() + private let sessionLock = NSLock() + private var sessionOwnership = AgentSessionOwnership() + private var lastConnectionID: UInt64 = 0 private var socketFd: Int32 = -1 private var isStopped = false - init(socketPath: String, token: String?) throws { + init( + socketPath: String, + token: String?, + onSessionClaimed: @escaping () -> Void, + onSessionClosed: @escaping () -> Void + ) throws { self.socketPath = socketPath self.token = token + self.onSessionClaimed = onSessionClaimed + self.onSessionClosed = onSessionClosed try bindSocket() } @@ -3587,14 +3624,35 @@ private final class SocketListener: @unchecked Sendable { } continue } + guard let connectionID = allocateConnectionID() else { + fputs("computer-use socket exhausted connection identities\n", stderr) + close(fd) + continue + } Thread.detachNewThread { [weak self] in - self?.handleConnection(fd) + self?.handleConnection(fd, connectionID: connectionID) } } } - private func handleConnection(_ fd: Int32) { - defer { close(fd) } + private func allocateConnectionID() -> AgentSessionConnectionID? { + sessionLock.lock() + defer { sessionLock.unlock() } + guard lastConnectionID < UInt64.max else { return nil } + lastConnectionID += 1 + return AgentSessionConnectionID(rawValue: lastConnectionID) + } + + private func handleConnection(_ fd: Int32, connectionID: AgentSessionConnectionID) { + var registeredSession = false + var hangupMonitor: AuthenticatedConnectionHangupMonitor? + defer { + hangupMonitor?.cancel() + if registeredSession { + disconnectSession(connectionID) + } + close(fd) + } let authorizedPeer = peerProcessId(fd).map(isAuthorizedAgentPeer) == true let decoder = JSONDecoder() while let line = readLine(from: fd) { @@ -3603,6 +3661,40 @@ private final class SocketListener: @unchecked Sendable { else { continue } + if !registeredSession && isAuthenticatedAgentSession( + expectedToken: token, + requestToken: request.token, + authorizedPeer: authorizedPeer + ) { + let monitor: AuthenticatedConnectionHangupMonitor + do { + monitor = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: fd, + onHangup: { [weak self] in + self?.disconnectSession(connectionID) + } + ) + } catch { + fputs("computer-use owner monitor failed: \(error)\n", stderr) + return + } + sessionLock.lock() + let registration = sessionOwnership.registerConnection( + connectionID, + authenticated: true + ) + sessionLock.unlock() + guard registration != .rejected else { + monitor.cancel() + return + } + registeredSession = true + hangupMonitor = monitor + monitor.start() + if registration == .claimed { + onSessionClaimed() + } + } let response = handleRequest( provider: provider, lock: providerLock, @@ -3613,6 +3705,15 @@ private final class SocketListener: @unchecked Sendable { writeJSON(response, to: fd) } } + + private func disconnectSession(_ connectionID: AgentSessionConnectionID) { + sessionLock.lock() + let shouldTerminate = sessionOwnership.disconnect(connectionID) + sessionLock.unlock() + if shouldTerminate { + onSessionClosed() + } + } } private func existingPathMode(_ path: String) -> mode_t? { diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AgentSessionOwnership.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AgentSessionOwnership.swift new file mode 100644 index 00000000000..d39543e9895 --- /dev/null +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AgentSessionOwnership.swift @@ -0,0 +1,50 @@ +public struct AgentSessionConnectionID: Hashable, Sendable { + public let rawValue: UInt64 + + public init(rawValue: UInt64) { + self.rawValue = rawValue + } +} + +public enum AgentSessionRegistration: Sendable { + case rejected + case claimed + case retained +} + +public struct AgentSessionOwnership: Sendable { + private var authenticatedConnections: Set = [] + private var wasClaimed = false + private var sessionClosed = false + + public init() {} + + public mutating func registerConnection( + _ connection: AgentSessionConnectionID, + authenticated: Bool + ) -> AgentSessionRegistration { + guard authenticated, !sessionClosed else { return .rejected } + let inserted = authenticatedConnections.insert(connection).inserted + guard inserted else { return .rejected } + guard !wasClaimed else { return .retained } + wasClaimed = true + return .claimed + } + + public mutating func disconnect(_ connection: AgentSessionConnectionID) -> Bool { + guard authenticatedConnections.remove(connection) != nil else { return false } + guard wasClaimed, authenticatedConnections.isEmpty else { return false } + sessionClosed = true + return true + } +} + +public func isAuthenticatedAgentSession( + expectedToken: String?, + requestToken: String?, + authorizedPeer: Bool +) -> Bool { + guard authorizedPeer else { return false } + guard let expectedToken else { return true } + return requestToken == expectedToken +} diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AuthenticatedConnectionHangupMonitor.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AuthenticatedConnectionHangupMonitor.swift new file mode 100644 index 00000000000..55e4ec91721 --- /dev/null +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/AuthenticatedConnectionHangupMonitor.swift @@ -0,0 +1,182 @@ +import Darwin +import Dispatch +import Foundation + +public final class AuthenticatedConnectionHangupMonitor: @unchecked Sendable { + typealias RegisterEvents = @Sendable ( + Int32, + UnsafePointer?, + Int32 + ) -> POSIXErrorCode? + typealias CloseDescriptor = @Sendable (Int32) -> Void + + private static let cancelEventIdentifier: UInt = 1 + + private let closeDescriptor: CloseDescriptor + private let eventQueue: DispatchQueue + private let eventQueueDescriptor: Int32 + private let onEvent: @Sendable () -> Void + private let stateLock = NSLock() + private let onHangup: @Sendable () -> Void + private var isCancelled = false + private var didReportHangup = false + private var isFinished = false + private var isStarted = false + + public convenience init( + fileDescriptor: Int32, + queue: DispatchQueue = DispatchQueue( + label: "com.stablyai.orca.computer-use-owner-hangup" + ), + onEvent: @escaping @Sendable () -> Void = {}, + onHangup: @escaping @Sendable () -> Void + ) throws { + try self.init( + fileDescriptor: fileDescriptor, + queue: queue, + registerEvents: { descriptor, events, count in + guard kevent(descriptor, events, count, nil, 0, nil) == 0 else { + return POSIXErrorCode(rawValue: errno) ?? .EIO + } + return nil + }, + closeDescriptor: { descriptor in + close(descriptor) + }, + onEvent: onEvent, + onHangup: onHangup + ) + } + + init( + fileDescriptor: Int32, + queue: DispatchQueue = DispatchQueue( + label: "com.stablyai.orca.computer-use-owner-hangup" + ), + registerEvents: RegisterEvents, + closeDescriptor: @escaping CloseDescriptor, + onEvent: @escaping @Sendable () -> Void = {}, + onHangup: @escaping @Sendable () -> Void + ) throws { + self.closeDescriptor = closeDescriptor + eventQueue = queue + self.onEvent = onEvent + self.onHangup = onHangup + let descriptor = kqueue() + guard descriptor >= 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + guard fileDescriptor >= 0 else { + closeDescriptor(descriptor) + throw POSIXError(.EBADF) + } + eventQueueDescriptor = descriptor + var registrations = [ + kevent( + ident: UInt(fileDescriptor), + filter: Int16(EVFILT_READ), + flags: UInt16(EV_ADD | EV_CLEAR), + fflags: UInt32(NOTE_LOWAT), + data: Int.max, + udata: nil + ), + kevent( + ident: Self.cancelEventIdentifier, + filter: Int16(EVFILT_USER), + flags: UInt16(EV_ADD | EV_CLEAR), + fflags: 0, + data: 0, + udata: nil + ) + ] + let registrationError = registrations.withUnsafeMutableBufferPointer { buffer in + registerEvents(descriptor, buffer.baseAddress, Int32(buffer.count)) + } + if let registrationError { + isFinished = true + closeDescriptor(descriptor) + throw POSIXError(registrationError) + } + } + + deinit { + cancel() + } + + public func start() { + stateLock.lock() + guard !isStarted, !isCancelled, !isFinished else { + stateLock.unlock() + return + } + isStarted = true + stateLock.unlock() + eventQueue.async { [self] in + waitForHangup() + } + } + + public func cancel() { + stateLock.lock() + guard !isFinished else { + stateLock.unlock() + return + } + isCancelled = true + guard isStarted else { + isFinished = true + closeDescriptor(eventQueueDescriptor) + stateLock.unlock() + return + } + var event = kevent( + ident: Self.cancelEventIdentifier, + filter: Int16(EVFILT_USER), + flags: 0, + fflags: UInt32(NOTE_TRIGGER), + data: 0, + udata: nil + ) + _ = kevent(eventQueueDescriptor, &event, 1, nil, 0, nil) + stateLock.unlock() + } + + private func waitForHangup() { + defer { finish() } + while true { + var event = kevent() + let result = kevent(eventQueueDescriptor, nil, 0, &event, 1, nil) + if result < 0 && errno == EINTR { + continue + } + guard result > 0 else { return } + onEvent() + if event.filter == Int16(EVFILT_USER) { + return + } + if event.flags & UInt16(EV_EOF | EV_ERROR) != 0 { + reportHangup() + return + } + } + } + + private func finish() { + stateLock.lock() + isFinished = true + closeDescriptor(eventQueueDescriptor) + stateLock.unlock() + } + + private func reportHangup() { + stateLock.lock() + guard !isCancelled, !didReportHangup else { + stateLock.unlock() + return + } + didReportHangup = true + stateLock.unlock() + + onHangup() + } +} diff --git a/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AgentSessionOwnershipTests.swift b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AgentSessionOwnershipTests.swift new file mode 100644 index 00000000000..8094501c919 --- /dev/null +++ b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AgentSessionOwnershipTests.swift @@ -0,0 +1,104 @@ +import OrcaComputerUseMacOSCore +import XCTest + +final class AgentSessionOwnershipTests: XCTestCase { + func testUnclaimedDisconnectDoesNotTerminateAgent() { + var ownership = AgentSessionOwnership() + + XCTAssertFalse(ownership.disconnect(connection(12))) + } + + func testUnauthenticatedConnectionCannotClaimOrRetainAgent() { + var ownership = AgentSessionOwnership() + + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: false), .rejected) + XCTAssertFalse(ownership.disconnect(connection(12))) + } + + func testLastAuthenticatedDisconnectTerminatesAgent() { + var ownership = AgentSessionOwnership() + + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: true), .claimed) + XCTAssertTrue(ownership.disconnect(connection(12))) + } + + func testAgentWaitsForEveryAuthenticatedConnectionToClose() { + var ownership = AgentSessionOwnership() + + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: true), .claimed) + XCTAssertEqual(ownership.registerConnection(connection(13), authenticated: true), .retained) + XCTAssertFalse(ownership.disconnect(connection(12))) + XCTAssertTrue(ownership.disconnect(connection(13))) + } + + func testDuplicateRegistrationDoesNotRetainAgent() { + var ownership = AgentSessionOwnership() + + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: true), .claimed) + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: true), .rejected) + XCTAssertTrue(ownership.disconnect(connection(12))) + } + + func testClosedSessionRejectsNewConnections() { + var ownership = AgentSessionOwnership() + + XCTAssertEqual(ownership.registerConnection(connection(12), authenticated: true), .claimed) + XCTAssertTrue(ownership.disconnect(connection(12))) + XCTAssertEqual(ownership.registerConnection(connection(13), authenticated: true), .rejected) + XCTAssertFalse(ownership.disconnect(connection(13))) + } + + func testStaleDisconnectCannotRemoveReusedFileDescriptorOwner() { + var ownership = AgentSessionOwnership() + let staleConnection = connection(12) + let otherConnection = connection(13) + let reusedDescriptorConnection = connection(14) + + XCTAssertEqual(ownership.registerConnection(staleConnection, authenticated: true), .claimed) + XCTAssertEqual(ownership.registerConnection(otherConnection, authenticated: true), .retained) + XCTAssertFalse(ownership.disconnect(staleConnection)) + XCTAssertEqual( + ownership.registerConnection(reusedDescriptorConnection, authenticated: true), + .retained + ) + + XCTAssertFalse(ownership.disconnect(staleConnection)) + XCTAssertFalse(ownership.disconnect(otherConnection)) + XCTAssertTrue(ownership.disconnect(reusedDescriptorConnection)) + } + + func testTokenlessSessionStillRequiresAuthorizedPeer() { + XCTAssertFalse(isAuthenticatedAgentSession( + expectedToken: nil, + requestToken: nil, + authorizedPeer: false + )) + XCTAssertTrue(isAuthenticatedAgentSession( + expectedToken: nil, + requestToken: nil, + authorizedPeer: true + )) + } + + func testTokenSessionRequiresAuthorizedPeerAndMatchingToken() { + XCTAssertFalse(isAuthenticatedAgentSession( + expectedToken: "expected", + requestToken: "expected", + authorizedPeer: false + )) + XCTAssertFalse(isAuthenticatedAgentSession( + expectedToken: "expected", + requestToken: "wrong", + authorizedPeer: true + )) + XCTAssertTrue(isAuthenticatedAgentSession( + expectedToken: "expected", + requestToken: "expected", + authorizedPeer: true + )) + } +} + +private func connection(_ rawValue: UInt64) -> AgentSessionConnectionID { + AgentSessionConnectionID(rawValue: rawValue) +} diff --git a/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AuthenticatedConnectionHangupMonitorTests.swift b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AuthenticatedConnectionHangupMonitorTests.swift new file mode 100644 index 00000000000..1d335e98bb6 --- /dev/null +++ b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/AuthenticatedConnectionHangupMonitorTests.swift @@ -0,0 +1,195 @@ +import Darwin +import Dispatch +@testable import OrcaComputerUseMacOSCore +import XCTest + +final class AuthenticatedConnectionHangupMonitorTests: XCTestCase { + func testRejectsInvalidDescriptorDuringSetup() { + XCTAssertThrowsError( + try AuthenticatedConnectionHangupMonitor( + fileDescriptor: -1, + onHangup: {} + ) + ) + } + + func testRegistrationFailureClosesEventQueueOnce() { + let closes = DescriptorCloseRecorder() + + XCTAssertThrowsError( + try AuthenticatedConnectionHangupMonitor( + fileDescriptor: 42, + registerEvents: { _, _, _ in .EBADF }, + closeDescriptor: { descriptor in + closes.recordAndClose(descriptor) + }, + onHangup: {} + ) + ) + XCTAssertEqual(closes.count, 1) + } + + func testReportsPeerCloseWhileRequestHandlingIsBlocked() throws { + let descriptors = try makeSocketPair() + let hangup = expectation(description: "peer hangup") + let processingStarted = expectation(description: "processing started") + let releaseProcessing = DispatchSemaphore(value: 0) + let processingFinished = DispatchSemaphore(value: 0) + let monitor = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: descriptors.local, + onHangup: { + hangup.fulfill() + } + ) + monitor.start() + + DispatchQueue.global().async { + processingStarted.fulfill() + releaseProcessing.wait() + processingFinished.signal() + } + wait(for: [processingStarted], timeout: 1) + close(descriptors.peer) + + wait(for: [hangup], timeout: 1) + XCTAssertEqual(processingFinished.wait(timeout: .now() + 0.05), .timedOut) + + releaseProcessing.signal() + XCTAssertEqual(processingFinished.wait(timeout: .now() + 1), .success) + monitor.cancel() + close(descriptors.local) + } + + func testReadableDataDoesNotLookLikeHangup() throws { + let descriptors = try makeSocketPair() + let hangup = expectation(description: "peer hangup") + let callbacks = CallbackRecorder() + let events = CallbackRecorder() + let monitor = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: descriptors.local, + onEvent: { + events.record() + }, + onHangup: { + callbacks.record() + hangup.fulfill() + } + ) + monitor.start() + var byte: UInt8 = 7 + + XCTAssertEqual(write(descriptors.peer, &byte, 1), 1) + usleep(50_000) + XCTAssertEqual(callbacks.count, 0) + XCTAssertEqual(events.count, 0) + close(descriptors.peer) + wait(for: [hangup], timeout: 1) + XCTAssertEqual(events.count, 1) + + monitor.cancel() + close(descriptors.local) + } + + func testCancelPreventsLaterHangupCallback() throws { + let descriptors = try makeSocketPair() + let hangup = expectation(description: "peer hangup") + hangup.isInverted = true + let monitor = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: descriptors.local, + onHangup: { + hangup.fulfill() + } + ) + monitor.start() + + monitor.cancel() + close(descriptors.peer) + + wait(for: [hangup], timeout: 0.1) + close(descriptors.local) + } + + func testCancelReleasesMonitorWhileSocketStaysOpen() throws { + let descriptors = try makeSocketPair() + var monitor: AuthenticatedConnectionHangupMonitor? = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: descriptors.local, + onHangup: {} + ) + weak var retainedMonitor = monitor + + monitor?.cancel() + monitor = nil + + let deadline = Date().addingTimeInterval(1) + while retainedMonitor != nil, Date() < deadline { + usleep(10_000) + } + XCTAssertNil(retainedMonitor) + close(descriptors.peer) + close(descriptors.local) + } + + func testStartReportsHangupQueuedAfterSuccessfulSetup() throws { + let descriptors = try makeSocketPair() + let hangup = expectation(description: "queued peer hangup") + let monitor = try AuthenticatedConnectionHangupMonitor( + fileDescriptor: descriptors.local, + onHangup: { + hangup.fulfill() + } + ) + + close(descriptors.peer) + monitor.start() + + wait(for: [hangup], timeout: 1) + monitor.cancel() + close(descriptors.local) + } +} + +private final class CallbackRecorder: @unchecked Sendable { + private let lock = NSLock() + private var recordedCount = 0 + + var count: Int { + lock.lock() + defer { lock.unlock() } + return recordedCount + } + + func record() { + lock.lock() + recordedCount += 1 + lock.unlock() + } +} + +private final class DescriptorCloseRecorder: @unchecked Sendable { + private let lock = NSLock() + private var closeCount = 0 + + var count: Int { + lock.lock() + defer { lock.unlock() } + return closeCount + } + + func recordAndClose(_ descriptor: Int32) { + lock.lock() + closeCount += 1 + let shouldClose = closeCount == 1 + lock.unlock() + if shouldClose { + close(descriptor) + } + } +} + +private func makeSocketPair() throws -> (local: Int32, peer: Int32) { + var descriptors: [Int32] = [0, 0] + guard socketpair(AF_UNIX, SOCK_STREAM, 0, &descriptors) == 0 else { + throw POSIXError(.EIO) + } + return (descriptors[0], descriptors[1]) +} diff --git a/package.json b/package.json index 4c1e8474517..3c37bc18efb 100644 --- a/package.json +++ b/package.json @@ -105,8 +105,10 @@ "test:e2e:terminal-ime-native": "node config/scripts/run-terminal-ibus-hangul-e2e.mjs", "test:e2e:computer": "vitest run --config tests/e2e/vitest.config.ts", "bench:idle-cpu": "pnpm run ensure:electron-runtime && node config/scripts/run-idle-cpu-benchmark.mjs", + "bench:macos-computer-helper-owner-loss": "node config/scripts/macos-computer-helper-owner-loss-benchmark.mjs", "bench:startup": "pnpm run ensure:electron-runtime && node tools/benchmarks/startup-time-bench.mjs", "bench:daemon-coldstart": "pnpm run ensure:electron-runtime && node tools/benchmarks/daemon-coldstart-bench.mjs", + "bench:hang-watchdog-memory": "pnpm run ensure:electron-runtime && node config/scripts/hang-watchdog-memory-benchmark.mjs", "bench:main-thread-jank": "pnpm run ensure:electron-runtime && node tools/benchmarks/main-thread-jank-bench.mjs", "bench:worktree-deletion": "node tools/benchmarks/worktree-deletion-dev-bench.mjs", "bench:zustand-selector-fanout": "node config/scripts/zustand-selector-fanout-benchmark.mjs", diff --git a/src/main/hang-watchdog/hang-detection-marker.ts b/src/main/hang-watchdog/hang-detection-marker.ts index a6e6fbddd08..a32d872b8ec 100644 --- a/src/main/hang-watchdog/hang-detection-marker.ts +++ b/src/main/hang-watchdog/hang-detection-marker.ts @@ -1,7 +1,7 @@ import { readFileSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' -// Why: written by the plain-Node watchdog child when main-thread heartbeats stop, and rewritten if +// Why: written by the watchdog worker when main-thread heartbeats stop, and rewritten if // they resume; consumed on the next launch to report how long the stall lasted and whether it ever // cleared. `selfRecovered` separates a real deadlock from a long-but-survivable stall — the two are // indistinguishable at detection time, and only the latter would have been a destructive kill. diff --git a/src/main/hang-watchdog/hang-watchdog-child-loop.test.ts b/src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts similarity index 96% rename from src/main/hang-watchdog/hang-watchdog-child-loop.test.ts rename to src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts index 8d390c04e8e..8d1c3ebfcd1 100644 --- a/src/main/hang-watchdog/hang-watchdog-child-loop.test.ts +++ b/src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { createHangWatchdogChildLoop } from './hang-watchdog-child-loop' +import { createHangWatchdogDetectionLoop } from './hang-watchdog-detection-loop' const TIMEOUT_MS = 45_000 const CHECK_INTERVAL_MS = 5_000 @@ -8,7 +8,7 @@ function loopWithClock(startAt = 0) { let now = startAt const onHangDetected = vi.fn() const onHangResolved = vi.fn() - const loop = createHangWatchdogChildLoop({ + const loop = createHangWatchdogDetectionLoop({ timeoutMs: TIMEOUT_MS, checkIntervalMs: CHECK_INTERVAL_MS, now: () => now, @@ -18,7 +18,7 @@ function loopWithClock(startAt = 0) { return { loop, onHangDetected, onHangResolved, advance: (ms: number) => (now += ms) } } -describe('createHangWatchdogChildLoop', () => { +describe('createHangWatchdogDetectionLoop', () => { it('does not fire while heartbeats keep arriving', () => { const { loop, onHangDetected, advance } = loopWithClock() for (let i = 0; i < 100; i++) { diff --git a/src/main/hang-watchdog/hang-watchdog-child-loop.ts b/src/main/hang-watchdog/hang-watchdog-detection-loop.ts similarity index 86% rename from src/main/hang-watchdog/hang-watchdog-child-loop.ts rename to src/main/hang-watchdog/hang-watchdog-detection-loop.ts index 3a8ebb34fdf..9d45cba7797 100644 --- a/src/main/hang-watchdog/hang-watchdog-child-loop.ts +++ b/src/main/hang-watchdog/hang-watchdog-detection-loop.ts @@ -1,4 +1,4 @@ -export type HangWatchdogChildLoopConfig = { +export type HangWatchdogDetectionLoopConfig = { timeoutMs: number checkIntervalMs: number now: () => number @@ -7,14 +7,14 @@ export type HangWatchdogChildLoopConfig = { onHangResolved: (unresponsiveMs: number) => void } -export type HangWatchdogChildLoop = { +export type HangWatchdogDetectionLoop = { recordHeartbeat: () => void tick: () => void } -export function createHangWatchdogChildLoop( - config: HangWatchdogChildLoopConfig -): HangWatchdogChildLoop { +export function createHangWatchdogDetectionLoop( + config: HangWatchdogDetectionLoopConfig +): HangWatchdogDetectionLoop { let lastHeartbeatAt = config.now() let lastTickAt = config.now() let detected = false diff --git a/src/main/hang-watchdog/hang-watchdog-entry-path.ts b/src/main/hang-watchdog/hang-watchdog-entry-path.ts deleted file mode 100644 index 74f8b1d0089..00000000000 --- a/src/main/hang-watchdog/hang-watchdog-entry-path.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { existsSync } from 'node:fs' -import { join } from 'node:path' - -export function resolveHangWatchdogEntryPath( - appPath: string, - isPackaged: boolean, - pathExists: (candidate: string) => boolean = existsSync -): string { - // Why: ELECTRON_RUN_AS_NODE bypasses asar integration, so the packaged entry must be forked from app.asar.unpacked. - const basePath = isPackaged ? appPath.replace('app.asar', 'app.asar.unpacked') : appPath - const adjacentBuildEntry = join(basePath, 'main-thread-hang-watchdog-entry.js') - // Why: electron-vite's unpackaged appPath is already out/main; appending out/main again would silently disable the watchdog in dev and E2E builds. - if (!isPackaged && pathExists(adjacentBuildEntry)) { - return adjacentBuildEntry - } - return join(basePath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') -} diff --git a/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts b/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts new file mode 100644 index 00000000000..3ade5b1cbf3 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts @@ -0,0 +1,26 @@ +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { resolveHangWatchdogWorkerPath } from './hang-watchdog-worker-path' + +describe('resolveHangWatchdogWorkerPath', () => { + it('resolves packaged workers inside app.asar', () => { + const appPath = join('/apps', 'orca', 'app.asar') + expect(resolveHangWatchdogWorkerPath(appPath, true)).toBe( + join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + }) + + it('uses an adjacent entry when the dev app path is out/main', () => { + const appPath = join('/repo', 'out', 'main') + const adjacent = join(appPath, 'main-thread-hang-watchdog-entry.js') + const pathExists = vi.fn((candidate: string) => candidate === adjacent) + expect(resolveHangWatchdogWorkerPath(appPath, false, pathExists)).toBe(adjacent) + }) + + it('resolves through out/main from a dev project root', () => { + const appPath = join('/repo', 'orca') + expect(resolveHangWatchdogWorkerPath(appPath, false, () => false)).toBe( + join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + }) +}) diff --git a/src/main/hang-watchdog/hang-watchdog-worker-path.ts b/src/main/hang-watchdog/hang-watchdog-worker-path.ts new file mode 100644 index 00000000000..7efc300cb45 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-path.ts @@ -0,0 +1,14 @@ +import { existsSync } from 'node:fs' +import { join } from 'node:path' + +export function resolveHangWatchdogWorkerPath( + appPath: string, + isPackaged: boolean, + pathExists: (candidate: string) => boolean = existsSync +): string { + const adjacentBuildEntry = join(appPath, 'main-thread-hang-watchdog-entry.js') + if (!isPackaged && pathExists(adjacentBuildEntry)) { + return adjacentBuildEntry + } + return join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') +} diff --git a/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts b/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts new file mode 100644 index 00000000000..14e6d007796 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts @@ -0,0 +1,12 @@ +export const HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS = 2_000 +export const HANG_WATCHDOG_TIMEOUT_MS = 45_000 +export const HANG_WATCHDOG_CHECK_INTERVAL_MS = 5_000 + +export type HangWatchdogWorkerData = { + parentPid: number + markerPath: string + timeoutMs: number + checkIntervalMs: number +} + +export type MainToHangWatchdogWorkerMessage = { type: 'heartbeat' } | { type: 'shutdown' } diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts index 0f36e8acad9..a1ececdadb4 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts @@ -12,7 +12,11 @@ vi.mock('node:child_process', () => ({ spawn: spawnMock })) -import { recordHangObservation } from './main-thread-hang-watchdog-entry' +import { + isHangWatchdogWorkerData, + recordHangObservation, + runWatchdog +} from './main-thread-hang-watchdog-entry' describe('recordHangObservation', () => { let dir: string @@ -67,10 +71,8 @@ describe('recordHangObservation', () => { }) }) - // Why: this is the safety contract of the whole PR. Observing a hang must never kill, relaunch, - // or exit — a false positive would SIGKILL a live main thread mid-write. If a future change - // reintroduces recovery, this test must fail loudly rather than ship silently. it('never spawns, signals, or exits', () => { + // Why: a false positive must never kill a live main thread while writes may be in flight. recordHangObservation({ parentPid: 4242, markerPath: join(dir, 'marker.json'), @@ -104,3 +106,75 @@ describe('recordHangObservation', () => { ).not.toThrow() }) }) + +describe('watchdog worker entry', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(0) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('accepts only complete positive worker data', () => { + expect( + isHangWatchdogWorkerData({ + parentPid: 42, + markerPath: '/tmp/marker', + timeoutMs: 100, + checkIntervalMs: 25 + }) + ).toBe(true) + for (const value of [ + null, + {}, + { parentPid: 0, markerPath: '/tmp/marker', timeoutMs: 100, checkIntervalMs: 25 }, + { parentPid: 42, markerPath: 7, timeoutMs: 100, checkIntervalMs: 25 }, + { parentPid: 42, markerPath: '/tmp/marker', timeoutMs: 0, checkIntervalMs: 25 }, + { parentPid: 42, markerPath: '/tmp/marker', timeoutMs: 100, checkIntervalMs: Infinity } + ]) { + expect(isHangWatchdogWorkerData(value)).toBe(false) + } + }) + + it('routes heartbeats and shuts down its timer and port', () => { + const markerPath = join(tmpdir(), `hang-watchdog-entry-${process.pid}.json`) + let onMessage: ((message: { type: 'heartbeat' | 'shutdown' }) => void) | undefined + const port = { + on: vi.fn( + (_event: 'message', listener: (message: { type: 'heartbeat' | 'shutdown' }) => void) => { + onMessage = listener + } + ), + close: vi.fn() + } + try { + runWatchdog( + { + parentPid: process.pid, + markerPath, + timeoutMs: 100, + checkIntervalMs: 25 + }, + port + ) + vi.advanceTimersByTime(75) + onMessage?.({ type: 'heartbeat' }) + vi.advanceTimersByTime(75) + expect(consumeHangDetectionMarker(markerPath)).toBeNull() + vi.advanceTimersByTime(50) + expect(consumeHangDetectionMarker(markerPath)).toMatchObject({ selfRecovered: false }) + + onMessage?.({ type: 'heartbeat' }) + expect(consumeHangDetectionMarker(markerPath)).toMatchObject({ selfRecovered: true }) + onMessage?.({ type: 'shutdown' }) + expect(port.close).toHaveBeenCalledOnce() + vi.advanceTimersByTime(1_000) + expect(port.close).toHaveBeenCalledOnce() + expect(consumeHangDetectionMarker(markerPath)).toBeNull() + } finally { + rmSync(markerPath, { force: true }) + } + }) +}) diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts index 8d45c725a72..2c60b17e9ff 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts @@ -1,20 +1,17 @@ -// Forked with ELECTRON_RUN_AS_NODE from the main process. Watches heartbeats from the main -// thread; if they stop (e.g. the macOS 26 AppKit scene-update deadlock) it records a marker so -// the next launch can report the stall, and rewrites it if the heartbeats come back. -// -// Why no kill: a deadlocked main thread has already lost whatever sat in the persistence debounce -// window, so killing it recovers nothing the user could not recover by force-quitting. A false -// positive, though, would SIGKILL a live main thread that was merely blocked — most plausibly on -// I/O, i.e. exactly when writes are in flight. All of the downside sits in the misfire, so this -// measures first: `selfRecovered` counts the stalls a killer would have gotten wrong. -// -// Must never import electron. -import { createHangWatchdogChildLoop } from './hang-watchdog-child-loop' +import { isMainThread, parentPort, workerData } from 'node:worker_threads' +import { createHangWatchdogDetectionLoop } from './hang-watchdog-detection-loop' import { writeHangDetectionMarker } from './hang-detection-marker' +import type { + HangWatchdogWorkerData, + MainToHangWatchdogWorkerMessage +} from './hang-watchdog-worker-protocol' -const DEFAULT_TIMEOUT_MS = 45_000 -const DEFAULT_CHECK_INTERVAL_MS = 5_000 +type HangWatchdogPort = { + on: (event: 'message', listener: (message: MainToHangWatchdogWorkerMessage) => void) => unknown + close: () => void +} +// Observation only: a false positive must never kill a live main thread mid-write. export function recordHangObservation(options: { parentPid: number markerPath: string @@ -36,37 +33,65 @@ export function recordHangObservation(options: { } } -function runWatchdog(parentPid: number): void { - const markerPath = process.env.ORCA_HANG_WATCHDOG_MARKER_PATH ?? '' - const timeoutMs = Number(process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS) || DEFAULT_TIMEOUT_MS - const checkIntervalMs = - Number(process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS) || DEFAULT_CHECK_INTERVAL_MS - - const loop = createHangWatchdogChildLoop({ - timeoutMs, - checkIntervalMs, +export function runWatchdog( + config: HangWatchdogWorkerData, + port: HangWatchdogPort | null = parentPort +): void { + if (!port) { + return + } + const loop = createHangWatchdogDetectionLoop({ + timeoutMs: config.timeoutMs, + checkIntervalMs: config.checkIntervalMs, now: () => Date.now(), onHangDetected: (unresponsiveMs) => - recordHangObservation({ parentPid, markerPath, unresponsiveMs, selfRecovered: false }), + recordHangObservation({ + parentPid: config.parentPid, + markerPath: config.markerPath, + unresponsiveMs, + selfRecovered: false + }), // Why: rewriting the marker keeps one observation per stall rather than two rows to reconcile. onHangResolved: (unresponsiveMs) => - recordHangObservation({ parentPid, markerPath, unresponsiveMs, selfRecovered: true }) + recordHangObservation({ + parentPid: config.parentPid, + markerPath: config.markerPath, + unresponsiveMs, + selfRecovered: true + }) }) - process.on('message', (message) => { - const type = (message as { type?: string } | null)?.type - if (type === 'heartbeat') { + let checkTimer: ReturnType | null = setInterval( + () => loop.tick(), + config.checkIntervalMs + ) + port.on('message', (message: MainToHangWatchdogWorkerMessage) => { + if (message.type === 'heartbeat') { loop.recordHeartbeat() - } else if (type === 'shutdown') { - process.exit(0) + } else if (message.type === 'shutdown') { + if (checkTimer) { + clearInterval(checkTimer) + checkTimer = null + } + port.close() } }) - // Why: a normal parent exit closes the IPC channel; the watchdog must not outlive it and misfire. - process.on('disconnect', () => process.exit(0)) - setInterval(() => loop.tick(), checkIntervalMs) } -const configuredParentPid = Number(process.env.ORCA_HANG_WATCHDOG_PARENT_PID) -if (Number.isInteger(configuredParentPid) && configuredParentPid > 0) { - runWatchdog(configuredParentPid) +export function isHangWatchdogWorkerData(value: unknown): value is HangWatchdogWorkerData { + const data = value as Partial | null + return ( + !!data && + Number.isInteger(data.parentPid) && + (data.parentPid ?? 0) > 0 && + typeof data.markerPath === 'string' && + Number.isFinite(data.timeoutMs) && + (data.timeoutMs ?? 0) > 0 && + Number.isFinite(data.checkIntervalMs) && + (data.checkIntervalMs ?? 0) > 0 + ) +} + +if (!isMainThread && isHangWatchdogWorkerData(workerData)) { + runWatchdog(workerData) } diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts b/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts index b5c1063a915..eaaea54225b 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts @@ -1,7 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' -const { forkMock, appMock } = vi.hoisted(() => ({ - forkMock: vi.fn(), +const { workerState, appMock } = vi.hoisted(() => ({ + workerState: { + calls: [] as unknown[][], + instance: null as object | null, + error: null as Error | null + }, appMock: { isPackaged: true, getAppPath: vi.fn(() => '/apps/orca/app.asar'), @@ -9,8 +14,16 @@ const { forkMock, appMock } = vi.hoisted(() => ({ } })) -vi.mock('node:child_process', () => ({ - fork: forkMock +vi.mock('node:worker_threads', () => ({ + Worker: class WorkerMock { + constructor(...args: unknown[]) { + workerState.calls.push(args) + if (workerState.error) { + throw workerState.error + } + return workerState.instance as WorkerMock + } + } })) vi.mock('electron', () => ({ @@ -29,28 +42,33 @@ function withPlatform(platform: NodeJS.Platform, run: () => T): T { } } -function fakeChild() { +function fakeWorker() { return { - connected: true, - stderr: { on: vi.fn() }, + postMessage: vi.fn(), + unref: vi.fn(), on: vi.fn(), - send: vi.fn(), - disconnect: vi.fn(), - kill: vi.fn() + once: vi.fn() } } describe('installMainThreadHangWatchdog', () => { beforeEach(() => { vi.useFakeTimers() - forkMock.mockReset() + workerState.calls = [] + workerState.instance = null + workerState.error = null appMock.on.mockReset() appMock.isPackaged = true delete process.env.ORCA_HANG_WATCHDOG_FORCE + delete process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS + delete process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS }) afterEach(() => { vi.useRealTimers() + delete process.env.ORCA_HANG_WATCHDOG_FORCE + delete process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS + delete process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS }) it('is a no-op off macOS', () => { @@ -60,7 +78,7 @@ describe('installMainThreadHangWatchdog', () => { expect( withPlatform('linux', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() - expect(forkMock).not.toHaveBeenCalled() + expect(workerState.calls).toHaveLength(0) }) it('is a no-op in unpackaged builds unless forced', () => { @@ -69,62 +87,98 @@ describe('installMainThreadHangWatchdog', () => { withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() process.env.ORCA_HANG_WATCHDOG_FORCE = '1' - const child = fakeChild() - forkMock.mockReturnValue(child) + const worker = fakeWorker() + workerState.instance = worker expect( withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).not.toBeNull() delete process.env.ORCA_HANG_WATCHDOG_FORCE }) - it('forks the watchdog as plain Node with pid, bundle, and marker config', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + it('starts a worker with pid, marker, and timing config', () => { + const worker = fakeWorker() + workerState.instance = worker const handle = withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' }) ) expect(handle).not.toBeNull() - const [, , options] = forkMock.mock.calls[0] - expect(options.env.ELECTRON_RUN_AS_NODE).toBe('1') - expect(options.env.ORCA_HANG_WATCHDOG_PARENT_PID).toBe(String(process.pid)) - expect(options.env.ORCA_HANG_WATCHDOG_MARKER_PATH).toContain('/ud') + const [workerPath, rawOptions] = workerState.calls[0] + const options = rawOptions as { + name: string + workerData: { + parentPid: number + markerPath: string + timeoutMs: number + checkIntervalMs: number + } + } + expect(workerPath).toBe( + join('/apps/orca/app.asar', 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + expect(options.name).toBe('orca-main-thread-hang-watchdog') + expect(options.workerData).toMatchObject({ + parentPid: process.pid, + markerPath: join('/ud', 'main-thread-hang.json'), + timeoutMs: 45_000, + checkIntervalMs: 5_000 + }) + expect(worker.unref).toHaveBeenCalled() }) - it('sends heartbeats on an interval and shutdown+disconnect on stop', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + it('sends heartbeats on an interval and shutdown on stop', () => { + const worker = fakeWorker() + workerState.instance = worker const handle = withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' }) ) vi.advanceTimersByTime(6_000) - const heartbeats = child.send.mock.calls.filter(([m]) => m.type === 'heartbeat') + const heartbeats = worker.postMessage.mock.calls.filter(([m]) => m.type === 'heartbeat') expect(heartbeats.length).toBe(3) handle?.stop() - expect(child.send.mock.calls.some(([m]) => m.type === 'shutdown')).toBe(true) - expect(child.disconnect).toHaveBeenCalled() + expect(worker.postMessage.mock.calls.some(([m]) => m.type === 'shutdown')).toBe(true) - // Why: quit fires will-quit twice; a second stop must not resend or throw. handle?.stop() - const shutdowns = child.send.mock.calls.filter(([m]) => m.type === 'shutdown') + const shutdowns = worker.postMessage.mock.calls.filter(([m]) => m.type === 'shutdown') expect(shutdowns.length).toBe(1) vi.advanceTimersByTime(10_000) - const heartbeatsAfterStop = child.send.mock.calls.filter(([m]) => m.type === 'heartbeat') + const heartbeatsAfterStop = worker.postMessage.mock.calls.filter( + ([m]) => m.type === 'heartbeat' + ) expect(heartbeatsAfterStop.length).toBe(3) }) + it('passes test timing overrides to the worker', () => { + process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS = '900' + process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS = '100' + workerState.instance = fakeWorker() + withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) + const options = workerState.calls[0][1] as { + workerData: { timeoutMs: number; checkIntervalMs: number } + } + expect(options.workerData).toMatchObject({ timeoutMs: 900, checkIntervalMs: 100 }) + }) + it('registers stop on will-quit', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + workerState.instance = fakeWorker() withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) expect(appMock.on).toHaveBeenCalledWith('will-quit', expect.any(Function)) }) - it('returns null and stays inert when the fork itself fails', () => { - forkMock.mockImplementation(() => { - throw new Error('spawn failure') - }) + it('stops heartbeat work when the worker exits', () => { + const worker = fakeWorker() + workerState.instance = worker + withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) + const exitListener = worker.once.mock.calls.find(([event]) => event === 'exit')?.[1] + expect(exitListener).toEqual(expect.any(Function)) + exitListener() + vi.advanceTimersByTime(6_000) + expect(worker.postMessage).not.toHaveBeenCalled() + }) + + it('returns null and stays inert when worker construction fails', () => { + workerState.error = new Error('worker failure') expect( withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog.ts b/src/main/hang-watchdog/main-thread-hang-watchdog.ts index f859f8b5745..46454429fc7 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog.ts @@ -1,19 +1,25 @@ -import { fork, type ChildProcess } from 'node:child_process' +import { Worker } from 'node:worker_threads' import { app } from 'electron' -import { resolveHangWatchdogEntryPath } from './hang-watchdog-entry-path' import { hangDetectionMarkerPath } from './hang-detection-marker' - -const HEARTBEAT_INTERVAL_MS = 2_000 +import { resolveHangWatchdogWorkerPath } from './hang-watchdog-worker-path' +import { + HANG_WATCHDOG_CHECK_INTERVAL_MS, + HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS, + HANG_WATCHDOG_TIMEOUT_MS, + type HangWatchdogWorkerData, + type MainToHangWatchdogWorkerMessage +} from './hang-watchdog-worker-protocol' export type MainThreadHangWatchdogHandle = { stop: () => void - child: ChildProcess + worker: Worker +} + +function positiveTiming(value: string | undefined, fallback: number): number { + const parsed = Number(value) + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback } -// Why: macOS 26 scene-backed AppKit windows can deadlock the main thread inside -// FrontBoardServices with no crash and no event loop left to self-report. A plain-Node sibling -// process watches heartbeats and records the stall so the next launch can report it. It observes -// only — see main-thread-hang-watchdog-entry.ts for why it does not kill the parent. export function installMainThreadHangWatchdog(options: { userDataPath: string }): MainThreadHangWatchdogHandle | null { @@ -24,58 +30,57 @@ export function installMainThreadHangWatchdog(options: { if (!app.isPackaged && process.env.ORCA_HANG_WATCHDOG_FORCE !== '1') { return null } - const entryPath = resolveHangWatchdogEntryPath(app.getAppPath(), app.isPackaged) - let child: ChildProcess + const workerPath = resolveHangWatchdogWorkerPath(app.getAppPath(), app.isPackaged) + const workerData: HangWatchdogWorkerData = { + parentPid: process.pid, + markerPath: hangDetectionMarkerPath(options.userDataPath), + timeoutMs: positiveTiming(process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS, HANG_WATCHDOG_TIMEOUT_MS), + checkIntervalMs: positiveTiming( + process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS, + HANG_WATCHDOG_CHECK_INTERVAL_MS + ) + } + let worker: Worker try { - child = fork(entryPath, [], { - stdio: ['ignore', 'ignore', 'pipe', 'ipc'], - env: { - ...process.env, - ELECTRON_RUN_AS_NODE: '1', - ORCA_HANG_WATCHDOG_PARENT_PID: String(process.pid), - ORCA_HANG_WATCHDOG_MARKER_PATH: hangDetectionMarkerPath(options.userDataPath) - } + // Why: the worker survives an AppKit main-thread deadlock without another Electron process. + worker = new Worker(workerPath, { + name: 'orca-main-thread-hang-watchdog', + workerData }) } catch (error) { - console.error('[hang-watchdog] failed to fork watchdog process:', error) + console.error('[hang-watchdog] failed to start watchdog worker:', error) return null } - child.stderr?.on('data', (chunk: Buffer) => { - console.error('[hang-watchdog]', String(chunk).trimEnd()) + worker.on('error', (error) => { + console.error('[hang-watchdog] watchdog worker failed:', error) }) - // Why: the watchdog is a safety net — if it dies, run without it; a restart loop here must never affect the app. - child.on('error', () => {}) - const heartbeatTimer = setInterval(() => { - if (child.connected) { - try { - child.send({ type: 'heartbeat' }, () => {}) - } catch { - // Channel raced closed between the check and the send. - } - } - }, HEARTBEAT_INTERVAL_MS) let stopped = false + const postMessage = (message: MainToHangWatchdogWorkerMessage): void => { + if (stopped && message.type === 'heartbeat') { + return + } + try { + worker.postMessage(message) + } catch { + // The worker already exited. + } + } + const heartbeatTimer = setInterval(() => { + postMessage({ type: 'heartbeat' }) + }, HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS) const stop = (): void => { if (stopped) { return } stopped = true clearInterval(heartbeatTimer) - if (child.connected) { - try { - child.send({ type: 'shutdown' }, () => {}) - } catch { - // Already disconnecting. - } - } - // Why: disconnect guarantees the child observes parent shutdown and exits instead of misreading quit as a hang. - try { - child.disconnect() - } catch { - // Channel already closed. - } + postMessage({ type: 'shutdown' }) } - // Why: will-quit fires twice during quit; stop is idempotent. + worker.once('exit', () => { + stopped = true + clearInterval(heartbeatTimer) + }) + worker.unref() app.on('will-quit', stop) - return { stop, child } + return { stop, worker } } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 8c41f25690a..3855ad4e37b 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -25026,6 +25026,117 @@ describe('OrcaRuntimeService', () => { ]) }) + it('keeps a live headed runtime-owned tab until its explicit close', async () => { + const ptyId = 'local-runtime-owned-pty' + const splitPtyId = 'local-runtime-owned-split-pty' + const tabId = 'runtime-session-tab' + const leafId = HEADLESS_LEAF_ID + const kill = vi.fn(() => true) + const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession({ + ...getDefaultWorkspaceSession(), + activeRepoId: TEST_REPO_ID, + activeWorktreeId: TEST_WORKTREE_ID, + tabsByWorktree: { + [TEST_WORKTREE_ID]: [ + { + id: tabId, + ptyId: null, + worktreeId: TEST_WORKTREE_ID, + title: 'Codex', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + launchAgent: 'codex' + } + ] + }, + terminalLayoutsByTabId: { + [tabId]: makeHeadlessTerminalLayout({ [leafId]: undefined }) + } + }) + const runtime = new OrcaRuntimeService(runtimeStore as never) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValueOnce({ id: ptyId }).mockResolvedValueOnce({ id: splitPtyId }), + write: () => true, + kill, + getForegroundProcess: async () => null, + listProcesses: async () => [ + { + id: ptyId, + cwd: TEST_WORKTREE_PATH, + title: 'Codex', + worktreeId: TEST_WORKTREE_ID + }, + { + id: splitPtyId, + cwd: TEST_WORKTREE_PATH, + title: 'Codex', + worktreeId: TEST_WORKTREE_ID + } + ] + }) + const publishRendererOmission = (snapshotVersion: number): void => { + runtime.syncWindowGraph(1, { + tabs: [], + leaves: [], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'headed-runtime', + snapshotVersion, + activeGroupId: 'group-1', + activeTabId: null, + activeTabType: null, + tabs: [] + } + ] + }) + } + runtime.attachWindow(1) + publishRendererOmission(1) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { send: vi.fn() } + }) + + const created = await runtime.createTerminal(`id:${TEST_WORKTREE_ID}`, { + presentation: 'background', + persistHostSessionBinding: true, + tabId, + leafId, + launchAgent: 'codex' + }) + const split = await runtime.splitTerminal(created.handle, { direction: 'vertical' }) + publishRendererOmission(2) + + expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: `${tabId}::${leafId}`, + parentTabId: tabId, + ptyId, + status: 'ready', + terminal: created.handle + }), + expect.objectContaining({ + parentTabId: tabId, + ptyId: splitPtyId, + status: 'ready', + terminal: split.handle + }) + ]) + ) + expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toHaveLength(2) + + await runtime.closeMobileSessionTab(`id:${TEST_WORKTREE_ID}`, tabId, { reason: 'user' }) + publishRendererOmission(3) + + expect(kill).toHaveBeenCalledWith(ptyId) + expect(kill).toHaveBeenCalledWith(splitPtyId) + expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) + }) + it('publishes laptop-created remote runtime terminals to phone session tabs', async () => { const spawn = vi.fn().mockResolvedValue({ id: 'laptop-created-pty' }) const runtime = new OrcaRuntimeService(store) @@ -29289,7 +29400,7 @@ describe('OrcaRuntimeService', () => { { tabId: 'tab-renderer', worktreeId: TEST_WORKTREE_ID, - leafId: 'pane:1', + leafId: HEADLESS_LEAF_ID, paneRuntimeId: 1, ptyId: 'pty-renderer', paneTitle: null @@ -29306,10 +29417,12 @@ describe('OrcaRuntimeService', () => { tabs: [ { type: 'terminal', - id: 'tab-renderer::pane:1', + id: `tab-renderer::${HEADLESS_LEAF_ID}`, parentTabId: 'tab-renderer', - leafId: 'pane:1', + leafId: HEADLESS_LEAF_ID, + ptyId: 'pty-renderer', title: 'Terminal', + viewMode: 'chat', isActive: false } ] @@ -29350,6 +29463,86 @@ describe('OrcaRuntimeService', () => { ) expect(focusTerminal).not.toHaveBeenCalled() expect(result.tab).toMatchObject({ parentTabId: 'tab-renderer', isActive: false }) + + runtime.syncWindowGraph(1, { + tabs: [], + leaves: [ + { + tabId: 'tab-renderer', + worktreeId: TEST_WORKTREE_ID, + leafId: HEADLESS_LEAF_ID, + paneRuntimeId: 1, + ptyId: 'pty-renderer', + paneTitle: null + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 2, + activeGroupId: 'group-1', + activeTabId: null, + activeTabType: null, + tabs: [] + } + ] + }) + + expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([ + expect.objectContaining({ parentTabId: 'tab-renderer', ptyId: 'pty-renderer' }) + ]) + + runtime.syncWindowGraph(1, { + tabs: [], + leaves: [ + { + tabId: 'tab-renderer', + worktreeId: TEST_WORKTREE_ID, + leafId: HEADLESS_SECOND_LEAF_ID, + paneRuntimeId: 1, + ptyId: 'pty-renderer', + paneTitle: null + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 3, + activeGroupId: 'group-1', + activeTabId: null, + activeTabType: null, + tabs: [] + } + ] + }) + runtime.syncWindowGraph(1, { + tabs: [], + leaves: [ + { + tabId: 'tab-renderer', + worktreeId: TEST_WORKTREE_ID, + leafId: HEADLESS_SECOND_LEAF_ID, + paneRuntimeId: 1, + ptyId: 'pty-renderer', + paneTitle: null + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 4, + activeGroupId: 'group-1', + activeTabId: null, + activeTabType: null, + tabs: [] + } + ] + }) + + expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) }) it('dedupes concurrent mobile terminal creates that share a clientMutationId', async () => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 1328dc5c6ed..57d80f75a59 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1216,6 +1216,7 @@ type RuntimePtyWorktreeRecord = { incarnationId: PtyIncarnationId | null worktreeId: string connectionId: string | null + runtimeSessionOwned: boolean // Why: a Windows host can own both native and WSL panes; preamble command // selection must follow the pane that executes it, not process.platform. isWsl: boolean | null @@ -5982,6 +5983,35 @@ export class OrcaRuntimeService { return boundPtyIds.some((ptyId) => persistedPtyIds.has(ptyId)) } + private hasLiveRuntimeSessionOwnedPtyBinding( + worktreeId: string, + tab: RuntimeMobileSessionTerminalTab + ): boolean { + const pty = this.findPtyForMobileTerminalTab(worktreeId, tab) + return pty?.connected === true && pty.runtimeSessionOwned + } + + private clearRuntimeSessionOwnershipForMobileTab( + worktreeId: string, + snapshot: RuntimeMobileSessionTabsSnapshot, + parentTabId: string + ): void { + for (const tab of snapshot.tabs) { + if (tab.type !== 'terminal' || tab.parentTabId !== parentTabId) { + continue + } + const ptyIds = [tab.ptyId, ...Object.values(tab.parentLayout?.ptyIdsByLeafId ?? {})].filter( + (ptyId): ptyId is string => typeof ptyId === 'string' + ) + for (const ptyId of ptyIds) { + const pty = this.ptysById.get(ptyId) + if (pty?.worktreeId === worktreeId && pty.tabId === parentTabId) { + pty.runtimeSessionOwned = false + } + } + } + } + // Why: a tab needs authoritative runtime teardown (kill + de-persist + prune) // only when the renderer can't durably tear it down: either it's serve/SSH // (preserved + re-hydrated, would resurrect) or the renderer graph never @@ -7440,6 +7470,7 @@ export class OrcaRuntimeService { this.notifyRendererOfHeadlessTerminalClose(tab.parentTabId) this.store?.flushOrThrow?.() } + this.clearRuntimeSessionOwnershipForMobileTab(worktreeId, snapshot!, tab.parentTabId) return { closed: true } } // Why: notifier implementations without the acknowledged relay may expose @@ -7467,6 +7498,7 @@ export class OrcaRuntimeService { // parent tab id. Closing that parent should close the desktop tab, not // just whichever leaf happened to be first in the session snapshot. this.notifier?.closeTerminal(tab.parentTabId) + this.clearRuntimeSessionOwnershipForMobileTab(worktreeId, snapshot!, tab.parentTabId) } } else if (tab.type === 'browser' && this.offscreenBrowserBackend) { // Why: headless browser tabs are offscreen WebContents with no renderer to @@ -7611,6 +7643,7 @@ export class OrcaRuntimeService { options: { killPtys?: boolean } = {} ): void { const closedParentTabId = tab.parentTabId + this.clearRuntimeSessionOwnershipForMobileTab(worktreeId, snapshot, closedParentTabId) const projectedPtyIds = this.removePersistedHeadlessTerminalTab(worktreeId, closedParentTabId) // Why: local provider ids can be reused after restart, so a dormant // persisted id is not kill authority. SSH relay ids remain durable exact @@ -8782,6 +8815,9 @@ export class OrcaRuntimeService { this.recordPtyWorktree(ptyId, worktreeId, { connected: true, connectionId, + ...(binding && this.pendingMobileTerminalCreatesByKey.has(`${worktreeId}::${binding.tabId}`) + ? { runtimeSessionOwned: true } + : {}), ...(isWsl !== undefined ? { isWsl } : {}), ...(binding && paneKey ? { tabId: binding.tabId, paneKey } : {}), ...(binding?.incarnationId ? { incarnationId: binding.incarnationId } : {}) @@ -12806,6 +12842,7 @@ export class OrcaRuntimeService { this.agentDetector?.onExit(ptyId) if (pty) { pty.connected = false + pty.runtimeSessionOwned = false pty.disconnectedAt = Date.now() pty.lastExitCode = exitCode this.resolvePtyExitWaiters(pty, ptyId) @@ -24193,6 +24230,9 @@ export class OrcaRuntimeService { }) const pty = this.getOrCreatePtyWorktreeRecord(result.id) if (pty) { + if (launchOpts.persistHostSessionBinding) { + pty.runtimeSessionOwned = true + } if (launchOpts.title) { const observedAt = this.nextTitleObservationSequence() pty.title = launchOpts.title @@ -25101,6 +25141,10 @@ export class OrcaRuntimeService { return null } const existing = this.findMobileTerminalSurface(worktreeId, tabId) + const pty = this.findLiveRegisteredPtyForRendererTab(worktreeId, tabId) + if (pty) { + pty.runtimeSessionOwned = true + } if ( existing && this.isReadyMobileTerminalSurface(existing) && @@ -25109,7 +25153,6 @@ export class OrcaRuntimeService { // Why: the renderer's ready publication already landed with the intended mode; only a pending shell needs the main-side rescue. return existing } - const pty = this.findLiveRegisteredPtyForRendererTab(worktreeId, tabId) const leafId = pty ? parsePaneKey(pty.paneKey ?? '')?.leafId : undefined if (!pty || !leafId) { return existing @@ -25554,6 +25597,7 @@ export class OrcaRuntimeService { if (createdPty) { createdPty.tabId = parentTabId createdPty.paneKey = paneKey + createdPty.runtimeSessionOwned = pty.runtimeSessionOwned } try { @@ -27428,6 +27472,7 @@ export class OrcaRuntimeService { | 'paneKey' | 'title' | 'connectionId' + | 'runtimeSessionOwned' | 'isWsl' | 'wslDistro' | 'incarnationId' @@ -27452,6 +27497,7 @@ export class OrcaRuntimeService { incarnationId: state.incarnationId ?? null, worktreeId, connectionId, + runtimeSessionOwned: state.runtimeSessionOwned ?? false, isWsl: state.isWsl ?? null, wslDistro, tabId: state.tabId ?? null, @@ -27509,6 +27555,9 @@ export class OrcaRuntimeService { this.wslDistroByPtyId.delete(ptyId) } } + if (state.runtimeSessionOwned !== undefined) { + pty.runtimeSessionOwned = state.runtimeSessionOwned + } if (state.isWsl !== undefined) { pty.isWsl = state.isWsl } @@ -28303,6 +28352,7 @@ export class OrcaRuntimeService { // are runtime-owned and preservable. return ( this.isHeadlessBuiltMobileSessionPublicationBase(snapshot.publicationEpoch) || + this.hasLiveRuntimeSessionOwnedPtyBinding(snapshot.worktree, tab) || this.hasLiveOrPersistedServeOrSshOwnedPtyBinding(snapshot.worktree, tab) ) } diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index e5b5d7387d8..cda226ef620 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -445,6 +445,7 @@ function App(): React.JSX.Element { toggleSidebar: s.toggleSidebar, fetchRepos: s.fetchRepos, fetchReposForAllHosts: s.fetchReposForAllHosts, + awaitLocalRepoCatalogSettlement: s.awaitLocalRepoCatalogSettlement, fetchProjectGroups: s.fetchProjectGroups, fetchProjectGroupsForAllHosts: s.fetchProjectGroupsForAllHosts, fetchFolderWorkspaces: s.fetchFolderWorkspaces, @@ -899,6 +900,9 @@ function App(): React.JSX.Element { await timeRendererStartupStep('fetch-repos-local', () => actions.fetchReposForAllHosts({ remoteHosts: 'skip' }) ) + await timeRendererStartupStep('repo-catalog-settlement', () => + actions.awaitLocalRepoCatalogSettlement() + ) // Why: folder workspaces merge against projectGroups (repos.ts fetchFolderWorkspacesForAllHosts), // so keep this chain ordered while overlapping it with session-scoped hydration. const localCatalogChain = (async () => { @@ -951,6 +955,9 @@ function App(): React.JSX.Element { } const sessionRead = sessionOutcome.value await keybindingsPromise + await timeRendererStartupStep('repo-catalog-final-settlement', () => + actions.awaitLocalRepoCatalogSettlement() + ) if (!cancelled) { const sessionHydrationOptions = { additionalValidWorkspaceKeys: collectFolderWorkspaceKeysFromSession(sessionRead.session) diff --git a/src/renderer/src/app-startup-routing.test.ts b/src/renderer/src/app-startup-routing.test.ts index 85dd64ba574..f638ae82083 100644 --- a/src/renderer/src/app-startup-routing.test.ts +++ b/src/renderer/src/app-startup-routing.test.ts @@ -20,6 +20,12 @@ describe('renderer startup runtime routing', () => { const localReposIndex = indexInStartupBlock( "actions.fetchReposForAllHosts({ remoteHosts: 'skip' })" ) + const repoCatalogSettlementIndex = indexInStartupBlock( + "timeRendererStartupStep('repo-catalog-settlement'" + ) + const finalRepoCatalogSettlementIndex = indexInStartupBlock( + "timeRendererStartupStep('repo-catalog-final-settlement'" + ) const localGroupsIndex = indexInStartupBlock( "actions.fetchProjectGroupsForAllHosts({ remoteHosts: 'skip' })" ) @@ -39,6 +45,10 @@ describe('renderer startup runtime routing', () => { expect(settingsIndex).toBeLessThan(uiGetIndex) expect(uiGetIndex).toBeLessThan(hydrateUiIndex) expect(hydrateUiIndex).toBeLessThan(localReposIndex) + expect(localReposIndex).toBeLessThan(repoCatalogSettlementIndex) + expect(repoCatalogSettlementIndex).toBeLessThan(sessionIndex) + expect(sessionIndex).toBeLessThan(finalRepoCatalogSettlementIndex) + expect(finalRepoCatalogSettlementIndex).toBeLessThan(startupBlockEnd) // The local catalog chain stays internally ordered (folders merge against project groups). expect(localReposIndex).toBeLessThan(localGroupsIndex) expect(localGroupsIndex).toBeLessThan(localFoldersIndex) @@ -75,6 +85,7 @@ describe('renderer startup runtime routing', () => { // The catalog and selective hydration chains overlap, but both settle before recovery or hydration. const joinStart = indexInStartupBlock('await Promise.allSettled([') expect(joinStart).toBeGreaterThan(hydrateUiIndex) + expect(joinStart).toBeLessThan(finalRepoCatalogSettlementIndex) const joinBlock = source.slice(joinStart, startupBlockEnd) expect(joinBlock).toContain('hydrationSessionChain') expect(joinBlock).toContain('localCatalogChain') diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx index 7f1576c720b..3106deda4c7 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx @@ -42,6 +42,10 @@ vi.mock('@/store', () => ({ ) })) +vi.mock('@/store/selectors', () => ({ + useAllWorktrees: () => mocks.state.allWorktrees() +})) + vi.mock('@/components/ui/dialog', () => ({ Dialog: ({ open, children }: { open: boolean; children: ReactNode }) => open ?
{children}
: null, diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx index 598f5bedd65..b89f3fef3fa 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx @@ -7,6 +7,7 @@ import { DialogTitle } from '@/components/ui/dialog' import { useAppStore } from '@/store' +import { useAllWorktrees } from '@/store/selectors' import { toast } from 'sonner' import { getConnectionId } from '@/lib/connection-context' import { getRuntimeGitStatus } from '@/runtime/runtime-git-client' @@ -36,7 +37,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const closeModal = useAppStore((s) => s.closeModal) const removeWorktree = useAppStore((s) => s.removeWorktree) const clearWorktreeDeleteState = useAppStore((s) => s.clearWorktreeDeleteState) - const allWorktrees = useAppStore((s) => s.allWorktrees) + const allWorktrees = useAllWorktrees() const repos = useAppStore((s) => s.repos) const worktreeLineageById = useAppStore((s) => s.worktreeLineageById) const updateSettings = useAppStore((s) => s.updateSettings) @@ -62,7 +63,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { ? (modalData.onDeleted as (worktreeIds: string[]) => void) : null const worktree = useMemo( - () => (worktreeId ? (allWorktrees().find((item) => item.id === worktreeId) ?? null) : null), + () => (worktreeId ? (allWorktrees.find((item) => item.id === worktreeId) ?? null) : null), [allWorktrees, worktreeId] ) const worktrees = useMemo(() => { @@ -70,7 +71,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { return [] } const selected = new Set(worktreeIds) - return allWorktrees().filter((item) => selected.has(item.id)) + return allWorktrees.filter((item) => selected.has(item.id)) }, [allWorktrees, worktreeIds]) const repoMap = useMemo(() => new Map(repos.map((repo) => [repo.id, repo])), [repos]) const isBatchDelete = worktreeIds.length > 1 @@ -90,7 +91,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const lineageDelete = useMemo( () => !isBatchDelete && worktree - ? getWorkspaceDeleteLineage(worktree, allWorktrees(), worktreeLineageById) + ? getWorkspaceDeleteLineage(worktree, allWorktrees, worktreeLineageById) : { descendants: [], deleteAllTargets: [] }, [allWorktrees, isBatchDelete, worktree, worktreeLineageById] ) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanCard.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanCard.tsx index 601bef497d6..cd7b94dd725 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanCard.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanCard.tsx @@ -7,6 +7,9 @@ import { translate } from '@/i18n/i18n' type WorkspaceKanbanCardProps = { worktree: Worktree + // Why: the lane virtualizes, so DOM order no longer implies list position. + // Drop-index math reads this instead of counting rendered siblings. + laneIndex: number repo: Repo | undefined isActive: boolean isSelected: boolean @@ -23,6 +26,7 @@ type WorkspaceKanbanCardProps = { function WorkspaceKanbanCard({ worktree, + laneIndex, repo, isActive, isSelected, @@ -40,6 +44,7 @@ function WorkspaceKanbanCard({
(null) const [dragOverStatus, setDragOverStatus] = useState(null) const [pinDragOver, setPinDragOver] = useState(false) + const [renderCards, setRenderCards] = useState(false) const { canCreateWorktree, createWorktreeForStatus } = useWorkspaceKanbanCreateWorktree() const visibleWorktreeIdSet = useVisibleWorkspaceKanbanWorktreeIds({ allWorktrees, @@ -191,6 +201,12 @@ export default function WorkspaceKanbanDrawer({ })), [worktreesByStatus, workspaceStatuses] ) + useLayoutEffect(() => { + if (!open) { + return + } + return registerWorkspaceKanbanSidebarDropGroups(boardDragGroups) + }, [boardDragGroups, open]) const laneFullWorktreeIds = useMemo( () => new Map(boardDragGroups.map((group) => [group.key, group.worktreeIds])), [boardDragGroups] @@ -692,6 +708,28 @@ export default function WorkspaceKanbanDrawer({ } ) + // Why: mounting every lane's cards in the same commit that opens the sheet + // blocks the slide-in for the whole card render. Paint the board chrome + // first, then fill the lanes in a non-blocking transition. + useEffect(() => { + if (!open) { + setRenderCards(false) + return + } + let cancelled = false + const frameId = window.requestAnimationFrame(() => { + startTransition(() => { + if (!cancelled) { + setRenderCards(true) + } + }) + }) + return () => { + cancelled = true + window.cancelAnimationFrame(frameId) + } + }, [open]) + useWorkspaceKanbanShiftWheelScroll(boardRef, laneScrollerRef, open, isPointerDragActiveRef) useWorkspaceKanbanOutsideDismiss({ open, boardRef, preserveOpenForMenu, onOpenChange }) useContextualTour('workspace-board', open && !dragPreview, 'workspace_board_visible') @@ -871,6 +909,7 @@ export default function WorkspaceKanbanDrawer({ isResizingColumn={isResizingColumn} dragOverStatus={dragOverStatus} canCreateWorktree={canCreateWorktree} + renderCards={renderCards} selectedWorktreeIds={selectedWorktreeIds} selectedWorktrees={renderedSelectedWorktrees} onDragOver={handleDragOver} diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsx new file mode 100644 index 00000000000..d5b9b5c4176 --- /dev/null +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsx @@ -0,0 +1,119 @@ +// @vitest-environment happy-dom +/** + * Why a render test: the board's open cost was mounting every card in every + * lane at once. These assert the lane renders only the virtual window, and that + * each rendered card still advertises its true lane index — drop-index math + * reads that attribute now that DOM order no longer implies list order. + * + * The virtualizer itself is mocked (as the sidebar list tests do): it needs real + * layout, which the test DOM has none of, and the contract under test is ours. + */ +import { cleanup, render } from '@testing-library/react' +import { createRef } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Repo, Worktree } from '../../../../shared/types' + +const WINDOW_START = 20 +const WINDOW_END = 25 +const TOTAL_SIZE = 21_992 + +vi.mock('@tanstack/react-virtual', () => ({ + useVirtualizer: ({ count }: { count: number }) => { + const start = count === 0 ? 0 : Math.min(WINDOW_START, count - 1) + const end = count === 0 ? -1 : Math.min(WINDOW_END, count - 1) + return { + getTotalSize: () => (count === 0 ? 0 : count * 44 - 8), + measurementsCache: Array.from({ length: count }, (_, index) => ({ + index, + start: index * 44, + end: index * 44 + 36 + })), + getVirtualItems: () => + Array.from({ length: Math.max(0, end - start + 1) }, (_, offset) => { + const index = start + offset + return { index, key: `w${index}`, start: index * 44 } + }), + measureElement: () => {} + } + } +})) + +vi.mock('./WorkspaceKanbanCard', () => ({ + default: ({ worktree, laneIndex }: { worktree: Worktree; laneIndex: number }) => ( +
+ ) +})) + +const { default: WorkspaceKanbanLaneCardList } = await import('./WorkspaceKanbanLaneCardList') + +const REPO_MAP = new Map() + +function renderLane(count: number): HTMLElement { + const scrollRef = createRef() + const { container } = render( +
+ ({ id: `w${index}` }) as Worktree)} + repoMap={REPO_MAP} + activeWorktreeId={null} + scrollRef={scrollRef} + selectedWorktreeIds={new Set()} + selectedWorktrees={[]} + nativeDragEnabled={false} + onActivate={() => {}} + onSelectionGesture={() => false} + onContextMenuSelect={() => []} + /> +
+ ) + return container +} + +afterEach(cleanup) + +describe('WorkspaceKanbanLaneCardList', () => { + it('mounts only the virtual window, not the whole lane', () => { + const container = renderLane(500) + + const ids = Array.from(container.querySelectorAll('[data-workspace-board-card-id]')).map( + (card) => card.getAttribute('data-workspace-board-card-id') + ) + expect(ids).toEqual(['w20', 'w21', 'w22', 'w23', 'w24', 'w25']) + }) + + it('gives every rendered card its lane index, not its rendered position', () => { + const container = renderLane(500) + + const indexes = Array.from(container.querySelectorAll('[data-workspace-board-card-index]')).map( + (card) => Number(card.getAttribute('data-workspace-board-card-index')) + ) + expect(indexes).toEqual([20, 21, 22, 23, 24, 25]) + }) + + it('reserves the full lane height so the scrollbar spans the whole list', () => { + const container = renderLane(500) + + const spacer = container.querySelector('[data-workspace-board-card-id]') + ?.parentElement?.parentElement + expect(spacer?.style.height).toBe(`${TOTAL_SIZE}px`) + }) + + it('positions each card at its virtual offset', () => { + const container = renderLane(500) + + const first = container.querySelector( + '[data-workspace-board-card-id]' + )?.parentElement + expect(first?.style.transform).toBe(`translateY(${WINDOW_START * 44}px)`) + }) + + it('renders nothing for an empty lane and a single card at index 0', () => { + const empty = renderLane(0) + expect(empty.querySelectorAll('[data-workspace-board-card-id]')).toHaveLength(0) + + const single = renderLane(1) + const card = single.querySelector('[data-workspace-board-card-id]') + expect(card?.getAttribute('data-workspace-board-card-id')).toBe('w0') + expect(card?.getAttribute('data-workspace-board-card-index')).toBe('0') + }) +}) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsx new file mode 100644 index 00000000000..49b9232839b --- /dev/null +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsx @@ -0,0 +1,119 @@ +import React, { useCallback, useLayoutEffect, useMemo, useRef } from 'react' +import { useVirtualizer } from '@tanstack/react-virtual' +import type { Repo, WorkspaceStatus, Worktree } from '../../../../shared/types' +import WorkspaceKanbanCard from './WorkspaceKanbanCard' +import { registerWorkspaceKanbanVirtualLaneLayout } from './workspace-kanban-virtual-lane-layout' + +// Why: board cards are uniform one-line rows; a close estimate keeps the first +// virtual window right so the lane does not reflow once cards measure. +const WORKSPACE_BOARD_CARD_ESTIMATED_HEIGHT = 36 +// Matches the `space-y-2` rhythm the lane used before virtualization. +const WORKSPACE_BOARD_CARD_GAP = 8 +const WORKSPACE_BOARD_CARD_OVERSCAN = 6 + +function estimateWorkspaceBoardCardSize(): number { + return WORKSPACE_BOARD_CARD_ESTIMATED_HEIGHT +} + +type WorkspaceKanbanLaneCardListProps = { + items: readonly Worktree[] + repoMap: Map + activeWorktreeId: string | null + scrollRef: React.RefObject + selectedWorktreeIds: ReadonlySet + selectedWorktrees: readonly Worktree[] + nativeDragEnabled: boolean + onActivate: () => void + onSelectionGesture: (event: React.MouseEvent, worktreeId: string) => boolean + onContextMenuSelect: ( + event: React.MouseEvent, + worktree: Worktree + ) => readonly Worktree[] + onAssignWorkspaceStatus?: (worktreeIds: readonly string[], status: WorkspaceStatus) => void +} + +function WorkspaceKanbanLaneCardList({ + items, + repoMap, + activeWorktreeId, + scrollRef, + selectedWorktreeIds, + selectedWorktrees, + nativeDragEnabled, + onActivate, + onSelectionGesture, + onContextMenuSelect, + onAssignWorkspaceStatus +}: WorkspaceKanbanLaneCardListProps): React.JSX.Element { + const spacerRef = useRef(null) + const itemIds = useMemo(() => items.map((item) => item.id), [items]) + const virtualizer = useVirtualizer({ + count: items.length, + getScrollElement: () => scrollRef.current, + estimateSize: estimateWorkspaceBoardCardSize, + getItemKey: useCallback((index: number) => items[index]?.id ?? index, [items]), + overscan: WORKSPACE_BOARD_CARD_OVERSCAN, + gap: WORKSPACE_BOARD_CARD_GAP, + // Why: sync-flushing rich card renders inside the scroll listener stalls the + // wheel; async + overscan keeps the lane filled without blocking input. + useFlushSync: false + }) + + useLayoutEffect(() => { + const scrollElement = scrollRef.current + const spacerElement = spacerRef.current + if (!scrollElement || !spacerElement) { + return + } + return registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => itemIds, + getMeasurements: () => virtualizer.measurementsCache + }) + // eslint-disable-next-line react-hooks/exhaustive-deps -- virtualizer is a stable instance (useVirtualizer holds it in useState), and getMeasurements reads measurementsCache off it live. + }, [itemIds, scrollRef]) + + return ( +
+ {virtualizer.getVirtualItems().map((virtualItem) => { + const worktree = items[virtualItem.index] + if (!worktree) { + return null + } + const isSelected = selectedWorktreeIds.has(worktree.id) + return ( +
+ 0 ? selectedWorktrees : undefined + } + onActivate={onActivate} + onSelectionGesture={onSelectionGesture} + onContextMenuSelect={onContextMenuSelect} + onAssignWorkspaceStatus={onAssignWorkspaceStatus} + /> +
+ ) + })} +
+ ) +} + +export default React.memo(WorkspaceKanbanLaneCardList) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsx index 0b258d4dcf7..12506678b63 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsx @@ -8,6 +8,9 @@ import type { import type { WorkspaceKanbanLaneView } from './workspace-kanban-search' import WorkspaceKanbanStatusLane from './WorkspaceKanbanStatusLane' +// Why: a fresh [] per render would defeat the memoized lane on empty lanes. +const EMPTY_LANE_ITEMS: readonly Worktree[] = [] + type WorkspaceKanbanLaneGridProps = { statuses: readonly WorkspaceStatusDefinition[] laneViews: ReadonlyMap @@ -19,6 +22,7 @@ type WorkspaceKanbanLaneGridProps = { isResizingColumn: boolean dragOverStatus: WorkspaceStatus | null canCreateWorktree: boolean + renderCards: boolean selectedWorktreeIds: ReadonlySet selectedWorktrees: readonly Worktree[] onDragOver: (event: React.DragEvent, statusId: string) => void @@ -47,6 +51,7 @@ export default function WorkspaceKanbanLaneGrid({ isResizingColumn, dragOverStatus, canCreateWorktree, + renderCards, selectedWorktreeIds, selectedWorktrees, onDragOver, @@ -72,7 +77,7 @@ export default function WorkspaceKanbanLaneGrid({ ({ - default: ({ worktree }: { worktree: Worktree }) => ( -
+vi.mock('./WorkspaceKanbanLaneCardList', () => ({ + default: ({ items }: { items: readonly Worktree[] }) => ( + <> + {items.map((item) => ( +
+ ))} + ) })) @@ -51,6 +55,7 @@ function renderLane(props: { isResizingColumn={false} isDragTarget={false} canCreateWorktree={true} + renderCards={true} selectedWorktreeIds={new Set()} selectedWorktrees={[]} onDragOver={vi.fn()} diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx index 40e9f1e8360..cceda5e12ee 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react' +import React, { useMemo, useRef } from 'react' import { Plus } from 'lucide-react' import type { Repo, @@ -13,7 +13,7 @@ import { import { cn } from '@/lib/utils' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import WorkspaceKanbanCard from './WorkspaceKanbanCard' +import WorkspaceKanbanLaneCardList from './WorkspaceKanbanLaneCardList' import { serializeWorkspaceLaneFullIds } from './workspace-kanban-filtered-drop-index' import { getWorkspaceStatusVisualMeta } from './workspace-status' import { translate } from '@/i18n/i18n' @@ -32,6 +32,7 @@ type WorkspaceKanbanStatusLaneProps = { isDragTarget: boolean canCreateWorktree: boolean nativeDragEnabled?: boolean + renderCards: boolean selectedWorktreeIds: ReadonlySet selectedWorktrees: readonly Worktree[] onDragOver: (event: React.DragEvent, statusId: string) => void @@ -49,7 +50,7 @@ type WorkspaceKanbanStatusLaneProps = { onColumnResizeKeyDown: (event: React.KeyboardEvent) => void } -export default function WorkspaceKanbanStatusLane({ +function WorkspaceKanbanStatusLane({ status, items, totalCount, @@ -62,6 +63,7 @@ export default function WorkspaceKanbanStatusLane({ isDragTarget, canCreateWorktree, nativeDragEnabled = true, + renderCards, selectedWorktreeIds, selectedWorktrees, onDragOver, @@ -75,6 +77,7 @@ export default function WorkspaceKanbanStatusLane({ onColumnResizeStart, onColumnResizeKeyDown }: WorkspaceKanbanStatusLaneProps): React.JSX.Element { + const laneScrollRef = useRef(null) const meta = getWorkspaceStatusVisualMeta(status) // Why: a lane that is empty on its own merits is still "Empty" under a query — // only a lane whose cards were filtered away has anything to say about matches. @@ -180,32 +183,26 @@ export default function WorkspaceKanbanStatusLane({
{items.length > 0 ? ( -
- {items.map((worktree) => { - const isSelected = selectedWorktreeIds.has(worktree.id) - return ( - 0 ? selectedWorktrees : undefined - } - onActivate={onActivate} - onSelectionGesture={onSelectionGesture} - onContextMenuSelect={onContextMenuSelect} - onAssignWorkspaceStatus={onAssignWorkspaceStatus} - /> - ) - })} -
+ renderCards ? ( + + ) : null ) : (
{isFiltered @@ -241,3 +238,5 @@ export default function WorkspaceKanbanStatusLane({ ) } + +export default React.memo(WorkspaceKanbanStatusLane) diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts index 69771ef31f1..a188f16c83b 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts @@ -53,6 +53,7 @@ vi.mock('@/store', () => ({ })) vi.mock('@/store/selectors', () => ({ + getAllWorktreesFromState: () => Array.from(mocks.state.worktreeMap.values()), getWorktreeMapFromState: () => mocks.state.worktreeMap })) @@ -141,6 +142,16 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' }) }) + it('treats duplicate selected ids as one delete target', () => { + setWorktrees([{ id: 'wt-1' }]) + + const started = runWorktreeBatchDelete(['wt-1', 'wt-1']) + + expect(started).toBe(true) + expect(mocks.state.clearWorktreeDeleteState).toHaveBeenCalledTimes(1) + expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' }) + }) + it('keeps batch deletes behind confirmation when confirmation is skipped', () => { mocks.state.settings = { skipDeleteWorktreeConfirm: true } setWorktrees([ diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.ts index f5a653a42b9..b2dc1dbc1a6 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.ts @@ -1,6 +1,6 @@ import { toast } from 'sonner' import { useAppStore } from '@/store' -import { getWorktreeMapFromState } from '@/store/selectors' +import { getAllWorktreesFromState, getWorktreeMapFromState } from '@/store/selectors' import { findRepoForHost } from '@/store/slices/repo-host-identity' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete' @@ -46,16 +46,18 @@ export async function runWorktreeDeletesInParallel( targets: readonly Pick[], options: WorktreeDeleteWithToastOptions = {} ): Promise { + // Why: refresh races can leave duplicate rows, but a destructive command must run once per identity. + const uniqueTargets = Array.from(new Map(targets.map((target) => [target.id, target])).values()) // Why: capture the viewed workspace before any delete so we can focus one survivor after the batch settles, not per delete. const activeWorktreeIdBefore = useAppStore.getState().activeWorktreeId const commitBatchFocus = activeWorktreeIdBefore ? prepareActiveWorktreeFocusAfterDelete(activeWorktreeIdBefore) : null // Why: mark every target deleting up front for immediate in-flight feedback, even though deletes serialize per repo. - useAppStore.getState().markWorktreesDeleting(targets.map((target) => target.id)) + useAppStore.getState().markWorktreesDeleting(uniqueTargets.map((target) => target.id)) // Why: worktree remove/prune/branch -D race on shared ref locks; group by repoId to serialize per repo (cross-repo stays parallel). - const groups = new Map() - for (const target of targets) { + const groups = new Map() + for (const target of uniqueTargets) { const group = groups.get(target.repoId) if (group) { group.push(target) @@ -95,7 +97,7 @@ export async function runWorktreeDeletesInParallel( if (activeWorktreeIdBefore && deletedSet.has(activeWorktreeIdBefore)) { commitBatchFocus?.() } - return targets.filter((target) => deletedSet.has(target.id)).map((target) => target.id) + return uniqueTargets.filter((target) => deletedSet.has(target.id)).map((target) => target.id) } /** @@ -249,8 +251,8 @@ export function runWorktreeDelete(worktreeId: string): void { } const hasLineageChildren = - getWorkspaceDeleteLineage(target, state.allWorktrees(), state.worktreeLineageById).descendants - .length > 0 + getWorkspaceDeleteLineage(target, getAllWorktreesFromState(state), state.worktreeLineageById) + .descendants.length > 0 const skipConfirm = state.settings?.skipDeleteWorktreeConfirm ?? false if (skipConfirm && !hasLineageChildren) { void runWorktreeDeleteWithToast(worktreeId, target.displayName) @@ -268,7 +270,7 @@ export function runWorktreeBatchDelete( ): boolean { const state = useAppStore.getState() const worktreeMap = getWorktreeMapFromState(state) - const targets = worktreeIds + const targets = Array.from(new Set(worktreeIds)) .map((id) => worktreeMap.get(id) ?? null) .filter((worktree): worktree is Worktree => worktree != null && !worktree.isMainWorktree) @@ -295,8 +297,11 @@ export function runWorktreeBatchDelete( // Why: bulk cleanup can destroy many directories at once, so batch/Space deletes keep an explicit confirmation step. const singleTargetHasLineageChildren = targets.length === 1 && - getWorkspaceDeleteLineage(targets[0], state.allWorktrees(), state.worktreeLineageById) - .descendants.length > 0 + getWorkspaceDeleteLineage( + targets[0], + getAllWorktreesFromState(state), + state.worktreeLineageById + ).descendants.length > 0 const skipConfirm = !options.forceConfirm && targets.length === 1 && diff --git a/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts b/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts index e20214d6633..817a02f2040 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts @@ -31,6 +31,7 @@ vi.mock('@/store', () => ({ })) vi.mock('@/store/selectors', () => ({ + getAllWorktreesFromState: () => Array.from(mocks.state.worktreeMap.values()), getWorktreeMapFromState: () => mocks.state.worktreeMap })) @@ -145,6 +146,25 @@ describe('runWorktreeDeletesInParallel', () => { expect(mocks.state.removeWorktree).toHaveBeenNthCalledWith(2, 'wt-2', true) }) + it('deletes a duplicated target identity only once', async () => { + const target = { + id: 'wt-1', + displayName: 'one', + repoId: 'repo-a', + path: '/workspaces/one' + } + mocks.state.removeWorktree + .mockResolvedValueOnce({ ok: true }) + .mockResolvedValueOnce({ ok: false, error: 'selector_not_found' }) + + await expect(runWorktreeDeletesInParallel([target, target])).resolves.toEqual(['wt-1']) + + expect(mocks.state.markWorktreesDeleting).toHaveBeenCalledWith(['wt-1']) + expect(mocks.state.removeWorktree).toHaveBeenCalledTimes(1) + expect(mocks.state.removeWorktree).toHaveBeenCalledWith('wt-1', false) + expect(toast.error).not.toHaveBeenCalled() + }) + it('clears a pending ancestor when a nested descendant delete fails', async () => { mocks.state.removeWorktree.mockImplementationOnce(async (worktreeId: string) => { mocks.state.deleteStateByWorktreeId[worktreeId] = { diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-area-selection.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-area-selection.ts index a767e73fe95..36f4d5af69c 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-area-selection.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-area-selection.ts @@ -1,18 +1,20 @@ /* eslint-disable max-lines -- Why: marquee selection coordinates pointer capture, lane-scroll refresh, auto-scroll, preview cleanup, and final commit against one drag state. Splitting those phases would make the interaction easier to desynchronize. */ import React, { useCallback, useEffect, useRef } from 'react' +import { + getAreaSelectionCardRects, + type AreaSelectionCardRect +} from './workspace-kanban-area-selection-card-rects' import { clearPreviewSelection, getAreaSelectionAutoScrollDelta, getAreaSelectionCardIds, - getAreaSelectionCardRects, getAreaSelectionRect, getAreaSelectionScrollContainer, getAreaSelectionScrollStartContentYByElement, isScrollbarPointerDown, setOverlayRect, shouldIgnoreAreaSelectionStart, - updatePreviewSelection, - type AreaSelectionCardRect + updatePreviewSelection } from './workspace-kanban-area-selection-dom' type AreaSelectionDragState = { @@ -110,6 +112,20 @@ export function useWorkspaceKanbanAreaSelection({ state.started = true + // Why: deferred card mount can start a marquee against empty rects; pick up + // virtual layout once lanes register without waiting for a scroll event. + if (state.cardRects.length === 0) { + const board = boardRef.current + if (board) { + state.boardRect = board.getBoundingClientRect() + state.cardRects = getAreaSelectionCardRects(board) + state.scrollStartContentYByElement = getAreaSelectionScrollStartContentYByElement( + board, + state.startY + ) + } + } + const viewportRect = getAreaSelectionRect( state.startX, state.startY, @@ -153,7 +169,7 @@ export function useWorkspaceKanbanAreaSelection({ state.additive, areaIds ) - }, [overlayRef]) + }, [boardRef, overlayRef]) const refreshAreaSelectionMeasurements = useCallback(() => { const state = dragRef.current @@ -233,6 +249,7 @@ export function useWorkspaceKanbanAreaSelection({ window.cancelAnimationFrame(state.scrollFrameId) state.scrollFrameId = null } + refreshAreaSelectionMeasurements() flushAreaSelectionDrag() if (shouldCommitWorkspaceKanbanAreaSelection(state)) { updateSelectionForAreaRef.current( @@ -246,7 +263,7 @@ export function useWorkspaceKanbanAreaSelection({ dragRef.current = null setOverlayRect(overlayRef.current, null) }, - [flushAreaSelectionDrag, overlayRef] + [flushAreaSelectionDrag, overlayRef, refreshAreaSelectionMeasurements] ) const handleAreaSelectionPointerDown = useCallback( diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.ts index 14f635896fc..19a67a31663 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.ts @@ -11,7 +11,6 @@ import { } from './workspace-kanban-card-pointer-drag-dom' import { createDragPreview, - getDraggedCards, setDragDocumentStyles, setDraggedCardsDragging, updateDragPreviewPosition @@ -33,7 +32,6 @@ type DragState = { currentY: number worktreeIds: string[] sourceCard: HTMLElement - draggedCards: HTMLElement[] preview: HTMLElement | null previewOffsetX: number previewOffsetY: number @@ -117,7 +115,11 @@ export function useWorkspaceKanbanCardPointerDrag({ if (state.frameId !== null) { window.cancelAnimationFrame(state.frameId) } - setDraggedCardsDragging(state.draggedCards, false) + setDraggedCardsDragging({ + board: boardRef.current, + worktreeIds: state.worktreeIds, + enabled: false + }) removeCardDropIndicator() state.preview?.remove() setDragDocumentStyles(false) @@ -146,13 +148,20 @@ export function useWorkspaceKanbanCardPointerDrag({ [boardRef, clearDragTarget] ) - const startPointerDrag = useCallback((state: DragState) => { - state.started = true - isPointerDragActiveRef.current = true - setDraggedCardsDragging(state.draggedCards, true) - state.preview = createDragPreview(state) - setDragDocumentStyles(true) - }, []) + const startPointerDrag = useCallback( + (state: DragState) => { + state.started = true + isPointerDragActiveRef.current = true + setDraggedCardsDragging({ + board: boardRef.current, + worktreeIds: state.worktreeIds, + enabled: true + }) + state.preview = createDragPreview(state) + setDragDocumentStyles(true) + }, + [boardRef] + ) const updatePointerDragTarget = useCallback( (state: DragState) => { @@ -289,10 +298,9 @@ export function useWorkspaceKanbanCardPointerDrag({ return } - const selectedIds = selectedWorktreeIdsRef.current const selectedWorktrees = selectedWorktreesRef.current const worktreeIds = - selectedIds.has(worktreeId) && selectedWorktrees.length > 1 + selectedWorktreeIdsRef.current.has(worktreeId) && selectedWorktrees.length > 1 ? selectedWorktrees.map((worktree) => worktree.id) : [worktreeId] dragRef.current = { @@ -303,7 +311,6 @@ export function useWorkspaceKanbanCardPointerDrag({ currentY: event.clientY, worktreeIds, sourceCard: card, - draggedCards: getDraggedCards(board, worktreeIds, card), preview: null, previewOffsetX: 0, previewOffsetY: 0, diff --git a/src/renderer/src/components/sidebar/workspace-kanban-area-selection-card-rects.ts b/src/renderer/src/components/sidebar/workspace-kanban-area-selection-card-rects.ts new file mode 100644 index 00000000000..95d315966f2 --- /dev/null +++ b/src/renderer/src/components/sidebar/workspace-kanban-area-selection-card-rects.ts @@ -0,0 +1,99 @@ +import { getWorkspaceKanbanVirtualLaneItemRects } from './workspace-kanban-virtual-lane-layout' + +type AreaSelectionCardContentRect = { + top: number + bottom: number + containerTop: number + scrollTop: number +} + +export type AreaSelectionViewportRect = { + left: number + top: number + right: number + bottom: number +} + +export type AreaSelectionCardRect = { + id: string + element: HTMLElement | null + rect: AreaSelectionViewportRect + scrollContainer: HTMLElement | null + contentRect: AreaSelectionCardContentRect | null +} + +export const AREA_SELECTION_SCROLL_CONTAINER_SELECTOR = '[data-workspace-board-lane-scroll]' + +export function getAreaSelectionCardRects(board: HTMLElement): AreaSelectionCardRect[] { + const cardRects = new Map() + const scrollMetrics = new Map() + const scrollContainers = board.querySelectorAll( + AREA_SELECTION_SCROLL_CONTAINER_SELECTOR + ) + for (const scrollContainer of scrollContainers) { + const virtualRects = getWorkspaceKanbanVirtualLaneItemRects(scrollContainer) + if (!virtualRects) { + continue + } + const containerTop = scrollContainer.getBoundingClientRect().top + const scrollTop = scrollContainer.scrollTop + scrollMetrics.set(scrollContainer, { containerTop, scrollTop }) + for (const virtualRect of virtualRects) { + cardRects.set(virtualRect.id, { + id: virtualRect.id, + element: null, + rect: { + left: virtualRect.left, + top: virtualRect.top, + right: virtualRect.right, + bottom: virtualRect.bottom + }, + scrollContainer, + contentRect: { + top: virtualRect.contentTop, + bottom: virtualRect.contentBottom, + containerTop, + scrollTop + } + }) + } + } + + const cards = board.querySelectorAll('[data-workspace-board-card-id]') + for (const card of cards) { + const id = card.dataset.workspaceBoardCardId + if (!id) { + continue + } + const rect = card.getBoundingClientRect() + const scrollContainer = card.closest(AREA_SELECTION_SCROLL_CONTAINER_SELECTOR) + let metrics = scrollContainer ? scrollMetrics.get(scrollContainer) : undefined + if (scrollContainer && !metrics) { + metrics = { + containerTop: scrollContainer.getBoundingClientRect().top, + scrollTop: scrollContainer.scrollTop + } + scrollMetrics.set(scrollContainer, metrics) + } + cardRects.set(id, { + id, + element: card, + rect: { + left: rect.left, + top: rect.top, + right: rect.right, + bottom: rect.bottom + }, + scrollContainer, + contentRect: metrics + ? { + top: rect.top - metrics.containerTop + metrics.scrollTop, + bottom: rect.bottom - metrics.containerTop + metrics.scrollTop, + containerTop: metrics.containerTop, + scrollTop: metrics.scrollTop + } + : null + }) + } + return Array.from(cardRects.values()) +} diff --git a/src/renderer/src/components/sidebar/workspace-kanban-area-selection-dom.ts b/src/renderer/src/components/sidebar/workspace-kanban-area-selection-dom.ts index a0a5c9d46b3..d023c468fbd 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-area-selection-dom.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-area-selection-dom.ts @@ -1,3 +1,8 @@ +import { + AREA_SELECTION_SCROLL_CONTAINER_SELECTOR, + type AreaSelectionCardRect +} from './workspace-kanban-area-selection-card-rects' + export type AreaSelectionRect = { left: number top: number @@ -5,21 +10,6 @@ export type AreaSelectionRect = { height: number } -export type AreaSelectionCardRect = { - id: string - element: HTMLElement - rect: DOMRect - scrollContainer: HTMLElement | null - contentRect: AreaSelectionCardContentRect | null -} - -type AreaSelectionCardContentRect = { - top: number - bottom: number - containerTop: number - scrollTop: number -} - type AreaSelectionAutoScrollParams = { pointerY: number containerTop: number @@ -37,7 +27,6 @@ type AreaSelectionCardIdOptions = { } const AREA_SELECTED_ATTR = 'data-workspace-board-card-area-selected' -export const AREA_SELECTION_SCROLL_CONTAINER_SELECTOR = '[data-workspace-board-lane-scroll]' export const AREA_SELECTION_AUTO_SCROLL_EDGE_SIZE = 48 export const AREA_SELECTION_AUTO_SCROLL_MAX_DELTA = 22 @@ -93,45 +82,6 @@ export function isScrollbarPointerDown( return hitsVerticalScrollbar || hitsHorizontalScrollbar } -export function getAreaSelectionCardRects(board: HTMLElement): AreaSelectionCardRect[] { - const cardRects: AreaSelectionCardRect[] = [] - const seen = new Set() - const scrollMetrics = new Map() - const cards = board.querySelectorAll('[data-workspace-board-card-id]') - for (const card of cards) { - const id = card.dataset.workspaceBoardCardId - if (!id || seen.has(id)) { - continue - } - const rect = card.getBoundingClientRect() - const scrollContainer = card.closest(AREA_SELECTION_SCROLL_CONTAINER_SELECTOR) - let metrics = scrollContainer ? scrollMetrics.get(scrollContainer) : undefined - if (scrollContainer && !metrics) { - metrics = { - containerTop: scrollContainer.getBoundingClientRect().top, - scrollTop: scrollContainer.scrollTop - } - scrollMetrics.set(scrollContainer, metrics) - } - cardRects.push({ - id, - element: card, - rect, - scrollContainer, - contentRect: metrics - ? { - top: rect.top - metrics.containerTop + metrics.scrollTop, - bottom: rect.bottom - metrics.containerTop + metrics.scrollTop, - containerTop: metrics.containerTop, - scrollTop: metrics.scrollTop - } - : null - }) - seen.add(id) - } - return cardRects -} - export function getAreaSelectionCardIds( cardRects: readonly AreaSelectionCardRect[], selectionRect: AreaSelectionRect, @@ -256,7 +206,8 @@ export function clearPreviewSelection( previewIds: Set ): void { for (const card of cardRects) { - if (previewIds.has(card.id)) { + // Why: virtual remounts replace the element; clear via live connected node when present. + if (card.element?.isConnected && previewIds.has(card.id)) { card.element.removeAttribute(AREA_SELECTED_ATTR) } } @@ -276,16 +227,29 @@ export function updatePreviewSelection( } for (const card of cardRects) { + const element = card.element?.isConnected ? card.element : null + if (!element) { + if (!nextIds.has(card.id)) { + previewIds.delete(card.id) + } + continue + } const shouldPreview = nextIds.has(card.id) - const isPreviewed = previewIds.has(card.id) + // Why: virtualization can remount the same card id; trust the live attr, not previewIds alone. + const isPreviewed = element.getAttribute(AREA_SELECTED_ATTR) === 'true' if (shouldPreview === isPreviewed) { + if (shouldPreview) { + previewIds.add(card.id) + } else { + previewIds.delete(card.id) + } continue } if (shouldPreview) { - card.element.setAttribute(AREA_SELECTED_ATTR, 'true') + element.setAttribute(AREA_SELECTED_ATTR, 'true') previewIds.add(card.id) } else { - card.element.removeAttribute(AREA_SELECTED_ATTR) + element.removeAttribute(AREA_SELECTED_ATTR) previewIds.delete(card.id) } } diff --git a/src/renderer/src/components/sidebar/workspace-kanban-area-selection.test.ts b/src/renderer/src/components/sidebar/workspace-kanban-area-selection.test.ts index d04f34ca121..86a51e1e8a2 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-area-selection.test.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-area-selection.test.ts @@ -1,8 +1,10 @@ +// @vitest-environment happy-dom import { describe, expect, it } from 'vitest' +import type { AreaSelectionCardRect } from './workspace-kanban-area-selection-card-rects' import { getAreaSelectionAutoScrollDelta, getAreaSelectionCardIds, - type AreaSelectionCardRect + updatePreviewSelection } from './workspace-kanban-area-selection-dom' import { shouldCommitWorkspaceKanbanAreaSelection } from './use-workspace-kanban-area-selection' @@ -166,16 +168,52 @@ function makeRect({ top: number right: number bottom: number -}): DOMRect { - return { - left, - top, - right, - bottom, - width: right - left, - height: bottom - top, - x: left, - y: top, - toJSON: () => ({}) - } as DOMRect +}): AreaSelectionCardRect['rect'] { + return { left, top, right, bottom } } + +describe('workspace kanban area selection preview remount', () => { + it('re-applies the preview attribute when a card remounts under the same id', () => { + const first = document.createElement('div') + first.setAttribute('data-workspace-board-card-id', 'card-a') + Object.defineProperty(first, 'isConnected', { value: true }) + const previewIds = new Set() + updatePreviewSelection( + [ + { + id: 'card-a', + element: first, + rect: makeRect({ left: 0, top: 0, right: 100, bottom: 40 }), + scrollContainer: null, + contentRect: null + } + ], + previewIds, + new Set(), + false, + ['card-a'] + ) + expect(first.getAttribute('data-workspace-board-card-area-selected')).toBe('true') + expect(previewIds.has('card-a')).toBe(true) + + const remounted = document.createElement('div') + remounted.setAttribute('data-workspace-board-card-id', 'card-a') + Object.defineProperty(remounted, 'isConnected', { value: true }) + updatePreviewSelection( + [ + { + id: 'card-a', + element: remounted, + rect: makeRect({ left: 0, top: 0, right: 100, bottom: 40 }), + scrollContainer: null, + contentRect: null + } + ], + previewIds, + new Set(), + false, + ['card-a'] + ) + expect(remounted.getAttribute('data-workspace-board-card-area-selected')).toBe('true') + }) +}) diff --git a/src/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.ts b/src/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.ts index 887d7ebcac2..58dafc5db3f 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.ts @@ -37,12 +37,27 @@ export function getDraggedCards( return cards.length > 0 ? cards : [fallbackCard] } -export function setDraggedCardsDragging(cards: readonly HTMLElement[], enabled: boolean): void { - for (const card of cards) { - if (enabled) { - card.setAttribute(POINTER_CARD_DRAGGING_ATTR, 'true') - } else { +export function setDraggedCardsDragging(args: { + board: HTMLElement | null + worktreeIds: readonly string[] + enabled: boolean +}): void { + const { board, worktreeIds, enabled } = args + if (!board) { + return + } + // Why: virtual lanes remount cards during scroll; query live nodes by id + // instead of holding HTMLElement references across unmounts. + if (!enabled) { + board.querySelectorAll(`[${POINTER_CARD_DRAGGING_ATTR}]`).forEach((card) => { card.removeAttribute(POINTER_CARD_DRAGGING_ATTR) + }) + return + } + const ids = new Set(worktreeIds) + for (const card of board.querySelectorAll(CARD_SELECTOR)) { + if (ids.has(card.dataset.workspaceBoardCardId ?? '')) { + card.setAttribute(POINTER_CARD_DRAGGING_ATTR, 'true') } } } diff --git a/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.test.ts b/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.test.ts index 5a8698a1cb8..f8ab1df0f94 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.test.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { resolveWorkspaceKanbanCardDropCommitTarget, resolveWorkspaceCardDropIndexFromRects, + resolveWorkspaceCardDropIndicatorY, resolveWorkspaceStatusDropTargetFromRects } from './workspace-kanban-card-pointer-drag-dom' @@ -41,6 +42,47 @@ describe('workspace kanban pointer drag card drop index', () => { it('inserts at the end when the pointer is below every card midpoint', () => { expect(resolveWorkspaceCardDropIndexFromRects(cardRects, 140)).toBe(3) }) + + // Why: a virtualized lane only renders its window, so rendered position is + // not lane position. Without the carried index a drop into a scrolled lane + // would land near the top of the list. + it('uses the lane index of a rendered card rather than its rendered position', () => { + const virtualized = [ + { top: 0, bottom: 40, index: 20 }, + { top: 48, bottom: 88, index: 21 }, + { top: 96, bottom: 136, index: 22 } + ] + expect(resolveWorkspaceCardDropIndexFromRects(virtualized, 10)).toBe(20) + expect(resolveWorkspaceCardDropIndexFromRects(virtualized, 70)).toBe(22) + expect(resolveWorkspaceCardDropIndexFromRects(virtualized, 140)).toBe(23) + }) +}) + +describe('workspace kanban drop indicator placement', () => { + const cardRects = [ + { top: 100, bottom: 140 }, + { top: 148, bottom: 188 } + ] + + it('places the line above, between, and below rendered cards', () => { + expect(resolveWorkspaceCardDropIndicatorY(cardRects, 0, 90)).toBe(95) + expect(resolveWorkspaceCardDropIndicatorY(cardRects, 1, 90)).toBe(144) + expect(resolveWorkspaceCardDropIndicatorY(cardRects, 2, 90)).toBe(193) + }) + + it('falls back to the lane top when the lane renders no cards', () => { + expect(resolveWorkspaceCardDropIndicatorY([], 0, 90)).toBe(104) + }) + + it('anchors to the rendered card that owns the lane index', () => { + const virtualized = [ + { top: 100, bottom: 140, index: 20 }, + { top: 148, bottom: 188, index: 21 } + ] + expect(resolveWorkspaceCardDropIndicatorY(virtualized, 20, 90)).toBe(95) + expect(resolveWorkspaceCardDropIndicatorY(virtualized, 21, 90)).toBe(144) + expect(resolveWorkspaceCardDropIndicatorY(virtualized, 22, 90)).toBe(193) + }) }) describe('workspace kanban pointer drag commit target', () => { diff --git a/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.ts b/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.ts index 9114a47a881..dd59d430ab9 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-card-pointer-drag-dom.ts @@ -1,4 +1,8 @@ import type { WorkspaceStatus } from '../../../../shared/types' +import { + resolveWorkspaceKanbanVirtualLaneDropIndex, + resolveWorkspaceKanbanVirtualLaneDropIndicatorY +} from './workspace-kanban-virtual-lane-layout' export const CARD_SELECTOR = '[data-workspace-board-card-id]' export const STATUS_DROP_TARGET = '[data-workspace-status-drop-target]' @@ -19,6 +23,9 @@ export type WorkspaceKanbanStatusDropRect = { export type WorkspaceKanbanCardDropRect = { top: number bottom: number + // Why: lanes virtualize, so a rendered card's DOM position is not its index + // in the lane. Carry the lane index when the card advertises one. + index?: number } export type WorkspaceKanbanLaneDropRect = { @@ -31,6 +38,7 @@ export type WorkspaceKanbanCardDropTarget = { status: WorkspaceStatus | null isPinDrop: boolean dropIndex: number + dropIndicatorY?: number laneRect?: WorkspaceKanbanLaneDropRect cardRects?: readonly WorkspaceKanbanCardDropRect[] } @@ -75,10 +83,51 @@ export function resolveWorkspaceCardDropIndexFromRects( for (let index = 0; index < rects.length; index++) { const rect = rects[index]! if (y < (rect.top + rect.bottom) / 2) { - return index + return rect.index ?? index } } - return rects.length + const last = rects.at(-1) + return last?.index !== undefined ? last.index + 1 : rects.length +} + +function getRenderedCardLanePosition(rect: WorkspaceKanbanCardDropRect, fallback: number): number { + return rect.index ?? fallback +} + +export function resolveWorkspaceCardDropIndicatorY( + rects: readonly WorkspaceKanbanCardDropRect[], + dropIndex: number, + laneTop: number +): number { + if (rects.length === 0) { + return laneTop + 14 + } + // Why: with a virtualized lane the drop index counts unrendered cards too, + // so place the line relative to the rendered card that owns that slot. + const slot = rects.findIndex( + (rect, index) => getRenderedCardLanePosition(rect, index) >= dropIndex + ) + if (slot === -1) { + return rects.at(-1)!.bottom + 5 + } + if (slot === 0) { + return rects[0]!.top - 5 + } + return (rects[slot - 1]!.bottom + rects[slot]!.top) / 2 +} + +function getLaneCardDropRects(lane: HTMLElement): WorkspaceKanbanCardDropRect[] { + return Array.from(lane.querySelectorAll(CARD_SELECTOR)) + .filter((card) => card.offsetParent !== null) + .map((card) => { + const rect = card.getBoundingClientRect() + const index = Number.parseInt(card.dataset.workspaceBoardCardIndex ?? '', 10) + return { + top: rect.top, + bottom: rect.bottom, + ...(Number.isInteger(index) ? { index } : {}) + } + }) } function hasWorkspaceKanbanCardDropTarget(target: WorkspaceKanbanCardDropTarget): boolean { @@ -176,16 +225,18 @@ export function getCardDropTarget( const laneContent = lane.querySelector('[data-workspace-board-lane-scroll]') const laneClientRect = (laneContent ?? lane).getBoundingClientRect() - const cardRects = Array.from(lane.querySelectorAll(CARD_SELECTOR)) - .filter((card) => card.offsetParent !== null) - .map((card) => { - const rect = card.getBoundingClientRect() - return { top: rect.top, bottom: rect.bottom } - }) - const dropIndex = resolveWorkspaceCardDropIndexFromRects(cardRects, y) + const cardRects = getLaneCardDropRects(lane) + const virtualDropIndex = laneContent + ? resolveWorkspaceKanbanVirtualLaneDropIndex(laneContent, y) + : null + const dropIndex = virtualDropIndex ?? resolveWorkspaceCardDropIndexFromRects(cardRects, y) + const dropIndicatorY = laneContent + ? resolveWorkspaceKanbanVirtualLaneDropIndicatorY(laneContent, dropIndex) + : null return { ...target, dropIndex, + ...(dropIndicatorY !== null ? { dropIndicatorY } : {}), laneRect: { left: laneClientRect.left, top: laneClientRect.top, @@ -245,23 +296,10 @@ export function updateCardDropIndicator( width: fallbackLaneRect.width } : { left: 0, top: 0, width: 0 }) - const cardRects = - target.cardRects ?? - Array.from(lane.querySelectorAll(CARD_SELECTOR)) - .filter((card) => card.offsetParent !== null) - .map((card) => { - const rect = card.getBoundingClientRect() - return { top: rect.top, bottom: rect.bottom } - }) - const boundedDropIndex = Math.max(0, Math.min(cardRects.length, target.dropIndex)) + const cardRects = target.cardRects ?? getLaneCardDropRects(lane) const y = - cardRects.length === 0 - ? laneRect.top + 14 - : boundedDropIndex === 0 - ? cardRects[0]!.top - 5 - : boundedDropIndex >= cardRects.length - ? cardRects.at(-1)!.bottom + 5 - : (cardRects[boundedDropIndex - 1]!.bottom + cardRects[boundedDropIndex]!.top) / 2 + target.dropIndicatorY ?? + resolveWorkspaceCardDropIndicatorY(cardRects, Math.max(0, target.dropIndex), laneRect.top) const horizontalInset = 8 const indicator = getOrCreateDropIndicator() indicator.dataset.workspaceStatus = target.status diff --git a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts index 3feefdd62da..08f627cc0b4 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts @@ -7,9 +7,11 @@ import { getWorkspaceKanbanSidebarDropGroups, getWorkspaceKanbanSidebarDropTarget, isWorkspaceKanbanSidebarDropPointInBoard, + registerWorkspaceKanbanSidebarDropGroups, resolveWorkspaceKanbanSidebarFullLaneDropIndex, updateWorkspaceKanbanSidebarDropTargetVisual } from './workspace-kanban-sidebar-drop' +import { registerWorkspaceKanbanVirtualLaneLayout } from './workspace-kanban-virtual-lane-layout' const workspaceStatuses: WorkspaceStatusDefinition[] = [ { id: 'todo', label: 'Todo' }, @@ -29,6 +31,7 @@ class FakeElement extends FakeNode { } } offsetParent: FakeElement | null = null + scrollTop = 0 private readonly attributes = new Map() private rect: Pick = { left: 0, @@ -159,6 +162,7 @@ class FakeDocument { } let fakeDocument: FakeDocument +let unregisterSidebarDropGroups: (() => void) | null = null function setRect( element: HTMLElement, @@ -221,6 +225,8 @@ function worktree(args: { } afterEach(() => { + unregisterSidebarDropGroups?.() + unregisterSidebarDropGroups = null clearWorkspaceKanbanSidebarDropTargetVisual() vi.unstubAllGlobals() vi.restoreAllMocks() @@ -249,6 +255,71 @@ describe('workspace kanban sidebar drop DOM bridge', () => { ]) }) + it('prefers complete registered groups and restores the DOM fallback on cleanup', () => { + appendBoard() + unregisterSidebarDropGroups = registerWorkspaceKanbanSidebarDropGroups([ + { key: 'doing', worktreeIds: ['doing-a', 'doing-b', 'doing-c', 'doing-d'] } + ]) + + expect(getWorkspaceKanbanSidebarDropGroups()).toEqual([ + { key: 'doing', worktreeIds: ['doing-a', 'doing-b', 'doing-c', 'doing-d'] } + ]) + + unregisterSidebarDropGroups() + unregisterSidebarDropGroups = null + expect(getWorkspaceKanbanSidebarDropGroups()).toEqual([ + { key: 'doing', worktreeIds: ['doing-a', 'doing-b'] } + ]) + }) + + it('uses the full virtual layout when the mounted card window is stale', () => { + const { lane } = appendBoard() + const laneScroll = document.createElement('div') + laneScroll.setAttribute('data-workspace-board-lane-scroll', '') + setRect(laneScroll, { + left: 0, + top: 0, + right: 200, + bottom: 220, + width: 200, + height: 220 + }) + const spacer = document.createElement('div') + setRect(spacer, { + left: 8, + top: -4260, + right: 192, + bottom: 220, + width: 184, + height: 4480 + }) + laneScroll.append(spacer) + lane.append(laneScroll) + setElementFromPoint(lane) + const unregister = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement: laneScroll, + spacerElement: spacer, + getItemIds: () => Array.from({ length: 102 }, (_, index) => `doing-${index}`), + getMeasurements: () => + Array.from({ length: 102 }, (_, index) => ({ + index, + start: index * 44, + end: index * 44 + 36 + })) + }) + + expect(getWorkspaceKanbanSidebarDropTarget(24, 210)).toMatchObject({ + status: 'doing', + dropIndex: 102, + dropIndicatorY: 225, + cardRects: [ + { top: 8, bottom: 48 }, + { top: 56, bottom: 96 } + ] + }) + unregister() + }) + it('prefers the published full lane membership over the rendered card scan', () => { const { lane } = appendBoard() lane.dataset.workspaceLaneFullIds = diff --git a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts index c48d5312ad1..363038bd529 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts @@ -29,6 +29,7 @@ const BOARD_SHEET_SELECTOR = '[data-workspace-board-sheet]' const EXTERNAL_DRAG_TARGET_ATTR = 'data-workspace-board-external-drag-target' let externalDragTargetElement: HTMLElement | null = null +let logicalDropGroupsRegistration: { groups: readonly WorktreeDragGroup[] } | null = null function getWorkspaceKanbanBoardElement(): HTMLElement | null { return document.querySelector(BOARD_SELECTOR) @@ -97,7 +98,26 @@ export function clearWorkspaceKanbanSidebarDropTargetVisual(): void { removeCardDropIndicator() } +export function registerWorkspaceKanbanSidebarDropGroups( + groups: readonly WorktreeDragGroup[] +): () => void { + const registration = { groups } + logicalDropGroupsRegistration = registration + return () => { + if (logicalDropGroupsRegistration === registration) { + logicalDropGroupsRegistration = null + } + } +} + export function getWorkspaceKanbanSidebarDropGroups(): WorktreeDragGroup[] { + if (logicalDropGroupsRegistration) { + return logicalDropGroupsRegistration.groups.map((group) => ({ + key: group.key, + worktreeIds: [...group.worktreeIds] + })) + } + const board = getWorkspaceKanbanBoardElement() if (!board) { return [] diff --git a/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.test.ts b/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.test.ts new file mode 100644 index 00000000000..003cdf73784 --- /dev/null +++ b/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it } from 'vitest' +import { getAreaSelectionCardRects } from './workspace-kanban-area-selection-card-rects' +import { getAreaSelectionCardIds } from './workspace-kanban-area-selection-dom' +import { + getWorkspaceKanbanVirtualLaneItemRects, + registerWorkspaceKanbanVirtualLaneLayout, + resolveWorkspaceKanbanVirtualLaneDropIndex, + resolveWorkspaceKanbanVirtualLaneDropIndicatorY +} from './workspace-kanban-virtual-lane-layout' + +const ITEM_COUNT = 102 +const ITEM_STRIDE = 44 +const ITEM_HEIGHT = 36 + +function createLayoutElements(): { + scrollElement: HTMLElement + spacerElement: HTMLElement +} { + const scrollElement = { + scrollTop: 0, + getBoundingClientRect: () => ({ + left: 20, + top: 100, + right: 220, + bottom: 600, + width: 200, + height: 500 + }) + } as unknown as HTMLElement + const spacerElement = { + getBoundingClientRect: () => ({ + left: 26, + top: 108 - scrollElement.scrollTop, + right: 214, + bottom: 108 - scrollElement.scrollTop + ITEM_COUNT * ITEM_STRIDE, + width: 188, + height: ITEM_COUNT * ITEM_STRIDE + }) + } as unknown as HTMLElement + return { scrollElement, spacerElement } +} + +function registerLayout(scrollElement: HTMLElement, spacerElement: HTMLElement): () => void { + return registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => Array.from({ length: ITEM_COUNT }, (_, index) => `w${index}`), + getMeasurements: () => + Array.from({ length: ITEM_COUNT }, (_, index) => ({ + index, + start: index * ITEM_STRIDE, + end: index * ITEM_STRIDE + ITEM_HEIGHT + })) + }) +} + +describe('workspace kanban virtual lane layout', () => { + it('resolves mid-lane and bottom drops immediately after a large scroll jump', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregister = registerLayout(scrollElement, spacerElement) + scrollElement.scrollTop = 3_980 + + expect(resolveWorkspaceKanbanVirtualLaneDropIndex(scrollElement, 370)).toBe(97) + expect(resolveWorkspaceKanbanVirtualLaneDropIndex(scrollElement, 590)).toBe(102) + expect(resolveWorkspaceKanbanVirtualLaneDropIndicatorY(scrollElement, 102)).toBe(613) + unregister() + }) + + it('exposes every logical item even when no card is mounted', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregister = registerLayout(scrollElement, spacerElement) + scrollElement.scrollTop = 3_980 + + const rects = getWorkspaceKanbanVirtualLaneItemRects(scrollElement) + expect(rects).toHaveLength(ITEM_COUNT) + expect(rects?.[50]).toMatchObject({ + id: 'w50', + index: 50, + contentTop: 2_208, + contentBottom: 2_244 + }) + unregister() + }) + + it('selects intermediate never-mounted cards across a jumped lane range', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregister = registerLayout(scrollElement, spacerElement) + scrollElement.scrollTop = 3_980 + const board = { + querySelectorAll: (selector: string) => + selector === '[data-workspace-board-lane-scroll]' ? [scrollElement] : [] + } as unknown as HTMLElement + + const cardRects = getAreaSelectionCardRects(board) + const selectedIds = getAreaSelectionCardIds( + cardRects, + { left: 0, top: 110, width: 300, height: 480 }, + { + scrollStartContentYByElement: new Map([[scrollElement, 18]]), + currentY: 590 + } + ) + + expect(cardRects.every((card) => card.element === null)).toBe(true) + expect(selectedIds).toHaveLength(ITEM_COUNT) + expect(selectedIds).toContain('w50') + unregister() + }) + + it('does not let stale cleanup remove a newer lane registration', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregisterFirst = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => ['old'], + getMeasurements: () => [{ index: 0, start: 0, end: ITEM_HEIGHT }] + }) + const unregisterSecond = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => ['new'], + getMeasurements: () => [{ index: 0, start: 0, end: ITEM_HEIGHT }] + }) + + unregisterFirst() + expect(getWorkspaceKanbanVirtualLaneItemRects(scrollElement)?.map((item) => item.id)).toEqual([ + 'new' + ]) + unregisterSecond() + expect(getWorkspaceKanbanVirtualLaneItemRects(scrollElement)).toBeNull() + }) + + it('places empty-lane and boundary drop indicators from the spacer', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregisterEmpty = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => [], + getMeasurements: () => [] + }) + expect(resolveWorkspaceKanbanVirtualLaneDropIndex(scrollElement, 200)).toBe(0) + expect(resolveWorkspaceKanbanVirtualLaneDropIndicatorY(scrollElement, 0)).toBe(114) + unregisterEmpty() + + const unregister = registerLayout(scrollElement, spacerElement) + expect(resolveWorkspaceKanbanVirtualLaneDropIndex(scrollElement, 90)).toBe(0) + expect(resolveWorkspaceKanbanVirtualLaneDropIndicatorY(scrollElement, 0)).toBe(103) + expect(resolveWorkspaceKanbanVirtualLaneDropIndicatorY(scrollElement, 1)).toBe(148) + unregister() + }) + + it('returns null when measurements are incomplete or mismatched', () => { + const { scrollElement, spacerElement } = createLayoutElements() + const unregisterShort = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => ['a', 'b'], + getMeasurements: () => [{ index: 0, start: 0, end: ITEM_HEIGHT }] + }) + expect(getWorkspaceKanbanVirtualLaneItemRects(scrollElement)).toBeNull() + expect(resolveWorkspaceKanbanVirtualLaneDropIndex(scrollElement, 120)).toBeNull() + unregisterShort() + + const unregisterBadIndex = registerWorkspaceKanbanVirtualLaneLayout({ + scrollElement, + spacerElement, + getItemIds: () => ['a'], + getMeasurements: () => [{ index: 3, start: 0, end: ITEM_HEIGHT }] + }) + expect(getWorkspaceKanbanVirtualLaneItemRects(scrollElement)).toBeNull() + unregisterBadIndex() + }) +}) diff --git a/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.ts b/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.ts new file mode 100644 index 00000000000..e142b3617d6 --- /dev/null +++ b/src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.ts @@ -0,0 +1,159 @@ +import type { VirtualItem } from '@tanstack/react-virtual' + +type VirtualLaneLayoutRegistration = { + spacerElement: HTMLElement + getItemIds: () => readonly string[] + getMeasurements: () => readonly Pick[] +} + +type VirtualLaneLayoutSnapshot = { + registration: VirtualLaneLayoutRegistration + itemIds: readonly string[] + measurements: readonly Pick[] +} + +export type WorkspaceKanbanVirtualLaneItemRect = { + id: string + index: number + left: number + top: number + right: number + bottom: number + contentTop: number + contentBottom: number +} + +const virtualLaneLayouts = new WeakMap() + +export function registerWorkspaceKanbanVirtualLaneLayout(args: { + scrollElement: HTMLElement + spacerElement: HTMLElement + getItemIds: () => readonly string[] + getMeasurements: () => readonly Pick[] +}): () => void { + const registration = { + spacerElement: args.spacerElement, + getItemIds: args.getItemIds, + getMeasurements: args.getMeasurements + } + virtualLaneLayouts.set(args.scrollElement, registration) + return () => { + if (virtualLaneLayouts.get(args.scrollElement) === registration) { + virtualLaneLayouts.delete(args.scrollElement) + } + } +} + +export function getWorkspaceKanbanVirtualLaneItemRects( + scrollElement: HTMLElement +): WorkspaceKanbanVirtualLaneItemRect[] | null { + const snapshot = getVirtualLaneLayoutSnapshot(scrollElement) + if (!snapshot) { + return null + } + + const spacerRect = snapshot.registration.spacerElement.getBoundingClientRect() + const containerRect = scrollElement.getBoundingClientRect() + const contentOffset = spacerRect.top - containerRect.top + scrollElement.scrollTop + const rects: WorkspaceKanbanVirtualLaneItemRect[] = [] + for (let index = 0; index < snapshot.itemIds.length; index++) { + const measurement = snapshot.measurements[index] + if (!isValidMeasurement(measurement, index)) { + return null + } + rects.push({ + id: snapshot.itemIds[index]!, + index, + left: spacerRect.left, + top: spacerRect.top + measurement.start, + right: spacerRect.right, + bottom: spacerRect.top + measurement.end, + contentTop: contentOffset + measurement.start, + contentBottom: contentOffset + measurement.end + }) + } + return rects +} + +export function resolveWorkspaceKanbanVirtualLaneDropIndex( + scrollElement: HTMLElement, + pointerY: number +): number | null { + const snapshot = getVirtualLaneLayoutSnapshot(scrollElement) + if (!snapshot) { + return null + } + + const spacerTop = snapshot.registration.spacerElement.getBoundingClientRect().top + let low = 0 + let high = snapshot.itemIds.length + while (low < high) { + const middle = Math.floor((low + high) / 2) + const measurement = snapshot.measurements[middle] + if (!isValidMeasurement(measurement, middle)) { + return null + } + if (pointerY < spacerTop + (measurement.start + measurement.end) / 2) { + high = middle + } else { + low = middle + 1 + } + } + return low +} + +export function resolveWorkspaceKanbanVirtualLaneDropIndicatorY( + scrollElement: HTMLElement, + dropIndex: number +): number | null { + const snapshot = getVirtualLaneLayoutSnapshot(scrollElement) + if (!snapshot) { + return null + } + if (snapshot.itemIds.length === 0) { + return scrollElement.getBoundingClientRect().top + 14 + } + + const spacerTop = snapshot.registration.spacerElement.getBoundingClientRect().top + const boundedIndex = Math.max(0, Math.min(snapshot.itemIds.length, dropIndex)) + if (boundedIndex === 0) { + const first = snapshot.measurements[0] + return isValidMeasurement(first, 0) ? spacerTop + first.start - 5 : null + } + if (boundedIndex === snapshot.itemIds.length) { + const lastIndex = snapshot.itemIds.length - 1 + const last = snapshot.measurements[lastIndex] + return isValidMeasurement(last, lastIndex) ? spacerTop + last.end + 5 : null + } + const previous = snapshot.measurements[boundedIndex - 1] + const next = snapshot.measurements[boundedIndex] + if (!isValidMeasurement(previous, boundedIndex - 1) || !isValidMeasurement(next, boundedIndex)) { + return null + } + return spacerTop + (previous.end + next.start) / 2 +} + +function getVirtualLaneLayoutSnapshot( + scrollElement: HTMLElement +): VirtualLaneLayoutSnapshot | null { + const registration = virtualLaneLayouts.get(scrollElement) + if (!registration) { + return null + } + const itemIds = registration.getItemIds() + const measurements = registration.getMeasurements() + return measurements.length >= itemIds.length ? { registration, itemIds, measurements } : null +} + +function isValidMeasurement( + measurement: Pick | undefined, + index: number +): measurement is Pick { + return Boolean( + measurement && + measurement.index === index && + Number.isFinite(measurement.start) && + Number.isFinite(measurement.end) && + measurement.end >= measurement.start + ) +} diff --git a/src/renderer/src/runtime/web-session-tabs-sync.test.ts b/src/renderer/src/runtime/web-session-tabs-sync.test.ts index b2049c38936..fcdfbddecb3 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.test.ts @@ -1866,7 +1866,10 @@ describe('applyWebSessionTabsSnapshot', () => { stateStartedAt: NOW - 1_000, agentType: 'codex', paneKey: hostPaneKey, + tabId: 'host-tab-1', + worktreeId: WT, terminalTitle: 'codex [working]', + providerSession: { key: 'session_id', id: 'session-1' }, stateHistory: [] } } @@ -1882,6 +1885,9 @@ describe('applyWebSessionTabsSnapshot', () => { prompt: 'fix web parity', agentType: 'codex', paneKey: mirroredPaneKey, + tabId: mirroredId, + worktreeId: WT, + providerSession: { key: 'session_id', id: 'session-1' }, terminalTitle: 'codex [working]' }) expect(patch.agentStatusByPaneKey?.[hostPaneKey]).toBeUndefined() @@ -1889,7 +1895,7 @@ describe('applyWebSessionTabsSnapshot', () => { expect(patch.sortEpoch).toBe(1) }) - it('bumps aggregate epochs when a mirrored same-state entry gains attribution', () => { + it('repairs mirrored same-state attribution and retains identity from an older snapshot', () => { const hostPaneKey = makePaneKey('host-tab-1', LEAF_ID) const snapshot = makeSnapshot([ { @@ -1910,6 +1916,7 @@ describe('applyWebSessionTabsSnapshot', () => { paneKey: hostPaneKey, worktreeId: WT, tabId: 'host-tab-1', + providerSession: { key: 'session_id', id: 'session-1' }, stateHistory: [] } } @@ -1922,11 +1929,15 @@ describe('applyWebSessionTabsSnapshot', () => { ) as Partial const mirroredPaneKey = Object.keys(initial.agentStatusByPaneKey ?? {})[0]! const existing = initial.agentStatusByPaneKey![mirroredPaneKey]! - const patch = applyWebSessionTabsSnapshot( + const attributionPatch = applyWebSessionTabsSnapshot( makeState({ ...initial, agentStatusByPaneKey: { - [mirroredPaneKey]: { ...existing, worktreeId: 'stale-worktree', tabId: 'stale-tab' } + [mirroredPaneKey]: { + ...existing, + worktreeId: 'stale-worktree', + tabId: 'stale-tab' + } }, agentStatusEpoch: 7, sortEpoch: 11 @@ -1936,8 +1947,109 @@ describe('applyWebSessionTabsSnapshot', () => { NOW ) as Partial - expect(patch.agentStatusEpoch).toBe(8) - expect(patch.sortEpoch).toBe(12) + expect(attributionPatch.agentStatusByPaneKey?.[mirroredPaneKey]).toMatchObject({ + worktreeId: existing.worktreeId, + tabId: existing.tabId + }) + expect(attributionPatch.agentStatusEpoch).toBe(8) + expect(attributionPatch.sortEpoch).toBe(12) + + const fresherAttributionPatch = applyWebSessionTabsSnapshot( + makeState({ + ...initial, + agentStatusByPaneKey: { + [mirroredPaneKey]: { + ...existing, + updatedAt: NOW, + worktreeId: 'stale-worktree', + tabId: 'stale-tab', + providerSession: undefined + } + }, + agentStatusEpoch: 7, + sortEpoch: 11 + }), + { ...snapshot, snapshotVersion: 3 }, + ENV, + NOW + ) as Partial + + expect(fresherAttributionPatch.agentStatusByPaneKey?.[mirroredPaneKey]).toMatchObject({ + worktreeId: existing.worktreeId, + tabId: existing.tabId, + updatedAt: NOW, + providerSession: { key: 'session_id', id: 'session-1' } + }) + expect(fresherAttributionPatch.agentStatusEpoch).toBe(8) + expect(fresherAttributionPatch.sortEpoch).toBe(12) + + const identityPatch = applyWebSessionTabsSnapshot( + makeState({ + ...initial, + ...attributionPatch, + agentStatusByPaneKey: { + [mirroredPaneKey]: { + ...attributionPatch.agentStatusByPaneKey![mirroredPaneKey]!, + updatedAt: NOW, + providerSession: undefined + } + } + }), + { ...snapshot, snapshotVersion: 4 }, + ENV, + NOW + ) as Partial + + expect(identityPatch.agentStatusByPaneKey?.[mirroredPaneKey]?.providerSession).toEqual({ + key: 'session_id', + id: 'session-1' + }) + expect(identityPatch.agentStatusByPaneKey?.[mirroredPaneKey]?.updatedAt).toBe(NOW) + expect(identityPatch.agentStatusEpoch).toBe(8) + expect(identityPatch.sortEpoch).toBe(12) + + const nextTurnPatch = applyWebSessionTabsSnapshot( + makeState({ + ...initial, + agentStatusByPaneKey: { + [mirroredPaneKey]: { + ...existing, + state: 'working', + updatedAt: NOW, + stateStartedAt: NOW, + worktreeId: 'stale-worktree', + tabId: 'stale-tab', + providerSession: undefined + } + } + }), + { + ...snapshot, + snapshotVersion: 5, + tabs: snapshot.tabs.map((tab) => + tab.type === 'terminal' && tab.agentStatus + ? { + ...tab, + agentStatus: { + ...tab.agentStatus, + state: 'done', + providerSession: { key: 'session_id', id: 'previous-session' } + } + } + : tab + ) + }, + ENV, + NOW + ) as Partial + + expect(nextTurnPatch.agentStatusByPaneKey?.[mirroredPaneKey]).toMatchObject({ + state: 'working', + stateStartedAt: NOW, + worktreeId: WT, + tabId: existing.tabId + }) + expect(nextTurnPatch.agentStatusByPaneKey?.[mirroredPaneKey]?.providerSession).toBeUndefined() }) it('keeps mirrored OMP tabs from repainting to Pi-compatible titles', () => { diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 022ba9503b5..ca5f56a2ae0 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -8,6 +8,7 @@ import { AGENT_STATUS_STALE_AFTER_MS, type AgentStatusEntry } from '../../../shared/agent-status-types' +import { agentProviderSessionsEqual } from '../../../shared/agent-session-resume' import type { RuntimeMobileSessionTabsResult, RuntimeMobileSessionBrowserTab, @@ -674,7 +675,8 @@ function remapHostAgentStatus(surface: TerminalSurface): AgentStatusEntry | null }) return { ...normalizeCompatibleAgentStatusEntryForOwner(surface.agentStatus, ownerAgent), - paneKey + paneKey, + tabId: toWebTerminalSurfaceTabId(surface.parentTabId) } } @@ -711,10 +713,23 @@ function buildMirroredAgentStatusPatch( continue } const existing = state.agentStatusByPaneKey[entry.paneKey] - // Why: an active web stream can report a fresher OSC 9999 status before the next host snapshot, so don't rewind it with an older host publication. + // Why: keep fresher OSC state while taking remapped ownership metadata from the authoritative host snapshot. + const hostIdentityPredatesCurrentTurn = + existing !== undefined && + entry.state === 'done' && + existing.state !== 'done' && + existing.stateStartedAt > entry.stateStartedAt const nextEntry = existing && existing.updatedAt > entry.updatedAt - ? normalizeCompatibleAgentStatusEntryForOwner(existing, entry.agentType) + ? { + ...normalizeCompatibleAgentStatusEntryForOwner(existing, entry.agentType), + paneKey: entry.paneKey, + worktreeId: entry.worktreeId ?? existing.worktreeId, + tabId: entry.tabId, + providerSession: + existing.providerSession ?? + (hostIdentityPredatesCurrentTurn ? undefined : entry.providerSession) + } : entry nextByPaneKey.set(entry.paneKey, nextEntry) } @@ -1346,6 +1361,7 @@ function agentStatusEntryEqual(a: AgentStatusEntry | undefined, b: AgentStatusEn a.lastAssistantMessage === b.lastAssistantMessage && a.interrupted === b.interrupted && a.promptInteractionKey === b.promptInteractionKey && + agentProviderSessionsEqual(a.agentType, a.providerSession, b.providerSession) && sameAgentStateHistory(a.stateHistory, b.stateHistory) ) } diff --git a/src/renderer/src/store/slices/repos-all-hosts-generation.test.ts b/src/renderer/src/store/slices/repos-all-hosts-generation.test.ts index a6859b21bb3..24e2c770dce 100644 --- a/src/renderer/src/store/slices/repos-all-hosts-generation.test.ts +++ b/src/renderer/src/store/slices/repos-all-hosts-generation.test.ts @@ -23,14 +23,29 @@ const remoteRepo: Repo = { addedAt: 1 } +const staleRemoteRepo: Repo = { + ...remoteRepo, + path: '/remote-stale', + displayName: 'Remote stale' +} + +const freshRemoteRepo: Repo = { + ...remoteRepo, + path: '/remote-fresh', + displayName: 'Remote fresh' +} + const runtimeEnvironmentCall = vi.fn() +const reposList = vi.fn() beforeEach(() => { clearRuntimeCompatibilityCacheForTests() runtimeEnvironmentCall.mockReset() + reposList.mockReset() + reposList.mockResolvedValue([localRepo]) vi.stubGlobal('window', { api: { - repos: { list: vi.fn().mockResolvedValue([localRepo]) }, + repos: { list: reposList }, projects: { list: vi.fn().mockResolvedValue([]), listHostSetups: vi.fn().mockResolvedValue([]) @@ -46,6 +61,185 @@ beforeEach(() => { }) describe('fetchReposForAllHosts generation', () => { + it('settles startup hydration after the local catalog without waiting for remotes', async () => { + let resolveRemote!: (value: unknown) => void + let markRemoteStarted!: () => void + const remote = new Promise((resolve) => { + resolveRemote = resolve + }) + const remoteStarted = new Promise((resolve) => { + markRemoteStarted = resolve + }) + runtimeEnvironmentCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + if (args.method === 'repo.list') { + markRemoteStarted() + return remote + } + return { + id: 'rpc-other', + ok: true, + result: { projects: [], setups: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + const store = createTestStore() + + const load = store.getState().fetchReposForAllHosts() + await remoteStarted + await store.getState().awaitLocalRepoCatalogSettlement() + + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo']) + + resolveRemote({ + id: 'rpc-repo-list', + ok: true, + result: { repos: [remoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + }) + await load + }) + + it('does not let a remote refresh supersede or delay local startup settlement', async () => { + let resolveLocal!: (repos: Repo[]) => void + let resolveRemote!: (value: unknown) => void + const local = new Promise((resolve) => { + resolveLocal = resolve + }) + const remote = new Promise((resolve) => { + resolveRemote = resolve + }) + reposList.mockReturnValueOnce(local) + runtimeEnvironmentCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + if (args.method === 'repo.list') { + return remote + } + return { + id: 'rpc-other', + ok: true, + result: { projects: [], setups: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + const store = createTestStore() + store.setState({ settings: { activeRuntimeEnvironmentId: 'env-1' } as never }) + + const startup = store.getState().fetchReposForAllHosts({ remoteHosts: 'skip' }) + const remoteRefresh = store.getState().fetchRepos() + let remoteSettled = false + void remoteRefresh.then(() => { + remoteSettled = true + }) + + resolveLocal([localRepo]) + await startup + await store.getState().awaitLocalRepoCatalogSettlement() + + expect(remoteSettled).toBe(false) + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo']) + + resolveRemote({ + id: 'rpc-repo-list', + ok: true, + result: { repos: [remoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + }) + await remoteRefresh + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo', 'remote-repo']) + }) + + it('keeps a newer Connect-flow catalog when an older all-host response resolves last', async () => { + let resolveOlder!: (value: unknown) => void + let markOlderStarted!: () => void + const older = new Promise((resolve) => { + resolveOlder = resolve + }) + const olderStarted = new Promise((resolve) => { + markOlderStarted = resolve + }) + let repoListCalls = 0 + runtimeEnvironmentCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + if (args.method !== 'repo.list') { + return { + id: 'rpc-other', + ok: true, + result: { projects: [], setups: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + } + repoListCalls++ + if (repoListCalls === 1) { + markOlderStarted() + return older + } + return { + id: 'rpc-fresh', + ok: true, + result: { repos: [freshRemoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + const store = createTestStore() + + const allHosts = store.getState().fetchReposForAllHosts() + await olderStarted + await store.getState().fetchRuntimeEnvironmentRepos('env-1') + + resolveOlder({ + id: 'rpc-stale', + ok: true, + result: { repos: [staleRemoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + }) + await allHosts + + expect(store.getState().repos.find((repo) => repo.id === remoteRepo.id)?.path).toBe( + freshRemoteRepo.path + ) + }) + + it('keeps a newer all-host catalog when an older Connect-flow response resolves last', async () => { + let resolveOlder!: (value: unknown) => void + const older = new Promise((resolve) => { + resolveOlder = resolve + }) + let repoListCalls = 0 + runtimeEnvironmentCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + if (args.method !== 'repo.list') { + return { + id: 'rpc-other', + ok: true, + result: { projects: [], setups: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + } + repoListCalls++ + return repoListCalls === 1 + ? older + : { + id: 'rpc-fresh', + ok: true, + result: { repos: [freshRemoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + const store = createTestStore() + + const connect = store.getState().fetchRuntimeEnvironmentRepos('env-1') + await store.getState().fetchReposForAllHosts() + + resolveOlder({ + id: 'rpc-stale', + ok: true, + result: { repos: [staleRemoteRepo] }, + _meta: { runtimeId: 'runtime-remote' } + }) + await connect + + expect(store.getState().repos.find((repo) => repo.id === remoteRepo.id)?.path).toBe( + freshRemoteRepo.path + ) + }) + it('does not validate repo UI from a superseded refresh', async () => { let resolveOlderRemote!: (value: unknown) => void let resolveNewerRemote!: (value: unknown) => void diff --git a/src/renderer/src/store/slices/repos-stale-fetch.test.ts b/src/renderer/src/store/slices/repos-stale-fetch.test.ts index a536224b860..c3c73d204ce 100644 --- a/src/renderer/src/store/slices/repos-stale-fetch.test.ts +++ b/src/renderer/src/store/slices/repos-stale-fetch.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' -import { createTestStore } from './store-test-helpers' -import type { Repo } from '../../../../shared/types' +import { createTestStore, makeLayout, makeTab } from './store-test-helpers' +import type { Repo, WorkspaceSessionState } from '../../../../shared/types' +import { getDefaultWorkspaceSession } from '../../../../shared/constants' import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' const localRepo: Repo = { @@ -20,6 +21,34 @@ const remoteRepo: Repo = { } const reposList = vi.fn() +const promotionWorktreeId = 'local-repo::/local' +const promotionTabId = 'restored-tab' +const promotionPtyId = 'original-daemon-pty' + +function makePromotionSession(): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + activeRepoId: localRepo.id, + activeWorktreeId: promotionWorktreeId, + activeTabId: promotionTabId, + activeWorktreeIdsOnShutdown: [promotionWorktreeId], + tabsByWorktree: { + [promotionWorktreeId]: [ + makeTab({ + id: promotionTabId, + worktreeId: promotionWorktreeId, + ptyId: promotionPtyId + }) + ] + }, + terminalLayoutsByTabId: { + [promotionTabId]: { + ...makeLayout(), + ptyIdsByLeafId: { 'pane:1': promotionPtyId } + } + } + } +} beforeEach(() => { clearRuntimeCompatibilityCacheForTests() @@ -74,4 +103,96 @@ describe('repos slice stale-fetch race (#7020)', () => { await stale expect(store.getState().repos).toEqual([]) }) + + it('waits for the superseding catalog fetch before startup hydration', async () => { + let resolveStartup!: (repos: Repo[]) => void + let resolveRefresh!: (repos: Repo[]) => void + const startupRepos = new Promise((resolve) => { + resolveStartup = resolve + }) + const refreshedRepos = new Promise((resolve) => { + resolveRefresh = resolve + }) + reposList.mockReturnValueOnce(startupRepos).mockReturnValueOnce(refreshedRepos) + const store = createTestStore() + + const startup = store.getState().fetchReposForAllHosts({ remoteHosts: 'skip' }) + const refresh = store.getState().fetchRepos() + resolveStartup([localRepo]) + await startup + + let settled = false + const settlement = store + .getState() + .awaitLocalRepoCatalogSettlement() + .then(() => { + settled = true + }) + await Promise.resolve() + expect(settled).toBe(false) + + resolveRefresh([localRepo]) + await Promise.all([refresh, settlement]) + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo']) + + store.getState().hydrateWorkspaceSession(makePromotionSession()) + + expect(store.getState().activeWorktreeId).toBe(promotionWorktreeId) + expect(store.getState().pendingReconnectPtyIdByTabId).toEqual({ + [promotionTabId]: promotionPtyId + }) + expect(store.getState().terminalLayoutsByTabId[promotionTabId]?.ptyIdsByLeafId).toEqual({ + 'pane:1': promotionPtyId + }) + }) + + it('waits for a refresh started after initial settlement before startup hydration', async () => { + let resolveRefresh!: (repos: Repo[]) => void + const refreshedRepos = new Promise((resolve) => { + resolveRefresh = resolve + }) + reposList.mockResolvedValueOnce([localRepo]).mockReturnValueOnce(refreshedRepos) + const store = createTestStore() + + await store.getState().fetchReposForAllHosts({ remoteHosts: 'skip' }) + await store.getState().awaitLocalRepoCatalogSettlement() + const refresh = store.getState().fetchRepos() + let hydrated = false + const hydration = (async () => { + await store.getState().awaitLocalRepoCatalogSettlement() + store.getState().hydrateWorkspaceSession(makePromotionSession()) + hydrated = true + })() + + await Promise.resolve() + expect(hydrated).toBe(false) + + resolveRefresh([localRepo]) + await Promise.all([refresh, hydration]) + expect(store.getState().activeWorktreeId).toBe(promotionWorktreeId) + expect(store.getState().pendingReconnectPtyIdByTabId[promotionTabId]).toBe(promotionPtyId) + }) + + it('rejects startup settlement when the superseding local catalog fails', async () => { + let resolveStartup!: (repos: Repo[]) => void + const startupRepos = new Promise((resolve) => { + resolveStartup = resolve + }) + reposList.mockReturnValueOnce(startupRepos).mockRejectedValueOnce(new Error('catalog failed')) + const store = createTestStore() + + const startup = store.getState().fetchReposForAllHosts({ remoteHosts: 'skip' }) + const refresh = store.getState().fetchRepos() + resolveStartup([localRepo]) + await Promise.all([startup, refresh]) + + const hydration = async (): Promise => { + await store.getState().awaitLocalRepoCatalogSettlement() + store.getState().hydrateWorkspaceSession(makePromotionSession()) + } + await expect(hydration()).rejects.toThrow('catalog failed') + expect(store.getState().tabsByWorktree).toEqual({}) + expect(store.getState().terminalLayoutsByTabId).toEqual({}) + expect(store.getState().pendingReconnectPtyIdByTabId).toEqual({}) + }) }) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index fc55fa9148d..dac56c84102 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -1530,12 +1530,13 @@ export type RepoSlice = { folderWorkspaces: FolderWorkspace[] folderWorkspacePathStatuses: Record activeRepoId: string | null - // Monotonic sequence so an overlapping fetchRepos can drop its own stale result (#7020). + // Monotonic sequence so overlapping catalog fetches can drop stale same-host results (#7020). reposFetchGeneration: number pendingSshRepoReadoptions: SshRepoReadoption[] recordSshRepoReadoptions: (readoptions: SshRepoReadoption[]) => void fetchRepos: () => Promise fetchReposForAllHosts: (options?: AllHostCatalogFetchOptions) => Promise + awaitLocalRepoCatalogSettlement: () => Promise fetchRuntimeEnvironmentRepos: (environmentId: string) => Promise fetchProjectGroups: () => Promise fetchProjectGroupsForAllHosts: (options?: AllHostCatalogFetchOptions) => Promise @@ -1633,6 +1634,63 @@ export type RepoSlice = { reorderRepos: (orderedIds: string[]) => Promise } +type LocalRepoCatalogFetchOutcome = + | { status: 'fulfilled' } + | { status: 'rejected'; reason: unknown } + +const latestLocalRepoCatalogFetchByStore = new WeakMap< + () => AppState, + Promise +>() +const latestRepoCatalogGenerationByHostByStore = new WeakMap<() => AppState, Map>() +const latestAllHostRepoCatalogGenerationByStore = new WeakMap<() => AppState, number>() + +function startLocalRepoCatalogFetch( + get: () => AppState +): (outcome: LocalRepoCatalogFetchOutcome) => void { + let settle: (outcome: LocalRepoCatalogFetchOutcome) => void = () => undefined + const settlement = new Promise((resolve) => { + settle = resolve + }) + latestLocalRepoCatalogFetchByStore.set(get, settlement) + return settle +} + +async function awaitLatestLocalRepoCatalogFetch(get: () => AppState): Promise { + while (true) { + const pending = latestLocalRepoCatalogFetchByStore.get(get) + if (!pending) { + return + } + const outcome = await pending + if (latestLocalRepoCatalogFetchByStore.get(get) === pending) { + if (outcome.status === 'rejected') { + throw outcome.reason + } + return + } + } +} + +function claimRepoCatalogGeneration(get: () => AppState, hostId: string, generation: number): void { + let generations = latestRepoCatalogGenerationByHostByStore.get(get) + if (!generations) { + generations = new Map() + latestRepoCatalogGenerationByHostByStore.set(get, generations) + } + if ((generations.get(hostId) ?? 0) < generation) { + generations.set(hostId, generation) + } +} + +function isLatestRepoCatalogGeneration( + get: () => AppState, + hostId: string, + generation: number +): boolean { + return latestRepoCatalogGenerationByHostByStore.get(get)?.get(hostId) === generation +} + export const createRepoSlice: StateCreator = (set, get) => ({ repos: [], projects: [], @@ -1670,17 +1728,22 @@ export const createRepoSlice: StateCreator = (set, }), fetchRepos: async () => { + const target = getActiveRuntimeTarget(get().settings) + const settleLocalCatalog: (outcome: LocalRepoCatalogFetchOutcome) => void = + target.kind === 'local' ? startLocalRepoCatalogFetch(get) : () => undefined + let localCatalogOutcome: LocalRepoCatalogFetchOutcome = { status: 'fulfilled' } // Why: overlapping repos:changed fetches can resolve out of order; a stale one must not overwrite a newer result and resurrect deleted projects (#7020). let generation = 0 set((s) => { generation = s.reposFetchGeneration + 1 return { reposFetchGeneration: generation } }) + const targetHostId = getRuntimeTargetHostId(target) + claimRepoCatalogGeneration(get, targetHostId, generation) try { - const target = getActiveRuntimeTarget(get().settings) const catalog = await fetchRepoCatalogForTarget(target) - // A newer fetchRepos superseded us while we awaited — drop this stale result. - if (get().reposFetchGeneration !== generation) { + // A newer same-host fetch superseded us while we awaited — drop this stale result. + if (!isLatestRepoCatalogGeneration(get, targetHostId, generation)) { return } let finalizedHostRepos: Repo[] = [] @@ -1731,7 +1794,10 @@ export const createRepoSlice: StateCreator = (set, }) scheduleSafeAutoForkSync(get, finalizedHostRepos) } catch (err) { + localCatalogOutcome = { status: 'rejected', reason: err } console.error('Failed to fetch repos:', err) + } finally { + settleLocalCatalog(localCatalogOutcome) } }, @@ -1740,11 +1806,19 @@ export const createRepoSlice: StateCreator = (set, runtimeRepoFetchGenerationByEnvironment.set(environmentId, requestGeneration) const connectionGeneration = getEnvironmentSshStateGeneration(environmentId) const runtimeConnectionGeneration = getRuntimeEnvironmentConnectionGeneration(environmentId) + let catalogGeneration = 0 + set((s) => { + catalogGeneration = s.reposFetchGeneration + 1 + return { reposFetchGeneration: catalogGeneration } + }) + const target = { kind: 'environment' as const, environmentId } + const targetHostId = getRuntimeTargetHostId(target) + claimRepoCatalogGeneration(get, targetHostId, catalogGeneration) try { - const target = { kind: 'environment' as const, environmentId } const catalog = await fetchRepoCatalogForTarget(target) if ( runtimeRepoFetchGenerationByEnvironment.get(environmentId) !== requestGeneration || + !isLatestRepoCatalogGeneration(get, targetHostId, catalogGeneration) || getEnvironmentSshStateGeneration(environmentId) !== connectionGeneration || getRuntimeEnvironmentConnectionGeneration(environmentId) !== runtimeConnectionGeneration ) { @@ -1754,6 +1828,7 @@ export const createRepoSlice: StateCreator = (set, set((s) => { if ( runtimeRepoFetchGenerationByEnvironment.get(environmentId) !== requestGeneration || + !isLatestRepoCatalogGeneration(get, targetHostId, catalogGeneration) || getEnvironmentSshStateGeneration(environmentId) !== connectionGeneration || getRuntimeEnvironmentConnectionGeneration(environmentId) !== runtimeConnectionGeneration ) { @@ -1810,15 +1885,21 @@ export const createRepoSlice: StateCreator = (set, }, fetchReposForAllHosts: async (options) => { + const settleLocalCatalog = startLocalRepoCatalogFetch(get) let generation = 0 set((s) => { generation = s.reposFetchGeneration + 1 return { reposFetchGeneration: generation } }) + latestAllHostRepoCatalogGenerationByStore.set(get, generation) + claimRepoCatalogGeneration(get, LOCAL_EXECUTION_HOST_ID, generation) // Why: fetching only the active host hides every other host's repos ("my projects vanished"); load local + all runtime envs, each failing soft. const applyCatalog = (catalog: FetchedRepoCatalog): void => { // Why: a concurrent all-host refresh must not let the older catalog resurrect a migrated SSH owner. - if (get().reposFetchGeneration !== generation) { + if ( + latestAllHostRepoCatalogGenerationByStore.get(get) !== generation || + !isLatestRepoCatalogGeneration(get, catalog.hostId, generation) + ) { return } let hostRepos: Repo[] = [] @@ -1880,13 +1961,20 @@ export const createRepoSlice: StateCreator = (set, // Local first so local repos are present even if a remote fetch stalls. let failed = false + let localCatalogOutcome: LocalRepoCatalogFetchOutcome = { status: 'fulfilled' } try { applyCatalog(await fetchRepoCatalogForTarget({ kind: 'local' })) } catch (err) { failed = true + localCatalogOutcome = { status: 'rejected', reason: err } console.error('Failed to fetch local repos for all-host load:', err) } - if (get().reposFetchGeneration !== generation) { + // Why: startup hydration needs the newest local catalog, not unreachable remote hosts. + settleLocalCatalog(localCatalogOutcome) + if ( + get().reposFetchGeneration !== generation && + !isLatestRepoCatalogGeneration(get, LOCAL_EXECUTION_HOST_ID, generation) + ) { return } if (options?.remoteHosts === 'skip') { @@ -1897,13 +1985,13 @@ export const createRepoSlice: StateCreator = (set, // Why: unreachable remotes can spend the full connect timeout; merge each resolved host via the state updater so parallel loads don't clobber. await Promise.all( environments.map(async (environment) => { + const target = { + kind: 'environment' as const, + environmentId: environment.id + } + claimRepoCatalogGeneration(get, getRuntimeTargetHostId(target), generation) try { - applyCatalog( - await fetchRepoCatalogForTarget({ - kind: 'environment', - environmentId: environment.id - }) - ) + applyCatalog(await fetchRepoCatalogForTarget(target)) } catch (err) { failed = true console.warn(`Skipped repos for runtime environment ${environment.id}:`, err) @@ -1916,6 +2004,8 @@ export const createRepoSlice: StateCreator = (set, } }, + awaitLocalRepoCatalogSettlement: () => awaitLatestLocalRepoCatalogFetch(get), + fetchProjectGroups: async () => { try { const target = getActiveRuntimeTarget(get().settings) diff --git a/tests/e2e/headless-serve-desktop-activation.spec.ts b/tests/e2e/headless-serve-desktop-activation.spec.ts index 1a15cfd2897..e19e0dabf9f 100644 --- a/tests/e2e/headless-serve-desktop-activation.spec.ts +++ b/tests/e2e/headless-serve-desktop-activation.spec.ts @@ -30,6 +30,8 @@ import type { RuntimeTerminalRead } from '../../src/shared/runtime-types' import { PROTOCOL_VERSION } from '../../src/main/daemon/types' +import { parsePaneKey } from '../../src/shared/stable-pane-id' +import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' const electronPackageDir = path.join(process.cwd(), 'node_modules', 'electron') const electronPath = path.join( @@ -68,6 +70,35 @@ function readDaemonPid(userDataDir: string): number { return parsed.pid } +function readPersistedPromotionBinding( + userDataDir: string, + worktreeId: string, + tabId: string, + leafId: string +): { tabId: string; leafId: string; ptyId: string } | null { + try { + const persisted = JSON.parse( + readFileSync( + path.join(userDataDir, 'profiles', DEFAULT_LOCAL_ORCA_PROFILE_ID, 'orca-data.json'), + 'utf8' + ) + ) as { + workspaceSession?: { + tabsByWorktree?: Record + terminalLayoutsByTabId?: Record }> + } + } + const tab = persisted.workspaceSession?.tabsByWorktree?.[worktreeId]?.find( + (candidate) => candidate.id === tabId + ) + const ptyId = + persisted.workspaceSession?.terminalLayoutsByTabId?.[tabId]?.ptyIdsByLeafId?.[leafId] + return tab && ptyId ? { tabId, leafId, ptyId } : null + } catch { + return null + } +} + async function waitForProcessExit(child: ChildProcess, timeoutMs: number): Promise { if (child.exitCode !== null || child.signalCode !== null) { return true @@ -146,8 +177,11 @@ test('promotes the headless owner without replacing its daemon terminal', async title: 'Serve promotion continuity' }) const terminal = created.result.terminal - if (!terminal.ptyId) { - throw new Error('Headless terminal did not expose its daemon PTY id') + const originalPtyId = terminal.ptyId + const originalTabId = terminal.tabId + const paneIdentity = terminal.paneKey ? parsePaneKey(terminal.paneKey) : null + if (!originalPtyId || !originalTabId || !paneIdentity) { + throw new Error('Headless terminal did not expose its durable pane and daemon PTY identity') } const beforeMarker = `SERVE_PROMOTION_BEFORE_${Date.now()}` @@ -190,6 +224,26 @@ test('promotes the headless owner without replacing its daemon terminal', async }) } + await expect + .poll( + () => + readPersistedPromotionBinding( + userDataDir, + terminal.worktreeId, + originalTabId, + paneIdentity.leafId + ), + { + timeout: 15_000, + message: 'headless terminal binding was not persisted during desktop promotion' + } + ) + .toEqual({ + tabId: originalTabId, + leafId: paneIdentity.leafId, + ptyId: originalPtyId + }) + const page = await serveApp.firstWindow({ timeout: 60_000 }) await page.waitForLoadState('domcontentloaded') await page.waitForFunction(() => Boolean(window.__store), null, { timeout: 30_000 }) @@ -223,6 +277,6 @@ test('promotes the headless owner without replacing its daemon terminal', async await closeElectronAppForE2E(serveApp) } await cleanupE2EDaemons(userDataDir) - rmSync(userDataDir, { recursive: true, force: true }) + rmSync(userDataDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }) } }) diff --git a/tests/e2e/workspace-board-lane-virtualization.spec.ts b/tests/e2e/workspace-board-lane-virtualization.spec.ts new file mode 100644 index 00000000000..850a83024a7 --- /dev/null +++ b/tests/e2e/workspace-board-lane-virtualization.spec.ts @@ -0,0 +1,238 @@ +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +const SEEDED_WORKSPACE_COUNT = 300 +const MARQUEE_WORKSPACE_COUNT = 102 + +/** + * Why: the board used to mount every workspace card in every lane in one + * commit, which blocked the sheet open for seconds on a large workspace set. + * These assert the lane renders a window instead, and that each rendered card + * still carries its true lane index so drop targeting stays correct. + */ +test.describe('Workspace board lane virtualization', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + }) + + test('mounts a window of cards for a large lane and keeps lane indexes', async ({ orcaPage }) => { + await orcaPage.evaluate((count) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const repo = state.repos[0] + if (!repo) { + throw new Error('Expected a seeded e2e repo') + } + + const now = Date.now() + const seeded = state.worktreesByRepo[repo.id] ?? [] + const synthetic = Array.from({ length: count }, (_, index) => { + const suffix = String(index).padStart(3, '0') + return { + id: `${repo.id}::/virtual-board-${suffix}`, + instanceId: `virtual-board-${suffix}`, + repoId: repo.id, + path: `${repo.path}/../virtual-board-${suffix}`, + displayName: `Virtual board ${suffix}`, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 10_000 - index, + lastActivityAt: now - index - 100, + head: '0000000000000000000000000000000000000000', + branch: `virtual-board-${suffix}`, + isBare: false, + isMainWorktree: false, + workspaceStatus: 'in-progress' + } + }) + + state.setSidebarOpen(true) + state.setShowSleepingWorkspaces(true) + state.setHideDefaultBranchWorkspace(false) + state.setFilterRepoIds([]) + store.setState({ + sortBy: 'manual', + worktreesByRepo: { ...state.worktreesByRepo, [repo.id]: [...seeded, ...synthetic] } + }) + }, SEEDED_WORKSPACE_COUNT) + + await orcaPage.getByRole('button', { name: 'Workspace board' }).click() + + const cards = orcaPage.locator('[data-workspace-board-card-id]') + // Why: an empty window would also satisfy "fewer than seeded"; the point of + // the change is a filled window, not a blank board. + await expect.poll(() => cards.count(), { timeout: 15_000 }).toBeGreaterThan(3) + expect(await cards.count()).toBeLessThan(SEEDED_WORKSPACE_COUNT / 2) + + const indexes = await cards.evaluateAll((elements) => + elements.map((element) => + Number((element as HTMLElement).dataset.workspaceBoardCardIndex ?? -1) + ) + ) + expect(indexes.every((index) => Number.isInteger(index) && index >= 0)).toBe(true) + expect(new Set(indexes).size).toBe(indexes.length) + }) + + test('renders later lane indexes after the lane scrolls', async ({ orcaPage }) => { + await orcaPage.evaluate((count) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const repo = state.repos[0] + if (!repo) { + throw new Error('Expected a seeded e2e repo') + } + const now = Date.now() + const seeded = state.worktreesByRepo[repo.id] ?? [] + const synthetic = Array.from({ length: count }, (_, index) => ({ + id: `${repo.id}::/virtual-scroll-${index}`, + instanceId: `virtual-scroll-${index}`, + repoId: repo.id, + path: `${repo.path}/../virtual-scroll-${index}`, + displayName: `Virtual scroll ${index}`, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 10_000 - index, + lastActivityAt: now - index - 100, + head: '0000000000000000000000000000000000000000', + branch: `virtual-scroll-${index}`, + isBare: false, + isMainWorktree: false, + workspaceStatus: 'in-progress' + })) + state.setSidebarOpen(true) + state.setShowSleepingWorkspaces(true) + state.setFilterRepoIds([]) + store.setState({ + sortBy: 'manual', + worktreesByRepo: { ...state.worktreesByRepo, [repo.id]: [...seeded, ...synthetic] } + }) + }, SEEDED_WORKSPACE_COUNT) + + await orcaPage.getByRole('button', { name: 'Workspace board' }).click() + + const cards = orcaPage.locator('[data-workspace-board-card-id]') + await expect.poll(() => cards.count(), { timeout: 15_000 }).toBeGreaterThan(3) + + const readMaxIndex = (): Promise => + cards.evaluateAll((elements) => + Math.max( + ...elements.map((element) => + Number((element as HTMLElement).dataset.workspaceBoardCardIndex ?? -1) + ) + ) + ) + const before = await readMaxIndex() + + await orcaPage + .locator('[data-workspace-status="in-progress"] [data-workspace-board-lane-scroll]') + .first() + .evaluate((element) => { + element.scrollTop = element.scrollHeight + element.dispatchEvent(new Event('scroll', { bubbles: true })) + }) + + await expect.poll(readMaxIndex, { timeout: 15_000 }).toBeGreaterThan(before) + }) + + test('selects the full lane across a single large marquee scroll jump', async ({ orcaPage }) => { + const statusId = 'virtual-marquee' + await orcaPage.evaluate( + ({ count, status }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const repo = state.repos[0] + if (!repo) { + throw new Error('Expected a seeded e2e repo') + } + const now = Date.now() + const seeded = state.worktreesByRepo[repo.id] ?? [] + const synthetic = Array.from({ length: count }, (_, index) => ({ + id: `${repo.id}::/virtual-marquee-${index}`, + instanceId: `virtual-marquee-${index}`, + repoId: repo.id, + path: `${repo.path}/../virtual-marquee-${index}`, + displayName: `Virtual marquee ${index}`, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 10_000 - index, + manualOrder: 10_000 - index, + lastActivityAt: now - index - 100, + head: '0000000000000000000000000000000000000000', + branch: `virtual-marquee-${index}`, + isBare: false, + isMainWorktree: false, + workspaceStatus: status + })) + + state.setSidebarOpen(true) + state.setShowSleepingWorkspaces(true) + state.setFilterRepoIds([]) + store.setState({ + sortBy: 'manual', + worktreesByRepo: { ...state.worktreesByRepo, [repo.id]: [...seeded, ...synthetic] } + }) + state.setWorkspaceStatuses([ + { id: status, label: 'Virtual marquee' }, + ...state.workspaceStatuses.filter((entry) => entry.id !== status) + ]) + }, + { count: MARQUEE_WORKSPACE_COUNT, status: statusId } + ) + + await orcaPage.getByRole('button', { name: 'Workspace board' }).click() + + const lane = orcaPage.locator(`[data-workspace-status="${statusId}"]`) + await expect(lane.getByText(String(MARQUEE_WORKSPACE_COUNT), { exact: true })).toBeVisible() + const laneCards = lane.locator('[data-workspace-board-card-id]') + await expect.poll(() => laneCards.count(), { timeout: 15_000 }).toBeGreaterThan(3) + const laneScroll = lane.locator('[data-workspace-board-lane-scroll]') + const box = await laneScroll.boundingBox() + if (!box) { + throw new Error('Expected the marquee lane to have a bounding box') + } + + await orcaPage.mouse.move(box.x + 2, box.y + 12) + await orcaPage.mouse.down() + await orcaPage.mouse.move(box.x + box.width - 18, box.y + 80) + await laneScroll.evaluate(async (element) => { + for (let pass = 0; pass < 4; pass++) { + element.scrollTop = element.scrollHeight + element.dispatchEvent(new Event('scroll', { bubbles: true })) + await new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + }) + } + }) + await orcaPage.mouse.move(box.x + box.width - 18, box.y + box.height - 12) + await orcaPage.mouse.up() + + await expect( + orcaPage.getByText(`${MARQUEE_WORKSPACE_COUNT} selected`, { exact: true }) + ).toBeVisible() + }) +})