Fix Windows CI flake hardening (#1325)

* fix windows ci test flake hardening

* restore required test check status
This commit is contained in:
Tabish Bidiwale
2026-07-08 11:37:27 +00:00
committed by GitHub
parent 871dece1be
commit 296ecbc20a
13 changed files with 173 additions and 84 deletions
+24 -42
View File
@@ -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
+6 -3
View File
@@ -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);
});
+6 -4
View File
@@ -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 <name> --store ${STORE_ID}`);
});
}, JOURNEY_TIMEOUT_MS);
it('end state is just normal OpenSpec files in both checkouts', async () => {
for (const root of [storeRoot, cloneRoot]) {
+5 -2
View File
@@ -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);
+2 -1
View File
@@ -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 {
+5 -2
View File
@@ -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<string, string> {
@@ -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');
+2 -1
View File
@@ -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 {
+4 -2
View File
@@ -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', () => {
+2 -1
View File
@@ -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 {
+6 -5
View File
@@ -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 {
+92 -11
View File
@@ -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<void> | undefined;
const activeCliChildren = new Set<ChildProcess>();
interface RunCommandOptions {
cwd?: string;
@@ -53,6 +55,65 @@ function runCommand(command: string, args: string[], options: RunCommandOptions
});
}
function mergeEnv(
...sources: Array<NodeJS.ProcessEnv | undefined>
): 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<RunCLIResult>((resolve, reject) => {
const timeoutMs = options.timeoutMs ?? DEFAULT_CLI_TIMEOUT_MS;
const child = spawn(process.execPath, [cliEntry, ...finalArgs], {
cwd: options.cwd ?? projectRoot,
env: {
...process.env,
env: mergeEnv(
process.env,
{
OPENSPEC_TELEMETRY: '0',
OPEN_SPEC_INTERACTIVE: '0',
...options.env,
},
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(() => {
const timeout = setTimeout(() => {
timedOut = true;
child.kill('SIGKILL');
}, options.timeoutMs)
: undefined;
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,
+14
View File
@@ -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,
});
}
+2 -7
View File
@@ -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();
}