test(wire): make release checkouts race safe

.github/workflows/pr.yml

@@ -539,6 +539,7 @@ jobs:
       - name: Old/new client and server compatibility journeys
         run: >-
           pnpm exec vitest run --config config/vitest.config.ts
+          tests/e2e/cross-version-wire/release-checkout.unit.test.ts
           tests/e2e/cross-version-wire/cross-version-browser-placement.unit.test.ts
           tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts
           tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts

tests/e2e/cross-version-wire/cross-version-browser-placement.unit.test.ts

@@ -4,7 +4,11 @@ import {
   BROWSER_NETWORK_TUNNEL_RUNTIME_CAPABILITY
 } from '../../../src/shared/protocol-version'
 import { BrowserTabCreateParams } from '../../../src/main/runtime/rpc/methods/browser-tab-create-schema'
-import { materializeReleaseCheckout } from './release-checkout'
+import {
+  importReleaseCheckoutModule,
+  materializeReleaseCheckout,
+  type ReleaseCheckout
+} from './release-checkout'
 
 type Schema = { parse: (value: unknown) => Record<string, unknown> }
 
@@ -19,12 +23,13 @@ const BASELINE_TAB_CREATE_SOURCES = [
   ['browser-schemas.ts', 'TabCreate']
 ] as const
 
