From b16a71c7fb044af96b29dc2a2c4f86ee7b9d0122 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:52:22 -0700 Subject: [PATCH] ci: reuse Windows hook builds and shorten test setup and shutdown (#29439) * ci: remove Windows build and persistence test overhead * ci: collect unit phase diagnostics during PR checks * test(ci): seed hook reuse fixture from the canonical prebuild The hook reuse regression first built into a missing private --output, so it always reached Cargo (74s warm on Windows). Build through the default output instead, which reuses a warm canonical prebuild, then copy its executable and fingerprint into the private fixture root. * fix(lint): clear inherited max-lines and broad object tab parameter after main merge Combine the duplicate shared/execution-host imports in runtime-host-services (301 -> 300 lines) and type the overlap test's tab mapper with the RuntimeMobileSessionClientTab owner type instead of object. * fix(ci): align inherited runtime and workflow contracts * test: establish changed runtime probe signatures deterministically * test: await asynchronous daemon profile cleanup * test: shut down owned daemons before deleting profiles * test: clean up oracle daemons during final app close * test: recognize verified Linux zombie exit during daemon teardown * fix: preserve folder SSH ownership when project groups conflict --- .github/workflows/pr.yml | 1 + config/scripts/build-windows-agent-hook.mjs | 2 +- .../build-windows-cli-launcher.test.mjs | 87 ++++++++ .../scripts/check-readme-local-links.test.mjs | 7 + .../check-root-directory-entries.test.mjs | 10 + .../ci-dependency-download-cache.test.mjs | 16 +- config/scripts/ci-draft-pr-admission.test.mjs | 2 + config/scripts/node-server-test-paths.mjs | 1 + config/scripts/vitest-node-runtime-files.mjs | 1 + src/main/amp/hook-service.test.ts | 7 - src/main/command-code/hook-service.test.ts | 2 - .../ipc/runtime-environments-pairing.test.ts | 3 +- src/main/opencode-usage/scanner.test.ts | 21 +- src/main/opencode/hook-service.test.ts | 4 - .../orcad/orcad-daemon-teardown-fixture.ts | 98 +++++++-- .../orcad/orcad-daemon-teardown-linux.test.ts | 186 ++++++++++++++++++ .../orcad-serve-parity.integration.test.ts | 57 ++++-- .../profile-state-access-windows.test.ts | 2 + .../profile-state-access.test.ts | 8 +- ...e-state-automation-storage-upgrade.test.ts | 7 + ...remote-node-runtime-recorded-probe.test.ts | 22 ++- ...agent-session-execution-host-scope.test.ts | 21 ++ .../sleeping-record-execution-host-scope.ts | 2 +- tests/e2e/helpers/orca-restart.ts | 10 +- .../workspace-layout-oracle-session.ts | 5 +- 25 files changed, 512 insertions(+), 70 deletions(-) create mode 100644 src/main/orcad/orcad-daemon-teardown-linux.test.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 45dada409ed..58c18ae68d7 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -896,6 +896,7 @@ jobs: with: node_versions: '["24"]' runner: blacksmith-4vcpu-ubuntu-2404 + phase_diagnostics: true shards: ${{ needs.preflight.outputs.shards }} run_shards: ${{ needs.code_paths.outputs.mobile_ci_tools != 'true' }} browser_contract: ${{ needs.code_paths.outputs.orcad_browser == 'true' }} diff --git a/config/scripts/build-windows-agent-hook.mjs b/config/scripts/build-windows-agent-hook.mjs index 0e4bd1d81f3..02f97e41797 100644 --- a/config/scripts/build-windows-agent-hook.mjs +++ b/config/scripts/build-windows-agent-hook.mjs @@ -19,7 +19,7 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) binName: 'orca-agent-hook', label: 'Windows agent hook', fingerprintInputs: [iconPath, join(repoRoot, 'config/scripts/build-windows-agent-hook.mjs')], - arch: readArg('--arch'), + arch: readArg('--arch') ?? process.arch, outputPath: readArg('--output') ?? join(crateRoot, '.build', 'orca-agent-hook.exe'), buildEnv: (version) => ({ ORCA_HOOK_BUILD_VERSION: version, diff --git a/config/scripts/build-windows-cli-launcher.test.mjs b/config/scripts/build-windows-cli-launcher.test.mjs index ba7ff5adade..24d75862a1f 100644 --- a/config/scripts/build-windows-cli-launcher.test.mjs +++ b/config/scripts/build-windows-cli-launcher.test.mjs @@ -88,6 +88,93 @@ function copySharedLauncher(target) { } describe('Windows CLI launcher', () => { + itWindows('reuses the hook host target while invalidating changed inputs', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-hook-target-')) + try { + const hookScript = join('config', 'scripts', 'build-windows-agent-hook.mjs') + const crate = join('native', 'windows-agent-hook') + const outputFile = join(crate, '.build', 'orca-agent-hook.exe') + const outputPath = join(root, outputFile) + const fingerprintPath = `${outputPath}.sha256` + // Why the default output: it reuses a warm canonical prebuild; Cargo runs only when that is missing or stale. + const first = runProcessSync({ + program: process.execPath, + args: [hookScript], + cwd: projectRoot, + timeoutMs: 300_000 + }) + expect(first.code, `${first.stdout}\n${first.stderr}`).toBe(0) + for (const file of [ + outputFile, + `${outputFile}.sha256`, + 'package.json', + hookScript, + join('config', 'scripts', 'windows-rust-executable-build.mjs'), + join('config', 'scripts', 'windows-pe-imports.mjs'), + join('resources', 'build', 'icon.ico'), + ...[ + 'src/main.rs', + 'build.rs', + 'Cargo.toml', + 'Cargo.lock', + 'app.manifest', + '.cargo/config.toml' + ].map((file) => join(crate, ...file.split('/'))) + ]) { + mkdirSync(dirname(join(root, file)), { recursive: true }) + copyFileSync(join(projectRoot, file), join(root, file)) + } + const fingerprint = readFileSync(fingerprintPath, 'utf8') + const binary = readFileSync(outputPath) + // A reuse hit must succeed even when Cargo is unavailable. + const environment = Object.fromEntries( + Object.entries(process.env).filter(([key]) => key.toLowerCase() !== 'path') + ) + environment.PATH = '' + const invoke = (args = []) => + runProcessSync({ + program: process.execPath, + args: [hookScript, ...args], + cwd: root, + env: environment, + timeoutMs: 10_000 + }) + for (const args of [[], ['--arch', process.arch]]) { + const reused = invoke(args) + expect(reused.code, `${reused.stdout}\n${reused.stderr}`).toBe(0) + expect(reused.stdout).toContain('reusing Windows agent hook') + expect(readFileSync(fingerprintPath, 'utf8')).toBe(fingerprint) + expect(readFileSync(outputPath)).toEqual(binary) + } + const otherArchitecture = process.arch === 'x64' ? 'arm64' : 'x64' + const crossTarget = invoke(['--arch', otherArchitecture]) + expect(crossTarget.code).not.toBe(0) + expect(crossTarget.stderr).toContain('Unable to run cargo') + const unsupported = invoke(['--arch', 'ia32']) + expect(unsupported.code).not.toBe(0) + expect(unsupported.stderr).toContain('Unsupported Windows executable architecture: ia32') + for (const file of [join(crate, 'src', 'main.rs'), 'package.json']) { + const path = join(root, file) + const original = readFileSync(path, 'utf8') + writeFileSync(fingerprintPath, fingerprint) + writeFileSync( + path, + file === 'package.json' + ? JSON.stringify({ ...JSON.parse(original), version: '99.0.0' }) + : `${original}\n// changed fixture source\n` + ) + const changed = invoke() + expect(changed.code).not.toBe(0) + expect(changed.stderr).toContain('Unable to run cargo') + writeFileSync(path, original) + } + writeFileSync(fingerprintPath, fingerprint) + expect(invoke().code).toBe(0) + } finally { + removeFixtureTree(root) + } + }) + it('reuses restored builds only while all embedded inputs and the release version match', () => { const root = mkdtempSync(join(tmpdir(), 'orca-cli-launcher-reuse-')) try { diff --git a/config/scripts/check-readme-local-links.test.mjs b/config/scripts/check-readme-local-links.test.mjs index 0fdc6199698..98818ba2a8c 100644 --- a/config/scripts/check-readme-local-links.test.mjs +++ b/config/scripts/check-readme-local-links.test.mjs @@ -2,6 +2,7 @@ import { execFileSync } from 'node:child_process' import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import path from 'node:path' +import { runInNewContext } from 'node:vm' import { afterEach, describe, expect, it, vi } from 'vitest' import { parse } from 'yaml' import { findBrokenReadmeLinks, main } from './check-readme-local-links.mjs' @@ -162,7 +163,13 @@ describe('README local link check', () => { const workflow = parse(readFileSync(path.join(projectDir, '.github/workflows/pr.yml'), 'utf8')) const guardJob = workflow.jobs.code_paths const step = guardJob.steps.find((candidate) => candidate.name === 'Check README local links') + const runsFor = (draft) => + runInNewContext(guardJob.if, { + github: { event_name: 'pull_request', event: { pull_request: { draft } } } + }) + expect(runsFor(false)).toBe(true) + expect(runsFor(true)).toBe(false) expect(guardJob.needs).toBeUndefined() expect(step.if).toBeUndefined() expect(step.run).toBe('node config/scripts/check-readme-local-links.mjs') diff --git a/config/scripts/check-root-directory-entries.test.mjs b/config/scripts/check-root-directory-entries.test.mjs index a79fe1567b2..771ef9ac3fe 100644 --- a/config/scripts/check-root-directory-entries.test.mjs +++ b/config/scripts/check-root-directory-entries.test.mjs @@ -2,6 +2,7 @@ import { execFileSync, spawnSync } from 'node:child_process' import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join, resolve } from 'node:path' +import { runInNewContext } from 'node:vm' import { afterEach, describe, expect, it } from 'vitest' import { parse } from 'yaml' @@ -237,6 +238,15 @@ describe('root directory guard', () => { (step) => step.name === 'Reject new root-level files and folders' ) + // Only draft PRs skip the guard; verify skips with it, so no draft reports a result. + const runsFor = (expression, draft) => + runInNewContext(expression.replace(/^\$\{\{(.*)\}\}$/s, '$1'), { + github: { event_name: 'pull_request', event: { pull_request: { draft } } }, + cancelled: () => false + }) + expect(runsFor(guardJob.if, false)).toBe(true) + expect(runsFor(guardJob.if, true)).toBe(false) + expect(runsFor(workflow.jobs.verify.if, true)).toBe(false) expect(guardStep.if).toBeUndefined() // Why >= 2 rather than 0: the guard compares the merge commit's first parent against the // merged tree, so it needs both parents present but no history beyond them. Depth 1 would diff --git a/config/scripts/ci-dependency-download-cache.test.mjs b/config/scripts/ci-dependency-download-cache.test.mjs index 01221cc5ffe..706cd018e43 100644 --- a/config/scripts/ci-dependency-download-cache.test.mjs +++ b/config/scripts/ci-dependency-download-cache.test.mjs @@ -349,13 +349,19 @@ describe('CI dependency download caches', () => { const shared = workflow('ci-cache-warmup') const platformWorkflow = workflow('ci-cache-warmup-platforms') const platforms = read(platformWorkflow.jobs['warm-platforms'].uses) - const context = { github: { event_name: event, event: { pull_request: { draft } } } } + const contextFor = (isDraft) => ({ + github: { + event_name: event, + event: event === 'pull_request' ? { pull_request: { draft: isDraft } } : {} + } + }) + const context = contextFor(draft) expect(runInNewContext(platforms.jobs['warm-linux-runtime'].if, context)).toBe(armRuns) expect(runInNewContext(shared.jobs['warm-platforms'].if, context)).toBe(sharedPlatforms) - expect(runInNewContext(shared.jobs.warm.if ?? 'true', context)).toBe(admitted) - expect(runInNewContext(platformWorkflow.jobs['warm-platforms'].if ?? 'true', context)).toBe( - admitted - ) + for (const guard of [shared.jobs.warm.if, platformWorkflow.jobs['warm-platforms'].if]) { + expect(runInNewContext(guard, context)).toBe(admitted) + expect(runInNewContext(guard, contextFor(!draft))).toBe(event !== 'pull_request' || draft) + } expect(platforms.jobs['warm-windows'].if).toBeUndefined() expect(platforms.jobs['warm-windows'].strategy.matrix.os).toEqual([ 'windows-2022', diff --git a/config/scripts/ci-draft-pr-admission.test.mjs b/config/scripts/ci-draft-pr-admission.test.mjs index 8c84d693fa9..a925717234d 100644 --- a/config/scripts/ci-draft-pr-admission.test.mjs +++ b/config/scripts/ci-draft-pr-admission.test.mjs @@ -73,6 +73,8 @@ it('keeps the root and README guards on ready docs-only PRs without another runn const { workflow: prWorkflow } = workflows.find(({ file }) => file === 'pr.yml') const detector = prWorkflow.jobs.code_paths const eligible = compileAggregateExpression(detector.if ?? 'true', ['github', 'steps', 'needs']) + expect(admitted(detector.if, true)).toBe(false) + expect(admitted(prWorkflow.jobs.verify.if, true)).toBe(false) for (const shouldRun of ['false', 'true']) { expect( eligible({ diff --git a/config/scripts/node-server-test-paths.mjs b/config/scripts/node-server-test-paths.mjs index b66d83f6405..7dc21a1e530 100644 --- a/config/scripts/node-server-test-paths.mjs +++ b/config/scripts/node-server-test-paths.mjs @@ -29,6 +29,7 @@ export function nodeServerTestPaths({ 'src/main/orcad/orcad-stop-request-listener.test.ts', 'src/main/orcad/orcad-managed-stop.test.ts', 'src/main/orcad/orcad-managed-stop-cancellation.test.ts', + 'src/main/orcad/orcad-daemon-teardown-linux.test.ts', // The directory, not a prefix: its siblings are POSIX-host unit tests pr.yml already runs. 'src/main/daemon/pty-subprocess/', 'src/main/daemon/pty-subprocess-spawn-file-foreground.test.ts', diff --git a/config/scripts/vitest-node-runtime-files.mjs b/config/scripts/vitest-node-runtime-files.mjs index 6dc9051e8cb..1e50492bf60 100644 --- a/config/scripts/vitest-node-runtime-files.mjs +++ b/config/scripts/vitest-node-runtime-files.mjs @@ -19,6 +19,7 @@ export const NODE_RUNTIME_INCLUDE = [ // Electron main bridges streams with Node's implementation. 'src/main/host/electron-http-client.test.ts', 'src/main/daemon/daemon-init-provider-installation.test.ts', + 'src/main/orcad/orcad-daemon-teardown-linux.test.ts', 'src/renderer/src/components/terminal-pane/pty-input-write-queue.test.ts', 'src/renderer/src/lib/react-commit-cascade-observer.test.ts', 'src/renderer/src/lib/react-commit-cascade-telemetry.test.ts', diff --git a/src/main/amp/hook-service.test.ts b/src/main/amp/hook-service.test.ts index 5ed0f6c5e3e..48875d122b0 100644 --- a/src/main/amp/hook-service.test.ts +++ b/src/main/amp/hook-service.test.ts @@ -42,13 +42,6 @@ describe('AmpHookService', () => { }) const source = readFileSync(status.configPath, 'utf-8') - expect(source).toContain(_internals.AMP_PLUGIN_MARKER) - expect(source).toContain('/hook/amp') - expect(source).toContain("amp.on('session.start'") - expect(source).toContain("amp.on('agent.start'") - expect(source).toContain("amp.on('tool.call'") - expect(source).toContain("amp.on('tool.result'") - expect(source).toContain("amp.on('agent.end'") expect(source).toContain('return { action: "allow" }') expect(source).toContain('const MAX_PENDING_POSTS = 50') expect(source).toContain('let postQueue: QueuedPost[] = []') diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index 41402ae03af..1bc21c16997 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -96,8 +96,6 @@ describe('CommandCodeHookService', () => { expect(script).toContain('orca-dev\\agent-hooks') expect(script).toContain('set ORCA_AGENT_HOOK_PORT=') } else { - expect(script).toContain('Command Code strips TOKEN-like env vars') - expect(script).toContain('Command Code sanitizes hook subprocess env') expect(script).toContain('__orca_read_ancestor_var') expect(script).toContain('__orca_fill_from_endpoint_file') expect(script).toContain('[ "$__orca_endpoint_port" != "$ORCA_AGENT_HOOK_PORT" ]') diff --git a/src/main/ipc/runtime-environments-pairing.test.ts b/src/main/ipc/runtime-environments-pairing.test.ts index a7bb7ecf541..a5ae600ea4f 100644 --- a/src/main/ipc/runtime-environments-pairing.test.ts +++ b/src/main/ipc/runtime-environments-pairing.test.ts @@ -141,7 +141,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { }) it('replaces stale registrations and keeps one binary listener when registered again', async () => { - const channels = new Set(['runtimeEnvironments:list']) + const channels = new Set(['runtimeEnvironments:list', 'runtimeEnvironments:restartOrcad']) const listeners = new EventEmitter() listeners.on('runtimeEnvironments:subscriptionBinary', vi.fn()) handleMock.mockImplementation((channel: string) => { @@ -166,6 +166,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { registerRuntimeEnvironmentHandlers(store as never) expect(listeners.listenerCount('runtimeEnvironments:subscriptionBinary')).toBe(1) + expect(channels.has('runtimeEnvironments:restartOrcad')).toBe(true) const list = handler('runtimeEnvironments:list') expect(await list(null, undefined)).toEqual([]) } finally { diff --git a/src/main/opencode-usage/scanner.test.ts b/src/main/opencode-usage/scanner.test.ts index 9ca0d8e73eb..123b29d997b 100644 --- a/src/main/opencode-usage/scanner.test.ts +++ b/src/main/opencode-usage/scanner.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, rmSync, unlinkSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, statSync, unlinkSync, utimesSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' @@ -437,6 +437,17 @@ describe('scanOpenCodeUsageDatabases', () => { return path } + function appendLiveSessionRow(path: string, sessionId: string, inputTokens: number): void { + const previousMtimeMs = statSync(path).mtimeMs + const db = new Database(path) + insertSessionTotalsRow(db, sessionId, inputTokens) + db.close() + // Why: the row fits the existing page and can land in the same coarse mtime + // tick, so pin a later mtime to change the scanner's mtime+size fingerprint. + const changedAt = new Date(previousMtimeMs + 1000) + utimesSync(path, changedAt, changedAt) + } + it('does not scan disk databases when OPENCODE_DB uses memory', async () => { writeSessionTotalsDb('opencode.db', []) process.env.OPENCODE_DB = ':memory:' @@ -491,9 +502,7 @@ describe('scanOpenCodeUsageDatabases', () => { // The canonical db keeps growing while the backup stays cached; session-1 // must stay owned by the canonical db. - const db = new Database(canonicalPath) - insertSessionTotalsRow(db, 'session-2', 200) - db.close() + appendLiveSessionRow(canonicalPath, 'session-2', 200) const third = await scanOpenCodeUsageDatabases([], second.processedDatabases) expect( @@ -517,9 +526,7 @@ describe('scanOpenCodeUsageDatabases', () => { ) expect(firstBackup?.ownedSessionIds).toEqual([]) - const db = new Database(canonicalPath) - insertSessionTotalsRow(db, 'session-2', 200) - db.close() + appendLiveSessionRow(canonicalPath, 'session-2', 200) const second = await scanOpenCodeUsageDatabases([], first.processedDatabases) const secondBackup = second.processedDatabases.find((database) => diff --git a/src/main/opencode/hook-service.test.ts b/src/main/opencode/hook-service.test.ts index 482df3671f7..6eee14d66e4 100644 --- a/src/main/opencode/hook-service.test.ts +++ b/src/main/opencode/hook-service.test.ts @@ -130,7 +130,6 @@ describe('OpenCodeHookService buildPtyEnv / clearPty round-trip', () => { expect(existsSync(pluginPath)).toBe(true) // Sanity-check the file has plugin source, not a stray write. const pluginSource = readFileSync(pluginPath, 'utf8') - expect(pluginSource).toContain('OrcaOpenCodeStatusPlugin') expect(pluginSource).toContain('messageID: part.messageID') // Legacy attach loads an object TUI entry; the server export stays callable. const tuiEntry = join( @@ -139,9 +138,6 @@ describe('OpenCodeHookService buildPtyEnv / clearPty round-trip', () => { 'orca-opencode-status-tui', 'tui.js' ) - expect(readFileSync(tuiEntry, 'utf8')).toContain('tui: setupLegacyOpenCodeTui') - expect(readFileSync(tuiEntry, 'utf8')).toContain('setup: setupOpenCode2Status') - expect(pluginSource).not.toContain('export default { id:') const past = new Date('2020-01-01T00:00:00Z') utimesSync(tuiEntry, past, past) service.buildPtyEnv(daemonSessionId) diff --git a/src/main/orcad/orcad-daemon-teardown-fixture.ts b/src/main/orcad/orcad-daemon-teardown-fixture.ts index ac524676de0..64fec467e75 100644 --- a/src/main/orcad/orcad-daemon-teardown-fixture.ts +++ b/src/main/orcad/orcad-daemon-teardown-fixture.ts @@ -7,8 +7,12 @@ */ import { existsSync, readdirSync } from 'node:fs' import { join } from 'node:path' -import { removeTreeSync } from '../../shared/windows-transient-lock-removal' +import { removeTree } from '../../shared/windows-transient-lock-removal' +import { parseLinuxStartTicks } from '../agent-hooks/managed-hook-owner-identity' +import { parseLinuxProcessState } from '../daemon/daemon-incarnation-evidence' import { readDaemonPidRecord } from '../daemon/daemon-endpoint-incarnation' +import { readLinuxStat } from '../daemon/daemon-process-inspection' +import { cleanupDaemonForProtocol } from '../daemon/daemon-protocol-cleanup' import type { spawnProcess } from '@orca/process-host' const EXIT_WAIT_MS = 10_000 @@ -16,6 +20,12 @@ const POLL_MS = 50 const REMOVE_ATTEMPTS = 8 const REMOVE_RETRY_MS = 150 +type ProcessExitDependencies = { + platform?: NodeJS.Platform + signalProcess?: (pid: number) => void + readStat?: typeof readLinuxStat +} + const sleep = (ms: number): Promise => new Promise((settle) => setTimeout(settle, ms)) function errorCode(error: unknown): string | undefined { @@ -27,17 +37,62 @@ function errorCode(error: unknown): string | undefined { : undefined } -function hasExited(pid: number): boolean { +async function hasExited(pid: number, dependencies: ProcessExitDependencies): Promise { + const signalProcess = dependencies.signalProcess ?? ((pid) => process.kill(pid, 0)) try { - process.kill(pid, 0) - return false + signalProcess(pid) } catch (error) { // EPERM still proves the process exists. return errorCode(error) === 'ESRCH' } + if ((dependencies.platform ?? process.platform) !== 'linux') { + return false + } + const stat = await (dependencies.readStat ?? readLinuxStat)(pid) + // An unreaped zombie has exited even though kill(pid, 0) still succeeds. + return ( + stat.status === 'present' && + stat.value.startsWith(`${pid} (`) && + /^\d+$/.test(parseLinuxStartTicks(stat.value) ?? '') && + parseLinuxProcessState(stat.value) === 'Z' + ) } -/** SIGKILLs each pid and waits, bounded, until the OS no longer knows it. */ +/** Requires process exit; Linux zombies have exited but still await their parent's reaper. */ +export async function waitForProcessExit( + pids: Iterable, + dependencies: ProcessExitDependencies = {} +): Promise { + let remaining = [...pids] + const deadline = Date.now() + EXIT_WAIT_MS + let deadlineTimer: ReturnType | undefined + const timedOut = new Promise((settle) => { + deadlineTimer = setTimeout(() => settle(null), EXIT_WAIT_MS) + }) + try { + while (remaining.length > 0) { + const exited = await Promise.race([ + Promise.all(remaining.map((pid) => hasExited(pid, dependencies))), + timedOut + ]) + if (exited === null) { + break + } + remaining = remaining.filter((_pid, index) => !exited[index]) + if (remaining.length === 0 || Date.now() >= deadline) { + break + } + await sleep(Math.min(POLL_MS, Math.max(0, deadline - Date.now()))) + } + } finally { + clearTimeout(deadlineTimer) + } + if (remaining.length > 0) { + throw new Error(`Processes did not exit: ${remaining.join(', ')}`) + } +} + +/** SIGKILLs each pid and requires the OS to confirm its exit. */ export async function killAndAwaitExit(pids: Iterable): Promise { const pending = [...pids] for (const pid of pending) { @@ -45,13 +100,10 @@ export async function killAndAwaitExit(pids: Iterable): Promise { process.kill(pid, 'SIGKILL') } catch {} } - const deadline = Date.now() + EXIT_WAIT_MS - while (pending.some((pid) => !hasExited(pid)) && Date.now() < deadline) { - await sleep(POLL_MS) - } + await waitForProcessExit(pending) } -/** Kills a spawned child and waits for its exit event, so nothing it spawned races teardown. */ +/** Kills a spawned child and waits for its exit event. */ export async function killChildAndWait(child: ReturnType): Promise { if (child.exitCode !== null || child.signalCode !== null) { return @@ -61,23 +113,33 @@ export async function killChildAndWait(child: ReturnType): await exited } -/** Kills every terminal daemon `/daemon` names and waits until each has exited. */ +/** Authenticates each profile daemon, shuts down its PTYs, and requires process exit. */ export async function killProfileDaemons(userData: string): Promise { const daemonDir = join(userData, 'daemon') - const pids = (existsSync(daemonDir) ? readdirSync(daemonDir) : []).flatMap((name) => { - const pid = /^daemon-v\d+\.pid$/u.test(name) - ? readDaemonPidRecord(join(daemonDir, name))?.pid - : undefined - return pid ? [pid] : [] + const daemons = (existsSync(daemonDir) ? readdirSync(daemonDir) : []).flatMap((name) => { + const match = /^daemon-v(\d+)\.pid$/u.exec(name) + const pid = match ? readDaemonPidRecord(join(daemonDir, name))?.pid : undefined + return pid && match ? [{ pid, protocolVersion: Number(match[1]) }] : [] }) - await killAndAwaitExit(pids) + const results = await Promise.allSettled( + daemons.map(async ({ pid, protocolVersion }) => { + await cleanupDaemonForProtocol(daemonDir, protocolVersion, pid) + await waitForProcessExit([pid]) + }) + ) + const failures = results.flatMap((result) => + result.status === 'rejected' ? [result.reason] : [] + ) + if (failures.length > 0) { + throw new AggregateError(failures, 'Profile daemon shutdown incomplete') + } } /** Removes a test root, retrying a late writer's ENOTEMPTY/EBUSY on every platform. */ export async function removeTestRoot(root: string): Promise { for (let attempt = 1; ; attempt += 1) { try { - removeTreeSync(root) + await removeTree(root) return } catch (error) { if ( diff --git a/src/main/orcad/orcad-daemon-teardown-linux.test.ts b/src/main/orcad/orcad-daemon-teardown-linux.test.ts new file mode 100644 index 00000000000..2ea4a8399c2 --- /dev/null +++ b/src/main/orcad/orcad-daemon-teardown-linux.test.ts @@ -0,0 +1,186 @@ +import { spawnProcess } from '@orca/process-host' +import { describe, expect, it, vi } from 'vitest' +import { readAgentProcess } from '../../shared/agent-process-presence-probe' +import type { LinuxStatEvidence } from '../daemon/daemon-incarnation-evidence-types' +import { killAndAwaitExit, waitForProcessExit } from './orcad-daemon-teardown-fixture' + +const ZOMBIE_PARENT = ` +import os, select, sys +pid = os.fork() +if pid == 0: + os._exit(0) +try: + print(pid, flush=True) + select.select([sys.stdin], [], [], 25) +finally: + os.waitpid(pid, 0) +` + +describe('daemon teardown exit proof', () => { + it.skipIf(process.platform !== 'linux')( + 'accepts an unreaped zombie while its owned parent remains alive, then reaps it', + async () => { + const parent = spawnProcess({ + program: process.env.PYTHON || 'python3', + args: ['-c', ZOMBIE_PARENT], + env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1' }, + timeoutMs: null + }) + let stdout = '' + let stderr = '' + parent.stdout.on('data', (chunk: Buffer) => (stdout += chunk.toString())) + parent.stderr.on('data', (chunk: Buffer) => (stderr += chunk.toString())) + const closed = new Promise((resolve, reject) => { + parent.once('error', reject) + parent.once('close', resolve) + }) + let zombiePid = 0 + try { + await vi.waitFor( + async () => { + zombiePid = Number(stdout.trim()) + expect(Number.isSafeInteger(zombiePid) && zombiePid > 0).toBe(true) + expect(await readAgentProcess(zombiePid)).toMatchObject({ + verdict: 'live', + zombie: true + }) + }, + { timeout: 5_000, interval: 25 } + ) + expect(() => process.kill(zombiePid, 0)).not.toThrow() + await waitForProcessExit([zombiePid]) + await killAndAwaitExit([zombiePid]) + expect(parent.exitCode).toBeNull() + expect(() => process.kill(zombiePid, 0)).not.toThrow() + expect(await readAgentProcess(zombiePid)).toMatchObject({ verdict: 'live', zombie: true }) + } finally { + // EOF releases waitpid even when an assertion fails; never orphan the zombie. + parent.stdin.end() + expect(await closed).toBe(0) + expect(stderr).toBe('') + if (zombiePid > 0) { + expect(() => process.kill(zombiePid, 0)).toThrow( + expect.objectContaining({ code: 'ESRCH' }) + ) + } + } + }, + 20_000 + ) + + it('still rejects a live process at the unchanged ten-second deadline', async () => { + vi.useFakeTimers() + try { + const rejected = expect(waitForProcessExit([process.pid])).rejects.toThrow( + `Processes did not exit: ${process.pid}` + ) + await vi.advanceTimersByTimeAsync(10_000) + await rejected + expect(() => process.kill(process.pid, 0)).not.toThrow() + } finally { + vi.useRealTimers() + } + }) + + const zombieStat = `42 (daemon) Z ${Array(18).fill('0').join(' ')} 100` + it.each<[string, LinuxStatEvidence]>([ + ['missing proc entry after occupied signal', { status: 'missing' }], + ['unavailable proc entry', { status: 'unavailable' }], + ['truncated zombie entry', { status: 'present', value: '42 (daemon) Z' }], + ['malformed start ticks', { status: 'present', value: zombieStat.replace(/100$/, 'bad') }], + ['different pid', { status: 'present', value: zombieStat.replace(/^42/, '43') }], + ['live proc entry', { status: 'present', value: zombieStat.replace(') Z', ') S') }] + ])('does not accept %s as exit proof', async (_name, stat) => { + vi.useFakeTimers() + try { + const rejected = expect( + waitForProcessExit([42], { + platform: 'linux', + signalProcess: () => {}, + readStat: async () => stat + }) + ).rejects.toThrow('Processes did not exit: 42') + await vi.advanceTimersByTimeAsync(10_000) + await rejected + } finally { + vi.useRealTimers() + } + }) + + it('bounds a stalled proc read without treating timeout as exit', async () => { + vi.useFakeTimers() + try { + const rejected = expect( + waitForProcessExit([42], { + platform: 'linux', + signalProcess: () => {}, + readStat: () => new Promise(() => {}) + }) + ).rejects.toThrow('Processes did not exit: 42') + await vi.advanceTimersByTimeAsync(10_000) + await rejected + } finally { + vi.useRealTimers() + } + }) + + it.each(['EPERM', 'EACCES'])( + 'keeps signal %s pending without accepting proc evidence', + async (code) => { + vi.useFakeTimers() + const readStat = vi.fn(async (): Promise => ({ + status: 'present', + value: zombieStat + })) + try { + const rejected = expect( + waitForProcessExit([42], { + platform: 'linux', + signalProcess: () => { + throw Object.assign(new Error(code), { code }) + }, + readStat + }) + ).rejects.toThrow('Processes did not exit: 42') + await vi.advanceTimersByTimeAsync(10_000) + await rejected + expect(readStat).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + } + ) + + it('preserves ESRCH exit proof without requiring proc evidence', async () => { + const readStat = vi.fn() + await waitForProcessExit([42], { + platform: 'linux', + signalProcess: () => { + throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' }) + }, + readStat + }) + expect(readStat).not.toHaveBeenCalled() + }) + + it.each(['darwin', 'win32'])( + 'keeps occupied %s processes pending', + async (platform) => { + vi.useFakeTimers() + const readStat = vi.fn(async (): Promise => ({ + status: 'present', + value: zombieStat + })) + try { + const rejected = expect( + waitForProcessExit([42], { platform, signalProcess: () => {}, readStat }) + ).rejects.toThrow('Processes did not exit: 42') + await vi.advanceTimersByTimeAsync(10_000) + await rejected + expect(readStat).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + } + ) +}) diff --git a/src/main/orcad/orcad-serve-parity.integration.test.ts b/src/main/orcad/orcad-serve-parity.integration.test.ts index e59beb8d123..74f6445f000 100644 --- a/src/main/orcad/orcad-serve-parity.integration.test.ts +++ b/src/main/orcad/orcad-serve-parity.integration.test.ts @@ -21,7 +21,8 @@ import { killAndAwaitExit, killChildAndWait, killProfileDaemons, - removeTestRoot + removeTestRoot, + waitForProcessExit } from './orcad-daemon-teardown-fixture' const slotDir = resolve('out/orcad') @@ -32,10 +33,28 @@ const skip = skipForMissingInputs('artifact', runtime ? [] : ['a Node orcad slot const roots: string[] = [] afterEach(async () => { - for (const root of roots.splice(0)) { - // The terminal daemon outlives orcad by design; it must be gone before its profile is. - await killProfileDaemons(root) - await removeTestRoot(root) + const failures: unknown[] = [] + // Stop every profile before removing any project root it may still use. + for (const root of roots) { + try { + await killProfileDaemons(root) + } catch (error) { + failures.push(error) + } + } + if (failures.length > 0) { + throw new AggregateError(failures, 'Profile daemon shutdown incomplete') + } + for (const root of roots.slice()) { + try { + await removeTestRoot(root) + roots.splice(roots.indexOf(root), 1) + } catch (error) { + failures.push(error) + } + } + if (failures.length > 0) { + throw new AggregateError(failures, 'Test root removal incomplete') } }) @@ -161,9 +180,10 @@ describe.skipIf(skip)('orca serve on orcad', () => { return JSON.parse(result.stdout.trim().split('\n').at(-1) ?? '{}') } const first = await serveOnce(userData) - let daemonPid: number | null = first + let shellPid: number | null = null try { const created = await session('create', 'BEFORE') + shellPid = created.pid expect(created).toMatchObject({ isReattach: false, output: true }) await stopServe(userData) // The same daemon, in the same \`/daemon\` Electron serve uses, is adopted. @@ -174,11 +194,7 @@ describe.skipIf(skip)('orca serve on orcad', () => { output: true }) } finally { - await stopServe(userData) - if (daemonPid) { - await killAndAwaitExit([daemonPid]) - daemonPid = null - } + await stopServeProfile(userData, shellPid) } }, 300_000) }) @@ -224,3 +240,22 @@ async function stopServe(userData: string): Promise { child.kill('SIGTERM') await exited } + +async function stopServeProfile(userData: string, shellPid: number | null): Promise { + try { + await stopServe(userData) + await killProfileDaemons(userData) + if (shellPid) { + await waitForProcessExit([shellPid]) + } + } catch (error) { + if (shellPid) { + try { + await killAndAwaitExit([shellPid]) + } catch (cleanupError) { + throw new AggregateError([error, cleanupError], 'Terminal shutdown incomplete') + } + } + throw error + } +} diff --git a/src/main/persistence/profile-state/profile-state-access-windows.test.ts b/src/main/persistence/profile-state/profile-state-access-windows.test.ts index 02d210523e0..93923dcfc81 100644 --- a/src/main/persistence/profile-state/profile-state-access-windows.test.ts +++ b/src/main/persistence/profile-state/profile-state-access-windows.test.ts @@ -32,6 +32,8 @@ const platform = Object.getOwnPropertyDescriptor(process, 'platform') let root: string beforeEach(() => { + // Occupied owner fixtures need real rename retries, but no elapsed backoff. + vi.spyOn(Atomics, 'wait').mockReturnValue('timed-out') root = mkdtempSync(join(tmpdir(), 'orca-state-access-windows-')) Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) identity.boot.mockReset().mockReturnValue(null) diff --git a/src/main/persistence/profile-state/profile-state-access.test.ts b/src/main/persistence/profile-state/profile-state-access.test.ts index b551a44f053..0652f7bfe9b 100644 --- a/src/main/persistence/profile-state/profile-state-access.test.ts +++ b/src/main/persistence/profile-state/profile-state-access.test.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs' import { randomUUID } from 'node:crypto' import { tmpdir, hostname } from 'node:os' import { join } from 'node:path' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { acquireProfileStateMaintenance, acquireProfileStateRuntimeAdmission, @@ -28,6 +28,11 @@ function root(): string { return path } +beforeEach(() => { + // Occupied owner fixtures need real rename retries, but no elapsed backoff. + vi.spyOn(Atomics, 'wait').mockReturnValue('timed-out') +}) + afterEach(() => { vi.restoreAllMocks() for (const path of roots.splice(0)) { @@ -388,7 +393,6 @@ describe('profile state owner reclamation', () => { throw Object.assign(new Error('scanner holds directory'), { code: 'EPERM' }) }) .mockImplementation(rename) - vi.spyOn(Atomics, 'wait').mockReturnValue('timed-out') try { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) const admission = acquireProfileStateRuntimeAdmission(path) diff --git a/src/main/persistence/profile-state/profile-state-automation-storage-upgrade.test.ts b/src/main/persistence/profile-state/profile-state-automation-storage-upgrade.test.ts index bb28c491f0e..a018b4af635 100644 --- a/src/main/persistence/profile-state/profile-state-automation-storage-upgrade.test.ts +++ b/src/main/persistence/profile-state/profile-state-automation-storage-upgrade.test.ts @@ -63,6 +63,7 @@ function seedLegacyDatabase( const path = databasePath() const db = new Database(path) try { + db.exec('BEGIN') db.exec(` CREATE TABLE profile_state_meta (key TEXT PRIMARY KEY NOT NULL, value TEXT NOT NULL); CREATE TABLE profile_state_documents ( @@ -116,6 +117,12 @@ function seedLegacyDatabase( } } db.pragma(`user_version = ${version}`) + db.exec('COMMIT') + } catch (error) { + if (db.isTransaction) { + db.exec('ROLLBACK') + } + throw error } finally { db.close() } diff --git a/src/main/ssh/orcad-remote-node-runtime-recorded-probe.test.ts b/src/main/ssh/orcad-remote-node-runtime-recorded-probe.test.ts index a636dff9206..8a6d78a9143 100644 --- a/src/main/ssh/orcad-remote-node-runtime-recorded-probe.test.ts +++ b/src/main/ssh/orcad-remote-node-runtime-recorded-probe.test.ts @@ -1,4 +1,12 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + utimesSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -99,9 +107,17 @@ describe.skipIf(process.platform === 'win32')('recorded runtime probe', () => { expect(hashCount()).toBe(1) }) - it('hashes again after the executable is rewritten in place at the same size', async () => { + it('hashes a same-size in-place rewrite after its stat signature changes', async () => { await ensure(true) - writeFileSync(posixNodeRuntimeExecutable(host, runtimeDir), 'NODE') + const executable = posixNodeRuntimeExecutable(host, runtimeDir) + const before = statSync(executable) + writeFileSync(executable, 'NODE') + // Fast writes can share a filesystem timestamp; establish a changed cache signature. + utimesSync(executable, before.atime, new Date(before.mtimeMs + 1_000)) + const after = statSync(executable) + expect(after.ino).toBe(before.ino) + expect(after.size).toBe(before.size) + expect(after.mtimeMs).toBeGreaterThan(before.mtimeMs) await ensure(true) expect(hashCount()).toBe(2) }) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts index fb954f5941d..fcb8d562fb1 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts @@ -21,6 +21,7 @@ import { makeWorktree, TEST_REPO } from '../store/slices/worktrees-slice-test-fi import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' import { agentResumeOriginNamesAnotherExecutionHost, + sleepingRecordHostAllowsSessionClaim, sleepingRecordNamesAnotherExecutionHost } from './sleeping-record-execution-host-scope' import type { WorktreeRuntimeOwnerState } from './worktree-runtime-owner-state' @@ -142,6 +143,26 @@ describe('sleepingRecordNamesAnotherExecutionHost', () => { }) }) +describe('folder-owned SSH with conflicting inherited group owners', () => { + it.each(['local first', 'SSH first'] as const)('uses the folder target with %s', (order) => { + const groups = [ + { id: 'group', executionHostId: 'local' }, + { id: 'group', executionHostId: 'ssh:other-target' } + ] as const + const state: WorktreeRuntimeOwnerState = { + folderWorkspaces: [{ id: 'folder', projectGroupId: 'group', connectionId: TARGET_ID }], + projectGroups: order === 'local first' ? groups : groups.toReversed() + } + const local = makeRecord({ worktreeId: 'folder:folder', connectionId: null }) + const matching = makeRecord({ worktreeId: 'folder:folder', connectionId: TARGET_ID }) + + expect(sleepingRecordNamesAnotherExecutionHost(local, state)).toBe(true) + expect(sleepingRecordNamesAnotherExecutionHost(matching, state)).toBe(false) + expect(sleepingRecordHostAllowsSessionClaim(local, matching, state)).toBe(false) + expect(sleepingRecordHostAllowsSessionClaim(matching, matching, state)).toBe(true) + }) +}) + describe('agentResumeOriginNamesAnotherExecutionHost', () => { // The pane cold-restore path asks the same question against the transport the pane is attached to // rather than the catalog, so the host-pair form is exported and pinned separately. diff --git a/src/renderer/src/lib/sleeping-record-execution-host-scope.ts b/src/renderer/src/lib/sleeping-record-execution-host-scope.ts index 97ac63b31f8..c62047eb959 100644 --- a/src/renderer/src/lib/sleeping-record-execution-host-scope.ts +++ b/src/renderer/src/lib/sleeping-record-execution-host-scope.ts @@ -165,7 +165,7 @@ function ownerExecutionHostIdForRecord( if (workspaceScope?.type === 'folder') { const folder = findFolderWorkspaceOwner(state, workspaceScope.folderWorkspaceId) const group = folder?.projectGroupId - ? state.projectGroups?.find((candidate) => candidate.id === folder.projectGroupId) + ? findIndexedProjectGroupOwner(state.projectGroups, folder.projectGroupId) : undefined const explicit = parseExecutionHostId(folder?.executionHostId ?? group?.executionHostId)?.id const connectionId = folder?.connectionId?.trim() || group?.connectionId?.trim() diff --git a/tests/e2e/helpers/orca-restart.ts b/tests/e2e/helpers/orca-restart.ts index 15213287412..546e65552df 100644 --- a/tests/e2e/helpers/orca-restart.ts +++ b/tests/e2e/helpers/orca-restart.ts @@ -46,8 +46,8 @@ type RestartSession = { launchUntilExit: (executablePath: string) => Promise /** Gracefully close a launch, letting beforeunload flush session state. */ close: (app: ElectronApplication) => Promise - /** Remove the shared userDataDir after the test is done. */ - dispose: () => Promise + /** Close the final launch and remove the shared userDataDir after the test is done. */ + dispose: (app?: ElectronApplication) => Promise } async function delay(ms: number): Promise { @@ -217,8 +217,10 @@ export function createRestartSession( }) } - const dispose = async (): Promise => { - await cleanupE2EDaemons(userDataDir) + const dispose = async (app?: ElectronApplication): Promise => { + await (app + ? closeElectronAppForE2E(app, { cleanupDaemonsForUserDataDir: userDataDir }) + : cleanupE2EDaemons(userDataDir)) if (process.env.ORCA_E2E_PRESERVE_RESTART_PROFILE === '1') { console.log(`[e2e] Preserved restart profile at ${userDataDir}`) return diff --git a/tests/e2e/helpers/workspace-layout-oracle-session.ts b/tests/e2e/helpers/workspace-layout-oracle-session.ts index e2cdb58bf08..c0d054c957e 100644 --- a/tests/e2e/helpers/workspace-layout-oracle-session.ts +++ b/tests/e2e/helpers/workspace-layout-oracle-session.ts @@ -151,10 +151,7 @@ export async function runOracleScenario( }) return run.oracle.findings } finally { - if (app) { - await session.close(app) - } - await session.dispose() + await session.dispose(app ?? undefined) } }