diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 845d0936780..f0234f50286 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1435,6 +1435,9 @@ jobs: run: node config/scripts/build-windows-cli-launcher.mjs - name: Test Windows-specific boundaries + env: + ORCA_BACKGROUND_LAUNCH: '1' + ORCA_REAL_WINDOWS_SKILL_TEST: '1' run: >- pnpm exec vitest run --config config/vitest.config.ts src/main/zcode/zcode-plan-api-key-store-native-windows.test.ts @@ -1511,6 +1514,7 @@ jobs: src/main/ipc/preflight-provider-command-selection.test.ts src/main/ipc/preflight-runnable-local-cli.test.ts src/main/managed-server/install/remote-node-runtime-store-windows.test.ts + src/main/skills/skill-windows-rename-contention.integration.test.ts # Why the :parallel variant: identical to build:release except the three # electron-vite targets overlap instead of running back to back. The Linux package diff --git a/config/scripts/pr-package-test-paths.mjs b/config/scripts/pr-package-test-paths.mjs index fb5e11da5da..9b379e906c6 100644 --- a/config/scripts/pr-package-test-paths.mjs +++ b/config/scripts/pr-package-test-paths.mjs @@ -79,5 +79,6 @@ export const WINDOWS_PACKAGE_TESTS = [ 'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts', 'src/main/ipc/preflight-provider-command-selection.test.ts', 'src/main/ipc/preflight-runnable-local-cli.test.ts', - 'src/main/managed-server/install/remote-node-runtime-store-windows.test.ts' + 'src/main/managed-server/install/remote-node-runtime-store-windows.test.ts', + 'src/main/skills/skill-windows-rename-contention.integration.test.ts' ] diff --git a/config/scripts/win32-test-lane-registration.test.mjs b/config/scripts/win32-test-lane-registration.test.mjs index 4cc207f1edf..f8d0235e6f7 100644 --- a/config/scripts/win32-test-lane-registration.test.mjs +++ b/config/scripts/win32-test-lane-registration.test.mjs @@ -109,9 +109,7 @@ const MANUAL_OPT_IN = [ 'src/main/git/runner-wsl-linked-gitdir-windows.test.ts', // `runRealWsl = … && ORCA_REAL_WSL_BANNER_TEST === '1'`; needs a real distro. 'src/main/local-worktree-filesystem-wsl-banner.wsl.test.ts', - // `RUN_REAL_WINDOWS = platform === 'win32' && ORCA_REAL_WINDOWS_SKILL_TEST === '1'`. - 'src/main/skills/skill-windows-rename-contention.integration.test.ts', - // Same flag; installs into a real Windows workspace. + // `ORCA_REAL_WINDOWS_SKILL_TEST === '1'`; installs into a real Windows workspace. 'src/main/skills/skill-windows-workspace.integration.test.ts', // `RUN_REAL_WSL = … && ORCA_REAL_WSL_SKILL_TEST === '1'`; real distro filesystem. 'src/main/skills/skill-wsl-delete.integration.test.ts', @@ -129,7 +127,7 @@ const MANUAL_OPT_IN = [ /** Caps so growing either list is two deliberate edits, not one. */ const UNREGISTERED_MAX = 8 -const MANUAL_OPT_IN_MAX = 10 +const MANUAL_OPT_IN_MAX = 9 /** * Floor for the Windows-gated population, so a broken walk or a regex that @@ -350,6 +348,18 @@ function sourceOf(path) { } describe('Windows-gated test files are registered in the Windows CI lane', () => { + it('enables real Windows rename contention tests in the registered step', () => { + const workflow = parse(readFileSync(join(projectDir, '.github/workflows/pr.yml'), 'utf8')) + const step = workflow.jobs.package_windows.steps.find( + (candidate) => candidate.name === 'Test Windows-specific boundaries' + ) + expect(step.env.ORCA_REAL_WINDOWS_SKILL_TEST).toBe('1') + expect(step.env.ORCA_BACKGROUND_LAUNCH).toBe('1') + expect(step.run).toContain( + 'src/main/skills/skill-windows-rename-contention.integration.test.ts' + ) + }) + it('scans a plausible number of test files', () => { // A broken root or extension filter would make every assertion below vacuous. expect(scannedTestFiles.length).toBeGreaterThan(5000) diff --git a/src/main/skills/skill-windows-rename-contention.integration.test.ts b/src/main/skills/skill-windows-rename-contention.integration.test.ts index 01101a420ce..da6211c1c55 100644 --- a/src/main/skills/skill-windows-rename-contention.integration.test.ts +++ b/src/main/skills/skill-windows-rename-contention.integration.test.ts @@ -1,37 +1,55 @@ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' -import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises' +import type { ChildProcessWithoutNullStreams } from 'node:child_process' +import type * as FsPromises from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rename, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, describe, expect, it } from 'vitest' +import { spawnProcess } from '@orca/process-host' +import { afterEach, describe, expect, it, vi } from 'vitest' import { renameSkillPathWithWindowsRetry } from './skill-filesystem-retry' const RUN_REAL_WINDOWS = process.platform === 'win32' && process.env.ORCA_REAL_WINDOWS_SKILL_TEST === '1' const roots: string[] = [] -const lockers: ChildProcessWithoutNullStreams[] = [] +const lockers: { + child: ChildProcessWithoutNullStreams + closed: Promise +}[] = [] +let onRenameError: ((error: unknown) => Promise) | undefined + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + rename: vi.fn(async (...args: Parameters) => { + try { + await actual.rename(...args) + } catch (error) { + await onRenameError?.(error) + throw error + } + }) + } +}) const LOCK_SCRIPT = [ '$stream = [System.IO.File]::Open($env:ORCA_SKILL_LOCK_PATH, [System.IO.FileMode]::Open,', ' [System.IO.FileAccess]::Read, [System.IO.FileShare]::None)', "[Console]::Out.WriteLine('LOCKED')", - 'Start-Sleep -Milliseconds ([int]$env:ORCA_SKILL_LOCK_DURATION_MS)', + '$null = [Console]::In.ReadLine()', '$stream.Dispose()' ].join('\n') -function holdFile(path: string, durationMs: number) { - const child = spawn( - 'powershell.exe', - ['-NoProfile', '-NonInteractive', '-Command', LOCK_SCRIPT], - { - windowsHide: true, - env: { - ...process.env, - ORCA_SKILL_LOCK_PATH: path, - ORCA_SKILL_LOCK_DURATION_MS: String(durationMs) - } +function holdFile(path: string) { + const child = spawnProcess({ + program: 'powershell.exe', + args: ['-NoProfile', '-NonInteractive', '-Command', LOCK_SCRIPT], + env: { + ...process.env, + ORCA_SKILL_LOCK_PATH: path } - ) - lockers.push(child) + }) + const closed = new Promise((resolve) => child.once('close', resolve)) + lockers.push({ child, closed }) let ready = false const locked = new Promise((resolve, reject) => { child.once('error', reject) @@ -47,19 +65,15 @@ function holdFile(path: string, durationMs: number) { } }) }) - const released = new Promise((resolve, reject) => { - child.once('error', reject) - child.once('exit', (code) => { - if (code === 0) { - resolve() - } else if (ready) { - reject(new Error(`windows-locker-exited-${code ?? 'unknown'}`)) - } else { - resolve() + return { + locked, + release: async () => { + if (child.exitCode === null && child.signalCode === null) { + child.stdin.end('\n') } - }) - }) - return { locked, released } + expect(await closed).toBe(0) + } + } } async function fixture(label: string) { @@ -74,29 +88,40 @@ async function fixture(label: string) { } afterEach(async () => { - for (const child of lockers.splice(0)) { - if (child.exitCode === null) { - child.kill() - } - } + onRenameError = undefined + vi.clearAllMocks() + await Promise.all( + lockers.splice(0).map(async ({ child, closed }) => { + if (child.exitCode === null && child.signalCode === null) { + child.kill() + } + await closed + }) + ) await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) }) describe.runIf(RUN_REAL_WINDOWS)('real Windows rename contention', () => { it('retries a transient exclusive file lock', async () => { const value = await fixture('transient') - const lock = holdFile(value.file, 300) + const lock = holdFile(value.file) await lock.locked + // Release only after a real sharing violation, outside the retry timer budget. + onRenameError = async (error) => { + expect(error).toMatchObject({ code: expect.stringMatching(/^(EACCES|EBUSY|EPERM)$/) }) + onRenameError = undefined + await lock.release() + } await renameSkillPathWithWindowsRetry(value.source, value.target) - await lock.released + expect(vi.mocked(rename).mock.calls.length).toBeGreaterThanOrEqual(2) expect(await readFile(join(value.target, 'SKILL.md'), 'utf8')).toBe('private skill') }) it('preserves the source after bounded retry exhaustion', async () => { const value = await fixture('exhaustion') - const lock = holdFile(value.file, 2_000) + const lock = holdFile(value.file) await lock.locked await expect(renameSkillPathWithWindowsRetry(value.source, value.target)).rejects.toMatchObject( @@ -105,7 +130,8 @@ describe.runIf(RUN_REAL_WINDOWS)('real Windows rename contention', () => { } ) expect((await stat(value.source)).isDirectory()).toBe(true) - await lock.released + expect(rename).toHaveBeenCalledTimes(6) + await lock.release() expect(await readFile(value.file, 'utf8')).toBe('private skill') await expect(