-async function importBaselineTabCreate(root: string): Promise<Schema> {
+async function importBaselineTabCreate(checkout: ReleaseCheckout): Promise<Schema> {
   const attempted: string[] = []
   for (const [file, exportName] of BASELINE_TAB_CREATE_SOURCES) {
     attempted.push(`${file}#${exportName}`)
-    const loaded = await import(
-      /* @vite-ignore */ `${root}/src/main/runtime/rpc/methods/${file}`
+    const loaded = await importReleaseCheckoutModule(
+      checkout,
+      `/src/main/runtime/rpc/methods/${file}`
     ).catch(() => null)
     const schema = loaded?.[exportName] as Schema | undefined
     if (schema?.parse) {
@@ -32,7 +37,7 @@ async function importBaselineTabCreate(root: string): Promise<Schema> {
     }
   }
   throw new Error(
-    `Baseline release at ${root} exposes no tab-create schema (tried ${attempted.join(', ')}).`
+    `Baseline release at ${checkout.root} exposes no tab-create schema (tried ${attempted.join(', ')}).`
   )
 }
 
@@ -52,11 +57,11 @@ let baselineProtocol: Record<string, unknown>
 
 beforeAll(async () => {
   baselineRef = LEGACY_BROWSER_PLACEMENT_RELEASE_REF
-  const checkout = materializeReleaseCheckout(baselineRef)
+  const checkout = await materializeReleaseCheckout(baselineRef)
   baselineRevision = checkout.commit
   const [tabCreate, protocol] = await Promise.all([
-    importBaselineTabCreate(checkout.root),
-    import(/* @vite-ignore */ `${checkout.root}/src/shared/protocol-version.ts`)
+    importBaselineTabCreate(checkout),
+    importReleaseCheckoutModule(checkout, '/src/shared/protocol-version.ts')
   ])
   baselineTabCreate = tabCreate
   baselineProtocol = protocol

tests/e2e/cross-version-wire/release-checkout.ts

@@ -1,6 +1,7 @@
 import { execFileSync } from 'node:child_process'
 import {
   existsSync,
+  mkdtempSync,
   mkdirSync,
   readFileSync,
   readdirSync,
@@ -8,13 +9,14 @@ import {
   rmSync,
   writeFileSync
 } from 'node:fs'
-import { dirname, join, relative, resolve } from 'node:path'
+import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'
+import { lock } from 'proper-lockfile'
 
 export const REPO_ROOT = resolve(import.meta.dirname, '..', '..', '..')
-const CACHE_ROOT = join(REPO_ROOT, 'tests', 'e2e', '.cross-version-checkouts')
+const DEFAULT_CACHE_ROOT = join(REPO_ROOT, 'tests', 'e2e', '.cross-version-checkouts')
 
 // Bump when extraction or the alias rewrite changes so cached trees are rebuilt.
-const CHECKOUT_FORMAT = 1
+const CHECKOUT_FORMAT = 2
 
 // Why: the wire endpoints only need the runtime RPC host, the renderer client, and
 // the shared codec. Skipping cli/relay keeps a cold CI extraction a few seconds.
@@ -34,6 +36,10 @@ export type ReleaseCheckout = {
   root: string
 }
 
+export type MaterializeReleaseCheckoutOptions = {
+  cacheRoot?: string
+}
+
 function git(args: string[]): string {
   return execFileSync('git', args, {
     cwd: REPO_ROOT,
@@ -191,24 +197,75 @@ function readStamp(root: string): CheckoutStamp | null {
   }
 }
 
+function checkoutMatches(root: string, commit: string): boolean {
+  const stamp = readStamp(root)
+  return stamp?.commit === commit && stamp.format === CHECKOUT_FORMAT
+}
+
+function checkoutModulePath(checkout: ReleaseCheckout, rootRelativePath: string): string {
+  const fromRoot = rootRelativePath.replace(/^[/\\]+/, '')
+  const absolute = resolve(checkout.root, fromRoot)
+  const fromCheckout = relative(checkout.root, absolute)
+  if (
+    !fromRoot ||
+    fromCheckout === '..' ||
+    fromCheckout.startsWith(`..${sep}`) ||
+    isAbsolute(fromCheckout)
+  ) {
+    throw new Error(
+      `Cross-version module path must stay inside the release checkout: ${rootRelativePath}`
+    )
+  }
+  return absolute.split('\\').join('/')
+}
+
+/** Import a source module with `/src/...` anchored to the extracted release root. */
+export function importReleaseCheckoutModule(
+  checkout: ReleaseCheckout,
+  rootRelativePath: string
+): Promise<Record<string, unknown>> {
+  const specifier = checkoutModulePath(checkout, rootRelativePath)
+  return import(/* @vite-ignore */ specifier) as Promise<Record<string, unknown>>
+}
+
 /**
  * Extract `src/` at `ref` into a cached, gitignored checkout the test can import.
  * Cached by resolved commit, so a moved tag or a bumped rewrite format re-extracts.
  */
-export function materializeReleaseCheckout(ref: string): ReleaseCheckout {
+export async function materializeReleaseCheckout(
+  ref: string,
+  options: MaterializeReleaseCheckoutOptions = {}
+): Promise<ReleaseCheckout> {
   const commit = resolveCommit(ref)
   const label = ref.replace(/[^A-Za-z0-9._-]/g, '_')
-  const root = join(CACHE_ROOT, label)
-  const stamp = readStamp(root)
-  if (stamp?.commit === commit && stamp.format === CHECKOUT_FORMAT) {
+  const cacheRoot = options.cacheRoot ?? DEFAULT_CACHE_ROOT
+  const root = join(cacheRoot, label, `${commit}-format-${CHECKOUT_FORMAT}`)
+  if (checkoutMatches(root, commit)) {
     return { ref, commit, label, root }
   }
 
-  mkdirSync(CACHE_ROOT, { recursive: true })
-  const staging = join(CACHE_ROOT, `.staging-${label}-${process.pid}`)
-  rmSync(staging, { recursive: true, force: true })
-  mkdirSync(staging, { recursive: true })
+  mkdirSync(dirname(root), { recursive: true })
+  let releaseLock: (() => Promise<void>) | undefined
   try {
+    releaseLock = await lock(root, {
+      realpath: false,
+      stale: 60_000,
+      update: 10_000,
+      retries: { retries: 480, factor: 1, minTimeout: 250, maxTimeout: 250, randomize: true }
+    })
+  } catch (error) {
+    if (checkoutMatches(root, commit)) {
+      return { ref, commit, label, root }
+    }
+    throw new Error(`Cross-version harness could not lock ${ref} (${commit}): ${String(error)}`)
+  }
+
+  let staging: string | undefined
+  try {
+    if (checkoutMatches(root, commit)) {
+      return { ref, commit, label, root }
+    }
+    staging = mkdtempSync(join(dirname(root), `.staging-${commit.slice(0, 12)}-`))
     // `git archive | tar -x` keeps the extraction independent of the working tree,
     // so an injected violation in the working tree cannot leak into the old side.
     execFileSync(
@@ -223,12 +280,17 @@ export function materializeReleaseCheckout(ref: string): ReleaseCheckout {
     )
     rmSync(root, { recursive: true, force: true })
     renameSync(staging, root)
+    staging = undefined
   } catch (error) {
-    rmSync(staging, { recursive: true, force: true })
-    if (readStamp(root)?.commit === commit) {
+    if (checkoutMatches(root, commit)) {
       return { ref, commit, label, root }
     }
     throw new Error(`Cross-version harness failed to extract ${ref} (${commit}): ${String(error)}`)
+  } finally {
+    if (staging) {
+      rmSync(staging, { recursive: true, force: true })
+    }
+    await releaseLock()
   }
 
   if (!existsSync(join(root, 'src', 'shared', 'terminal-stream-protocol.ts'))) {

tests/e2e/cross-version-wire/release-checkout.unit.test.ts

@@ -0,0 +1,76 @@
+import { execFileSync } from 'node:child_process'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join, relative } from 'node:path'
+import { afterEach, describe, expect, it } from 'vitest'
+import {
+  importReleaseCheckoutModule,
+  materializeReleaseCheckout,
+  REPO_ROOT
+} from './release-checkout'
+const temporaryRoots: string[] = []
+
+function temporaryCacheRoot(): string {
+  const root = mkdtempSync(join(tmpdir(), 'orca-cross-version-checkout-'))
+  temporaryRoots.push(root)
+  return root
+}
+
+function git(args: string[]): string {
+  return execFileSync('git', args, { cwd: REPO_ROOT, encoding: 'utf8' }).trim()
+}
+
+afterEach(() => {
+  for (const root of temporaryRoots.splice(0)) {
+    rmSync(root, { recursive: true, force: true })
+  }
+})
+
+describe('release checkout materialization', () => {
+  it('single-flights concurrent consumers of one release identity', async () => {
+    const cacheRoot = temporaryCacheRoot()
+    const checkouts = await Promise.all([
+      materializeReleaseCheckout('v1.4.190', { cacheRoot }),
+      materializeReleaseCheckout('v1.4.190', { cacheRoot }),
+      materializeReleaseCheckout('v1.4.190', { cacheRoot })
+    ])
+
+    expect(new Set(checkouts.map(({ root }) => root))).toHaveLength(1)
+    expect(relative(cacheRoot, checkouts[0]!.root)).not.toMatch(/^\.\./)
+  })
+
+  it('loads a baseline module whose source imports another checkout-root file', async () => {
+    const cacheRoot = temporaryCacheRoot()
+    const checkout = await materializeReleaseCheckout('v1.4.190', { cacheRoot })
+    const protocol = await importReleaseCheckoutModule(checkout, '/src/shared/protocol-version.ts')
+
+    expect(protocol.REMOTE_SERVER_UPDATE_CAPABILITY).toBe('updater.remote-control.v1')
+    expect(relative(cacheRoot, checkout.root)).not.toMatch(/^\.\./)
+  })
+
+  it('keeps an import live while another colliding release label materializes', async () => {
+    const merge = git(['rev-list', '--merges', '-1', 'HEAD'])
+    const firstRef = `${merge}~2`
+    const secondRef = `${merge}^2`
+    expect(git(['rev-parse', `${firstRef}^{commit}`])).not.toBe(
+      git(['rev-parse', `${secondRef}^{commit}`])
+    )
+
+    const cacheRoot = temporaryCacheRoot()
+    const first = await materializeReleaseCheckout(firstRef, { cacheRoot })
+    const dependency = join(first.root, 'delayed-dependency.mjs')
+    const entry = join(first.root, 'delayed-entry.mjs')
+    writeFileSync(dependency, "export const loaded = 'first-release'\n")
+    writeFileSync(
+      entry,
+      'await new Promise((resolve) => setTimeout(resolve, 100))\n' +
+        "export const loaded = (await import('./delayed-dependency.mjs')).loaded\n"
+    )
+
+    const loading = importReleaseCheckoutModule(first, '/delayed-entry.mjs')
+    const second = await materializeReleaseCheckout(secondRef, { cacheRoot })
+
+    await expect(loading).resolves.toMatchObject({ loaded: 'first-release' })
+    expect(first.root).not.toBe(second.root)
+  })
+})

tests/e2e/cross-version-wire/versioned-agent-session-wire.ts

@@ -1,7 +1,11 @@
 import { readFileSync, readdirSync } from 'node:fs'
 import { join } from 'node:path'
-import { pathToFileURL } from 'node:url'
-import { materializeReleaseCheckout, REPO_ROOT, type ReleaseCheckout } from './release-checkout'
+import {
+  importReleaseCheckoutModule,
+  materializeReleaseCheckout,
+  REPO_ROOT,
+  type ReleaseCheckout
+} from './release-checkout'
 
 /**
  * The two things that decide whether a structured agent session exists for a given
@@ -118,20 +122,11 @@ async function loadWorkingTreeBuild(): Promise<AgentSessionWireBuild> {
   }
 }
 
-// Why @vite-ignore: the checkout is created at run time, so Vite cannot glob it at
-// transform time. Vite-node still resolves and transforms the target on demand.
-function importFromCheckout(specifier: string): Promise<Record<string, unknown>> {
-  return import(/* @vite-ignore */ pathToFileURL(specifier).href) as Promise<
-    Record<string, unknown>
-  >
-}
-
 async function loadReleaseBuild(checkout: ReleaseCheckout): Promise<AgentSessionWireBuild> {
-  const base = `${checkout.root}/src`
   const [protocol, dispatcher, terminalMethods] = await Promise.all([
-    importFromCheckout(`${base}/shared/protocol-version.ts`),
-    importFromCheckout(`${base}/main/runtime/rpc/dispatcher.ts`),
-    importFromCheckout(`${base}/main/runtime/rpc/methods/terminal.ts`)
+    importReleaseCheckoutModule(checkout, '/src/shared/protocol-version.ts'),
+    importReleaseCheckoutModule(checkout, '/src/main/runtime/rpc/dispatcher.ts'),
+    importReleaseCheckoutModule(checkout, '/src/main/runtime/rpc/methods/terminal.ts')
   ])
   const module = dispatcher as unknown as DispatcherModule
   return {
@@ -156,5 +151,5 @@ export async function loadAgentSessionWireBuild(ref: string): Promise<AgentSessi
   if (ref === WORKING_TREE) {
     return loadWorkingTreeBuild()
   }
-  return loadReleaseBuild(materializeReleaseCheckout(ref))
+  return loadReleaseBuild(await materializeReleaseCheckout(ref))
 }

tests/e2e/cross-version-wire/versioned-terminal-wire.ts

@@ -1,4 +1,8 @@
-import { materializeReleaseCheckout, type ReleaseCheckout } from './release-checkout'
+import {
+  importReleaseCheckoutModule,
+  materializeReleaseCheckout,
+  type ReleaseCheckout
+} from './release-checkout'
 
 /**
  * Structural views of the three modules that make up the remote terminal wire.
@@ -112,19 +116,15 @@ async function loadWorkingTreeBuild(): Promise<TerminalWireBuild> {
   }
 }
 
-// Why @vite-ignore: the checkout is created at run time, so Vite cannot glob it at
-// transform time. Vite-node still resolves and transforms the target on demand.
-function importFromCheckout(specifier: string): Promise<Record<string, unknown>> {
-  return import(/* @vite-ignore */ specifier) as Promise<Record<string, unknown>>
-}
-
 async function loadReleaseBuild(checkout: ReleaseCheckout): Promise<TerminalWireBuild> {
-  const base = `${checkout.root}/src`
   const [codec, dispatcher, terminalMethods, client] = await Promise.all([
-    importFromCheckout(`${base}/shared/terminal-stream-protocol.ts`),
-    importFromCheckout(`${base}/main/runtime/rpc/dispatcher.ts`),
-    importFromCheckout(`${base}/main/runtime/rpc/methods/terminal.ts`),
-    importFromCheckout(`${base}/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts`)
+    importReleaseCheckoutModule(checkout, '/src/shared/terminal-stream-protocol.ts'),
+    importReleaseCheckoutModule(checkout, '/src/main/runtime/rpc/dispatcher.ts'),
+    importReleaseCheckoutModule(checkout, '/src/main/runtime/rpc/methods/terminal.ts'),
+    importReleaseCheckoutModule(
+      checkout,
+      '/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts'
+    )
   ])
   return {
     label: checkout.ref,
@@ -147,5 +147,5 @@ export async function loadTerminalWireBuild(ref: string): Promise<TerminalWireBu
   if (ref === WORKING_TREE) {
     return loadWorkingTreeBuild()
   }
-  return loadReleaseBuild(materializeReleaseCheckout(ref))
+  return loadReleaseBuild(await materializeReleaseCheckout(ref))
 }