mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
## 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 <noreply@anthropic.com>
90 lines
3.1 KiB
JavaScript
90 lines
3.1 KiB
JavaScript
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}`);
|
|
}
|
|
}
|
|
}
|