diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 6dba5ac1ff6..d00b4e500fe 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -716,6 +716,7 @@ jobs: # message and tag name explicitly (avoids npm's `v1.2.3` prefix # assumptions and any lifecycle scripts that would run on bump). npm version "$VERSION" --no-git-tag-version --allow-same-version + npm --prefix resources/npm-server version "$VERSION" --no-git-tag-version --allow-same-version # Why: the cut is the only point where committed skill bytes become a # released revision. Without this row the ledger never advances, so the # next skill change rebuilds the revision this tag ships over different @@ -728,7 +729,7 @@ jobs: echo "::error::Refusing to record release provenance for v$VERSION: the committed skill artifacts do not match this ref. Land a regeneration on main, then re-run the cut." >&2 exit 1 fi - git add package.json resources/skills/release-mapping.json + git add package.json resources/npm-server/package.json resources/skills/release-mapping.json commit_message="release: v$VERSION" if [[ "$EVENT_NAME" == "schedule" ]]; then commit_message="$commit_message [rc-slot:$SLOT]" @@ -752,9 +753,9 @@ jobs: # -m --first-parent: plain diff-tree prints NOTHING for a merge commit, # which would make this guard pass silently rather than fail closed. committed="$(git diff-tree --no-commit-id --name-only -r -m --first-parent HEAD | - grep -vxF -e 'package.json' -e 'resources/skills/release-mapping.json' || true)" + grep -vxF -e 'package.json' -e 'resources/npm-server/package.json' -e 'resources/skills/release-mapping.json' || true)" if [[ -n "$committed" ]]; then - echo "::error::Release commit carries unexpected paths: $(echo "$committed" | tr '\n' ' ')Only package.json and the skill release-mapping row may ship in a version commit." >&2 + echo "::error::Release commit carries unexpected paths: $(echo "$committed" | tr '\n' ' ')Only package manifests and the skill release-mapping row may ship in a version commit." >&2 exit 1 fi git tag -a "v$VERSION" -m "v$VERSION" diff --git a/config/scripts/generate-skill-bundle-manifest.test.mjs b/config/scripts/generate-skill-bundle-manifest.test.mjs index e8a88b4636c..7753595aea1 100644 --- a/config/scripts/generate-skill-bundle-manifest.test.mjs +++ b/config/scripts/generate-skill-bundle-manifest.test.mjs @@ -565,13 +565,13 @@ describe('skill bundle manifest generator', () => { const bumpStep = runSteps.find((step) => step.name === 'Bump package.json and tag') // The load-bearing check: whatever staged it and however the commit was - // spelled, only these two paths may ship. Asserted on the commit rather than + // spelled, only these paths may ship. Asserted on the commit rather than // the index because `git commit -a/-i/--only/` bypasses the index. // -F is part of the contract; without it `.` admits a path like packageXjson. // Flags pinned, not just the command: a `--diff-filter` slipped in here would // silence modifications, and dropping -m makes a merge commit report nothing. expect(bumpStep.run).toMatch( - /git diff-tree --no-commit-id --name-only -r -m --first-parent HEAD\s*\|\s*grep -vxF -e 'package\.json' -e 'resources\/skills\/release-mapping\.json'/ + /git diff-tree --no-commit-id --name-only -r -m --first-parent HEAD\s*\|\s*grep -vxF -e 'package\.json' -e 'resources\/npm-server\/package\.json' -e 'resources\/skills\/release-mapping\.json'/ ) expect(bumpStep.run.indexOf('grep -vxF')).toBeLessThan(bumpStep.run.indexOf('git tag')) // ...and that it aborts. A guard degraded to a warning still reads as covered. diff --git a/config/scripts/node-server-runtime-verifier.ts b/config/scripts/node-server-runtime-verifier.ts index 4d8dc8cce26..6f38ca66919 100644 --- a/config/scripts/node-server-runtime-verifier.ts +++ b/config/scripts/node-server-runtime-verifier.ts @@ -13,6 +13,7 @@ import { startNodeServerVerifierProcess, stopNodeServerVerifierProcess, terminateNodeServerVerifierProcess, + verifyNodeServerStartupCancellation, type NodeServerVerifierProcess } from './node-server-verifier-server-process' const cliPath = resolve(readArgument('--cli') ?? 'resources/npm-server/dist/cli.js') @@ -29,6 +30,7 @@ async function main(): Promise { try { initializeNodeServerVerifierGitWorkspace(gitPath) mkdirSync(folderPath) + await verifyNodeServerStartupCancellation({ cliPath, dataPath }) const first = await startNodeServerVerifierProcess({ cliPath, dataPath }) activeServer = first diff --git a/config/scripts/node-server-verifier-server-process.ts b/config/scripts/node-server-verifier-server-process.ts index 7592cb55062..10f610e486c 100644 --- a/config/scripts/node-server-verifier-server-process.ts +++ b/config/scripts/node-server-verifier-server-process.ts @@ -22,30 +22,60 @@ export type NodeServerVerifierProcess = { stderr: string[] } +export async function verifyNodeServerStartupCancellation(args: { + cliPath: string + dataPath: string +}): Promise { + if (process.platform === 'win32') { + return + } + const child = spawn(process.execPath, serverArguments(args.cliPath), { + env: { + ...process.env, + ORCA_E2E_DAEMON_INIT_DELAY_MS: '30000', + ORCA_SERVER_DATA_DIR: args.dataPath, + ORCA_STARTUP_DIAGNOSTICS: '1' + }, + stdio: ['ignore', 'pipe', 'pipe'] + }) + let stdout = '' + let stderr = '' + child.stdout?.setEncoding('utf8') + child.stdout?.on('data', (chunk: string) => { + stdout += chunk + }) + child.stderr?.setEncoding('utf8') + child.stderr?.on('data', (chunk: string) => { + stderr += chunk + }) + + try { + await waitForDaemonInitMarker(child, () => stderr) + if (!child.kill('SIGTERM')) { + throw new Error('server rejected the startup shutdown signal') + } + const result = await waitForInstalledServerExit(child) + if (!isExpectedInstalledServerStopResult(result, process.platform)) { + throw new Error(`server startup shutdown failed (${result.code ?? result.signal}): ${stderr}`) + } + if (stdout.length > 0) { + throw new Error(`server published readiness during startup shutdown: ${stdout}`) + } + } finally { + await terminateNodeServerVerifierProcess(child) + } +} + 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'] - } - ) + const child = spawn(process.execPath, serverArguments(args.cliPath), { + 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) @@ -107,6 +137,54 @@ export function startNodeServerVerifierProcess(args: { }) } +function serverArguments(cliPath: string): string[] { + return [ + cliPath, + 'serve', + '--port', + '0', + '--listen', + '127.0.0.1', + '--pairing-address', + '127.0.0.1', + '--json' + ] +} + +function waitForDaemonInitMarker(child: ChildProcess, readStderr: () => string): Promise { + return new Promise((resolveMarker, rejectMarker) => { + const timeout = setTimeout( + () => finish(new Error(`daemon init marker timed out: ${readStderr()}`)), + 10_000 + ) + child.stderr?.on('data', onStderr) + child.once('error', finish) + child.once('exit', onExit) + + function onStderr(): void { + if (readStderr().includes('[startup] daemon-init-start')) { + finish() + } + } + + function onExit(code: number | null): void { + finish(new Error(`server exited before daemon init marker (${code}): ${readStderr()}`)) + } + + function finish(error?: unknown): void { + clearTimeout(timeout) + child.stderr?.off('data', onStderr) + child.off('error', finish) + child.off('exit', onExit) + if (error) { + rejectMarker(error) + } else { + resolveMarker() + } + } + }) +} + export async function terminateNodeServerVerifierProcess(child: ChildProcess): Promise { if (child.exitCode !== null || child.signalCode !== null) { return diff --git a/config/scripts/package-electron-runtime-contract.test.mjs b/config/scripts/package-electron-runtime-contract.test.mjs index 1b695086808..161d55b7b88 100644 --- a/config/scripts/package-electron-runtime-contract.test.mjs +++ b/config/scripts/package-electron-runtime-contract.test.mjs @@ -328,7 +328,7 @@ describe('Electron runtime package contract', () => { expect(afterInstallScript).not.toContain('chmod 0755 "$sandbox"') }) - it('advances only the skill release ledger in a taggable release-cut commit', () => { + it('keeps package manifests synchronized in a taggable release-cut commit', () => { const releaseWorkflow = readFileSync( join(projectDir, '.github/workflows/release-cut.yml'), 'utf8' @@ -354,15 +354,22 @@ describe('Electron runtime package contract', () => { const mentioned = new Set(commands.match(/resources[/\\]skills[^\s'"]*/g)) expect(checkoutStep.with['fetch-depth']).toBe(0) expect(bumpIndex).toBeGreaterThanOrEqual(0) + expect(bumpStep.run).toContain( + 'npm --prefix resources/npm-server version "$VERSION" --no-git-tag-version --allow-same-version' + ) // Why: the cut is the only point that advances the release ledger, so this // tag's revision is never rebuilt later — it appends that row, nothing else. expect(generateIndex).toBeGreaterThan(bumpIndex) expect(bumpStep.run.indexOf('git add package.json')).toBeGreaterThan(generateIndex) - expect(stagedPaths).toEqual(['package.json', 'resources/skills/release-mapping.json']) + expect(stagedPaths).toEqual([ + 'package.json', + 'resources/npm-server/package.json', + 'resources/skills/release-mapping.json' + ]) // Every distinct mention must be staged, so a copy, a redirect, or a path // held in a variable cannot reach the content-addressed artifacts. Matched // without a trailing slash so `dir="resources/skills"` still counts. - expect([...mentioned]).toEqual(stagedPaths.slice(1)) + expect([...mentioned]).toEqual(['resources/skills/release-mapping.json']) // Regeneration is banned job-wide by the generator suite. Here: `-a`, `-am`, // and `--all` sweep unstaged artifacts in; `--allow-empty` below must not. expect(commands).not.toMatch(/\bcommit\b[^\n]*(?:\s-[a-z]*a[a-z]*\b|\s--all\b)/) diff --git a/docs/reference/npm-server-design.md b/docs/reference/npm-server-design.md index d2f26f2408b..bd35e0191d9 100644 --- a/docs/reference/npm-server-design.md +++ b/docs/reference/npm-server-design.md @@ -202,10 +202,11 @@ considered only after the server build and native dependency matrix pass there. ## Shutdown and persistence -SIGINT, SIGTERM, and startup failure join one sequential teardown: stop RPC listeners; cancel local -or SSH prechecks, abort and drain headless automations, and wait for AgentHook shutdown through the -composition root; disconnect from the terminal daemon; flush the store; then release the profile -lock. A failed step is recorded while later steps still run. A normal server shutdown does not kill +SIGINT and SIGTERM synchronously cancel startup before joining its current phase. Teardown then +stops RPC listeners; cancels local or SSH prechecks, aborts and drains headless automations, and +waits for AgentHook shutdown through the composition root; disconnects from the terminal daemon; +flushes the store; and releases the profile lock. Startup failures join the same coordinator. A +failed step is recorded while later steps still run. A normal server shutdown does not kill daemon-owned terminals. Startup reconnects to the same daemon and device identity. Every listener, socket, timer, and signal handler installed by the Node composition root has a @@ -316,6 +317,8 @@ the terminal marker. - [x] Omit browser capabilities and fail remote browser requests explicitly. - [x] Join RPC stop, composition drain, daemon disconnect, store flush, and lock release in order on signals and startup errors. +- [x] Cancel startup before signal teardown and prove pre-readiness SIGTERM prints no credential, + exits cleanly, and leaves the profile reusable. ### CLI and networking @@ -397,46 +400,60 @@ the terminal marker. The installed-tarball history covers its inventory, executable, license, dependency, web-client, real E2EE, workspace, Git, PTY, shutdown, and restart-continuity oracle on macOS; Ubuntu 20.04, -22.04, and 24.04 amd64; and Ubuntu 20.04 arm64. The final package was rerun on macOS arm64 and -Ubuntu 20.04 amd64. The Ubuntu 20.04 runs used stock Git 2.25.1 and verified the active native PTY -against glibc 2.31 without a compiler, Electron, Chromium, Xvfb, or FUSE installed. +22.04, and 24.04 amd64; and Ubuntu 20.04 arm64. The final package was rerun on macOS arm64 and all +three amd64 Ubuntu versions. The Ubuntu 20.04 runs used stock Git 2.25.1 and verified the active +native PTY against glibc 2.31 without a compiler, Electron, Chromium, Xvfb, or FUSE installed. -The current focused selector passed 525 tests across 22 server, RPC, cross-version wire, onboarding, -browserless composition, automation, precheck, profile-lock, readiness, installed-process, -AgentHook, and shutdown files. The two workflow contract files passed 17 tests: +The current focused selector passed 552 tests across 31 server, RPC, cross-version wire, +onboarding, browserless composition, automation, precheck, profile-lock, readiness, +installed-process, AgentHook, and shutdown files. Four workflow and release-contract files passed +62 tests with one platform-specific skip: ```bash pnpm exec vitest run --config config/vitest.config.ts \ config/scripts/node-server-installed-process-harness.test.ts \ + config/scripts/node-server-verifier-daemon-cleanup.test.ts \ config/scripts/node-server-verifier-git-fixture.test.ts \ config/scripts/node-server-verifier-host-path.test.ts \ src/cli/runtime-client-deferral.test.ts \ src/main/agent-hooks/server.test.ts \ + src/main/automations/headless-completion-settlement.test.ts \ src/main/automations/headless-work-drain.test.ts \ src/main/automations/precheck-runner.test.ts \ src/main/automations/service-precheck.test.ts \ src/main/automations/service.test.ts \ src/main/runtime/pairing-endpoint.test.ts \ src/main/runtime/runtime-rpc.test.ts \ + src/main/server/serve-readiness.test.ts \ src/node-server/browserless-automation-dispatcher.test.ts \ src/node-server/browserless-runtime-composition.test.ts \ + src/node-server/package-cli-deferral.test.ts \ src/node-server/server-address-discovery.test.ts \ + src/node-server/server-bind-host.test.ts \ src/node-server/server-cli-arguments.test.ts \ src/node-server/server-paths.test.ts \ + src/node-server/server-profile-environment.test.ts \ src/node-server/server-profile-process-lock.test.ts \ + src/node-server/server-shutdown-coordinator.test.ts \ + src/node-server/server-signal-shutdown.test.ts \ src/node-server/server-websocket-readiness.test.ts \ src/node-server/server-window-graph.test.ts \ + src/renderer/src/components/settings/NodeServerSetupCallout.test.tsx \ src/renderer/src/components/settings/RuntimeHostAccessForm.test.tsx \ src/renderer/src/components/sidebar/AddRemoteHostFields.test.tsx \ tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts pnpm exec vitest run --config config/vitest.config.ts \ config/scripts/pr-workflow-parallelism.test.mjs \ - config/scripts/node-server-package-workflow-contract.test.mjs + config/scripts/node-server-package-workflow-contract.test.mjs \ + config/scripts/generate-skill-bundle-manifest.test.mjs \ + config/scripts/package-electron-runtime-contract.test.mjs ``` -The final macOS arm64 installed-tarball oracle passed with 18,114,920 packed bytes and 55,066,757 -unpacked bytes. Ubuntu 20.04, 22.04, and 24.04 amd64 passed the same installed runtime oracle in -Docker; Ubuntu 20.04 also verified the glibc 2.31 and `GLIBCXX_3.4.28` floors. +The final macOS arm64 installed-tarball oracle passed with 18,203,728 packed bytes and 55,879,495 +unpacked bytes. Its POSIX startup oracle also sent SIGTERM before readiness, observed no credential +output, and restarted the same profile successfully. Ubuntu 20.04, 22.04, and 24.04 amd64 passed +the same installed runtime oracle in Docker; Ubuntu 20.04 also verified the glibc 2.31 and +`GLIBCXX_3.4.28` floors. Ten fresh-profile installed-package trials on macOS arm64 reached the first schema-v1 readiness line in 352.705–748.323 ms, with a 369.474 ms median. Recursive process snapshots at one and three diff --git a/resources/npm-server/README.md b/resources/npm-server/README.md index c9d5d8b4d21..d8036533132 100644 --- a/resources/npm-server/README.md +++ b/resources/npm-server/README.md @@ -1,6 +1,6 @@ # Orca Server -Requires Node.js 24 or newer. +Requires Node.js 24.x. Run Orca on a development server without installing the Electron desktop application: diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 7b95d4a010c..7f5af90242a 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -928,7 +928,8 @@ export async function initDaemonPtyProvider( // Why: e2e coverage for the startup PTY gate (#5232) needs a daemon init that deterministically outlasts the first-window timeout. const e2eInitDelayMs = Number(process.env.ORCA_E2E_DAEMON_INIT_DELAY_MS) if (Number.isFinite(e2eInitDelayMs) && e2eInitDelayMs > 0) { - await new Promise((resolve) => setTimeout(resolve, e2eInitDelayMs)) + await waitForDaemonInitDelay(e2eInitDelayMs, signal) + signal?.throwIfAborted() } const runtimeDir = getRuntimeDir() @@ -1037,6 +1038,19 @@ export async function initDaemonPtyProvider( await reconcileSeededClaudeLivePtys(routedAdapter) } +function waitForDaemonInitDelay(delayMs: number, signal?: AbortSignal): Promise { + return new Promise((resolve) => { + const timeout = setTimeout(finish, delayMs) + signal?.addEventListener('abort', finish, { once: true }) + + function finish(): void { + clearTimeout(timeout) + signal?.removeEventListener('abort', finish) + resolve() + } + }) +} + // Why: release gate ids only for daemon-confirmed-dead sessions; keep seeds on listing failure since releasing early can rotate a live CLI's refresh token. async function reconcileSeededClaudeLivePtys(provider: DaemonProvider): Promise { if (!hasSeededUnconfirmedClaudePtys()) { diff --git a/src/main/server/serve-readiness.test.ts b/src/main/server/serve-readiness.test.ts index 7e037eba025..ebf702bcec8 100644 --- a/src/main/server/serve-readiness.test.ts +++ b/src/main/server/serve-readiness.test.ts @@ -22,6 +22,16 @@ const ready: ServeReadiness = { } describe('ServeReadinessPublisher', () => { + it('does not publish readiness after startup cancellation', async () => { + const write = vi.fn(async () => {}) + const publisher = new ServeReadinessPublisher(write) + + await expect( + publisher.publish(ready, { mode: 'human' }, AbortSignal.abort()) + ).rejects.toMatchObject({ name: 'AbortError' }) + expect(write).not.toHaveBeenCalled() + }) + it('writes one complete human-readable ready block', async () => { const write = vi.fn(async () => {}) const publisher = new ServeReadinessPublisher(write) diff --git a/src/main/server/serve-readiness.ts b/src/main/server/serve-readiness.ts index e4ab3db8925..b433fe72ab5 100644 --- a/src/main/server/serve-readiness.ts +++ b/src/main/server/serve-readiness.ts @@ -37,10 +37,15 @@ export class ServeReadinessPublisher { constructor(private readonly write: ReadinessWrite = writeStdout) {} - async publish(readiness: ServeReadiness, output: ServeReadinessOutput): Promise { + async publish( + readiness: ServeReadiness, + output: ServeReadinessOutput, + signal?: AbortSignal + ): Promise { if (this.state !== 'pending') { throw new Error(`Serve readiness publication already ${this.state}`) } + signal?.throwIfAborted() this.state = 'publishing' try { await this.write(`${renderServeReadiness(readiness, output)}\n`) diff --git a/src/node-server/browserless-automation-dispatcher.test.ts b/src/node-server/browserless-automation-dispatcher.test.ts index e89a40542de..9f903cf7de6 100644 --- a/src/node-server/browserless-automation-dispatcher.test.ts +++ b/src/node-server/browserless-automation-dispatcher.test.ts @@ -123,48 +123,43 @@ describe('browserless automation dispatcher', () => { expect(runtime.waitForTerminal).not.toHaveBeenCalled() }) - it('consumes a late worktree-create rejection without continuing dispatch', async () => { - vi.useFakeTimers() - try { - let rejectCreate!: (error: Error) => void - const create = new Promise((_resolve, reject) => { - rejectCreate = reject - }) - const runtime = { - createManagedWorktree: vi.fn(() => create), - launchAgentTerminal: vi.fn(), - showManagedWorktree: vi.fn(), - waitForTerminal: vi.fn(), - readTerminal: vi.fn() - } as unknown as OrcaRuntimeService - const controller = new AbortController() - const dispatch = createBrowserlessAutomationDispatcher(runtime) - const launched = dispatch({ - automation: { - workspaceMode: 'new_per_run', - agentId: 'claude', - prompt: 'Run checks' - }, - run: { id: 'run-1', title: 'Checks', scheduledFor: Date.now() }, - target: { repo: { id: 'repo-1' } }, - signal: controller.signal - } as never) - expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) - const rejection = expect(launched).rejects.toThrow( - 'Automation stopped during server shutdown.' - ) + it('joins an in-flight worktree creation before shutdown settles', async () => { + let rejectCreate!: (error: Error) => void + const create = new Promise((_resolve, reject) => { + rejectCreate = reject + }) + const runtime = { + createManagedWorktree: vi.fn(() => create), + launchAgentTerminal: vi.fn(), + showManagedWorktree: vi.fn(), + waitForTerminal: vi.fn(), + readTerminal: vi.fn() + } as unknown as OrcaRuntimeService + const controller = new AbortController() + const dispatch = createBrowserlessAutomationDispatcher(runtime) + let settled = false + const launched = dispatch({ + automation: { + workspaceMode: 'new_per_run', + agentId: 'claude', + prompt: 'Run checks' + }, + run: { id: 'run-1', title: 'Checks', scheduledFor: Date.now() }, + target: { repo: { id: 'repo-1' } }, + signal: controller.signal + } as never).finally(() => { + settled = true + }) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) - controller.abort() - await vi.runAllTimersAsync() - await rejection + controller.abort() + await new Promise((resolve) => setImmediate(resolve)) + expect(settled).toBe(false) - rejectCreate(new Error('late create failure')) - await Promise.resolve() - expect(runtime.launchAgentTerminal).not.toHaveBeenCalled() - expect(runtime.waitForTerminal).not.toHaveBeenCalled() - } finally { - vi.useRealTimers() - } + rejectCreate(new Error('late create failure')) + await expect(launched).rejects.toThrow('Automation stopped during server shutdown.') + expect(runtime.launchAgentTerminal).not.toHaveBeenCalled() + expect(runtime.waitForTerminal).not.toHaveBeenCalled() }) it('bounds shutdown while resolving the launched workspace display', async () => { diff --git a/src/node-server/browserless-automation-dispatcher.ts b/src/node-server/browserless-automation-dispatcher.ts index 0352030f46d..88cbfc7bc7f 100644 --- a/src/node-server/browserless-automation-dispatcher.ts +++ b/src/node-server/browserless-automation-dispatcher.ts @@ -31,7 +31,8 @@ export function createBrowserlessAutomationDispatcher( runtime.createManagedWorktree( buildHeadlessAutomationWorktreeCreateArgs({ automation, run, repo: target.repo }) ), - signal + signal, + 'join' ) terminalHandle = created.startupTerminal?.handle ?? '' terminalSessionId = created.startupTerminal?.tabId ?? null @@ -142,7 +143,8 @@ function shutdownCompletion(): { async function awaitRuntimeOperation( operation: () => Promise, - signal?: AbortSignal + signal?: AbortSignal, + shutdownSettlement: 'grace' | 'join' = 'grace' ): Promise { if (signal?.aborted) { throw new Error(SHUTDOWN_ERROR) @@ -171,9 +173,11 @@ async function awaitRuntimeOperation( return unwrapRuntimeOperationSettlement(first) } - // Why: some runtime operations have no cancellation contract. Observe their - // eventual outcome, but bound shutdown and never continue after the abort. - await waitForRuntimeOperationShutdownGrace(settlement) + // Why: read-only runtime operations may lack cancellation; observe their + // outcome without letting them block shutdown indefinitely. + await (shutdownSettlement === 'join' + ? settlement + : waitForRuntimeOperationShutdownGrace(settlement)) throw new Error(SHUTDOWN_ERROR) } diff --git a/src/node-server/index.ts b/src/node-server/index.ts index d6da3effe9b..99769bb782d 100644 --- a/src/node-server/index.ts +++ b/src/node-server/index.ts @@ -10,15 +10,22 @@ import { OrcaRuntimeRpcServer } from '../main/runtime/runtime-rpc' import { ServeReadinessPublisher } from '../main/server/serve-readiness' import { HEADLESS_RUNTIME_WINDOW_ID } from '../shared/runtime-types' import { configureNodeHostEnvironment } from './node-host-electron-facade' -import { createBrowserlessRuntimeComposition } from './browserless-runtime-composition' +import { + createBrowserlessRuntimeComposition, + type BrowserlessRuntimeComposition +} from './browserless-runtime-composition' import { discoverServerPairingAddress } from './server-address-discovery' +import { resolveServerBindHost } from './server-bind-host' import { parseServerCliArguments, renderServerHelp } from './server-cli-arguments' import { ensureServerDataPath, resolveServerDataPath } from './server-paths' +import { configureServerProfileEnvironment } from './server-profile-environment' import { acquireServerProfileProcessLock, ServerProfileProcessLockError } from './server-profile-process-lock' import { createServerWindowGraph } from './server-window-graph' +import { createServerShutdownCoordinator } from './server-shutdown-coordinator' +import { installServerSignalShutdown } from './server-signal-shutdown' import { requireServerWebSocketEndpoint } from './server-websocket-readiness' export async function runNodeServer(argv = process.argv.slice(2)): Promise { @@ -34,6 +41,7 @@ export async function runNodeServer(argv = process.argv.slice(2)): Promise const appPath = __dirname const dataPath = resolveServerDataPath(args.dataPath) + configureServerProfileEnvironment(dataPath) ensureServerDataPath(dataPath) configureNodeHostEnvironment({ appPath, dataPath, version: packageJson.version }) process.env.ORCA_APP_VERSION = packageJson.version @@ -43,24 +51,23 @@ export async function runNodeServer(argv = process.argv.slice(2)): Promise } throw error }) - initDataPath() - - const store = new Store({ dataFile: join(dataPath, 'orca-data.json') }) - const composition = createBrowserlessRuntimeComposition({ store, dataPath }) - const { runtime } = composition + let store: Store | null = null + let composition: BrowserlessRuntimeComposition | null = null let rpc: OrcaRuntimeRpcServer | null = null - let shuttingDown = false - const shutdown = async (exitCode: number): Promise => { - if (shuttingDown) { - return - } - shuttingDown = true + let startupPromise: Promise | null = null + const startupAbortController = new AbortController() + const shutdown = createServerShutdownCoordinator(async (getRequestedExitCode) => { const lifecycleResults = await settleLifecycleSteps([ + () => + startupPromise?.then( + () => undefined, + () => undefined + ) ?? Promise.resolve(), () => rpc?.stop() ?? Promise.resolve(), - () => composition.stop(), + () => composition?.stop() ?? Promise.resolve(), () => disconnectDaemon(), - () => store.flushAsync() + () => store?.flushAsync() ?? Promise.resolve() ]) const lifecycleFailed = lifecycleResults.some((result) => result.status === 'rejected') const lockReleased = await profileLock.release().then( @@ -72,49 +79,63 @@ export async function runNodeServer(argv = process.argv.slice(2)): Promise return false } ) - process.exitCode = lifecycleFailed || !lockReleased ? 1 : exitCode - } + process.exitCode = lifecycleFailed || !lockReleased ? 1 : getRequestedExitCode() + }) + installServerSignalShutdown({ + emitter: process, + shutdown, + exit: (exitCode) => process.exit(exitCode), + getExitCode: () => (typeof process.exitCode === 'number' ? process.exitCode : 0), + onSignal: () => startupAbortController.abort() + }) try { - await initDaemonPtyProvider(undefined, { macosLoginSessionWatch: false }) - await composition.startAfterDaemon() - runtime.syncWindowGraph(HEADLESS_RUNTIME_WINDOW_ID, createServerWindowGraph(store)) - const pairingAddress = await discoverServerPairingAddress(args.pairingAddress) - validatePairingAddress(pairingAddress.address) - rpc = new OrcaRuntimeRpcServer({ - runtime, - userDataPath: dataPath, - enableWebSocket: true, - preferPinnedWsPort: args.port !== 0, - wsBindHost: - args.listenHost ?? - (pairingAddress.source === 'tailscale' - ? pairingAddress.address - : pairingAddress.source === 'explicit' - ? '0.0.0.0' - : '127.0.0.1'), - wsPort: args.port, - webClientRoot: resolveWebClientRoot(appPath) - }) - await rpc.start() - requireServerWebSocketEndpoint(rpc.getWebSocketEndpoint()) - await publishReadiness(runtime, rpc, args, pairingAddress.address) - if (!args.json && pairingAddress.source === 'loopback' && process.env.SSH_CONNECTION) { - const resolvedPort = new URL(rpc.getWebSocketEndpoint()!).port - process.stdout.write( - `SSH tunnel: ssh -L ${resolvedPort}:127.0.0.1:${resolvedPort} ${process.env.USER ?? 'user'}@\n` - ) - } - const stopForSignal = (): void => { - void shutdown(0).then(() => - process.exit(typeof process.exitCode === 'number' ? process.exitCode : 0) - ) - } - process.once('SIGINT', stopForSignal) - process.once('SIGTERM', stopForSignal) + initDataPath() + store = new Store({ dataFile: join(dataPath, 'orca-data.json') }) + composition = createBrowserlessRuntimeComposition({ store, dataPath }) + const activeStore = store + const activeComposition = composition + const { runtime } = activeComposition + startupPromise = (async () => { + const { signal } = startupAbortController + signal.throwIfAborted() + await initDaemonPtyProvider(signal, { macosLoginSessionWatch: false }) + signal.throwIfAborted() + await activeComposition.startAfterDaemon() + signal.throwIfAborted() + runtime.syncWindowGraph(HEADLESS_RUNTIME_WINDOW_ID, createServerWindowGraph(activeStore)) + const pairingAddress = await discoverServerPairingAddress(args.pairingAddress) + signal.throwIfAborted() + validatePairingAddress(pairingAddress.address) + rpc = new OrcaRuntimeRpcServer({ + runtime, + userDataPath: dataPath, + enableWebSocket: true, + preferPinnedWsPort: args.port !== 0, + wsBindHost: resolveServerBindHost(args.listenHost, pairingAddress), + wsPort: args.port, + webClientRoot: resolveWebClientRoot(appPath) + }) + await rpc.start() + signal.throwIfAborted() + requireServerWebSocketEndpoint(rpc.getWebSocketEndpoint()) + await publishReadiness(runtime, rpc, args, pairingAddress.address, signal) + signal.throwIfAborted() + if (!args.json && pairingAddress.source === 'loopback' && process.env.SSH_CONNECTION) { + const resolvedPort = new URL(rpc.getWebSocketEndpoint()!).port + process.stdout.write( + `SSH tunnel (run locally with the same SSH target): ssh -L ${resolvedPort}:127.0.0.1:${resolvedPort} ${process.env.USER ?? 'user'}@\n` + ) + } + })() + await startupPromise } catch (error) { - await shutdown(1) - throw error + if (startupAbortController.signal.aborted) { + await shutdown(0) + } else { + await shutdown(1) + throw error + } } } @@ -149,8 +170,10 @@ async function publishReadiness( runtime: OrcaRuntimeService, rpc: OrcaRuntimeRpcServer, args: ReturnType, - pairingAddress: string + pairingAddress: string, + signal: AbortSignal ): Promise { + signal.throwIfAborted() const boundEndpoint = requireServerWebSocketEndpoint(rpc.getWebSocketEndpoint()) const advertised = resolveAdvertisedPairingEndpoint(boundEndpoint, pairingAddress) const pairing = args.noPairing @@ -160,6 +183,7 @@ async function publishReadiness( guidance: 'Restart without --no-pairing to create a client pairing offer.' } : rpc.createPairingOffer({ address: pairingAddress, name: 'Orca npm server', scope: 'runtime' }) + signal.throwIfAborted() await new ServeReadinessPublisher().publish( { runtimeId: runtime.getRuntimeId(), @@ -178,7 +202,8 @@ async function publishReadiness( } : pairing }, - { mode: args.json ? 'json' : 'human' } + { mode: args.json ? 'json' : 'human' }, + signal ) } diff --git a/src/node-server/package-cli-deferral.test.ts b/src/node-server/package-cli-deferral.test.ts new file mode 100644 index 00000000000..41311b1ec2e --- /dev/null +++ b/src/node-server/package-cli-deferral.test.ts @@ -0,0 +1,37 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const state = vi.hoisted(() => ({ + configureProfile: vi.fn(), + controlMain: vi.fn(), + serverEntryLoaded: false +})) + +vi.mock('../cli/cli-program', () => ({ main: state.controlMain })) +vi.mock('./server-profile-environment', () => ({ + configureServerControlProfileEnvironment: state.configureProfile +})) +vi.mock('./index', () => { + state.serverEntryLoaded = true + return { reportNodeServerFailure: vi.fn(), runNodeServer: vi.fn() } +}) + +describe('package CLI module deferral', () => { + beforeEach(() => { + state.controlMain.mockReset() + state.configureProfile.mockReset() + state.serverEntryLoaded = false + }) + + it('does not evaluate the server entry for control CLI help', async () => { + const originalArgv = process.argv + process.argv = [process.execPath, 'cli.js', '--help'] + try { + await import('./package-cli') + await vi.waitFor(() => expect(state.controlMain).toHaveBeenCalledWith(['--help'])) + expect(state.configureProfile).toHaveBeenCalledOnce() + expect(state.serverEntryLoaded).toBe(false) + } finally { + process.argv = originalArgv + } + }) +}) diff --git a/src/node-server/package-cli.ts b/src/node-server/package-cli.ts index 38bb8368560..2014e43045d 100644 --- a/src/node-server/package-cli.ts +++ b/src/node-server/package-cli.ts @@ -1,7 +1,5 @@ #!/usr/bin/env node -import { main as runControlCli } from '../cli/cli-program' -import { reportNodeServerFailure, runNodeServer } from './index' -import { resolveServerDataPath } from './server-paths' +export {} const argv = process.argv.slice(2) const serverCommand = @@ -12,8 +10,14 @@ const serverCommand = argv[0] === '-v' if (serverCommand) { - void runNodeServer(argv).catch(reportNodeServerFailure) + void import('./index').then(({ reportNodeServerFailure, runNodeServer }) => + runNodeServer(argv).catch(reportNodeServerFailure) + ) } else { - process.env.ORCA_USER_DATA_PATH ??= resolveServerDataPath() - void runControlCli(argv) + void Promise.all([import('../cli/cli-program'), import('./server-profile-environment')]).then( + ([{ main: runControlCli }, { configureServerControlProfileEnvironment }]) => { + configureServerControlProfileEnvironment() + return runControlCli(argv) + } + ) } diff --git a/src/node-server/server-bind-host.test.ts b/src/node-server/server-bind-host.test.ts new file mode 100644 index 00000000000..582b3a64a26 --- /dev/null +++ b/src/node-server/server-bind-host.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest' +import { resolveServerBindHost } from './server-bind-host' + +describe('server bind host', () => { + it('binds explicit IPv6 advertisements on an IPv6 listener', () => { + expect(resolveServerBindHost(undefined, { address: '::1', source: 'explicit' })).toBe('::') + expect( + resolveServerBindHost(undefined, { + address: 'ws://[2001:db8::4]:6768', + source: 'explicit' + }) + ).toBe('::') + }) + + it('preserves explicit listeners and existing IPv4 defaults', () => { + expect(resolveServerBindHost('127.0.0.2', { address: '::1', source: 'explicit' })).toBe( + '127.0.0.2' + ) + expect(resolveServerBindHost(undefined, { address: 'host.example', source: 'explicit' })).toBe( + '0.0.0.0' + ) + expect(resolveServerBindHost(undefined, { address: '100.64.1.2', source: 'tailscale' })).toBe( + '100.64.1.2' + ) + expect(resolveServerBindHost(undefined, { address: '127.0.0.1', source: 'loopback' })).toBe( + '127.0.0.1' + ) + }) +}) diff --git a/src/node-server/server-bind-host.ts b/src/node-server/server-bind-host.ts new file mode 100644 index 00000000000..899485d23dd --- /dev/null +++ b/src/node-server/server-bind-host.ts @@ -0,0 +1,24 @@ +import { isIPv6 } from 'node:net' +import { resolveAdvertisedPairingHostname } from '../main/runtime/pairing-endpoint' + +type PairingAddress = { + address: string + source: 'explicit' | 'tailscale' | 'loopback' +} + +export function resolveServerBindHost( + listenHost: string | undefined, + pairingAddress: PairingAddress +): string { + if (listenHost) { + return listenHost + } + if (pairingAddress.source === 'tailscale') { + return pairingAddress.address + } + if (pairingAddress.source === 'loopback') { + return '127.0.0.1' + } + const advertisedHostname = resolveAdvertisedPairingHostname(pairingAddress.address) + return advertisedHostname && isIPv6(advertisedHostname) ? '::' : '0.0.0.0' +} diff --git a/src/node-server/server-profile-environment.test.ts b/src/node-server/server-profile-environment.test.ts new file mode 100644 index 00000000000..3d6da9026d1 --- /dev/null +++ b/src/node-server/server-profile-environment.test.ts @@ -0,0 +1,54 @@ +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { resolveOrcaManagedCodexHomePath } from '../main/codex/codex-home-paths' +import { + configureServerControlProfileEnvironment, + configureServerProfileEnvironment +} from './server-profile-environment' + +const originalEnvironment = { + serverDataPath: process.env.ORCA_SERVER_DATA_DIR, + userDataPath: process.env.ORCA_USER_DATA_PATH +} + +afterEach(() => { + if (originalEnvironment.userDataPath === undefined) { + delete process.env.ORCA_USER_DATA_PATH + } else { + process.env.ORCA_USER_DATA_PATH = originalEnvironment.userDataPath + } + if (originalEnvironment.serverDataPath === undefined) { + delete process.env.ORCA_SERVER_DATA_DIR + } else { + process.env.ORCA_SERVER_DATA_DIR = originalEnvironment.serverDataPath + } +}) + +describe('server profile environment', () => { + it('roots managed Codex state in the selected server profile', () => { + const dataPath = join('custom', 'server-profile') + + configureServerProfileEnvironment(dataPath) + + expect(resolveOrcaManagedCodexHomePath()).toBe(join(dataPath, 'codex-runtime-home', 'home')) + }) + + it('gives an explicit server profile precedence over an inherited desktop profile', () => { + process.env.ORCA_USER_DATA_PATH = join('desktop', 'profile') + process.env.ORCA_SERVER_DATA_DIR = join('server', 'profile') + + configureServerControlProfileEnvironment() + + expect(process.env.ORCA_USER_DATA_PATH).toBe(join(process.cwd(), 'server', 'profile')) + }) + + it('preserves an inherited profile when no server override is explicit', () => { + const inherited = join('desktop', 'profile') + process.env.ORCA_USER_DATA_PATH = inherited + delete process.env.ORCA_SERVER_DATA_DIR + + configureServerControlProfileEnvironment() + + expect(process.env.ORCA_USER_DATA_PATH).toBe(inherited) + }) +}) diff --git a/src/node-server/server-profile-environment.ts b/src/node-server/server-profile-environment.ts new file mode 100644 index 00000000000..7915d7d7ee0 --- /dev/null +++ b/src/node-server/server-profile-environment.ts @@ -0,0 +1,11 @@ +import { resolveServerDataPath } from './server-paths' + +export function configureServerProfileEnvironment(dataPath: string): void { + process.env.ORCA_USER_DATA_PATH = dataPath +} + +export function configureServerControlProfileEnvironment(): void { + if (process.env.ORCA_SERVER_DATA_DIR || !process.env.ORCA_USER_DATA_PATH) { + process.env.ORCA_USER_DATA_PATH = resolveServerDataPath() + } +} diff --git a/src/node-server/server-profile-process-lock.test.ts b/src/node-server/server-profile-process-lock.test.ts index 2b0230f1078..2c8703341e1 100644 --- a/src/node-server/server-profile-process-lock.test.ts +++ b/src/node-server/server-profile-process-lock.test.ts @@ -14,6 +14,7 @@ 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 { isManagedHookLockOwnerActive } from '../main/agent-hooks/managed-hook-lock-records' import { readManagedHookHostIdentity, readManagedHookProcessIdentity @@ -300,12 +301,17 @@ describe('server profile process lock', () => { it('retries release after a transient removal failure', async () => { const root = await createRoot() const lock = await acquireServerProfileProcessLock(root) + const owner = JSON.parse(await readFile(paths(root).lock, 'utf8')) as { token: string } + const removeManagedHookLock = managedHookLockClaims.removeManagedHookLock const removeLock = vi.spyOn(managedHookLockClaims, 'removeManagedHookLock') - removeLock.mockResolvedValueOnce('unverifiable') + removeLock.mockResolvedValueOnce('unverifiable').mockImplementationOnce(async (...args) => { + expect(isManagedHookLockOwnerActive(owner.token)).toBe(true) + return removeManagedHookLock(...args) + }) - await expect(lock.release()).rejects.toMatchObject({ reason: 'release_failed' }) await expect(lock.release()).resolves.toBeUndefined() expect(removeLock).toHaveBeenCalledTimes(2) + expect(isManagedHookLockOwnerActive(owner.token)).toBe(false) }) it('surfaces a typed error when the current process cannot be identified', async () => { diff --git a/src/node-server/server-profile-process-lock.ts b/src/node-server/server-profile-process-lock.ts index 581c60497c9..df8afc9c67c 100644 --- a/src/node-server/server-profile-process-lock.ts +++ b/src/node-server/server-profile-process-lock.ts @@ -14,6 +14,7 @@ import { } from '../main/agent-hooks/managed-hook-owner-identity' const MAX_RECOVERY_ATTEMPTS = 32 +const RELEASE_RETRY_DELAYS_MS = [0, 25, 100] as const export type ServerProfileProcessLock = { ownerPid: number @@ -166,25 +167,37 @@ function createOwnedLock( if (released) { return } + let lastFailure: unknown try { - const removal = await removeManagedHookLock( - lockPath, - lockParent, - owner, - hostIdentity, - processIdentity - ) - if (removal !== 'removed') { - throw new ServerProfileProcessLockError( - 'release_failed', - `Could not release the Orca server profile lock: ${removal}`, - owner.pid - ) + for (const delayMs of RELEASE_RETRY_DELAYS_MS) { + if (delayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, delayMs)) + } + try { + const removal = await removeManagedHookLock( + lockPath, + lockParent, + owner, + hostIdentity, + processIdentity + ) + if (removal === 'removed') { + released = true + return + } + lastFailure = new ServerProfileProcessLockError( + 'release_failed', + `Could not release the Orca server profile lock: ${removal}`, + owner.pid + ) + } catch (error) { + lastFailure = error + } } - released = true } finally { deactivateManagedHookLockOwner(owner.token) } + throw lastFailure } } } diff --git a/src/node-server/server-shutdown-coordinator.test.ts b/src/node-server/server-shutdown-coordinator.test.ts new file mode 100644 index 00000000000..620ce948f09 --- /dev/null +++ b/src/node-server/server-shutdown-coordinator.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from 'vitest' +import { createServerShutdownCoordinator } from './server-shutdown-coordinator' + +describe('server shutdown coordinator', () => { + it('joins concurrent shutdown requests and retains a later failure code', async () => { + let finish!: () => void + const barrier = new Promise((resolve) => { + finish = resolve + }) + const observedExitCodes: number[] = [] + const run = vi.fn(async (getRequestedExitCode: () => number) => { + await barrier + observedExitCodes.push(getRequestedExitCode()) + }) + const shutdown = createServerShutdownCoordinator(run) + + const first = shutdown(0) + const second = shutdown(1) + + expect(second).toBe(first) + expect(run).toHaveBeenCalledTimes(1) + finish() + await first + expect(observedExitCodes).toEqual([1]) + }) +}) diff --git a/src/node-server/server-shutdown-coordinator.ts b/src/node-server/server-shutdown-coordinator.ts new file mode 100644 index 00000000000..67dc4d01b22 --- /dev/null +++ b/src/node-server/server-shutdown-coordinator.ts @@ -0,0 +1,11 @@ +export function createServerShutdownCoordinator( + run: (getRequestedExitCode: () => number) => Promise +): (exitCode: number) => Promise { + let requestedExitCode = 0 + let shutdownPromise: Promise | null = null + return (exitCode) => { + requestedExitCode = Math.max(requestedExitCode, exitCode) + shutdownPromise ??= run(() => requestedExitCode) + return shutdownPromise + } +} diff --git a/src/node-server/server-signal-shutdown.test.ts b/src/node-server/server-signal-shutdown.test.ts new file mode 100644 index 00000000000..c54d61cec7c --- /dev/null +++ b/src/node-server/server-signal-shutdown.test.ts @@ -0,0 +1,33 @@ +import { EventEmitter } from 'node:events' +import { describe, expect, it, vi } from 'vitest' +import { createServerShutdownCoordinator } from './server-shutdown-coordinator' +import { installServerSignalShutdown } from './server-signal-shutdown' + +describe('server signal shutdown', () => { + it('retains signal handlers until repeated signals join teardown', async () => { + const emitter = new EventEmitter() + let finish!: () => void + const barrier = new Promise((resolve) => { + finish = resolve + }) + const run = vi.fn(async () => barrier) + const shutdown = createServerShutdownCoordinator(async () => run()) + const exit = vi.fn() + const onSignal = vi.fn() + installServerSignalShutdown({ emitter, shutdown, exit, getExitCode: () => 0, onSignal }) + + emitter.emit('SIGTERM') + emitter.emit('SIGTERM') + expect(onSignal).toHaveBeenCalledTimes(2) + expect(run).toHaveBeenCalledTimes(1) + expect(exit).not.toHaveBeenCalled() + expect(emitter.listenerCount('SIGTERM')).toBe(1) + + finish() + await barrier + await new Promise((resolve) => setImmediate(resolve)) + expect(exit).toHaveBeenCalledOnce() + expect(emitter.listenerCount('SIGINT')).toBe(0) + expect(emitter.listenerCount('SIGTERM')).toBe(0) + }) +}) diff --git a/src/node-server/server-signal-shutdown.ts b/src/node-server/server-signal-shutdown.ts new file mode 100644 index 00000000000..42288e99f34 --- /dev/null +++ b/src/node-server/server-signal-shutdown.ts @@ -0,0 +1,28 @@ +type SignalEmitter = { + on(signal: NodeJS.Signals, listener: () => void): unknown + off(signal: NodeJS.Signals, listener: () => void): unknown +} + +export function installServerSignalShutdown(args: { + emitter: SignalEmitter + shutdown: (exitCode: number) => Promise + exit: (exitCode: number) => void + getExitCode: () => number + onSignal?: () => void +}): void { + let exiting = false + const stopForSignal = (): void => { + args.onSignal?.() + void args.shutdown(0).then(() => { + if (exiting) { + return + } + exiting = true + args.emitter.off('SIGINT', stopForSignal) + args.emitter.off('SIGTERM', stopForSignal) + args.exit(args.getExitCode()) + }) + } + args.emitter.on('SIGINT', stopForSignal) + args.emitter.on('SIGTERM', stopForSignal) +}