From 0903758747b3f86386ebcc0900033ed4216adab2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 15:29:21 +0000 Subject: [PATCH 1/2] test: make parallel suite stable without a full rerun Stop load-sensitive flakes from forcing a 605-file rerun at 2 workers. Batch cheap files, isolate server/spawn/HOME-mutating tests, and retry only failed files. Convert the doctor CLI spawn to in-process status collection and keep the spawned status JSON contract in cli-status-json. Co-authored-by: pavelhov --- .github/workflows/ci.yml | 2 +- AGENTS.md | 5 + README.md | 3 +- docs-site/src/content/docs/contributing.md | 8 +- scripts/test-parallel.ts | 146 ++++++++++++++++----- scripts/test-plan.ts | 93 +++++++++++++ src/AGENTS.md | 3 +- structure/06_docs-and-release.md | 2 +- tests/ci-workflows.test.ts | 4 +- tests/claude-models-discovery.test.ts | 5 +- tests/cli-status-json.test.ts | 24 +++- tests/codex-plugins-doctor.test.ts | 38 +++--- tests/test-runner.test.ts | 51 ++++++- 13 files changed, 312 insertions(+), 72 deletions(-) create mode 100644 scripts/test-plan.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e53cb65ad9..02b4305c7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: run: cd gui && bun run build - name: Test suite - run: bun test --isolate tests + run: bun run test:parallel - name: CLI help smoke run: bun run src/cli/index.ts help diff --git a/AGENTS.md b/AGENTS.md index c04cd50be8..ee4d0e6afc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,11 @@ Run `bun run typecheck` and `bun run test:parallel` (fall back to `bun run test` if the parallel runner misbehaves) before proposing or approving any non-trivial change. CI runs these on Linux, Windows, and macOS. +If `test:parallel` reports failed files, rerun **only those files** +(`bun run test:parallel tests/foo.test.ts …`). The runner already retries +failed files once on a single worker. Do not recover from a load flake by +rerunning the entire suite at `CCX_TEST_PARALLEL_WORKERS=2`. + ## Issues and pull requests (agents) Agent-created issues must use the repository templates. Pull requests use diff --git a/README.md b/README.md index 587ec70b83..69ef786898 100644 --- a/README.md +++ b/README.md @@ -313,7 +313,8 @@ Source development requires the `bun` CLI on your `PATH`. cd /path/to/CodexCommander bun install bun run typecheck -bun run test:parallel # preferred — much faster (parallel runner) +bun run test:parallel # preferred — batches cheap files, isolates server/spawn files, + # retries only failures (do not rerun the whole suite on a flake) bun run test # serial fallback ``` diff --git a/docs-site/src/content/docs/contributing.md b/docs-site/src/content/docs/contributing.md index a34ec3d2bb..7f7f216797 100644 --- a/docs-site/src/content/docs/contributing.md +++ b/docs-site/src/content/docs/contributing.md @@ -36,8 +36,9 @@ bun run prepare:package # refresh package launchers/assets Most tests are flat `tests/*.test.ts` Bun tests. `tests/helpers/` contains shared fixtures and `tests/e2e-style/` contains broader native-parity scenarios. Keep a focused regression near the -existing tests for the subsystem you change; run the full suite for shared routing, adapters, config, -or server behavior. +existing tests for the subsystem you change; run `bun run test:parallel` for shared routing, +adapters, config, or server behavior. If that reports failed files, rerun only those files — not +the entire suite. The parallel runner retries failed files once automatically. The docs site you're reading lives in `docs-site/` (Astro + Starlight): @@ -78,7 +79,8 @@ contributor work. and results). Empty or placeholder-only descriptions are not enough for review. - If the change touches the dashboard UI, include a screenshot in the description. - Behavior changes need a focused regression near the existing tests for that subsystem. - Shared routing, adapter, config, or server changes need the full suite green. + Shared routing, adapter, config, or server changes need `bun run test:parallel` green. + On a flake, rerun only the failed files; do not rerun the entire suite. The retired dual-track Go native port is not part of this repository. Bun-native TypeScript on `main` is the single runtime line. diff --git a/scripts/test-parallel.ts b/scripts/test-parallel.ts index 63cbc541f4..d42ee6fe90 100644 --- a/scripts/test-parallel.ts +++ b/scripts/test-parallel.ts @@ -1,28 +1,33 @@ /** * Parallel full-suite runner. * - * `scripts/test.ts` runs one fresh process per test file (shard size 1) because - * Bun 1.3.x on macOS can leave the event loop spinning after a server-heavy file - * completes, stalling the next file in the same process. That isolation is - * preserved here: each file still gets its own `bun test --isolate` process, - * but up to `CCX_TEST_PARALLEL_WORKERS` of them run at the same time. + * Cheap unit files share a process (batched, `--no-isolate`, one test at a + * time). Server/subprocess files still get a dedicated `bun test --isolate` + * process — Bun 1.3.x on macOS can leave the event loop spinning after those, + * which is why the serial runner uses shard size 1. * - * On a multi-core machine this turns the serial ~40-minute full suite into a - * few minutes. Default worker count is min(4, CPU count); override with - * `CCX_TEST_PARALLEL_WORKERS`. Optional positional args restrict the run to the - * given test files (useful for smoke probes). + * Default worker count is min(4, CPU count); override with + * `CCX_TEST_PARALLEL_WORKERS`. Batch size defaults to 8 (`CCX_TEST_BATCH_SIZE`). + * `CCX_TEST_FORCE_ISOLATE=1` restores one process per file. * - * The same exclusivity queue as the serial runner applies: this script waits - * for any other `bun test --isolate` runner to finish before starting, so two - * suites never fight over the CPU. + * Failed items are retried once, file-by-file, on a single worker. That is the + * load-flake recovery path: do not rerun the entire suite. Optional positional + * args restrict the run to the given test files (including retry-only reruns). + * + * The same exclusivity queue as the serial runner applies. */ import { availableParallelism } from "node:os"; +import { relative } from "node:path"; import { createIsolatedTestEnvironment, - findCompetingTestRunners, listRepositoryTestFiles, waitForExclusiveRun, } from "./test"; +import { + filesFromFailedPlanItems, + planParallelTests, + resolveTestBatchSize, +} from "./test-plan"; export function resolveWorkerCount( raw = process.env.CCX_TEST_PARALLEL_WORKERS, @@ -38,11 +43,38 @@ export function resolveWorkerCount( return parsed; } -function runIsolatedFile(file: string): Promise { +export function resolveRetryCount(raw = process.env.CCX_TEST_RETRY): number { + if (raw === undefined || raw.trim() === "") return 1; + const parsed = Number(raw); + if (!Number.isSafeInteger(parsed) || parsed < 0) { + throw new Error(`CCX_TEST_RETRY must be a non-negative integer, received ${JSON.stringify(raw)}`); + } + return parsed; +} + +interface WorkItem { + files: string[]; + isolate: boolean; +} + +function displayPath(file: string): string { + const rel = relative(process.cwd(), file); + return rel && !rel.startsWith("..") ? rel : file; +} + +function displayItem(item: WorkItem): string { + if (item.files.length === 1) return displayPath(item.files[0]!); + return `${item.files.length} files (${displayPath(item.files[0]!)} …)`; +} + +function runWorkItem(item: WorkItem): Promise { const isolated = createIsolatedTestEnvironment(); const memoryArgs = process.env.CCX_TEST_SMOL === "1" ? ["--smol"] : []; + const isolateArgs = item.isolate + ? ["--isolate"] + : ["--no-isolate", "--max-concurrency=1"]; return Bun.spawn( - [process.execPath, "test", "--isolate", ...memoryArgs, file], + [process.execPath, "test", ...isolateArgs, ...memoryArgs, ...item.files], { env: isolated.env, stdin: "inherit", @@ -55,44 +87,88 @@ function runIsolatedFile(file: string): Promise { }); } +async function runQueue(items: WorkItem[], workers: number, label: string): Promise { + const workerCount = Math.min(workers, Math.max(1, items.length)); + let next = 0; + const failures: WorkItem[] = []; + + const workerLoop = async (): Promise => { + for (;;) { + const index = next++; + if (index >= items.length) return; + const item = items[index]!; + const code = await runWorkItem(item); + if (code !== 0) failures.push(item); + console.warn( + `[test:parallel] ${label} ${index + 1}/${items.length} ${code === 0 ? "ok" : "FAIL"} ${displayItem(item)}`, + ); + } + }; + + await Promise.all(Array.from({ length: workerCount }, () => workerLoop())); + return failures; +} + if (import.meta.main) { const requested = process.argv.slice(2); await waitForExclusiveRun(process.pid); const files = requested.length > 0 ? requested : listRepositoryTestFiles(); if (files.length === 0) throw new Error("no test files found"); - const workers = Math.min(resolveWorkerCount(), files.length); + + const plan = planParallelTests(files); + const items: WorkItem[] = [ + ...plan.batches.map(batch => ({ files: batch, isolate: false })), + ...plan.isolate.map(file => ({ files: [file], isolate: true })), + ]; + const workers = Math.min(resolveWorkerCount(), items.length); + const retryCount = resolveRetryCount(); console.warn( - `[test:parallel] ${files.length} file(s) across ${workers} worker(s)` - + (requested.length > 0 ? " (explicit file list)" : " (CCX_TEST_PARALLEL_WORKERS to override)"), + `[test:parallel] ${files.length} file(s): ${plan.isolate.length} isolated, ${plan.batches.length} batch(es) ` + + `of up to ${resolveTestBatchSize()} across ${workers} worker(s)` + + (requested.length > 0 ? " (explicit file list)" : " (CCX_TEST_PARALLEL_WORKERS / CCX_TEST_BATCH_SIZE to override)"), ); const startedAt = Date.now(); - let next = 0; - let failed = 0; - const failures: string[] = []; + let failures = await runQueue(items, workers, "run"); + const recovered: string[] = []; - const workerLoop = async (): Promise => { - for (;;) { - const index = next++; - if (index >= files.length) return; - const file = files[index]!; - const code = await runIsolatedFile(file); - if (code !== 0) { - failed += 1; - failures.push(file); + if (failures.length > 0 && retryCount > 0) { + const retryFiles = filesFromFailedPlanItems(failures); + console.warn( + `[test:parallel] ${retryFiles.length} file(s) failed; retrying only those files (${retryCount} pass(es), 1 worker):`, + ); + for (const file of retryFiles) console.warn(` ${displayPath(file)}`); + + const stillFailing = new Set(retryFiles); + for (let attempt = 1; attempt <= retryCount; attempt += 1) { + const retryItems: WorkItem[] = [...stillFailing].map(file => ({ files: [file], isolate: true })); + const retryFailures = await runQueue(retryItems, 1, `retry ${attempt}/${retryCount}`); + const failedThisPass = new Set(filesFromFailedPlanItems(retryFailures)); + for (const file of stillFailing) { + if (!failedThisPass.has(file)) recovered.push(file); } - console.warn(`[test:parallel] ${index + 1}/${files.length} ${code === 0 ? "ok" : "FAIL"} ${file}`); + stillFailing.clear(); + for (const file of failedThisPass) stillFailing.add(file); + if (stillFailing.size === 0) break; } - }; + failures = [...stillFailing].map(file => ({ files: [file], isolate: true })); + } - await Promise.all(Array.from({ length: workers }, () => workerLoop())); const minutes = ((Date.now() - startedAt) / 60_000).toFixed(1); + if (recovered.length > 0) { + console.warn(`[test:parallel] recovered after retry (${recovered.length} file(s)):`); + for (const file of recovered) console.warn(` ${displayPath(file)}`); + } + if (failures.length > 0) { - console.error(`[test:parallel] ${failures.length} file(s) failed after ${minutes} min:`); - for (const file of failures) console.error(` ${file}`); + const failedFiles = filesFromFailedPlanItems(failures); + console.error(`[test:parallel] ${failedFiles.length} file(s) failed after ${minutes} min:`); + for (const file of failedFiles) console.error(` ${displayPath(file)}`); + console.error("[test:parallel] rerun only the failed files, not the entire suite:"); + console.error(` bun run test:parallel ${failedFiles.map(displayPath).join(" ")}`); process.exit(1); } console.warn(`[test:parallel] all ${files.length} file(s) passed in ${minutes} min`); diff --git a/scripts/test-plan.ts b/scripts/test-plan.ts new file mode 100644 index 0000000000..39378dba5b --- /dev/null +++ b/scripts/test-plan.ts @@ -0,0 +1,93 @@ +import { readFileSync } from "node:fs"; + +/** + * Cheap files share a process; server/subprocess files stay isolated. + * + * `bun test --isolate` is a fresh global object, not a fresh OS process. The + * serial and parallel wrappers spawn one process per file because Bun 1.3.x on + * macOS can leave the event loop spinning after a server-heavy file, stalling + * whatever would run next in that process. That isolation is necessary — it is + * also why 600 one-file processes thrash a laptop. + * + * Files that never bind a server, spawn `process.execPath`, or write the + * process HOME sandbox do not hit that stall. Batching them with + * `--no-isolate --max-concurrency=1` keeps HOME sandboxing (preload still + * runs once per process) without paying a Bun boot per file. Isolate-needed + * files still get their own process. + */ +export const DEFAULT_TEST_BATCH_SIZE = 8; + +export type TestLane = "batch" | "isolate"; + +export interface ParallelTestPlan { + isolate: string[]; + batches: string[][]; +} + +/** + * Isolate anything that cannot safely share a process with the next file: + * - server binds / nested Bun CLI spawns (macOS event-loop stall + load timeouts) + * - writes to the process HOME sandbox (preload HOME is per-process, not per-file) + */ +const ISOLATE_SOURCE_RE = + /\bstartServer\s*\(|\bBun\.serve\s*\(|\bprocess\.execPath\b|\bsaveConfig\s*\(|\binstallIsolatedCodexHome\s*\(|process\.env\.(CODEXCOMMANDER_HOME|CODEX_HOME)\s*=/; + +export function classifyTestFile(filePath: string, source: string): TestLane { + const normalized = filePath.replaceAll("\\", "/"); + if (normalized.includes("/e2e-style/")) return "isolate"; + if (ISOLATE_SOURCE_RE.test(source)) return "isolate"; + return "batch"; +} + +export function resolveTestBatchSize(raw = process.env.CCX_TEST_BATCH_SIZE): number { + if (raw === undefined || raw.trim() === "") return DEFAULT_TEST_BATCH_SIZE; + const parsed = Number(raw); + if (!Number.isSafeInteger(parsed) || parsed < 1) { + throw new Error(`CCX_TEST_BATCH_SIZE must be a positive integer, received ${JSON.stringify(raw)}`); + } + return parsed; +} + +export function forceIsolateAllFiles(raw = process.env.CCX_TEST_FORCE_ISOLATE): boolean { + return raw === "1"; +} + +export function planParallelTests( + files: readonly string[], + options: { batchSize?: number; forceIsolate?: boolean; readSource?: (file: string) => string } = {}, +): ParallelTestPlan { + const batchSize = options.batchSize ?? resolveTestBatchSize(); + const forceIsolate = options.forceIsolate ?? forceIsolateAllFiles(); + const readSource = options.readSource ?? ((file: string) => readFileSync(file, "utf8")); + + const isolate: string[] = []; + const batchable: string[] = []; + for (const file of files) { + if (forceIsolate || classifyTestFile(file, readSource(file)) === "isolate") { + isolate.push(file); + } else { + batchable.push(file); + } + } + + const batches: string[][] = []; + for (let offset = 0; offset < batchable.length; offset += batchSize) { + batches.push(batchable.slice(offset, offset + batchSize)); + } + return { isolate, batches }; +} + +export function filesFromFailedPlanItems( + failed: readonly { files: readonly string[] }[], +): string[] { + const seen = new Set(); + const files: string[] = []; + for (const item of failed) { + for (const file of item.files) { + if (seen.has(file)) continue; + seen.add(file); + files.push(file); + } + } + return files; +} diff --git a/src/AGENTS.md b/src/AGENTS.md index ab7a5fef46..db4ceea1a7 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -23,6 +23,7 @@ This file applies to `src/` and inherits the repository-wide rules in `/AGENTS.m - Place focused regression coverage near the existing tests for the affected subsystem. - For focused behavior, run the relevant `bun test tests/.test.ts` and `bun run typecheck`. -- For shared routing, adapters, config, OAuth, or server behavior, also run `bun run test`. +- For shared routing, adapters, config, OAuth, or server behavior, also run `bun run test:parallel`. + If that reports failures, rerun only the failed files — not the entire suite. - For logging, requests, credentials, account data, or fixtures, also run `bun run privacy:scan`. - Update `docs-site/` when the change affects user-visible behavior or configuration. diff --git a/structure/06_docs-and-release.md b/structure/06_docs-and-release.md index 03d21dc81a..f6eb4bfae2 100644 --- a/structure/06_docs-and-release.md +++ b/structure/06_docs-and-release.md @@ -111,7 +111,7 @@ cd gui && bun run lint cd gui && bun run lint:i18n cd gui && bun test tests cd gui && bun run build -bun test --isolate tests +bun run test:parallel bun run src/cli/index.ts help ``` diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index b34244930d..dc09240eee 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -122,7 +122,7 @@ describe("ci.yml is the only pull-request check", () => { for (const step of [ "bun x tsc --noEmit", "bun run privacy:scan", - "bun test --isolate tests", + "bun run test:parallel", "bun run lint", "bun run lint:i18n", "bun run build", @@ -130,7 +130,7 @@ describe("ci.yml is the only pull-request check", () => { expect(ci).toContain(step); } // The suite serves gui/dist, so the build must precede the test run. - expect(ci.indexOf("bun run build")).toBeLessThan(ci.indexOf("bun test --isolate tests")); + expect(ci.indexOf("bun run build")).toBeLessThan(ci.indexOf("bun run test:parallel")); }); test("grants read-only permissions and keeps no checkout credentials", () => { diff --git a/tests/claude-models-discovery.test.ts b/tests/claude-models-discovery.test.ts index 2a0d2bbffe..5d684dcf85 100644 --- a/tests/claude-models-discovery.test.ts +++ b/tests/claude-models-discovery.test.ts @@ -6,10 +6,11 @@ import { saveConfig } from "../src/config"; import { startServer } from "../src/server"; import type { CodexCommanderConfig } from "../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { SERVER_BUDGET_MS } from "./helpers/test-budget"; -// Full-suite Windows load: startServer + discovery GETs exceed the default 5s budget +// startServer + discovery GETs exceed Bun's 5s default under parallel load // (same flake class as 810fa115 / claude-management-api). -setDefaultTimeout(30_000); +setDefaultTimeout(SERVER_BUDGET_MS); let testDir = ""; let previousHome: string | undefined; diff --git a/tests/cli-status-json.test.ts b/tests/cli-status-json.test.ts index 2513494817..ae07007ad5 100644 --- a/tests/cli-status-json.test.ts +++ b/tests/cli-status-json.test.ts @@ -6,20 +6,27 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { resolveStatusPid, selectListenTarget } from "../src/cli/status"; import { createCodexRuntimeFixture } from "./helpers/codex-runtime-fixture"; +import { SPAWN_BUDGET_MS } from "./helpers/test-budget"; -setDefaultTimeout(30_000); +setDefaultTimeout(SPAWN_BUDGET_MS); const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const cliPath = join(repoRoot, "src", "cli", "index.ts"); -function runStatusJson(codexCommanderHome: string) { +function runStatusJson(codexCommanderHome: string, extraEnv: Record = {}) { const runtimeDir = mkdtempSync(join(tmpdir(), "ccx-status-runtime-")); try { const codexCliPath = createCodexRuntimeFixture(runtimeDir); return spawnSync(process.execPath, [cliPath, "status", "--json"], { cwd: repoRoot, - env: { ...process.env, CODEXCOMMANDER_HOME: codexCommanderHome, CODEX_CLI_PATH: codexCliPath }, + env: { + ...process.env, + CODEXCOMMANDER_HOME: codexCommanderHome, + CODEX_CLI_PATH: codexCliPath, + ...extraEnv, + }, encoding: "utf8", + timeout: SPAWN_BUDGET_MS - 5_000, }); } finally { rmSync(runtimeDir, { recursive: true, force: true }); @@ -29,6 +36,8 @@ function runStatusJson(codexCommanderHome: string) { describe("CLI status JSON", () => { test("status --json prints valid read-only diagnostics without secrets", () => { const codexCommanderHome = mkdtempSync(join(tmpdir(), "ccx-status-json-")); + const codexHome = mkdtempSync(join(tmpdir(), "ccx-status-codex-home-")); + writeFileSync(join(codexHome, "config.toml"), `model = "gpt-5"\n`, "utf8"); try { const configPath = join(codexCommanderHome, "config.json"); writeFileSync(configPath, JSON.stringify({ @@ -47,12 +56,15 @@ describe("CLI status JSON", () => { }), "utf8"); const beforeFiles = readdirSync(codexCommanderHome).sort(); - const result = runStatusJson(codexCommanderHome); + const beforeCodexHome = readdirSync(codexHome).sort(); + const result = runStatusJson(codexCommanderHome, { CODEX_HOME: codexHome }); const afterFiles = readdirSync(codexCommanderHome).sort(); + const afterCodexHome = readdirSync(codexHome).sort(); expect(result.status).toBe(0); expect(result.stderr).toBe(""); expect(afterFiles).toEqual(beforeFiles); + expect(afterCodexHome).toEqual(beforeCodexHome); expect(existsSync(join(codexCommanderHome, "codexcommander.pid"))).toBe(false); const parsed = JSON.parse(result.stdout) as { @@ -78,6 +90,7 @@ describe("CLI status JSON", () => { config?: { source?: unknown; error?: unknown }; service?: { summary?: unknown }; codexShim?: { summary?: unknown }; + codexPlugins?: { applicable?: unknown }; codexRuntime?: { path?: unknown; version?: unknown; @@ -123,6 +136,8 @@ describe("CLI status JSON", () => { expect(parsed.config?.error).toBeNull(); expect(typeof parsed.service?.summary).toBe("string"); expect(typeof parsed.codexShim?.summary).toBe("string"); + expect(parsed.codexPlugins).toBeDefined(); + expect(typeof parsed.codexPlugins?.applicable).toBe("boolean"); expect(typeof parsed.codexRuntime?.path).toBe("string"); expect(typeof parsed.codexRuntime?.source).toBe("string"); expect(parsed.codexRuntime?.version === null || typeof parsed.codexRuntime?.version === "string").toBe(true); @@ -145,6 +160,7 @@ describe("CLI status JSON", () => { } } finally { rmSync(codexCommanderHome, { recursive: true, force: true }); + rmSync(codexHome, { recursive: true, force: true }); } }); diff --git a/tests/codex-plugins-doctor.test.ts b/tests/codex-plugins-doctor.test.ts index eb6f42cdb5..38e25219ad 100644 --- a/tests/codex-plugins-doctor.test.ts +++ b/tests/codex-plugins-doctor.test.ts @@ -1,13 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { spawnSync } from "node:child_process"; import { mkdtempSync, mkdirSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { join } from "node:path"; +import { collectStatus } from "../src/cli/status"; import { diagnoseCodexBundledPlugins, locateCurrentBundledMarketplace } from "../src/codex/plugins-doctor"; - -const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); -const cliPath = join(repoRoot, "src", "cli", "index.ts"); +import { SERVER_BUDGET_MS } from "./helpers/test-budget"; function makeConfig(body: string): { dir: string; configPath: string } { const dir = mkdtempSync(join(tmpdir(), "ccx-codex-home-")); @@ -291,8 +288,8 @@ describe("diagnose path-mismatch (current vs registered)", () => { }); }); -describe("ccx status --json codexPlugins (spawned, read-only)", () => { - test("status --json includes a codexPlugins block and never writes CODEX_HOME", () => { +describe("collectStatus codexPlugins (in-process, read-only)", () => { + test("status JSON includes a codexPlugins block and never writes CODEX_HOME", async () => { const codexCommanderHome = mkdtempSync(join(tmpdir(), "ccx-status-home-")); const codexHome = mkdtempSync(join(tmpdir(), "ccx-codex-home-")); writeFileSync(join(codexHome, "config.toml"), `model = "gpt-5"\n`, "utf8"); @@ -309,26 +306,25 @@ describe("ccx status --json codexPlugins (spawned, read-only)", () => { defaultProvider: "openai", codexAutoStart: false, }), "utf8"); + const previousCommanderHome = process.env.CODEXCOMMANDER_HOME; + const previousCodexHome = process.env.CODEX_HOME; + process.env.CODEXCOMMANDER_HOME = codexCommanderHome; + process.env.CODEX_HOME = codexHome; try { const before = readdirSync(codexHome).sort(); - const result = spawnSync(process.execPath, [cliPath, "status", "--json"], { - cwd: repoRoot, - env: { ...process.env, CODEXCOMMANDER_HOME: codexCommanderHome, CODEX_HOME: codexHome }, - encoding: "utf8", - }); + const status = await collectStatus(); const after = readdirSync(codexHome).sort(); - expect(result.status).toBe(0); expect(after).toEqual(before); // read-only: no files added to CODEX_HOME - - const parsed = JSON.parse(result.stdout) as { - codexPlugins?: { applicable?: unknown }; - }; - expect(parsed.codexPlugins).toBeDefined(); - expect(typeof parsed.codexPlugins?.applicable).toBe("boolean"); + expect(status.json.codexPlugins).toBeDefined(); + expect(typeof status.json.codexPlugins.applicable).toBe("boolean"); } finally { + if (previousCommanderHome === undefined) delete process.env.CODEXCOMMANDER_HOME; + else process.env.CODEXCOMMANDER_HOME = previousCommanderHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; rmSync(codexCommanderHome, { recursive: true, force: true }); rmSync(codexHome, { recursive: true, force: true }); } - }, { timeout: 20_000 }); + }, { timeout: SERVER_BUDGET_MS }); }); diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 9769c00fee..d77b1cac10 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -10,7 +10,14 @@ import { resolveTestShardSize, resolveTestStartShard, } from "../scripts/test"; -import { resolveWorkerCount } from "../scripts/test-parallel"; +import { resolveRetryCount, resolveWorkerCount } from "../scripts/test-parallel"; +import { + classifyTestFile, + DEFAULT_TEST_BATCH_SIZE, + filesFromFailedPlanItems, + planParallelTests, + resolveTestBatchSize, +} from "../scripts/test-plan"; describe("test runner isolation", () => { test("redirects user homes to a disposable root", () => { @@ -50,6 +57,48 @@ describe("test runner isolation", () => { expect(() => resolveWorkerCount("x", 10)).toThrow("positive integer"); }); + test("retries failed files once by default and validates overrides", () => { + expect(resolveRetryCount(undefined)).toBe(1); + expect(resolveRetryCount("0")).toBe(0); + expect(resolveRetryCount("2")).toBe(2); + expect(() => resolveRetryCount("-1")).toThrow("non-negative integer"); + expect(() => resolveRetryCount("1.5")).toThrow("non-negative integer"); + }); + + test("classifies server, spawn, and HOME-mutating files as isolate; cheap unit files as batch", () => { + expect(classifyTestFile("tests/adapter-resolve.test.ts", "expect(resolveAdapter()).toBe(\"x\")")).toBe("batch"); + expect(classifyTestFile("tests/server-auth.test.ts", "const server = startServer(0);")).toBe("isolate"); + expect(classifyTestFile("tests/cli-help.test.ts", "spawnSync(process.execPath, [cliPath]);")).toBe("isolate"); + expect(classifyTestFile("tests/config.test.ts", "saveConfig(config);")).toBe("isolate"); + expect(classifyTestFile("tests/e2e-style/native.test.ts", "expect(true).toBe(true);")).toBe("isolate"); + }); + + test("plans batches without dropping isolate files, and flattens failed batch items for retry", () => { + const original = process.env.CCX_TEST_BATCH_SIZE; + delete process.env.CCX_TEST_BATCH_SIZE; + try { + expect(DEFAULT_TEST_BATCH_SIZE).toBe(8); + expect(resolveTestBatchSize()).toBe(8); + } finally { + if (original === undefined) delete process.env.CCX_TEST_BATCH_SIZE; + else process.env.CCX_TEST_BATCH_SIZE = original; + } + + const plan = planParallelTests( + ["a.test.ts", "b.test.ts", "c.test.ts", "d.test.ts", "e.test.ts"], + { + batchSize: 2, + readSource: (file) => file === "c.test.ts" ? "startServer(0)" : "expect(1).toBe(1)", + }, + ); + expect(plan.isolate).toEqual(["c.test.ts"]); + expect(plan.batches).toEqual([["a.test.ts", "b.test.ts"], ["d.test.ts", "e.test.ts"]]); + expect(filesFromFailedPlanItems([{ files: ["a.test.ts", "b.test.ts"] }, { files: ["a.test.ts"] }])) + .toEqual(["a.test.ts", "b.test.ts"]); + expect(resolveTestBatchSize("12")).toBe(12); + expect(() => resolveTestBatchSize("0")).toThrow("positive integer"); + }); + test("uses a bounded default shard size and validates overrides", () => { const originalShardSize = process.env.CCX_TEST_SHARD_SIZE; delete process.env.CCX_TEST_SHARD_SIZE; From 9c4c195d07a7b0355e839c6dde1c635758a583e3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 15:41:59 +0000 Subject: [PATCH 2/2] test: keep Linux suite hermetic without a user systemd bus Route Back, sync, and ensure tests were reaching the host bus. With no user session that probe is unproven ownership / SERVICE_BLOCKED, which is not the catalog or lifecycle claim those files make. Pass diagnoseService through restore-back replacement, put the existing systemctl stub on child PATH, and treat unsupported summaries as the no-bus Linux case instead of requiring a log path they never embed. Co-authored-by: pavelhov --- tests/codex-models-cache-invalidate.test.ts | 1 + tests/codex-sync-api.test.ts | 7 +-- tests/helpers/owned-service-home.ts | 56 ++++++++++++++++----- tests/proxy-lifecycle-concurrency.test.ts | 2 + tests/proxy-lifecycle.test.ts | 2 + tests/service.test.ts | 13 +++-- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/tests/codex-models-cache-invalidate.test.ts b/tests/codex-models-cache-invalidate.test.ts index cb28cf9d6f..8af2dbff7a 100644 --- a/tests/codex-models-cache-invalidate.test.ts +++ b/tests/codex-models-cache-invalidate.test.ts @@ -163,6 +163,7 @@ describe("invalidateCodexModelsCache write gate (#476 / #518)", () => { saveConfig(emptyConfig); const syncResult = await syncModelsToCodex(10100, emptyConfig, null, { + admitCodexWrite: () => ({ kind: "admitted" }), prepareCodexTransitionState: () => ({ kind: "ready", state: { nativeGeneration: 0, currentTxId: null }, diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index 439c62f3ee..4fb79200fd 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -46,8 +46,8 @@ const config = { }, } as CodexCommanderConfig; -function claimTempHome(codexHome: string, ccxHome: string, home: string): void { - claimOwnedServiceHome(codexHome, ccxHome, home); +function claimTempHome(codexHome: string, ccxHome: string, home: string): Record { + return claimOwnedServiceHome(codexHome, ccxHome, home).env; } const admittedSync = () => ({ kind: "admitted" as const }); @@ -328,7 +328,7 @@ describe("GUI/CLI Codex sync backend", () => { try { writeFileSync(join(raceCodexHome, "config.toml"), 'model = "gpt-5"\n', "utf8"); writeFileSync(join(raceCodexCommanderHome, "config.json"), JSON.stringify(config)); - claimTempHome(raceCodexHome, raceCodexCommanderHome, raceHome); + const serviceManagerEnv = claimTempHome(raceCodexHome, raceCodexCommanderHome, raceHome); const script = [ 'const { spawnSync } = require("node:child_process");', 'const { loadConfig } = require("./src/config");', @@ -362,6 +362,7 @@ describe("GUI/CLI Codex sync backend", () => { USERPROFILE: raceHome, CODEX_HOME: raceCodexHome, CODEXCOMMANDER_HOME: raceCodexCommanderHome, + ...serviceManagerEnv, }, encoding: "utf8", }); diff --git a/tests/helpers/owned-service-home.ts b/tests/helpers/owned-service-home.ts index 72cee665d8..989fdc70a7 100644 --- a/tests/helpers/owned-service-home.ts +++ b/tests/helpers/owned-service-home.ts @@ -3,10 +3,47 @@ import { delimiter, join } from "node:path"; import { fileURLToPath } from "node:url"; export interface OwnedServiceHome { - /** Add this to child-process environments so Linux never reaches the host bus. */ + /** + * Add this to child-process `env` so Linux never reaches the host bus. + * In-process `PATH` mutation is not enough: Bun's `execSync` keeps the + * original lookup path, which is why these stubs are spawn-env only. + */ readonly env: Record; } +function linuxSystemctlStubEnv(home: string, lines: readonly string[]): Record { + const binDir = join(home, ".ccx-test-bin"); + mkdirSync(binDir, { recursive: true, mode: 0o700 }); + const systemctl = join(binDir, "systemctl"); + writeFileSync(systemctl, ["#!/bin/sh", ...lines].join("\n")); + chmodSync(systemctl, 0o700); + return { PATH: [binDir, process.env.PATH ?? ""].filter(Boolean).join(delimiter) }; +} + +/** + * Linux CI (and container runners) have `systemctl` on PATH but no user bus. + * The production probe correctly treats that as unproven ownership, and + * `diagnoseService` treats a failed query as an installed service that needs + * repair — which then blocks unmanaged `ensure`/`start`. + * + * Tests that need a fresh machine (no unit) put this on PATH so + * `systemctl --user show` answers not-found without touching the host bus. + */ +export function claimAbsentLinuxServiceBus(home: string): OwnedServiceHome { + if (process.platform !== "linux") return { env: {} }; + return { + env: linuxSystemctlStubEnv(home, [ + 'if [ "$1" = "--version" ]; then echo "systemd 255"; exit 0; fi', + 'if [ "$1" = "--user" ] && [ "$2" = "show-environment" ]; then exit 0; fi', + 'if [ "$1" = "--user" ] && [ "$2" = "show" ]; then', + " printf '%s\\n' 'LoadState=not-found' 'ActiveState=inactive' 'UnitFileState=' 'FragmentPath=' 'NeedDaemonReload=no'", + " exit 0", + "fi", + "exit 64", + ]), + }; +} + /** * Seed the same state and service-manager definition that an installed proxy * records, scoped entirely to a test home. @@ -51,15 +88,10 @@ export function claimOwnedServiceHome( `Environment=\"CODEXCOMMANDER_HOME=${codexCommanderHome}\"`, ].join("\n")); - const binDir = join(home, ".ccx-test-bin"); - mkdirSync(binDir, { recursive: true, mode: 0o700 }); - const systemctl = join(binDir, "systemctl"); - writeFileSync(systemctl, [ - "#!/bin/sh", - "if [ \"$1\" != \"--user\" ] || [ \"$2\" != \"show\" ] || [ \"$3\" != \"codexcommander-proxy\" ]; then exit 64; fi", - "printf '%s\\n' 'LoadState=loaded' 'ActiveState=inactive' 'FragmentPath=fixture' 'NeedDaemonReload=no'", - ].join("\n")); - chmodSync(systemctl, 0o700); - - return { env: { PATH: [binDir, process.env.PATH ?? ""].filter(Boolean).join(delimiter) } }; + return { + env: linuxSystemctlStubEnv(home, [ + 'if [ "$1" != "--user" ] || [ "$2" != "show" ] || [ "$3" != "codexcommander-proxy" ]; then exit 64; fi', + "printf '%s\\n' 'LoadState=loaded' 'ActiveState=inactive' 'FragmentPath=fixture' 'NeedDaemonReload=no'", + ]), + }; } diff --git a/tests/proxy-lifecycle-concurrency.test.ts b/tests/proxy-lifecycle-concurrency.test.ts index cf666c436d..e49fe400ba 100644 --- a/tests/proxy-lifecycle-concurrency.test.ts +++ b/tests/proxy-lifecycle-concurrency.test.ts @@ -10,6 +10,7 @@ import { import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; +import { claimAbsentLinuxServiceBus } from "./helpers/owned-service-home"; const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const cliPath = join(repoRoot, "src", "cli", "index.ts"); @@ -94,6 +95,7 @@ wire_api = "responses" CODEX_HOME: codexHome, CCX_DISABLE_COMPANION: "1", NO_COLOR: "1", + ...claimAbsentLinuxServiceBus(home).env, }; let livePid: number | null = null; try { diff --git a/tests/proxy-lifecycle.test.ts b/tests/proxy-lifecycle.test.ts index e156472b01..936de5e404 100644 --- a/tests/proxy-lifecycle.test.ts +++ b/tests/proxy-lifecycle.test.ts @@ -1014,6 +1014,7 @@ describe("shared proxy lifecycle authority", () => { acquireAuthority: async () => authority(), loadConfig: () => config(), findLive: async () => live, + diagnoseService: () => service(), attestLive: async () => ({ pid: 42, port: 10100, source: "runtime", baseUrl: "http://127.0.0.1:10100", lifecycleLockLeaseV1: true, runtimeVersion: "0.0.1", lifecycleCompatibilityGeneration: 0, runtimeRecordIdentity: "old-42", @@ -1048,6 +1049,7 @@ describe("shared proxy lifecycle authority", () => { acquireAuthority: async () => authority(), loadConfig: () => config(), findLive: async () => live, + diagnoseService: () => service(), attestLive: async () => ({ pid: 42, port: 10100, source: "runtime", baseUrl: "http://127.0.0.1:10100", lifecycleLockLeaseV1: true, runtimeVersion: "0.0.1", lifecycleCompatibilityGeneration: 0, runtimeRecordIdentity: "old-42" }), captureSignalIdentity: pid => ({ pid, argvSha256: "argv", birthIdentity: "birth", ownerIdentity: "uid:501" }), staleStopIo: { diagnoseService: () => service(), restoreNative: () => ({ success: true, changed: true, desiredChanged: true, configChanged: true, message: "native" }), stripGrok: () => ({ ok: true, changed: false, message: "native" }), readPid: () => 42, stopProxy: async () => { calls.push("stop"); live = null; }, findSurvivor: async () => null }, diff --git a/tests/service.test.ts b/tests/service.test.ts index 73714403ce..b071c29ed8 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -3,7 +3,7 @@ import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, readlinkSync import { join } from "node:path"; import { saveConfig } from "../src/config"; import { windowsEnvIndirectBatchValue } from "../src/lib/win-paths"; -import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, ensureLaunchdExecutable, launchdExecutableDiagnostic, launchdExecutablePath, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, launchctlLoadFailed, launchdJobMatchesPlist, parseServiceInstallState, probeLaunchdSupervisor, probeSystemdSupervisor, readWindowsSchedulerXmlState, removeLaunchdExecutable, repairService, resolveServiceListenPort, runLaunchctl, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, startOwnedSystemdUnit, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; +import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, diagnoseService, ensureLaunchdExecutable, launchdExecutableDiagnostic, launchdExecutablePath, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, launchctlLoadFailed, launchdJobMatchesPlist, parseServiceInstallState, probeLaunchdSupervisor, probeSystemdSupervisor, readWindowsSchedulerXmlState, removeLaunchdExecutable, repairService, resolveServiceListenPort, runLaunchctl, serviceDiagnosticsSummary, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, startOwnedSystemdUnit, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; import type { ServiceDiagnostic } from "../src/service"; import { normalizeServiceSubcommand, parseServiceArgs, prepareServiceRoutingForStart, prepareServiceRoutingForTermination, runServiceLifecycleCommand, type ServiceCommandDependencies } from "../src/cli/service-command"; import type { ProxyLifecycleAuthority } from "../src/server/proxy-lifecycle-authority"; @@ -1448,9 +1448,16 @@ describe("service diagnostics", () => { }); test("status summary exposes the service log path", () => { + // The log path lives on the diagnostics fragment. Platform summaries embed + // that fragment when a service manager can be asked; hosts without a user + // bus (Linux CI/containers) report unsupported instead of guessing a unit. + expectTextToContainPath(serviceDiagnosticsSummary(), serviceLogPath()); const summary = serviceStatusSummary(); - - expectTextToContainPath(summary, serviceLogPath()); + if (diagnoseService().supported) { + expectTextToContainPath(summary, serviceLogPath()); + } else { + expect(summary).toMatch(/unsupported/i); + } }); test("flags stale baked service paths recorded at install time", () => {