From 413398a2203b31ef2326a172ccc9df2dd4de128b Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 10 Aug 2026 02:25:56 -0400 Subject: [PATCH] fix(server): harden headless lifecycle review findings --- .github/workflows/pr.yml | 5 + config/reliability-gates.jsonc | 2 +- ...-server-package-workflow-contract.test.mjs | 4 +- .../scripts/node-server-runtime-verifier.ts | 120 +++----------- ...ode-server-verifier-daemon-cleanup.test.ts | 61 ++++++++ .../node-server-verifier-daemon-cleanup.ts | 8 +- .../node-server-verifier-server-process.ts | 147 ++++++++++++++++++ .../scripts/pr-workflow-parallelism.test.mjs | 6 + .../run-node-server-runtime-verifier.mjs | 5 +- resources/npm-server/README.md | 2 + .../headless-completion-settlement.test.ts | 47 ++++++ .../headless-completion-settlement.ts | 1 + .../automations/headless-work-drain.test.ts | 33 ++-- src/main/automations/headless-work-drain.ts | 7 +- src/main/automations/precheck-runner.test.ts | 39 ++++- src/main/automations/precheck-ssh-runner.ts | 21 +++ .../scheduled-automation-evaluation.ts | 47 ++++++ src/main/automations/service.test.ts | 115 ++++++++++++++ src/main/automations/service.ts | 53 +++---- .../browserless-runtime-composition.test.ts | 131 ++++++++++++++-- .../browserless-runtime-composition.ts | 64 +++++--- .../server-profile-process-lock.test.ts | 12 ++ .../server-profile-process-lock.ts | 2 +- .../settings/NodeServerSetupCallout.test.tsx | 43 +++++ .../settings/NodeServerSetupCallout.tsx | 12 +- .../settings/RuntimeHostAccessForm.test.tsx | 3 + 26 files changed, 791 insertions(+), 199 deletions(-) create mode 100644 config/scripts/node-server-verifier-daemon-cleanup.test.ts create mode 100644 config/scripts/node-server-verifier-server-process.ts create mode 100644 src/main/automations/headless-completion-settlement.test.ts create mode 100644 src/main/automations/scheduled-automation-evaluation.ts create mode 100644 src/renderer/src/components/settings/NodeServerSetupCallout.test.tsx diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 11d7f517ef3..747dab188da 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -288,6 +288,7 @@ jobs: node_server_package: name: node server package (${{ matrix.os }}) runs-on: ${{ matrix.os }} + timeout-minutes: 20 strategy: fail-fast: false matrix: @@ -304,6 +305,10 @@ jobs: - name: Build Node server package run: pnpm run build:node-server + - name: Verify Windows profile process lock + if: runner.os == 'Windows' + run: pnpm exec vitest run --config config/vitest.config.ts src/node-server/server-profile-process-lock.test.ts + - name: Verify installed package and runtime run: pnpm run verify:node-server-package diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 6313dae0d62..78d67e81db8 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -25,7 +25,7 @@ "platforms": ["macos", "linux", "windows"], "providers": ["local", "ssh", "paired-runtime-host"], "coveredPlatforms": ["macos"], - "coveredProviders": ["local", "ssh", "paired-runtime-host"], + "coveredProviders": ["local", "ssh"], "coverageNotes": "A real local child process and injected SSH acquisition/channel barriers cover cancellation, late fulfillment and rejection, process death, channel close, listener cleanup, and service drain. Physical Windows taskkill and live SSH transport remain uncollected.", "motivatingLinks": ["https://github.com/stablyai/orca/pull/13452"], "invariant": "Stopping a headless automation service must abort any scheduled precheck, terminate its local process tree or close its SSH channel, consume late SSH acquisition settlement, and drain without waiting for the configured precheck timeout or leaving an unobserved error event.", diff --git a/config/scripts/node-server-package-workflow-contract.test.mjs b/config/scripts/node-server-package-workflow-contract.test.mjs index b1c3fcb05d1..3fc1bbb7c61 100644 --- a/config/scripts/node-server-package-workflow-contract.test.mjs +++ b/config/scripts/node-server-package-workflow-contract.test.mjs @@ -1,9 +1,9 @@ import { readFileSync } from 'node:fs' -import { win32 } from 'node:path' +import { join, win32 } from 'node:path' import { describe, expect, it, vi } from 'vitest' import { resolveNodeServerNpmInvocation } from './node-server-npm-invocation.mjs' -const dockerfile = readFileSync('config/docker/node-server/Dockerfile', 'utf8') +const dockerfile = readFileSync(join('config', 'docker', 'node-server', 'Dockerfile'), 'utf8') describe('Node server package workflow contract', () => { it('verifies the selected Node archive before extracting it', () => { diff --git a/config/scripts/node-server-runtime-verifier.ts b/config/scripts/node-server-runtime-verifier.ts index 1aa56e11cef..4d8dc8cce26 100644 --- a/config/scripts/node-server-runtime-verifier.ts +++ b/config/scripts/node-server-runtime-verifier.ts @@ -1,4 +1,3 @@ -import { spawn, type ChildProcess } from 'node:child_process' import { mkdirSync, mkdtempSync, realpathSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' @@ -7,29 +6,18 @@ import { decodePairingOffer, type PairingOffer } from '../../src/shared/pairing' import { sendRemoteRuntimeRequest } from '../../src/shared/remote-runtime-client' import type { RuntimeStatus, RuntimeTerminalRead } from '../../src/shared/runtime-types' import { verifyInstalledCliProfileBehavior } from './node-server-installed-cli-oracle' -import { - isExpectedInstalledServerStopResult, - waitForInstalledServerExit -} from './node-server-installed-process-harness' import { stopNodeServerVerifierDaemons } from './node-server-verifier-daemon-cleanup' import { initializeNodeServerVerifierGitWorkspace } from './node-server-verifier-git-fixture' import { isSameExistingHostPath } from './node-server-verifier-host-path' - -type ReadyPayload = { - type: 'orca_server_ready' - schemaVersion: 1 - runtimeId: string - boundEndpoint: string - pairing: { - available: true - url: string - webClientUrl: string - } -} - -type RunningServer = { child: ChildProcess; ready: ReadyPayload; stderr: string[] } +import { + startNodeServerVerifierProcess, + stopNodeServerVerifierProcess, + terminateNodeServerVerifierProcess, + type NodeServerVerifierProcess +} from './node-server-verifier-server-process' const cliPath = resolve(readArgument('--cli') ?? 'resources/npm-server/dist/cli.js') const expectedVersion = readArgument('--expected-version') ?? packageVersionFromEnvironment() +// Keep Unix daemon socket paths below platform limits while preserving Windows-native temp paths. const shortTemporaryRoot = process.platform === 'win32' ? tmpdir() : '/tmp' const ownedRoot = realpathSync.native(mkdtempSync(join(shortTemporaryRoot, 'orca-nsv-'))) const dataPath = join(ownedRoot, 'state') @@ -37,12 +25,12 @@ const gitPath = join(ownedRoot, 'git-workspace') const folderPath = join(ownedRoot, 'folder-workspace') async function main(): Promise { - let activeServer: RunningServer | null = null + let activeServer: NodeServerVerifierProcess | null = null try { initializeNodeServerVerifierGitWorkspace(gitPath) mkdirSync(folderPath) - const first = await startServer() + const first = await startNodeServerVerifierProcess({ cliPath, dataPath }) activeServer = first const pairing = decodePairingOffer(first.ready.pairing.url) await verifyWebClient(first.ready.pairing.webClientUrl) @@ -59,10 +47,10 @@ async function main(): Promise { terminalHandle: terminal.handle, temporaryRoot: ownedRoot }) - await stopServer(first) + await stopNodeServerVerifierProcess(first) activeServer = null - const second = await startServer() + const second = await startNodeServerVerifierProcess({ cliPath, dataPath }) activeServer = second assert( second.ready.runtimeId !== first.ready.runtimeId, @@ -79,12 +67,14 @@ async function main(): Promise { const reattached = restored.terminals.find((candidate) => candidate.ptyId === terminal.ptyId) assert(reattached, 'terminal daemon session was not adopted') await waitForTerminalMarker(reconnectedPairing, reattached.handle, terminal.marker) - await stopServer(second) + await stopNodeServerVerifierProcess(second) activeServer = null process.stdout.write('Node server runtime verification passed\n') } finally { if (activeServer?.child.exitCode === null && activeServer.child.signalCode === null) { - await stopServer(activeServer).catch(() => activeServer?.child.kill('SIGKILL')) + await stopNodeServerVerifierProcess(activeServer).catch(() => + terminateNodeServerVerifierProcess(activeServer.child) + ) } await stopNodeServerVerifierDaemons(dataPath) await removeHostTree(ownedRoot) @@ -107,81 +97,8 @@ function packageVersionFromEnvironment(): string { return process.env.npm_package_version ?? '0.0.0-dev' } -function startServer(): Promise { - return new Promise((resolveStart, rejectStart) => { - const stderr: string[] = [] - const child = spawn( - process.execPath, - [ - cliPath, - 'serve', - '--port', - '0', - '--listen', - '127.0.0.1', - '--pairing-address', - '127.0.0.1', - '--json' - ], - { env: { ...process.env, ORCA_SERVER_DATA_DIR: dataPath }, stdio: ['ignore', 'pipe', 'pipe'] } - ) - let stdout = '' - const timeout = setTimeout(() => fail(new Error('server readiness timed out')), 30_000) - - child.stderr?.setEncoding('utf8') - child.stderr?.on('data', (chunk: string) => stderr.push(chunk)) - child.stdout?.setEncoding('utf8') - child.stdout?.on('data', (chunk: string) => { - stdout += chunk - const newline = stdout.indexOf('\n') - if (newline === -1) { - return - } - try { - const ready = JSON.parse(stdout.slice(0, newline)) as ReadyPayload - assert(ready.type === 'orca_server_ready', 'unexpected readiness type') - assert(ready.schemaVersion === 1, 'unexpected readiness schema') - assert(ready.pairing.available, 'pairing unavailable') - assert(new URL(ready.boundEndpoint).hostname === '127.0.0.1', 'listener is not loopback') - clearTimeout(timeout) - resolveStart({ child, ready, stderr }) - } catch (error) { - fail(error) - } - }) - child.once('error', fail) - child.once('exit', (code) => - fail(new Error(`server exited before readiness (${code}): ${stderr.join('')}`)) - ) - - function fail(error: unknown): void { - clearTimeout(timeout) - if (child.exitCode === null && child.signalCode === null) { - child.kill('SIGTERM') - } - rejectStart(error) - } - }) -} - -async function stopServer(server: RunningServer): Promise { - const { child } = server - if (child.exitCode !== null || child.signalCode !== null) { - throw new Error(`server exited unexpectedly (${child.exitCode ?? child.signalCode})`) - } - if (!child.kill('SIGTERM')) { - throw new Error('server rejected the shutdown signal') - } - const result = await waitForInstalledServerExit(child) - if (!isExpectedInstalledServerStopResult(result, process.platform)) { - throw new Error( - `server shutdown failed (${result.code ?? result.signal}): ${server.stderr.join('')}` - ) - } -} - async function verifyWebClient(url: string): Promise { - const response = await fetch(url) + const response = await fetch(url, { signal: AbortSignal.timeout(15_000) }) assert(response.ok, `web client returned ${response.status}`) assert((await response.text()).includes(''), 'web client HTML was not served') } @@ -201,7 +118,10 @@ async function verifyStatus( 'browser.headless.v1', 'browser.certificate-trust.v1' ]) { - assert(!capabilities.includes(capability as never), `browser capability leaked: ${capability}`) + assert( + !capabilities.some((candidate) => candidate === capability), + `browser capability leaked: ${capability}` + ) } } diff --git a/config/scripts/node-server-verifier-daemon-cleanup.test.ts b/config/scripts/node-server-verifier-daemon-cleanup.test.ts new file mode 100644 index 00000000000..5809c0862b6 --- /dev/null +++ b/config/scripts/node-server-verifier-daemon-cleanup.test.ts @@ -0,0 +1,61 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const clientState = vi.hoisted(() => ({ requests: [] as string[] })) + +vi.mock('../../src/main/daemon/client', () => ({ + DaemonClient: class { + async ensureConnectedWithin(): Promise {} + + async request(method: string): Promise { + clientState.requests.push(method) + return method === 'listSessions' ? { sessions: [] } : undefined + } + + disconnect(): void {} + } +})) + +import { stopNodeServerVerifierDaemons } from './node-server-verifier-daemon-cleanup' + +const temporaryPaths: string[] = [] + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + clientState.requests.length = 0 + for (const path of temporaryPaths.splice(0)) { + rmSync(path, { recursive: true, force: true }) + } +}) + +describe('stopNodeServerVerifierDaemons', () => { + it('waits for every shutdown attempt before aggregating cleanup failures', async () => { + vi.useFakeTimers() + const dataPath = mkdtempSync(join(tmpdir(), 'orca-daemon-cleanup-')) + temporaryPaths.push(dataPath) + const daemonPath = join(dataPath, 'daemon') + mkdirSync(daemonPath) + writeFileSync(join(daemonPath, 'daemon-v4.pid'), JSON.stringify({ pid: 4101 })) + writeFileSync(join(daemonPath, 'daemon-v5.pid'), JSON.stringify({ pid: 5101 })) + vi.spyOn(process, 'kill').mockImplementation(() => true) + + const outcome = stopNodeServerVerifierDaemons(dataPath).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(10_100) + + const error = await outcome + expect(error).toBeInstanceOf(AggregateError) + expect((error as AggregateError).errors).toHaveLength(2) + expect((error as AggregateError).errors).toEqual([ + expect.objectContaining({ + message: 'terminal daemon 4101 did not exit after the shutdown request' + }), + expect.objectContaining({ + message: 'terminal daemon 5101 did not exit after the shutdown request' + }) + ]) + expect(clientState.requests.filter((method) => method === 'shutdown')).toHaveLength(2) + }) +}) diff --git a/config/scripts/node-server-verifier-daemon-cleanup.ts b/config/scripts/node-server-verifier-daemon-cleanup.ts index da7f6ebacf8..291836728ad 100644 --- a/config/scripts/node-server-verifier-daemon-cleanup.ts +++ b/config/scripts/node-server-verifier-daemon-cleanup.ts @@ -23,7 +23,13 @@ export async function stopNodeServerVerifierDaemons(dataPath: string): Promise + result.status === 'rejected' ? [result.reason] : [] + ) + if (failures.length > 0) { + throw new AggregateError(failures, 'Terminal daemon cleanup failed') + } } async function shutdownDaemon( diff --git a/config/scripts/node-server-verifier-server-process.ts b/config/scripts/node-server-verifier-server-process.ts new file mode 100644 index 00000000000..7592cb55062 --- /dev/null +++ b/config/scripts/node-server-verifier-server-process.ts @@ -0,0 +1,147 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { + isExpectedInstalledServerStopResult, + waitForInstalledServerExit +} from './node-server-installed-process-harness' + +type ReadyPayload = { + type: 'orca_server_ready' + schemaVersion: 1 + runtimeId: string + boundEndpoint: string + pairing: { + available: true + url: string + webClientUrl: string + } +} + +export type NodeServerVerifierProcess = { + child: ChildProcess + ready: ReadyPayload + stderr: string[] +} + +export function startNodeServerVerifierProcess(args: { + cliPath: string + dataPath: string +}): Promise { + return new Promise((resolveStart, rejectStart) => { + const stderr: string[] = [] + const child = spawn( + process.execPath, + [ + args.cliPath, + 'serve', + '--port', + '0', + '--listen', + '127.0.0.1', + '--pairing-address', + '127.0.0.1', + '--json' + ], + { + env: { ...process.env, ORCA_SERVER_DATA_DIR: args.dataPath }, + stdio: ['ignore', 'pipe', 'pipe'] + } + ) + let stdout = '' + let settled = false + const timeout = setTimeout(() => fail(new Error('server readiness timed out')), 30_000) + + child.stderr?.setEncoding('utf8') + child.stderr?.on('data', (chunk: string) => stderr.push(chunk)) + child.stdout?.setEncoding('utf8') + child.stdout?.on('data', onStdout) + child.once('error', fail) + child.once('exit', onEarlyExit) + + function onStdout(chunk: string): void { + stdout += chunk + const newline = stdout.indexOf('\n') + if (newline === -1) { + return + } + try { + const ready = JSON.parse(stdout.slice(0, newline)) as ReadyPayload + assertReadyPayload(ready) + settled = true + stopObservingReadiness() + child.on('error', observeRuntimeError) + resolveStart({ child, ready, stderr }) + } catch (error) { + fail(error) + } + } + + function onEarlyExit(code: number | null): void { + fail(new Error(`server exited before readiness (${code}): ${stderr.join('')}`)) + } + + function observeRuntimeError(error: Error): void { + stderr.push(`${error.stack ?? error.message}\n`) + } + + function stopObservingReadiness(): void { + clearTimeout(timeout) + child.stdout?.off('data', onStdout) + child.off('error', fail) + child.off('exit', onEarlyExit) + } + + function fail(error: unknown): void { + if (settled) { + return + } + settled = true + stopObservingReadiness() + void terminateNodeServerVerifierProcess(child).then( + () => rejectStart(error), + (terminationError) => + rejectStart( + new AggregateError([error, terminationError], 'server readiness cleanup failed') + ) + ) + } + }) +} + +export async function terminateNodeServerVerifierProcess(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + return + } + if (!child.kill('SIGKILL')) { + throw new Error('server rejected forced termination') + } + await waitForInstalledServerExit(child) +} + +export async function stopNodeServerVerifierProcess( + server: NodeServerVerifierProcess +): Promise { + const { child } = server + if (child.exitCode !== null || child.signalCode !== null) { + throw new Error(`server exited unexpectedly (${child.exitCode ?? child.signalCode})`) + } + if (!child.kill('SIGTERM')) { + throw new Error('server rejected the shutdown signal') + } + const result = await waitForInstalledServerExit(child) + if (!isExpectedInstalledServerStopResult(result, process.platform)) { + throw new Error( + `server shutdown failed (${result.code ?? result.signal}): ${server.stderr.join('')}` + ) + } +} + +function assertReadyPayload(ready: ReadyPayload): void { + if ( + ready.type !== 'orca_server_ready' || + ready.schemaVersion !== 1 || + !ready.pairing.available || + new URL(ready.boundEndpoint).hostname !== '127.0.0.1' + ) { + throw new Error('server returned invalid readiness data') + } +} diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index b9af32ea84d..913f20a3b59 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -136,6 +136,7 @@ describe('PR workflow parallelism', () => { const job = workflow.jobs.node_server_package expect(job.strategy.matrix.os).toEqual(['ubuntu-latest', 'windows-2022']) + expect(job['timeout-minutes']).toBe(20) expect( job.steps.find((step) => step.uses === './.github/actions/install-node-dependencies').with ).toBeUndefined() @@ -145,6 +146,11 @@ describe('PR workflow parallelism', () => { expect(job.steps.find((step) => step.name === 'Verify installed package and runtime').run).toBe( 'pnpm run verify:node-server-package' ) + const windowsLockStep = job.steps.find( + (step) => step.name === 'Verify Windows profile process lock' + ) + expect(windowsLockStep.if).toBe("runner.os == 'Windows'") + expect(windowsLockStep.run).toContain('server-profile-process-lock.test.ts') const dockerStep = job.steps.find( (step) => step.name === 'Verify Ubuntu 20.04 and glibc 2.31 floor' ) diff --git a/config/scripts/run-node-server-runtime-verifier.mjs b/config/scripts/run-node-server-runtime-verifier.mjs index a704d67955e..0b2e5615fd2 100644 --- a/config/scripts/run-node-server-runtime-verifier.mjs +++ b/config/scripts/run-node-server-runtime-verifier.mjs @@ -4,14 +4,15 @@ import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { build } from 'esbuild' +const repoRoot = resolve(import.meta.dirname, '..', '..') const temporaryRoot = mkdtempSync(join(tmpdir(), 'orca-runtime-verifier-build-')) const verifierPath = join(temporaryRoot, 'verifier.cjs') try { await build({ - absWorkingDir: resolve('.'), + absWorkingDir: repoRoot, bundle: true, - entryPoints: ['config/scripts/node-server-runtime-verifier.ts'], + entryPoints: [join(repoRoot, 'config', 'scripts', 'node-server-runtime-verifier.ts')], format: 'cjs', outfile: verifierPath, platform: 'node', diff --git a/resources/npm-server/README.md b/resources/npm-server/README.md index 1c850c643fa..c9d5d8b4d21 100644 --- a/resources/npm-server/README.md +++ b/resources/npm-server/README.md @@ -1,5 +1,7 @@ # Orca Server +Requires Node.js 24 or newer. + Run Orca on a development server without installing the Electron desktop application: ```bash diff --git a/src/main/automations/headless-completion-settlement.test.ts b/src/main/automations/headless-completion-settlement.test.ts new file mode 100644 index 00000000000..ebef803cc15 --- /dev/null +++ b/src/main/automations/headless-completion-settlement.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AutomationDispatchResult, AutomationRun } from '../../shared/automations-types' +import { settleHeadlessAutomationCompletion } from './headless-completion-settlement' + +describe('settleHeadlessAutomationCompletion', () => { + it('retains the precheck result when completion fails', async () => { + const precheckResult = { + command: 'test -f ready', + exitCode: 0, + timedOut: false, + durationMs: 5, + stdout: 'ready', + stderr: '', + stdoutTruncated: false, + stderrTruncated: false, + error: null, + startedAt: 10, + completedAt: 15 + } + const mark = vi.fn( + async (result: AutomationDispatchResult) => result as unknown as AutomationRun + ) + + await settleHeadlessAutomationCompletion({ + completion: Promise.reject(new Error('terminal read failed')), + runId: 'run-1', + target: { + workspaceId: 'workspace-1', + workspaceDisplayName: 'Workspace', + terminalSessionId: 'session-1', + terminalPaneKey: 'pane-1', + terminalPtyId: 'pty-1' + }, + precheckResult, + mark + }) + + expect(mark).toHaveBeenCalledWith( + expect.objectContaining({ + runId: 'run-1', + status: 'dispatch_failed', + precheckResult, + error: 'terminal read failed' + }) + ) + }) +}) diff --git a/src/main/automations/headless-completion-settlement.ts b/src/main/automations/headless-completion-settlement.ts index 51a7e6aa4b6..3ca9cca8cac 100644 --- a/src/main/automations/headless-completion-settlement.ts +++ b/src/main/automations/headless-completion-settlement.ts @@ -44,6 +44,7 @@ export function settleHeadlessAutomationCompletion(args: { runId: args.runId, status: 'dispatch_failed', ...args.target, + precheckResult: args.precheckResult, error: error instanceof Error ? error.message : String(error) }) ) diff --git a/src/main/automations/headless-work-drain.test.ts b/src/main/automations/headless-work-drain.test.ts index 410fe89e5c4..2acfd335bfa 100644 --- a/src/main/automations/headless-work-drain.test.ts +++ b/src/main/automations/headless-work-drain.test.ts @@ -22,22 +22,25 @@ describe('HeadlessAutomationWorkDrain', () => { expect(drained).toBe(true) }) - it('observes rejected fire-and-forget work without an unhandled rejection', async () => { - const unhandled: unknown[] = [] - const onUnhandled = (reason: unknown): void => { - unhandled.push(reason) - } - process.on('unhandledRejection', onUnhandled) + it.each([true, false])( + 'observes rejected fire-and-forget work without an unhandled rejection (enabled=%s)', + async (enabled) => { + const unhandled: unknown[] = [] + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason) + } + process.on('unhandledRejection', onUnhandled) - try { - const drain = new HeadlessAutomationWorkDrain(true) - drain.track(Promise.reject(new Error('dispatch failed'))) + try { + const drain = new HeadlessAutomationWorkDrain(enabled) + drain.track(Promise.reject(new Error('dispatch failed'))) - await drain.drain() - await new Promise((resolve) => setImmediate(resolve)) - expect(unhandled).toEqual([]) - } finally { - process.off('unhandledRejection', onUnhandled) + await drain.drain() + await new Promise((resolve) => setImmediate(resolve)) + expect(unhandled).toEqual([]) + } finally { + process.off('unhandledRejection', onUnhandled) + } } - }) + ) }) diff --git a/src/main/automations/headless-work-drain.ts b/src/main/automations/headless-work-drain.ts index c550bb9dcf7..7b0a62cf838 100644 --- a/src/main/automations/headless-work-drain.ts +++ b/src/main/automations/headless-work-drain.ts @@ -4,9 +4,6 @@ export class HeadlessAutomationWorkDrain { constructor(private readonly enabled: boolean) {} track(work: Promise): Promise { - if (!this.enabled) { - return work - } let settlement: Promise settlement = work.then( () => { @@ -16,7 +13,9 @@ export class HeadlessAutomationWorkDrain { this.pending.delete(settlement) } ) - this.pending.add(settlement) + if (this.enabled) { + this.pending.add(settlement) + } return work } diff --git a/src/main/automations/precheck-runner.test.ts b/src/main/automations/precheck-runner.test.ts index d93c54c1e27..c27f6b758ae 100644 --- a/src/main/automations/precheck-runner.test.ts +++ b/src/main/automations/precheck-runner.test.ts @@ -19,7 +19,8 @@ vi.mock('../ipc/ssh', () => ({ const node = JSON.stringify(process.execPath) function nodeCommand(script: string): string { - return `${node} -e ${JSON.stringify(script)}` + const encodedScript = Buffer.from(script).toString('base64') + return `${node} -e "eval(Buffer.from('${encodedScript}','base64').toString())"` } function isProcessAlive(pid: number): boolean { @@ -100,7 +101,7 @@ describe('runAutomationPrecheck', () => { timedOut: false, error: 'Precheck cancelled.' }) - await vi.waitFor(() => expect(isProcessAlive(childPid)).toBe(false)) + await vi.waitFor(() => expect(isProcessAlive(childPid)).toBe(false), { timeout: 15_000 }) } finally { if (childPid && isProcessAlive(childPid)) { process.kill(childPid, 'SIGKILL') @@ -140,6 +141,11 @@ describe('runAutomationPrecheck', () => { expect(result.exitCode).toBe(0) expect(result.stdout).toContain('ready') expect(result.error).toBeNull() + expect(() => channel.emit('error', new Error('late channel error'))).not.toThrow() + expect(() => channel.stderr.emit('error', new Error('late stderr error'))).not.toThrow() + await new Promise((resolve) => setImmediate(resolve)) + expect(channel.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) }) it('closes an acquired SSH channel and settles when aborted', async () => { @@ -180,6 +186,35 @@ describe('runAutomationPrecheck', () => { expect(channel.stderr.listenerCount('error')).toBe(0) }) + it('consumes errors after an SSH failure until the channel closes', async () => { + const channel = Object.assign(new EventEmitter(), { + stderr: new PassThrough(), + close: vi.fn() + }) + sshManagerState.manager = { + getConnection: vi.fn(() => ({ + getState: () => ({ status: 'connected' }), + exec: vi.fn(async () => channel) + })) + } + + const resultPromise = runAutomationPrecheck({ + precheck: { command: 'failing-command', timeoutSeconds: 5 }, + target: { type: 'ssh', cwd: '/repo/path', connectionId: 'ssh-1' } + }) + await vi.waitFor(() => expect(channel.listenerCount('error')).toBe(1)) + channel.emit('error', new Error('SSH command failed')) + + await expect(resultPromise).resolves.toMatchObject({ error: 'SSH command failed' }) + expect(() => channel.emit('error', new Error('late channel error'))).not.toThrow() + expect(() => channel.stderr.emit('error', new Error('late stderr error'))).not.toThrow() + expect(channel.listenerCount('close')).toBe(1) + channel.emit('close') + expect(channel.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) + expect(channel.listenerCount('close')).toBe(0) + }) + it('bounds SSH channel acquisition and closes a channel that resolves after abort', async () => { let resolveExec!: ( channel: EventEmitter & { stderr: PassThrough; close: ReturnType } diff --git a/src/main/automations/precheck-ssh-runner.ts b/src/main/automations/precheck-ssh-runner.ts index b0a35e320f8..0724764f37f 100644 --- a/src/main/automations/precheck-ssh-runner.ts +++ b/src/main/automations/precheck-ssh-runner.ts @@ -15,6 +15,22 @@ type SshPrecheckTarget = { connectionId: string } +function consumeLateSshErrors(channel: ClientChannel, alreadyClosed: boolean): void { + const consumeError = (): void => {} + const stopObserving = (): void => { + channel.off('error', consumeError) + channel.stderr.off('error', consumeError) + channel.off('close', stopObserving) + } + channel.on('error', consumeError) + channel.stderr.on('error', consumeError) + if (alreadyClosed) { + setImmediate(stopObserving) + } else { + channel.once('close', stopObserving) + } +} + function closeSshPrecheckChannel(channel: ClientChannel): void { const consumeError = (): void => {} const stopObserving = (): void => { @@ -49,6 +65,7 @@ function runSshChannelPrecheck(args: { let exitCode: number | null = null let timedOut = false let cancelled = false + let closed = false let settled = false let timeout: ReturnType | null = null @@ -65,6 +82,7 @@ function runSshChannelPrecheck(args: { exitCode = typeof code === 'number' ? code : null } const onClose = (code?: number | null): void => { + closed = true if (typeof code === 'number') { exitCode = code } @@ -90,6 +108,9 @@ function runSshChannelPrecheck(args: { channel.stderr.off('data', collectStderr) channel.off('exit', onExit) channel.off('close', onClose) + if (!cancelled && !timedOut) { + consumeLateSshErrors(channel, closed) + } } const settle = (resultExitCode: number | null, error: string | null): void => { if (settled) { diff --git a/src/main/automations/scheduled-automation-evaluation.ts b/src/main/automations/scheduled-automation-evaluation.ts new file mode 100644 index 00000000000..75292ceae55 --- /dev/null +++ b/src/main/automations/scheduled-automation-evaluation.ts @@ -0,0 +1,47 @@ +import type { Automation, AutomationRun } from '../../shared/automations-types' +import type { Store } from '../persistence' + +export async function evaluateScheduledAutomation(args: { + store: Store + automation: Automation + now: number + requestDispatch: (automation: Automation, run: AutomationRun) => Promise +}): Promise { + const { store, automation, now, requestDispatch } = args + const scheduledFor = store.getLatestAutomationOccurrence(automation, now) + if (scheduledFor === null) { + store.advanceAutomationNextRun(automation.id, now) + return + } + const run = store.createAutomationRun(automation, scheduledFor) + try { + const graceMs = automation.missedRunGraceMinutes * 60 * 1000 + if (now - scheduledFor > graceMs) { + store.updateAutomationRun({ + runId: run.id, + status: 'skipped_missed', + workspaceId: automation.workspaceId, + error: 'Orca was unavailable during the missed-run grace window.' + }) + return + } + + await requestDispatch(automation, run) + } catch (error) { + try { + store.updateAutomationRun({ + runId: run.id, + status: 'dispatch_failed', + workspaceId: automation.workspaceId, + error: error instanceof Error ? error.message : String(error) + }) + } catch (persistenceError) { + throw new AggregateError( + [error, persistenceError], + `Automation ${automation.id} dispatch and failure persistence both failed` + ) + } + } finally { + store.advanceAutomationNextRun(automation.id, now) + } +} diff --git a/src/main/automations/service.test.ts b/src/main/automations/service.test.ts index dab39792584..7f61eb686ea 100644 --- a/src/main/automations/service.test.ts +++ b/src/main/automations/service.test.ts @@ -86,6 +86,76 @@ describe('AutomationService', () => { ) }) + it('fails one scheduled dispatch, advances it, and continues evaluating later due runs', async () => { + vi.setSystemTime(new Date('2026-05-13T08:59:00')) + const store = await createStore() + store.addRepo(makeRepo()) + const createDueAutomation = (name: string) => + store.createAutomation({ + name, + prompt: 'Check the repo', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-12T00:00:00').getTime() + }) + const failing = createDueAutomation('Failing dispatch') + const succeeding = createDueAutomation('Later dispatch') + vi.setSystemTime(new Date('2026-05-13T09:01:00')) + const send = vi.fn((_channel: string, payload: { automation: { id: string } }) => { + if (payload.automation.id === failing.id) { + throw new Error('renderer send failed') + } + }) + const service = new AutomationService(store) + service.setWebContents({ isDestroyed: () => false, send } as never) + + service.setRendererReady() + + await vi.waitFor(() => { + expect(store.listAutomationRuns(failing.id)[0]?.status).toBe('dispatch_failed') + expect(store.listAutomationRuns(succeeding.id)[0]?.status).toBe('dispatching') + }) + expect(store.listAutomationRuns(failing.id)[0]?.error).toBe('renderer send failed') + expect(store.listAutomations().find((entry) => entry.id === failing.id)?.nextRunAt).toBe( + new Date('2026-05-14T09:00:00').getTime() + ) + expect(store.listAutomations().find((entry) => entry.id === succeeding.id)?.nextRunAt).toBe( + new Date('2026-05-14T09:00:00').getTime() + ) + expect(send).toHaveBeenCalledTimes(2) + }) + + it('preserves manual run rejection semantics when renderer dispatch throws', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Manual failure', + prompt: 'Check the repo', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: Date.now() + 60_000 + }) + const service = new AutomationService(store) + service.setWebContents({ + isDestroyed: () => false, + send: () => { + throw new Error('renderer send failed') + } + } as never) + service.setRendererReady() + + await expect(service.runNow(automation.id)).rejects.toThrow('renderer send failed') + expect(store.listAutomationRuns(automation.id)[0]?.status).toBe('dispatching') + }) + it('returns the persisted status for manual runs after dispatch is requested', async () => { vi.setSystemTime(new Date('2026-05-13T08:00:00Z')) const store = await createStore() @@ -457,6 +527,51 @@ describe('AutomationService', () => { expect(store.listAutomationRuns(automation.id)[0]?.status).toBe('completed') }) + it('observes a rejected headless completion settlement', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Settlement failure', + prompt: 'Finish before shutdown', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: Date.now() + 60_000 + }) + const service = new AutomationService(store, { + allowRemoteHostScheduling: true, + headlessDispatcher: vi.fn().mockResolvedValue({ + workspaceId: 'wt1', + terminalSessionId: 'tab-1', + completion: Promise.resolve({ status: 'completed', outputSnapshot: null, error: null }) + }) + }) + vi.spyOn(service, 'markDispatchResult').mockRejectedValue(new Error('persistence failed')) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const unhandled: unknown[] = [] + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason) + } + process.on('unhandledRejection', onUnhandled) + + try { + await service.runNow(automation.id) + await vi.waitFor(() => + expect(consoleError).toHaveBeenCalledWith( + '[automations] headless completion settlement failed:', + expect.objectContaining({ message: 'persistence failed' }) + ) + ) + await vi.advanceTimersByTimeAsync(0) + expect(unhandled).toEqual([]) + } finally { + process.off('unhandledRejection', onUnhandled) + } + }) + it('attaches provider usage when a completed run can be attributed', async () => { vi.setSystemTime(new Date('2026-05-13T10:00:00')) const store = await createStore() diff --git a/src/main/automations/service.ts b/src/main/automations/service.ts index 8fea664f0b7..f189e67b62f 100644 --- a/src/main/automations/service.ts +++ b/src/main/automations/service.ts @@ -24,6 +24,7 @@ import { didAutomationPrecheckPass, formatAutomationPrecheckFailure } from '../../shared/automation-precheck' +import { evaluateScheduledAutomation } from './scheduled-automation-evaluation' const DEFAULT_TICK_MS = 60 * 1000 @@ -68,7 +69,7 @@ export class AutomationService { setRendererReady(): void { this.rendererReady = true - void this.headlessWork.track(this.evaluateDueRuns()) + this.trackBackgroundWork(this.evaluateDueRuns(), 'renderer-ready evaluation') } start(): void { @@ -76,12 +77,12 @@ export class AutomationService { return } this.timer = setInterval(() => { - void this.headlessWork.track(this.evaluateDueRuns()) + this.trackBackgroundWork(this.evaluateDueRuns(), 'scheduled evaluation') }, this.tickMs) // Why: headless serve never gets a renderer-ready IPC, but due runs still // need the same startup catch-up pass desktop gets after renderer attach. if (this.rendererReady || this.headlessDispatcher) { - void this.headlessWork.track(this.evaluateDueRuns()) + this.trackBackgroundWork(this.evaluateDueRuns(), 'startup evaluation') } } @@ -196,36 +197,22 @@ export class AutomationService { if (!automation.enabled || automation.nextRunAt > now) { continue } - await this.evaluateAutomation(automation, now) + try { + await evaluateScheduledAutomation({ + store: this.store, + automation, + now, + requestDispatch: (entry, run) => this.requestDispatch(entry, run) + }) + } catch (error) { + console.error(`[automations] Scheduled evaluation failed for ${automation.id}:`, error) + } } } finally { this.evaluating = false } } - private async evaluateAutomation(automation: Automation, now: number): Promise { - const scheduledFor = this.store.getLatestAutomationOccurrence(automation, now) - if (scheduledFor === null) { - this.store.advanceAutomationNextRun(automation.id, now) - return - } - const run = this.store.createAutomationRun(automation, scheduledFor) - const graceMs = automation.missedRunGraceMinutes * 60 * 1000 - if (now - scheduledFor > graceMs) { - this.store.updateAutomationRun({ - runId: run.id, - status: 'skipped_missed', - workspaceId: automation.workspaceId, - error: 'Orca was unavailable during the missed-run grace window.' - }) - this.store.advanceAutomationNextRun(automation.id, now) - return - } - - await this.requestDispatch(automation, run) - this.store.advanceAutomationNextRun(automation.id, now) - } - private async requestDispatch( automation: Automation, run: AutomationRun @@ -301,14 +288,15 @@ export class AutomationService { error: null }) if (launch.completion) { - void this.headlessWork.track( + this.trackBackgroundWork( settleHeadlessAutomationCompletion({ completion: launch.completion, runId: run.id, target: launchRunTarget, precheckResult, mark: (result) => this.markDispatchResult(result) - }) + }), + 'headless completion settlement' ) } return updated @@ -317,8 +305,15 @@ export class AutomationService { runId: run.id, status: 'dispatch_failed', workspaceId: automation.workspaceId, + precheckResult, error: error instanceof Error ? error.message : String(error) }) } } + + private trackBackgroundWork(work: Promise, context: string): void { + void this.headlessWork.track(work).catch((error) => { + console.error(`[automations] ${context} failed:`, error) + }) + } } diff --git a/src/node-server/browserless-runtime-composition.test.ts b/src/node-server/browserless-runtime-composition.test.ts index db5880bec92..63cb70d349a 100644 --- a/src/node-server/browserless-runtime-composition.test.ts +++ b/src/node-server/browserless-runtime-composition.test.ts @@ -7,6 +7,17 @@ import type { Store } from '../main/persistence' const state = vi.hoisted(() => { const calls: string[] = [] const signerKeyIds: string[] = [] + const inactiveResolvers = { + claude: null as (() => unknown) | null, + codex: null as (() => unknown) | null + } + const normalizationCalls = { claude: 0, codex: 0 } + const stopErrors = { + automation: null as Error | null, + rateLimits: null as Error | null, + targetSync: null as Error | null, + hook: null as Error | null + } class Runtime { private accountServices: { @@ -73,8 +84,12 @@ const state = vi.hoisted(() => { setClaudeAuthPreparationResolver(): void {} setCodexFetchTarget(): void {} setClaudeFetchTarget(): void {} - setInactiveClaudeAccountsResolver(): void {} - setInactiveCodexAccountsResolver(): void {} + setInactiveClaudeAccountsResolver(resolver: () => unknown): void { + inactiveResolvers.claude = resolver + } + setInactiveCodexAccountsResolver(resolver: () => unknown): void { + inactiveResolvers.codex = resolver + } ingestLiveClaudeRateLimits(): void {} getState(): unknown { return { codex: null, claude: null } @@ -84,6 +99,9 @@ const state = vi.hoisted(() => { } stop(): void { calls.push('rate-stop') + if (stopErrors.rateLimits) { + throw stopErrors.rateLimits + } } } @@ -99,10 +117,22 @@ const state = vi.hoisted(() => { } async stopAndDrain(): Promise { this.stop() + if (stopErrors.automation) { + throw stopErrors.automation + } } } - return { Automation, calls, RateLimits, Runtime, signerKeyIds } + return { + Automation, + calls, + inactiveResolvers, + normalizationCalls, + RateLimits, + Runtime, + signerKeyIds, + stopErrors + } }) vi.mock('../main/runtime/orca-runtime', () => ({ OrcaRuntimeService: state.Runtime })) @@ -174,17 +204,32 @@ vi.mock('../main/rate-limits/account-runtime-target-sync', () => ({ createAccountRuntimeTargetSettingsSync: () => async () => {} })) vi.mock('../main/codex-accounts/runtime-selection', () => ({ - normalizeCodexRuntimeSelection: () => ({ host: null, wsl: {} }) + normalizeCodexRuntimeSelection: () => { + state.normalizationCalls.codex += 1 + return { host: null, wsl: {} } + } })) vi.mock('../main/claude-accounts/runtime-selection', () => ({ - normalizeClaudeRuntimeSelection: () => ({ host: null, wsl: {} }) + normalizeClaudeRuntimeSelection: () => { + state.normalizationCalls.claude += 1 + return { host: null, wsl: {} } + } })) const hookServer = vi.hoisted(() => ({ start: vi.fn(async () => state.calls.push('hook-start')), - stop: vi.fn(() => state.calls.push('hook-stop')), - stopAndWait: vi.fn(async () => state.calls.push('hook-stop')), - setClaudeStatusLineListener: vi.fn(), + stop: vi.fn(() => state.calls.push('hook-stop-unjoined')), + stopAndWait: vi.fn(async () => { + state.calls.push('hook-stop-wait') + if (state.stopErrors.hook) { + throw state.stopErrors.hook + } + }), + setClaudeStatusLineListener: vi.fn((listener: unknown) => { + if (listener === null) { + state.calls.push('hook-listener-clear') + } + }), ingestTerminalStatus: vi.fn(), getStatusSnapshot: vi.fn(() => []), getStatusSnapshotForPane: vi.fn(() => []), @@ -214,6 +259,14 @@ const temporaryPaths: string[] = [] afterEach(() => { state.calls.length = 0 state.signerKeyIds.length = 0 + state.stopErrors.automation = null + state.stopErrors.rateLimits = null + state.stopErrors.targetSync = null + state.stopErrors.hook = null + state.inactiveResolvers.claude = null + state.inactiveResolvers.codex = null + state.normalizationCalls.claude = 0 + state.normalizationCalls.codex = 0 vi.clearAllMocks() for (const path of temporaryPaths.splice(0)) { rmSync(path, { force: true, recursive: true }) @@ -243,6 +296,17 @@ describe('browserless runtime composition', () => { expect(state.signerKeyIds[0]).toBe(state.signerKeyIds[1]) }) + it('normalizes each account runtime selection once per inactive-account resolution', () => { + const dataPath = mkdtempSync(join(tmpdir(), 'orca-browserless-composition-')) + temporaryPaths.push(dataPath) + createBrowserlessRuntimeComposition({ store: createStore(), dataPath }) + + state.inactiveResolvers.claude?.() + state.inactiveResolvers.codex?.() + + expect(state.normalizationCalls).toEqual({ claude: 1, codex: 1 }) + }) + it('orders startup recovery and joins owned lifecycle shutdown', async () => { const dataPath = mkdtempSync(join(tmpdir(), 'orca-browserless-composition-')) temporaryPaths.push(dataPath) @@ -262,8 +326,41 @@ describe('browserless runtime composition', () => { 'automation-start', 'automation-stop', 'rate-stop', - 'hook-stop' + 'target-sync-stop', + 'hook-listener-clear', + 'hook-stop-wait' ]) + expect(hookServer.stop).not.toHaveBeenCalled() + }) + + it('attempts every owned teardown in order and aggregates failures', async () => { + const dataPath = mkdtempSync(join(tmpdir(), 'orca-browserless-composition-')) + temporaryPaths.push(dataPath) + const composition = createBrowserlessRuntimeComposition({ store: createStore(), dataPath }) + state.stopErrors.automation = new Error('automation drain failed') + state.stopErrors.rateLimits = new Error('rate stop failed') + state.stopErrors.targetSync = new Error('target sync stop failed') + state.stopErrors.hook = new Error('hook stop failed') + + const error = await composition.stop().catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AggregateError) + expect((error as AggregateError).errors).toEqual([ + state.stopErrors.automation, + state.stopErrors.rateLimits, + state.stopErrors.targetSync, + state.stopErrors.hook + ]) + expect(state.calls).toEqual([ + 'prepare', + 'automation-stop', + 'rate-stop', + 'target-sync-stop', + 'hook-listener-clear', + 'hook-stop-wait' + ]) + await expect(composition.stop()).rejects.toBe(error) + expect(hookServer.stop).not.toHaveBeenCalled() }) it('cleans up once when startup fails', async () => { @@ -275,7 +372,14 @@ describe('browserless runtime composition', () => { await expect(composition.startAfterDaemon()).rejects.toThrow('hook bind failed') await composition.stop() - expect(state.calls).toEqual(['prepare', 'automation-stop', 'rate-stop', 'hook-stop']) + expect(state.calls).toEqual([ + 'prepare', + 'automation-stop', + 'rate-stop', + 'target-sync-stop', + 'hook-listener-clear', + 'hook-stop-wait' + ]) await expect(composition.startAfterDaemon()).rejects.toThrow('already stopped') }) }) @@ -284,6 +388,11 @@ function createStore(): Store { const settings = { claudeManagedAccounts: [], codexManagedAccounts: [] } return { getSettings: () => settings, - onSettingsChanged: () => () => {} + onSettingsChanged: () => () => { + state.calls.push('target-sync-stop') + if (state.stopErrors.targetSync) { + throw state.stopErrors.targetSync + } + } } as unknown as Store } diff --git a/src/node-server/browserless-runtime-composition.ts b/src/node-server/browserless-runtime-composition.ts index 550b419fa01..50206f14541 100644 --- a/src/node-server/browserless-runtime-composition.ts +++ b/src/node-server/browserless-runtime-composition.ts @@ -104,17 +104,18 @@ export function createBrowserlessRuntimeComposition(args: { }) let started = false - let stopped = false - const stopOwnedServices = async (): Promise => { - if (stopped) { - return + let stopPromise: Promise | null = null + const stopOwnedServices = (): Promise => { + if (!stopPromise) { + stopPromise = settleBrowserlessOwnedServiceStops([ + () => automations.stopAndDrain(), + () => rateLimits.stop(), + stopAccountRuntimeTargetSync, + () => agentHookServer.setClaudeStatusLineListener(null), + () => agentHookServer.stopAndWait() + ]) } - stopped = true - await automations.stopAndDrain() - rateLimits.stop() - stopAccountRuntimeTargetSync() - agentHookServer.setClaudeStatusLineListener(null) - await agentHookServer.stopAndWait() + return stopPromise } return { runtime, @@ -122,7 +123,7 @@ export function createBrowserlessRuntimeComposition(args: { if (started) { return } - if (stopped) { + if (stopPromise) { throw new Error('The browserless runtime composition is already stopped.') } started = true @@ -146,7 +147,14 @@ export function createBrowserlessRuntimeComposition(args: { automations.start() } catch (error) { started = false - await stopOwnedServices() + try { + await stopOwnedServices() + } catch (cleanupError) { + throw new AggregateError( + [error, cleanupError], + 'Browserless runtime startup and cleanup both failed' + ) + } throw error } }, @@ -156,6 +164,22 @@ export function createBrowserlessRuntimeComposition(args: { } } +async function settleBrowserlessOwnedServiceStops( + stops: readonly (() => void | Promise)[] +): Promise { + const failures: unknown[] = [] + for (const stop of stops) { + try { + await stop() + } catch (error) { + failures.push(error) + } + } + if (failures.length > 0) { + throw new AggregateError(failures, 'Browserless owned service shutdown failed') + } +} + function configureAccountServices( store: Store, rateLimits: RateLimitService, @@ -184,12 +208,8 @@ function configureAccountServices( function configureInactiveAccountResolvers(store: Store, rateLimits: RateLimitService): void { rateLimits.setInactiveClaudeAccountsResolver(() => { const settings = store.getSettings() - const activeIds = new Set( - [ - normalizeClaudeRuntimeSelection(settings).host, - ...Object.values(normalizeClaudeRuntimeSelection(settings).wsl) - ].filter(Boolean) - ) + const selection = normalizeClaudeRuntimeSelection(settings) + const activeIds = new Set([selection.host, ...Object.values(selection.wsl)].filter(Boolean)) return settings.claudeManagedAccounts .filter((account) => !activeIds.has(account.id)) .map((account) => ({ @@ -202,12 +222,8 @@ function configureInactiveAccountResolvers(store: Store, rateLimits: RateLimitSe }) rateLimits.setInactiveCodexAccountsResolver(() => { const settings = store.getSettings() - const activeIds = new Set( - [ - normalizeCodexRuntimeSelection(settings).host, - ...Object.values(normalizeCodexRuntimeSelection(settings).wsl) - ].filter(Boolean) - ) + const selection = normalizeCodexRuntimeSelection(settings) + const activeIds = new Set([selection.host, ...Object.values(selection.wsl)].filter(Boolean)) return settings.codexManagedAccounts .filter((account) => !activeIds.has(account.id)) .map((account) => ({ id: account.id, managedHomePath: account.managedHomePath })) diff --git a/src/node-server/server-profile-process-lock.test.ts b/src/node-server/server-profile-process-lock.test.ts index eabe2456424..2b0230f1078 100644 --- a/src/node-server/server-profile-process-lock.test.ts +++ b/src/node-server/server-profile-process-lock.test.ts @@ -13,6 +13,7 @@ import { import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' +import * as managedHookLockClaims from '../main/agent-hooks/managed-hook-lock-claims' import { readManagedHookHostIdentity, readManagedHookProcessIdentity @@ -296,6 +297,17 @@ describe('server profile process lock', () => { await recovered.release() }) + it('retries release after a transient removal failure', async () => { + const root = await createRoot() + const lock = await acquireServerProfileProcessLock(root) + const removeLock = vi.spyOn(managedHookLockClaims, 'removeManagedHookLock') + removeLock.mockResolvedValueOnce('unverifiable') + + await expect(lock.release()).rejects.toMatchObject({ reason: 'release_failed' }) + await expect(lock.release()).resolves.toBeUndefined() + expect(removeLock).toHaveBeenCalledTimes(2) + }) + it('surfaces a typed error when the current process cannot be identified', async () => { const root = await createRoot() const error = await acquireServerProfileProcessLock(root, { diff --git a/src/node-server/server-profile-process-lock.ts b/src/node-server/server-profile-process-lock.ts index d4405678d27..581c60497c9 100644 --- a/src/node-server/server-profile-process-lock.ts +++ b/src/node-server/server-profile-process-lock.ts @@ -166,7 +166,6 @@ function createOwnedLock( if (released) { return } - released = true try { const removal = await removeManagedHookLock( lockPath, @@ -182,6 +181,7 @@ function createOwnedLock( owner.pid ) } + released = true } finally { deactivateManagedHookLockOwner(owner.token) } diff --git a/src/renderer/src/components/settings/NodeServerSetupCallout.test.tsx b/src/renderer/src/components/settings/NodeServerSetupCallout.test.tsx new file mode 100644 index 00000000000..1d7139b8879 --- /dev/null +++ b/src/renderer/src/components/settings/NodeServerSetupCallout.test.tsx @@ -0,0 +1,43 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' + +import { cleanup, render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { toast } from 'sonner' +import { NodeServerSetupCallout } from './NodeServerSetupCallout' + +const writeClipboardText = vi.fn() + +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), success: vi.fn() } +})) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +beforeEach(() => { + vi.clearAllMocks() + Object.defineProperty(window, 'api', { + configurable: true, + value: { ui: { writeClipboardText } } + }) +}) + +afterEach(cleanup) + +describe('NodeServerSetupCallout', () => { + it('does not expose clipboard failure details', async () => { + writeClipboardText.mockRejectedValueOnce(new Error('private clipboard detail')) + render() + + await userEvent.click(screen.getByRole('button', { name: 'Copy server command' })) + + expect(toast.error).toHaveBeenCalledWith('Failed to copy server command.') + expect(JSON.stringify(vi.mocked(toast.error).mock.calls)).not.toContain( + 'private clipboard detail' + ) + }) +}) diff --git a/src/renderer/src/components/settings/NodeServerSetupCallout.tsx b/src/renderer/src/components/settings/NodeServerSetupCallout.tsx index 0605b4aa72a..24d26394a5c 100644 --- a/src/renderer/src/components/settings/NodeServerSetupCallout.tsx +++ b/src/renderer/src/components/settings/NodeServerSetupCallout.tsx @@ -15,14 +15,12 @@ export function NodeServerSetupCallout(): React.JSX.Element { 'Copied server command.' ) ) - } catch (error) { + } catch { toast.error( - error instanceof Error - ? error.message - : translate( - 'auto.components.settings.NodeServerSetupCallout.copyFailed', - 'Failed to copy server command.' - ) + translate( + 'auto.components.settings.NodeServerSetupCallout.copyFailed', + 'Failed to copy server command.' + ) ) } } diff --git a/src/renderer/src/components/settings/RuntimeHostAccessForm.test.tsx b/src/renderer/src/components/settings/RuntimeHostAccessForm.test.tsx index 9e695ec5dbc..2bcb469b04b 100644 --- a/src/renderer/src/components/settings/RuntimeHostAccessForm.test.tsx +++ b/src/renderer/src/components/settings/RuntimeHostAccessForm.test.tsx @@ -36,6 +36,9 @@ describe('RuntimeHostAccessForm', () => { expect(markup).toContain('Tailscale') expect(markup).toContain('SSH tunnel') expect(markup).toContain('Already have the Orca desktop app') + expect(markup.indexOf('npx @stablyai/orca@latest')).toBeLessThan( + markup.indexOf('Already have the Orca desktop app') + ) }) it('shows the sanitized destination without credential material', () => {