diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3d1f6235c..f983fc2749 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,48 +40,11 @@ jobs: - 'scripts/update-flake.sh' - '.github/workflows/ci.yml' - test_pr: - name: Test - runs-on: ubuntu-latest - timeout-minutes: 10 - if: github.event_name == 'pull_request' || github.event_name == 'merge_group' - - steps: - - name: Checkout code - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Setup pnpm - uses: pnpm/action-setup@v4 - - - name: Setup Node.js - uses: actions/setup-node@v4 - with: - node-version: '20.19.0' - cache: 'pnpm' - - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Build project - run: pnpm run build - - - name: Run tests - run: pnpm test - - - name: Upload test coverage - uses: actions/upload-artifact@v4 - with: - name: coverage-report-pr - path: coverage/ - retention-days: 7 - test_matrix: name: Test (${{ matrix.label }}) runs-on: ${{ matrix.os }} timeout-minutes: 15 - if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' + if: github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' strategy: fail-fast: false matrix: @@ -89,12 +52,15 @@ jobs: - os: ubuntu-latest shell: bash label: linux-bash + vitest_workers: 4 - os: macos-latest shell: bash label: macos-bash + vitest_workers: 4 - os: windows-latest shell: pwsh label: windows-pwsh + vitest_workers: 2 defaults: run: @@ -126,16 +92,32 @@ jobs: run: pnpm run build - name: Run tests + env: + VITEST_MAX_WORKERS: ${{ matrix.vitest_workers }} run: pnpm test - name: Upload test coverage if: matrix.os == 'ubuntu-latest' uses: actions/upload-artifact@v4 with: - name: coverage-report-main + name: coverage-report-${{ github.event_name }} path: coverage/ retention-days: 7 + test_pr_required: + name: Test + runs-on: ubuntu-latest + needs: [test_matrix] + if: always() && (github.event_name == 'pull_request' || github.event_name == 'merge_group') + steps: + - name: Verify matrix tests passed + run: | + if [[ "${{ needs.test_matrix.result }}" != "success" ]]; then + echo "Matrix test job failed" + exit 1 + fi + echo "All matrix tests passed!" + lint: name: Lint & Type Check runs-on: ubuntu-latest @@ -288,13 +270,13 @@ jobs: required-checks-pr: name: All checks passed runs-on: ubuntu-latest - needs: [test_pr, lint, nix-flake-validate] + needs: [test_matrix, lint, nix-flake-validate] if: always() && (github.event_name == 'pull_request' || github.event_name == 'merge_group') steps: - name: Verify all checks passed run: | - if [[ "${{ needs.test_pr.result }}" != "success" ]]; then - echo "Test job failed" + if [[ "${{ needs.test_matrix.result }}" != "success" ]]; then + echo "Matrix test job failed" exit 1 fi if [[ "${{ needs.lint.result }}" != "success" ]]; then diff --git a/test/cli-e2e/capstone-journeys.test.ts b/test/cli-e2e/capstone-journeys.test.ts index 5b411cbe2b..adc4271a73 100644 --- a/test/cli-e2e/capstone-journeys.test.ts +++ b/test/cli-e2e/capstone-journeys.test.ts @@ -6,6 +6,9 @@ import * as path from 'node:path'; import { getGlobalDataDir, registerStore } from '../../src/core/index.js'; import { runCLI } from '../helpers/run-cli.js'; import { createOpenSpecRoot, writeSpec } from '../helpers/openspec-fixtures.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; + +const JOURNEY_TIMEOUT_MS = 30_000; /** * Capstone persona journeys (6.1). Journey 1 (fresh team) lives in @@ -31,7 +34,7 @@ describe('capstone persona journeys (6.1)', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); it('journey 2 — layered flow: app-repo agent discovers, cites, designs locally', async () => { @@ -97,7 +100,7 @@ describe('capstone persona journeys (6.1)', () => { // The store stayed read-only context throughout. const storeChanges = fs.readdirSync(path.join(storeRoot, 'openspec', 'changes')); expect(storeChanges.filter((name) => name !== 'archive' && name !== '.gitkeep')).toEqual([]); - }); + }, JOURNEY_TIMEOUT_MS); it('journey 3 — externalized planning: pointer repo runs the lifecycle without --store', async () => { const storeRoot = path.join(tempDir, 'team-planning'); @@ -174,5 +177,5 @@ describe('capstone persona journeys (6.1)', () => { // The code repo never grew planning state. expect(fs.readdirSync(path.join(codeRepo, 'openspec'))).toEqual(['config.yaml']); - }); + }, JOURNEY_TIMEOUT_MS); }); diff --git a/test/cli-e2e/store-lifecycle.test.ts b/test/cli-e2e/store-lifecycle.test.ts index 5fc6735432..4f0acd99c4 100644 --- a/test/cli-e2e/store-lifecycle.test.ts +++ b/test/cli-e2e/store-lifecycle.test.ts @@ -5,6 +5,7 @@ import path from 'path'; import { tmpdir } from 'os'; import { promisify } from 'util'; import { runCLI } from '../helpers/run-cli.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; const execFileAsync = promisify(execFile); @@ -18,6 +19,7 @@ const execFileAsync = promisify(execFile); */ const STORE_ID = 'team-context'; +const JOURNEY_TIMEOUT_MS = 60_000; let base: string; let storeRoot: string; @@ -189,7 +191,7 @@ beforeAll(async () => { }, 120_000); afterAll(async () => { - await fs.rm(base, { recursive: true, force: true }); + cleanupTempPath(base); }); describe('standalone store lifecycle journey', () => { @@ -337,7 +339,7 @@ describe('standalone store lifecycle journey', () => { path.join(storeRoot, 'openspec', 'changes', 'archive') ); expect(archiveEntries.some((entry) => entry.endsWith(`-${changeId}`))).toBe(true); - }); + }, JOURNEY_TIMEOUT_MS); it('machine A: the project repo is byte-identical after the lifecycle', async () => { const after = await snapshotDirectory(projectDir); @@ -391,7 +393,7 @@ describe('standalone store lifecycle journey', () => { ); expect(shownSpec.exitCode).toBe(0); expect(shownSpec.stdout).toContain('billing SHALL work'); - }); + }, JOURNEY_TIMEOUT_MS); it('machine B: completes its own change through archive in the clone', async () => { const changeId = 'add-invoicing'; @@ -450,7 +452,7 @@ describe('standalone store lifecycle journey', () => { expect(failedApply.exitCode).not.toBe(0); expect(failedApply.stderr).toContain(`Using OpenSpec root: ${STORE_ID}`); expect(failedApply.stderr).toContain(`openspec new change --store ${STORE_ID}`); - }); + }, JOURNEY_TIMEOUT_MS); it('end state is just normal OpenSpec files in both checkouts', async () => { for (const root of [storeRoot, cloneRoot]) { diff --git a/test/commands/context.test.ts b/test/commands/context.test.ts index 709a366a1c..14471afadc 100644 --- a/test/commands/context.test.ts +++ b/test/commands/context.test.ts @@ -7,6 +7,9 @@ import { getGlobalDataDir, registerStore } from '../../src/core/index.js'; import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; import { createOpenSpecRoot } from '../helpers/openspec-fixtures.js'; import { snapshotDirectory as snapshot } from '../helpers/fs-snapshot.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; + +const CONTEXT_MATRIX_TIMEOUT_MS = 30_000; describe('openspec context (4.1)', () => { let tempDir: string; @@ -41,7 +44,7 @@ describe('openspec context (4.1)', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function parseJson(result: RunCLIResult): any { @@ -193,7 +196,7 @@ describe('openspec context (4.1)', () => { ); expect(jsonBadDir.exitCode).toBe(1); expect(JSON.parse(jsonBadDir.stdout).status[0].code).toBe('context_output_dir_missing'); - }); + }, CONTEXT_MATRIX_TIMEOUT_MS); it('is read-only except the requested file and fails with the null shape', async () => { const rootBefore = snapshot(storeRoot); diff --git a/test/commands/doctor.test.ts b/test/commands/doctor.test.ts index a62b6d0242..f677da01e9 100644 --- a/test/commands/doctor.test.ts +++ b/test/commands/doctor.test.ts @@ -7,6 +7,7 @@ import { getGlobalDataDir, registerStore } from '../../src/core/index.js'; import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; import { createOpenSpecRoot, writeSpec } from '../helpers/openspec-fixtures.js'; import { snapshotDirectory as snapshot } from '../helpers/fs-snapshot.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; describe('openspec doctor (3.6)', () => { let tempDir: string; @@ -30,7 +31,7 @@ describe('openspec doctor (3.6)', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function parseJson(result: RunCLIResult): any { diff --git a/test/commands/legacy-groups-removed.test.ts b/test/commands/legacy-groups-removed.test.ts index f7d527f88f..99c9700abc 100644 --- a/test/commands/legacy-groups-removed.test.ts +++ b/test/commands/legacy-groups-removed.test.ts @@ -6,6 +6,9 @@ import * as path from 'node:path'; import { getGlobalDataDir, registerStore } from '../../src/core/index.js'; import { runCLI } from '../helpers/run-cli.js'; import { createHealthyOpenSpecRoot } from '../helpers/store-git.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; + +const SURVIVING_COMMANDS_TIMEOUT_MS = 30_000; describe('legacy command groups are removed', () => { let tempDir: string; @@ -24,7 +27,7 @@ describe('legacy command groups are removed', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function snapshotDirectory(root: string): Map { @@ -139,7 +142,7 @@ describe('legacy command groups are removed', () => { expect(snapshotDirectory(path.join(storeRoot, 'initiatives'))).toEqual(initiativeBefore); expect(snapshotDirectory(path.join(projectDir, '.openspec-workspace'))).toEqual(viewBefore); - }); + }, SURVIVING_COMMANDS_TIMEOUT_MS); it('tolerates legacy initiative metadata without re-emitting it', async () => { const projectDir = path.join(tempDir, 'legacy-project'); diff --git a/test/commands/store-git.test.ts b/test/commands/store-git.test.ts index e8bb06e65f..49dbccd7fc 100644 --- a/test/commands/store-git.test.ts +++ b/test/commands/store-git.test.ts @@ -12,6 +12,7 @@ import { } from '../../src/core/index.js'; import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; import { createHealthyOpenSpecRoot, isolatedGitEnv } from '../helpers/store-git.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; vi.mock('@inquirer/prompts', () => ({ input: vi.fn(), @@ -82,7 +83,7 @@ describe('store git lifecycle', () => { consoleLogSpy?.mockRestore(); consoleErrorSpy?.mockRestore(); vi.clearAllMocks(); - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function mkdir(relativePath: string): string { diff --git a/test/commands/store-remote.test.ts b/test/commands/store-remote.test.ts index 5043d6f4c1..b51282c5a4 100644 --- a/test/commands/store-remote.test.ts +++ b/test/commands/store-remote.test.ts @@ -12,8 +12,10 @@ import { } from '../../src/core/index.js'; import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; import { createHealthyOpenSpecRoot, isolatedGitEnv } from '../helpers/store-git.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; const TEST_NET_URL = 'https://192.0.2.1/acme/team-context.git'; +const GIT_JOURNEY_TIMEOUT_MS = 60_000; describe('store canonical remote (3.3)', () => { let tempDir: string; @@ -33,7 +35,7 @@ describe('store canonical remote (3.3)', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function git(cwd: string, ...args: string[]): string { @@ -419,7 +421,7 @@ describe('store canonical remote (3.3)', () => { const resolvedEntry = parseJson(resolved).references[0]; expect(resolvedEntry.status).toEqual([]); expect(resolvedEntry.root).toBe(fs.realpathSync.native(expectedCheckout)); - }); + }, GIT_JOURNEY_TIMEOUT_MS); }); describe('doctor and resolution', () => { diff --git a/test/commands/store-root-selection.test.ts b/test/commands/store-root-selection.test.ts index 2079887d48..082e5cdb1c 100644 --- a/test/commands/store-root-selection.test.ts +++ b/test/commands/store-root-selection.test.ts @@ -9,6 +9,7 @@ import { } from '../../src/core/index.js'; import { writeStoreMetadataState } from '../../src/core/store/foundation.js'; import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; const VALID_DELTA_SPEC = `## ADDED Requirements @@ -69,7 +70,7 @@ describe('store root selection for normal commands', () => { }); afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function createOpenSpecRoot(rootDir: string): void { diff --git a/test/commands/workset.test.ts b/test/commands/workset.test.ts index 2bc01da49d..e1ad7321e9 100644 --- a/test/commands/workset.test.ts +++ b/test/commands/workset.test.ts @@ -16,6 +16,7 @@ import { import { runCLI, type RunCLIResult } from '../helpers/run-cli.js'; import { createFakeTool, envWithFakeTools, readLaunchLog } from '../helpers/fake-tool.js'; import { snapshotDirectory as snapshot } from '../helpers/fs-snapshot.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; describe('openspec workset (7.1)', () => { let tempDir: string; @@ -56,7 +57,7 @@ describe('openspec workset (7.1)', () => { afterEach(() => { delete process.env.OPENSPEC_ENABLE_CLI_AGENT_OPENERS; - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function parseJson(result: RunCLIResult): any { @@ -469,7 +470,7 @@ describe('openspec workset (7.1)', () => { it('skips a missing member and falls through to the next primary', async () => { await createPlatform(['--tool', 'claude']); const fakeClaude = createFakeTool(tempDir, 'claude'); - fs.rmSync(memberB, { recursive: true, force: true }); + cleanupTempPath(memberB); const result = await runCLI(['workset', 'open', 'platform'], { cwd: tempDir, @@ -494,7 +495,7 @@ describe('openspec workset (7.1)', () => { // Primary missing: the next surviving member becomes cwd, and // the reassignment is noted in the skip-line style. - fs.rmSync(memberA, { recursive: true, force: true }); + cleanupTempPath(memberA); const second = await runCLI(['workset', 'open', 'platform'], { cwd: tempDir, env: envWithFakeTools(env, [fakeClaude]), @@ -508,7 +509,7 @@ describe('openspec workset (7.1)', () => { ); // No member survives: a typed failure. - fs.rmSync(memberC, { recursive: true, force: true }); + cleanupTempPath(memberC); const third = await runCLI(['workset', 'open', 'platform'], { cwd: tempDir, env: envWithFakeTools(env, [fakeClaude]), @@ -871,7 +872,7 @@ describe('interactive compose cancellation (in-process)', () => { restoreTTY?.(); process.env = originalEnv; process.exitCode = originalExitCode; - fs.rmSync(tempDir, { recursive: true, force: true }); + cleanupTempPath(tempDir); }); function exitPromptError(): Error { diff --git a/test/helpers/run-cli.ts b/test/helpers/run-cli.ts index 69d67df7f2..6dd40304bd 100644 --- a/test/helpers/run-cli.ts +++ b/test/helpers/run-cli.ts @@ -1,4 +1,4 @@ -import { spawn } from 'child_process'; +import { type ChildProcess, spawn } from 'child_process'; import { existsSync } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; @@ -8,8 +8,10 @@ const __dirname = path.dirname(__filename); const projectRoot = path.resolve(__dirname, '..', '..'); const cliEntry = path.join(projectRoot, 'dist', 'cli', 'index.js'); +const DEFAULT_CLI_TIMEOUT_MS = 30_000; let buildPromise: Promise | undefined; +const activeCliChildren = new Set(); interface RunCommandOptions { cwd?: string; @@ -53,6 +55,65 @@ function runCommand(command: string, args: string[], options: RunCommandOptions }); } +function mergeEnv( + ...sources: Array +): NodeJS.ProcessEnv { + const merged: NodeJS.ProcessEnv = {}; + + for (const source of sources) { + if (!source) continue; + for (const [key, value] of Object.entries(source)) { + if (value === undefined) continue; + + if (process.platform === 'win32') { + const existingKey = Object.keys(merged).find( + (candidate) => candidate.toLowerCase() === key.toLowerCase() + ); + if (existingKey && existingKey !== key) { + delete merged[existingKey]; + } + } + + merged[key] = value; + } + } + + return merged; +} + +function terminateProcessTree(child: ChildProcess): void { + if (!child.pid || child.killed) { + return; + } + + if (process.platform === 'win32') { + spawn('taskkill', ['/pid', String(child.pid), '/t', '/f'], { + stdio: 'ignore', + windowsHide: true, + }).on('error', () => { + child.kill('SIGKILL'); + }); + return; + } + + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + child.kill('SIGKILL'); + } +} + +function formatOutputTail(output: string): string { + const lines = output.trimEnd().split(/\r?\n/); + return lines.slice(-20).join('\n'); +} + +export function terminateActiveCliChildren(): void { + for (const child of activeCliChildren) { + terminateProcessTree(child); + } +} + export async function ensureCliBuilt() { if (existsSync(cliEntry)) { return; @@ -79,30 +140,34 @@ export async function runCLI(args: string[] = [], options: RunCLIOptions = {}): const invocation = [cliEntry, ...finalArgs].join(' '); return new Promise((resolve, reject) => { + const timeoutMs = options.timeoutMs ?? DEFAULT_CLI_TIMEOUT_MS; const child = spawn(process.execPath, [cliEntry, ...finalArgs], { cwd: options.cwd ?? projectRoot, - env: { - ...process.env, - OPEN_SPEC_INTERACTIVE: '0', - ...options.env, - }, + env: mergeEnv( + process.env, + { + OPENSPEC_TELEMETRY: '0', + OPEN_SPEC_INTERACTIVE: '0', + }, + options.env + ), stdio: ['pipe', 'pipe', 'pipe'], + detached: process.platform !== 'win32', windowsHide: true, }); // Prevent child process from keeping the event loop alive child.unref(); + activeCliChildren.add(child); let stdout = ''; let stderr = ''; let timedOut = false; - const timeout = options.timeoutMs - ? setTimeout(() => { - timedOut = true; - child.kill('SIGKILL'); - }, options.timeoutMs) - : undefined; + const timeout = setTimeout(() => { + timedOut = true; + terminateProcessTree(child); + }, timeoutMs); child.stdout?.setEncoding('utf-8'); child.stdout?.on('data', (chunk) => { @@ -115,7 +180,8 @@ export async function runCLI(args: string[] = [], options: RunCLIOptions = {}): }); child.on('error', (error) => { - if (timeout) clearTimeout(timeout); + clearTimeout(timeout); + activeCliChildren.delete(child); // Explicitly destroy streams to prevent hanging handles child.stdout?.destroy(); child.stderr?.destroy(); @@ -124,11 +190,26 @@ export async function runCLI(args: string[] = [], options: RunCLIOptions = {}): }); child.on('close', (code, signal) => { - if (timeout) clearTimeout(timeout); + clearTimeout(timeout); + activeCliChildren.delete(child); // Explicitly destroy streams to prevent hanging handles child.stdout?.destroy(); child.stderr?.destroy(); child.stdin?.destroy(); + if (timedOut) { + reject( + new Error( + [ + `CLI command timed out after ${timeoutMs}ms: node ${invocation}`, + stderr ? `stderr tail:\n${formatOutputTail(stderr)}` : '', + stdout ? `stdout tail:\n${formatOutputTail(stdout)}` : '', + ] + .filter(Boolean) + .join('\n\n') + ) + ); + return; + } resolve({ exitCode: code, signal, diff --git a/test/helpers/temp-cleanup.ts b/test/helpers/temp-cleanup.ts new file mode 100644 index 0000000000..d1ffd1e58a --- /dev/null +++ b/test/helpers/temp-cleanup.ts @@ -0,0 +1,14 @@ +import * as fs from 'node:fs'; + +export function cleanupTempPath(target: string | undefined): void { + if (!target) { + return; + } + + fs.rmSync(target, { + recursive: true, + force: true, + maxRetries: 5, + retryDelay: 100, + }); +} diff --git a/vitest.setup.ts b/vitest.setup.ts index 1eea108ba1..f2f33da354 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -1,15 +1,10 @@ -import { ensureCliBuilt } from './test/helpers/run-cli.js'; +import { ensureCliBuilt, terminateActiveCliChildren } from './test/helpers/run-cli.js'; // Ensure the CLI bundle exists before tests execute export async function setup() { await ensureCliBuilt(); } -// Global teardown to ensure clean exit export async function teardown() { - // Force exit after a short grace period if the process hasn't exited cleanly. - // This handles cases where child processes or open handles keep the worker alive. - setTimeout(() => { - process.exit(0); - }, 1000).unref(); + terminateActiveCliChildren(); }