From fb3cf1a0727cd70f4517419b4cd43cf160b40a81 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 6 Nov 2026 16:43:02 -0800 Subject: [PATCH] feat(ci): retry a failed Vitest case once in PR CI and report each retried pass (#19637) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, so CI wall-clock time sets the pace of work > - PR CI runs Vitest shards with 4 workers on 4-vCPU runners, so timing-sensitive tests sometimes time out under load > - One such timeout fails the whole run, and a re-run costs another 15 minutes or more > - In one 7-hour window on 2026-11-06, about 30 failed jobs were test flakes on commits that passed on a later attempt > - This pull request retries a failed Vitest case once in PR CI only, and reports every pass that needed the retry > - The benefit is fewer whole-run re-runs, while each flaky test stays visible so that we can fix it ## Linked Issues or Issue Description **What existing behavior does this improve?** PR CI fails the whole run when one Vitest case fails once because of runner load. **Subsystem affected** CI: the PR Vitest lanes (general, serialized, workspace and Runner package lanes). **Current behavior** Vitest uses `retry: 0` everywhere. One timeout under load fails the run. A person must notice it and re-run the failed jobs. About 20% of recently merged PRs needed at least one re-run. **Proposed behavior** PR CI retries a failed Vitest case one time. A test that passes only on the retry gets a GitHub warning annotation and a row in the job summary. Local runs, release, cloud readiness and master keep `retry: 0`. **Reason and benefit** A re-run of one test case costs seconds. A re-run of the CI run costs 15 minutes or more. The report keeps flaky tests visible. Separate pull requests fix the known flaky tests. **Breaking changes** None. ## What Changed - Add `scripts/ci/vitest-retry.mjs`. It sets 1 retry when `GITHUB_ACTIONS=true` and `GITHUB_WORKFLOW=PR`, and 0 in other contexts. `PAPERCLIP_VITEST_RETRY` overrides the value. - Add `scripts/ci/retry-reporter.mjs`. It writes a `::warning` annotation and a step-summary table row for each test that passed only on a retry. - `scripts/run-vitest-stable.mjs` passes `--retry` to `vitest run`. The `vitest list` collection step does not retry. - The root `vitest.config.ts` and `packages/paperclip-runner/vitest.config.ts` add the retry reporter beside the existing reporters. The Runner config sets `retry` from the same gate. - `.github/workflows/pr-trusted.yml` reads the `CI_VITEST_RETRY` repository variable. Set it to `0` to turn retries off without a code change. - Add `scripts/ci/retry-reporter.test.mjs` to `test:release-registry`, and document the behavior in `doc/DEVELOPING.md`. ## Verification - `node --test scripts/ci/retry-reporter.test.mjs`: 11/11 pass. The tests cover the gate matrix, the reporter output, a real Vitest run with a test that fails once, the wrapper arguments, and the Runner config. - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs`: 39/39 pass. - `node --test .github/scripts/tests/*.test.mjs`: 744/744 pass. - Manual check: a temporary test that fails once ran through `run-vitest-stable.mjs` and through the Runner package config. With the PR environment it passed and wrote the annotation and the summary row. With `GITHUB_WORKFLOW=Release` it failed. The temporary test is removed. ## Risks - A retry can hide a flaky test. The annotation and the summary row reduce this risk. - A test that fails for a real reason now runs twice in PR CI, so a failed run takes slightly longer. - `pr.yml` loads `pr-trusted.yml` from master, so the `CI_VITEST_RETRY` switch works only after merge. The retry itself is in the scripts and starts with this pull request. - Low risk otherwise: release and master verification are not changed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent wrote the change, and the parent session reviewed it. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- .github/workflows/pr-trusted.yml | 3 + doc/DEVELOPING.md | 20 +++ package.json | 2 +- packages/paperclip-runner/vitest.config.ts | 8 +- scripts/ci/retry-reporter.mjs | 89 ++++++++++ scripts/ci/retry-reporter.test.mjs | 194 +++++++++++++++++++++ scripts/ci/vitest-retry.mjs | 33 ++++ scripts/run-vitest-stable.mjs | 4 +- vitest.config.ts | 5 +- 9 files changed, 354 insertions(+), 4 deletions(-) create mode 100644 scripts/ci/retry-reporter.mjs create mode 100644 scripts/ci/retry-reporter.test.mjs create mode 100644 scripts/ci/vitest-retry.mjs diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index bc737937f7..8a2c283131 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -15,6 +15,9 @@ permissions: # the Honeycomb credential, so fork PRs cannot execute with it. env: PAPERCLIP_CI_PROFILE_DIR: ${{ github.workspace }}/.ci-profile + # Empty keeps the PR default of one Vitest retry (scripts/ci/vitest-retry.mjs). + # Set the CI_VITEST_RETRY repository variable to 0 to switch retries off. + PAPERCLIP_VITEST_RETRY: ${{ vars.CI_VITEST_RETRY }} concurrency: group: pr-${{ github.event.pull_request.number }} diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index c7bb4aaea0..ce8bba7d7e 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1278,6 +1278,26 @@ family, including the separate deadline and settlement regressions, with: pnpm --filter @paperclipai/paperclip-runner exec vitest run runnerd-codex-transport ``` +#### PR Vitest retry + +PR CI retries a failing Vitest case once, so a single contention flake does not +force a re-run of the whole workflow. `scripts/ci/vitest-retry.mjs` owns the gate. +The default is one retry when `GITHUB_ACTIONS=true` and `GITHUB_WORKFLOW=PR`. +Reusable workflows inherit the caller's name, so `pr-trusted.yml` gets the retry +through `pr.yml`. Release, release verify, cloud readiness, master and local runs +keep zero retries. `PAPERCLIP_VITEST_RETRY` overrides the default in any context. +For example, set the `CI_VITEST_RETRY` repository variable to `0` to turn the retry +off, or set `PAPERCLIP_VITEST_RETRY=1` locally to reproduce the CI behavior. + +`scripts/run-vitest-stable.mjs` passes `--retry` to every `vitest run`. This +covers the general, workspace, chat, native Runner and serialized lanes and the +selected-file path. The Runner package config sets `retry` itself, because its +lane runs Vitest directly. A test that passes only on a retry is still a flake. +`scripts/ci/retry-reporter.mjs` adds a `Flaky test passed on retry` warning +annotation for each such test, and a row to the step summary with the file, test, +retry count and job. Fix a test that this reporter reports. Do not treat the +retry as the fix. Playwright retries stay at zero. + #### Embedded Postgres template cache To run the suite with one owned PostgreSQL server and a separate disposable diff --git a/package.json b/package.json index fb28fe11c5..55889627a0 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "smoke:notion-generic-live": "node scripts/smoke/notion-generic-live.mjs", "smoke:posthog-live": "node scripts/smoke/posthog-live.mjs", "smoke:terminal-bench-loop-skill": "node scripts/smoke/terminal-bench-loop-skill-smoke.mjs", - "test:release-registry": "node scripts/ci-profile/node-test.mjs scripts/ci-profile/reporters.test.mjs scripts/__tests__/runner-vitest-package-lanes.test.mjs scripts/__tests__/canary-startup-playwright.test.mjs scripts/__tests__/cursor-public-install-sandbox.test.mjs scripts/__tests__/grok-public-install-sandbox.test.mjs scripts/__tests__/public-install-closure.test.mjs scripts/__tests__/public-install-registry.test.mjs scripts/__tests__/prefetch-child.test.mjs scripts/__tests__/stage-bundled-packages.test.mjs scripts/__tests__/pack-staged-packages.test.mjs scripts/__tests__/retain-runner-qualification-packages.test.mjs scripts/__tests__/runner-release-crate-cache.test.mjs scripts/__tests__/runner-debug-crate-cache.test.mjs scripts/__tests__/agent-build-cache.test.mjs scripts/__tests__/vitest-worker-isolation.test.mjs scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs scripts/check-no-git-push.test.mjs scripts/release-lib.test.mjs scripts/release-registry-versions.test.mjs scripts/link-plugin-dev-sdk.test.js scripts/acpx-patch-packaging.test.mjs scripts/service-onboard-smoke.test.mjs scripts/docker-onboard-smoke.test.mjs scripts/preview-artifacts.test.mjs scripts/cloud-migrator-artifacts.test.mjs scripts/typecheck-test-programs.test.mjs scripts/__tests__/local-verify.test.mjs scripts/test-sandbox-providers.test.mjs", + "test:release-registry": "node scripts/ci-profile/node-test.mjs scripts/ci-profile/reporters.test.mjs scripts/ci/retry-reporter.test.mjs scripts/__tests__/runner-vitest-package-lanes.test.mjs scripts/__tests__/canary-startup-playwright.test.mjs scripts/__tests__/cursor-public-install-sandbox.test.mjs scripts/__tests__/grok-public-install-sandbox.test.mjs scripts/__tests__/public-install-closure.test.mjs scripts/__tests__/public-install-registry.test.mjs scripts/__tests__/prefetch-child.test.mjs scripts/__tests__/stage-bundled-packages.test.mjs scripts/__tests__/pack-staged-packages.test.mjs scripts/__tests__/retain-runner-qualification-packages.test.mjs scripts/__tests__/runner-release-crate-cache.test.mjs scripts/__tests__/runner-debug-crate-cache.test.mjs scripts/__tests__/agent-build-cache.test.mjs scripts/__tests__/vitest-worker-isolation.test.mjs scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs scripts/check-no-git-push.test.mjs scripts/release-lib.test.mjs scripts/release-registry-versions.test.mjs scripts/link-plugin-dev-sdk.test.js scripts/acpx-patch-packaging.test.mjs scripts/service-onboard-smoke.test.mjs scripts/docker-onboard-smoke.test.mjs scripts/preview-artifacts.test.mjs scripts/cloud-migrator-artifacts.test.mjs scripts/typecheck-test-programs.test.mjs scripts/__tests__/local-verify.test.mjs scripts/test-sandbox-providers.test.mjs", "storybook-visual:baseline": "node scripts/storybook-visual-baseline.mjs", "test:storybook-visual": "node scripts/storybook-visual-baseline.mjs download && node scripts/storybook-visual-baseline.mjs verify && pnpm build-storybook && npx playwright test --config tests/storybook-visual/playwright.config.ts", "test:storybook-visual:update": "node scripts/storybook-visual-baseline.mjs download && pnpm build-storybook && npx playwright test --config tests/storybook-visual/playwright.config.ts --update-snapshots && node scripts/storybook-visual-baseline.mjs pack", diff --git a/packages/paperclip-runner/vitest.config.ts b/packages/paperclip-runner/vitest.config.ts index 5c96b64dfb..247bc65033 100644 --- a/packages/paperclip-runner/vitest.config.ts +++ b/packages/paperclip-runner/vitest.config.ts @@ -4,12 +4,17 @@ import { fileURLToPath } from "node:url"; import { configDefaults, defineConfig } from "vitest/config"; import { RunnerSequencer } from "./scripts/vitest-sequencer.mjs"; +import { retryReporterEnabled, vitestRetryCount } from "../../scripts/ci/vitest-retry.mjs"; const packageRoot = fileURLToPath(new URL(".", import.meta.url)); const ciReporters = [ ...(process.env.GITHUB_ACTIONS === "true" ? [resolve(packageRoot, "../../scripts/ci/oversized-test-files.mjs")] : []), ...(process.env.PAPERCLIP_CI_PROFILE === "true" ? [resolve(packageRoot, "../../scripts/ci-profile/vitest-reporter.mjs")] : []), + ...(retryReporterEnabled() ? [resolve(packageRoot, "../../scripts/ci/retry-reporter.mjs")] : []), ]; +// One retry in PR CI only (run-pr-vitest-lane.mjs runs `vitest run` here +// without the root wrapper); see scripts/ci/vitest-retry.mjs. +const retry = vitestRetryCount(); export default defineConfig({ resolve: { @@ -19,7 +24,8 @@ export default defineConfig({ ], }, test: { - ...(ciReporters.length ? { reporters: [...configDefaults.reporters, ...ciReporters] } : {}), + ...(ciReporters.length > 0 ? { reporters: [...configDefaults.reporters, ...ciReporters] } : {}), + ...(retry > 0 ? { retry } : {}), environment: "node", // Hermetic files also launch Runner and sidecar processes. Reserve CPU // capacity for their startup within the original readiness deadlines. diff --git a/scripts/ci/retry-reporter.mjs b/scripts/ci/retry-reporter.mjs new file mode 100644 index 0000000000..e132a20644 --- /dev/null +++ b/scripts/ci/retry-reporter.mjs @@ -0,0 +1,89 @@ +import { appendFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +// A retried pass is still a flake. Keep each one visible as a GitHub warning +// annotation and a step-summary row so `retry` never hides a failure silently. +// The CI profile reporter already records `retries` per test case for traces. +const repoRoot = fileURLToPath(new URL("../../", import.meta.url)); + +export function flakyRecord(testCase, root = repoRoot) { + if (testCase.result()?.state !== "passed") return null; + const retries = testCase.diagnostic()?.retryCount ?? 0; + if (!(retries > 0)) return null; + return { + file: path.relative(root, testCase.module.moduleId).split(path.sep).join("/"), + line: testCase.location?.line ?? null, + name: testCase.fullName, + project: testCase.project?.name ?? null, + retries, + }; +} + +// GitHub workflow command escaping: data escapes %, CR and LF; property +// values additionally escape the `:` and `,` separators. +function escapeData(value) { + return String(value).replaceAll("%", "%25").replaceAll("\r", "%0D").replaceAll("\n", "%0A"); +} + +function escapeProperty(value) { + return escapeData(value).replaceAll(":", "%3A").replaceAll(",", "%2C"); +} + +export function annotation(record) { + const properties = [`file=${escapeProperty(record.file)}`]; + if (record.line) properties.push(`line=${record.line}`); + properties.push(`title=${escapeProperty("Flaky test passed on retry")}`); + return `::warning ${properties.join(",")}::${escapeData(`${record.name} (retried ${record.retries})`)}`; +} + +function cell(value) { + return String(value).replace(/\s+/g, " ").replaceAll("|", "\\|").trim(); +} + +export function summaryMarkdown(records, job = "") { + if (records.length === 0) return ""; + const rows = records.map((record) => `| \`${cell(record.file)}\` | ${cell(record.name)} | ${record.retries} | ${cell(job || "-")} |`); + // Lead with a blank line: Vitest's own job summary may end mid-list. + return [ + "", + "### Flaky tests passed on retry", + "", + "| File | Test | Retries | Job |", + "| --- | --- | --- | --- |", + ...rows, + "", + "", + ].join("\n"); +} + +export default class RetryReporter { + constructor({ env = process.env, root = env.GITHUB_WORKSPACE || repoRoot, write = (text) => process.stdout.write(text) } = {}) { + this.env = env; + this.root = root; + this.write = write; + this.records = []; + } + + onTestRunStart() { + this.records = []; + } + + onTestCaseResult(testCase) { + const record = flakyRecord(testCase, this.root); + if (record) this.records.push(record); + } + + onTestRunEnd() { + if (this.records.length === 0) return; + this.write(`${this.records.map(annotation).join("\n")}\n`); + const summaryFile = this.env.GITHUB_STEP_SUMMARY; + if (!summaryFile) return; + try { + appendFileSync(summaryFile, summaryMarkdown(this.records, this.env.PAPERCLIP_CI_PROFILE_JOB || this.env.GITHUB_JOB)); + } catch (error) { + // Reporting must not change the test result; the annotations already printed. + console.error(`retry reporter: unable to write the step summary: ${error.message}`); + } + } +} diff --git a/scripts/ci/retry-reporter.test.mjs b/scripts/ci/retry-reporter.test.mjs new file mode 100644 index 0000000000..35571a1d80 --- /dev/null +++ b/scripts/ci/retry-reporter.test.mjs @@ -0,0 +1,194 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; +import RetryReporter, { annotation, flakyRecord, summaryMarkdown } from "./retry-reporter.mjs"; +import { retryReporterEnabled, vitestRetryArgs, vitestRetryCount } from "./vitest-retry.mjs"; + +const root = fileURLToPath(new URL("../../", import.meta.url)); +const reporterPath = fileURLToPath(new URL("./retry-reporter.mjs", import.meta.url)); +const pr = { GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: "PR" }; + +function testCase({ state = "passed", retryCount = 0, file = path.join(root, "server/src/a.test.ts"), name = "suite > case", line } = {}) { + return { + fullName: name, + module: { moduleId: file }, + project: { name: "@paperclipai/server" }, + location: line === undefined ? undefined : { line, column: 3 }, + result: () => ({ state }), + diagnostic: () => ({ retryCount }), + }; +} + +test("retry defaults to one only in the PR workflow on GitHub Actions", () => { + assert.equal(vitestRetryCount({}), 0); + assert.equal(vitestRetryCount(pr), 1); + for (const workflow of ["Release", "Release Verify", "Cloud readiness", "Release Smoke", "Trusted PR CI", "Incremental CI qualification"]) { + assert.equal(vitestRetryCount({ GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: workflow }), 0, workflow); + } + assert.equal(vitestRetryCount({ GITHUB_WORKFLOW: "PR" }), 0, "a local shell that happens to set the name stays at 0"); + assert.deepEqual(vitestRetryArgs({}), []); + assert.deepEqual(vitestRetryArgs(pr), ["--retry=1"]); +}); + +test("PAPERCLIP_VITEST_RETRY overrides the default and rejects invalid values", () => { + assert.equal(vitestRetryCount({ ...pr, PAPERCLIP_VITEST_RETRY: "0" }), 0); + assert.deepEqual(vitestRetryArgs({ ...pr, PAPERCLIP_VITEST_RETRY: "0" }), []); + assert.equal(vitestRetryCount({ ...pr, PAPERCLIP_VITEST_RETRY: "" }), 1, "an unset repository variable keeps the PR default"); + assert.equal(vitestRetryCount({ ...pr, PAPERCLIP_VITEST_RETRY: " " }), 1); + assert.equal(vitestRetryCount({ PAPERCLIP_VITEST_RETRY: "2" }), 2); + for (const value of ["-1", "1.5", "yes"]) { + assert.throws(() => vitestRetryCount({ PAPERCLIP_VITEST_RETRY: value }), /non-negative integer/, value); + } +}); + +test("the reporter loads in GitHub Actions jobs and wherever retry is on", () => { + assert.equal(retryReporterEnabled({}), false); + assert.equal(retryReporterEnabled({ GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: "Release" }), true); + assert.equal(retryReporterEnabled({ PAPERCLIP_VITEST_RETRY: "1" }), true); +}); + +test("only passes that needed a retry are flaky records", () => { + assert.equal(flakyRecord(testCase()), null); + assert.equal(flakyRecord(testCase({ state: "failed", retryCount: 1 })), null); + assert.equal(flakyRecord(testCase({ state: "skipped", retryCount: 1 })), null); + assert.deepEqual(flakyRecord(testCase({ retryCount: 1, line: 12 })), { + file: "server/src/a.test.ts", line: 12, name: "suite > case", project: "@paperclipai/server", retries: 1, + }); +}); + +test("annotations escape workflow-command separators", () => { + const record = flakyRecord(testCase({ retryCount: 2, file: path.join(root, "ui/src/a,b:c.test.ts"), name: "50% > line\nbreak" })); + assert.equal(record.line, null); + assert.equal(annotation(record), + "::warning file=ui/src/a%2Cb%3Ac.test.ts,title=Flaky test passed on retry::50%25 > line%0Abreak (retried 2)"); + assert.equal(annotation({ ...record, line: 7 }), + "::warning file=ui/src/a%2Cb%3Ac.test.ts,line=7,title=Flaky test passed on retry::50%25 > line%0Abreak (retried 2)"); +}); + +test("the step summary is a Markdown table with the job name", () => { + assert.equal(summaryMarkdown([]), ""); + const markdown = summaryMarkdown([{ file: "server/src/a.test.ts", name: "a | b\nc", retries: 1 }], "General tests (server 1/4)"); + assert.equal(markdown, [ + "", + "### Flaky tests passed on retry", + "", + "| File | Test | Retries | Job |", + "| --- | --- | --- | --- |", + "| `server/src/a.test.ts` | a \\| b c | 1 | General tests (server 1/4) |", + "", ""].join("\n")); + assert.match(summaryMarkdown([{ file: "f", name: "n", retries: 1 }]), /\| 1 \| - \|$/m); +}); + +test("the reporter writes nothing for a clean run and both outputs for a flaky one", () => { + const directory = mkdtempSync(path.join(tmpdir(), "retry-reporter-")); + try { + const summary = path.join(directory, "summary.md"); + let output = ""; + const reporter = new RetryReporter({ env: { GITHUB_STEP_SUMMARY: summary, GITHUB_JOB: "general" }, root, write: (text) => { output += text; } }); + reporter.onTestRunStart(); + reporter.onTestCaseResult(testCase()); + reporter.onTestRunEnd(); + assert.equal(output, ""); + assert.throws(() => readFileSync(summary), { code: "ENOENT" }); + + reporter.onTestRunStart(); + reporter.onTestCaseResult(testCase({ retryCount: 1 })); + reporter.onTestCaseResult(testCase({ state: "failed", retryCount: 1, name: "still failing" })); + reporter.onTestRunEnd(); + assert.equal(output, "::warning file=server/src/a.test.ts,title=Flaky test passed on retry::suite > case (retried 1)\n"); + assert.match(readFileSync(summary, "utf8"), /^\| `server\/src\/a\.test\.ts` \| suite > case \| 1 \| general \|$/m); + } finally { rmSync(directory, { recursive: true, force: true }); } +}); + +function runFlakyFixture(env) { + const directory = realpathSync(mkdtempSync(path.join(tmpdir(), "retry-reporter-vitest-"))); + symlinkSync(path.join(root, "node_modules"), path.join(directory, "node_modules"), "dir"); + // The counter lives on disk because a retry reruns the case in the same worker. + writeFileSync(path.join(directory, "flaky.test.mjs"), `import { existsSync, writeFileSync } from "node:fs"; +import { test } from "vitest"; +test("fails on its first attempt", () => { + const marker = new URL("./attempted", import.meta.url); + if (!existsSync(marker)) { writeFileSync(marker, ""); throw new Error("first attempt fails"); } +}); +test("always passes", () => {}); +`); + const configFile = path.join(directory, "vitest.config.mjs"); + writeFileSync(configFile, `export default ${JSON.stringify({ test: { include: ["*.test.mjs"], reporters: ["default", reporterPath] } })};`); + const summary = path.join(directory, "summary.md"); + const childEnv = { ...process.env, ...env, GITHUB_STEP_SUMMARY: summary, GITHUB_WORKSPACE: directory, PAPERCLIP_CI_PROFILE_JOB: "probe" }; + delete childEnv.NODE_TEST_CONTEXT; + const result = spawnSync(process.execPath, [path.join(root, "node_modules/vitest/vitest.mjs"), "run", + "--config", configFile, "--root", directory, ...vitestRetryArgs(childEnv)], { + cwd: directory, encoding: "utf8", timeout: 60_000, env: childEnv, + }); + let summaryText = ""; + try { summaryText = readFileSync(summary, "utf8"); } catch {} + rmSync(directory, { recursive: true, force: true }); + return { result, summaryText }; +} + +test("a real PR-gated Vitest run retries the flake once and reports it", () => { + const { result, summaryText } = runFlakyFixture({ ...pr, PAPERCLIP_VITEST_RETRY: "" }); + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.match(result.stdout, /^::warning file=flaky\.test\.mjs,title=Flaky test passed on retry::fails on its first attempt \(retried 1\)$/m); + assert.doesNotMatch(result.stdout, /always passes \(retried/); + assert.match(summaryText, /^\| `flaky\.test\.mjs` \| fails on its first attempt \| 1 \| probe \|$/m); +}); + +test("outside the PR workflow the same flake fails without a retry", () => { + const { result, summaryText } = runFlakyFixture({ GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: "Release", PAPERCLIP_VITEST_RETRY: "" }); + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.doesNotMatch(result.stdout, /Flaky test passed on retry/); + assert.doesNotMatch(summaryText, /Flaky tests passed on retry/); +}); + +test("run-vitest-stable passes --retry to vitest run only in the PR workflow", () => { + const directory = mkdtempSync(path.join(tmpdir(), "retry-wrapper-")); + try { + const bin = path.join(directory, "bin"); + mkdirSync(bin); + writeFileSync(path.join(bin, "pnpm"), '#!/bin/sh\nprintf "%s\\n" "$@" >> "$RETRY_ARGS_FILE"\n'); + chmodSync(path.join(bin, "pnpm"), 0o755); + const invoke = (env) => { + const argsFile = path.join(directory, `args-${Math.random()}`); + const childEnv = { ...process.env, ...env, PATH: `${bin}${path.delimiter}${process.env.PATH}`, RETRY_ARGS_FILE: argsFile }; + for (const key of Object.keys(childEnv)) { + if (key === "PAPERCLIP_CI_TEST_SELECTION" || key === "PAPERCLIP_CI_PROFILE" || /^PAPERCLIP_.*_TEST_DATABASE_URL$/.test(key)) delete childEnv[key]; + } + const result = spawnSync(process.execPath, [path.join(root, "scripts/run-vitest-stable.mjs"), + "--mode", "general", "--group", "general-workspaces-c"], { cwd: root, encoding: "utf8", env: childEnv, timeout: 60_000 }); + assert.equal(result.status, 0, result.stdout + result.stderr); + return readFileSync(argsFile, "utf8").trim().split("\n"); + }; + assert.deepEqual(invoke({ ...pr, PAPERCLIP_VITEST_RETRY: "" }), + ["exec", "vitest", "run", "--exclude", "**/dist/**", "--retry=1", "--project", "@paperclipai/db"]); + assert.deepEqual(invoke({ ...pr, PAPERCLIP_VITEST_RETRY: "0" }), + ["exec", "vitest", "run", "--exclude", "**/dist/**", "--project", "@paperclipai/db"]); + assert.deepEqual(invoke({ GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: "Cloud readiness", PAPERCLIP_VITEST_RETRY: "" }), + ["exec", "vitest", "run", "--exclude", "**/dist/**", "--project", "@paperclipai/db"]); + } finally { rmSync(directory, { recursive: true, force: true }); } +}); + +test("the Runner package config retries only in the PR workflow", () => { + const base = { ...process.env }; + for (const key of ["GITHUB_ACTIONS", "GITHUB_WORKFLOW", "PAPERCLIP_VITEST_RETRY", "PAPERCLIP_CI_PROFILE"]) delete base[key]; + const load = (env) => { + const childEnv = { ...base, ...env }; + const result = spawnSync(process.execPath, ["--input-type=module", "-e", + "const { default: config } = await import('./packages/paperclip-runner/vitest.config.ts');" + + "process.stdout.write(JSON.stringify({ retry: config.test.retry ?? 0, reporters: config.test.reporters ?? [] }));"], + { cwd: root, encoding: "utf8", env: childEnv, timeout: 60_000 }); + assert.equal(result.status, 0, result.stderr); + return JSON.parse(result.stdout); + }; + const prConfig = load({ ...pr, PAPERCLIP_VITEST_RETRY: "" }); + assert.equal(prConfig.retry, 1); + assert.ok(prConfig.reporters.includes(reporterPath), JSON.stringify(prConfig.reporters)); + assert.equal(load({ GITHUB_ACTIONS: "true", GITHUB_WORKFLOW: "Release", PAPERCLIP_VITEST_RETRY: "" }).retry, 0); + assert.equal(load({ ...pr, PAPERCLIP_VITEST_RETRY: "0" }).retry, 0); + assert.deepEqual(load({}), { retry: 0, reporters: [] }, "local runs keep the unmodified config"); +}); diff --git a/scripts/ci/vitest-retry.mjs b/scripts/ci/vitest-retry.mjs new file mode 100644 index 0000000000..6a65aaca97 --- /dev/null +++ b/scripts/ci/vitest-retry.mjs @@ -0,0 +1,33 @@ +// One Vitest retry is a PR-only safety net for contention flakes: a test that +// fails once under a loaded runner otherwise forces a whole-workflow re-run. +// Every pass that needed a retry is surfaced by scripts/ci/retry-reporter.mjs. +// +// PR CI is the pr.yml caller of pr-trusted.yml. Reusable workflows inherit the +// caller's GITHUB_WORKFLOW, so release, cloud-readiness, master and local runs +// keep retry 0. PAPERCLIP_VITEST_RETRY overrides the default in any context; +// set it to 0 to switch retries off, or to N for a local reproduction. +export const vitestRetryEnvName = "PAPERCLIP_VITEST_RETRY"; +export const prWorkflowName = "PR"; +export const prDefaultVitestRetry = 1; + +export function vitestRetryCount(env = process.env) { + const configured = env[vitestRetryEnvName]?.trim(); + if (configured) { + const parsed = Number(configured); + if (!Number.isInteger(parsed) || parsed < 0) { + throw new Error(`${vitestRetryEnvName} must be a non-negative integer. Received "${configured}".`); + } + return parsed; + } + return env.GITHUB_ACTIONS === "true" && env.GITHUB_WORKFLOW === prWorkflowName ? prDefaultVitestRetry : 0; +} + +export function vitestRetryArgs(env = process.env) { + const count = vitestRetryCount(env); + return count > 0 ? [`--retry=${count}`] : []; +} + +// Report retried passes in any GitHub Actions job, and wherever retry is on. +export function retryReporterEnabled(env = process.env) { + return env.GITHUB_ACTIONS === "true" || vitestRetryCount(env) > 0; +} diff --git a/scripts/run-vitest-stable.mjs b/scripts/run-vitest-stable.mjs index dd1ec2da91..5c5709af2d 100644 --- a/scripts/run-vitest-stable.mjs +++ b/scripts/run-vitest-stable.mjs @@ -6,6 +6,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { loadShardDurations, selectGeneralServerShard } from "./general-server-shard.mjs"; import { invocationEnv } from "./ci-profile/record.mjs"; +import { vitestRetryArgs } from "./ci/vitest-retry.mjs"; import { assertSelectedTests, partitionTestLines } from "./test-line-shard.mjs"; import { loadSelection, selectedShard } from "./ci/test-selection-manifest.mjs"; @@ -469,7 +470,8 @@ async function runVitest(args, label, testShard = null, exactFiles = false) { await new Promise((resolve, reject) => { const command = exactFiles ? process.execPath : "pnpm"; const prefix = exactFiles ? [path.join(scriptsDir, "ci/run-selected-vitest.mjs")] : ["exec", "vitest", "run"]; - const child = spawn(command, [...prefix, ...sourceOnlyVitestArgs, ...args], { + // `vitest list` collection above never retries; only the run does. + const child = spawn(command, [...prefix, ...sourceOnlyVitestArgs, ...vitestRetryArgs(), ...args], { cwd: repoRoot, env: invocationEnv("vitest", env), stdio: "inherit", diff --git a/vitest.config.ts b/vitest.config.ts index f8408d06df..4961fc0eef 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,11 +1,14 @@ import { configDefaults, defineConfig } from "vitest/config"; +import { retryReporterEnabled } from "./scripts/ci/vitest-retry.mjs"; export default defineConfig({ test: { reporters: [...configDefaults.reporters, ...(process.env.GITHUB_ACTIONS === "true" ? ["./scripts/ci/test-timing-reporter.mjs", "./scripts/ci/oversized-test-files.mjs"] : []), - ...(process.env.PAPERCLIP_CI_PROFILE === "true" ? ["./scripts/ci-profile/vitest-reporter.mjs"] : [])], + ...(process.env.PAPERCLIP_CI_PROFILE === "true" ? ["./scripts/ci-profile/vitest-reporter.mjs"] : []), + // PR lanes pass --retry from scripts/run-vitest-stable.mjs; see scripts/ci/vitest-retry.mjs. + ...(retryReporterEnabled() ? ["./scripts/ci/retry-reporter.mjs"] : [])], projects: [ "packages/shared", "packages/skills-catalog",