Repository navigation
ci(repo): add flaky-test check workflow - #7058
Conversation
Runs one commit on many fresh runners with retries off and reports tests that both pass and fail. Nightly covers unit and integration on develop, weekly covers CLI and stack e2e, and the run-flaky-check PR label adds a focused suite that repeats the tests the PR affects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Reconciled all 10 findings into 9 entries, merging the duplicated failure-counting issue. Eight findings are confirmed; the artifact rerun claim remains uncertain because cross-attempt artifact naming behavior could not be verified. The three report tests pass, and targeted checks reproduce the counting, CDATA, and suite-selection bugs. The confirmed next-impact finding remains advisory.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/workflows/flaky-check.yml:231 |
correctness |
claude | The main @supabase/api integration project receives none of the appended JUnit, repeat, changed-test, or filter arguments, leaving its flaky tests absent from the named test report. |
| 🟠 MAJOR | .github/workflows/flaky-check.yml:250 |
workflow-correctness |
codex | Manual dispatches targeting commits predating this PR cannot collect results because the collector action is loaded from the tested commit. |
| 🟠 MAJOR | .github/scripts/flaky-report.ts:136 |
correctness |
claude+codex | Counting failure/error elements as failed executions can inflate failure rates and classify a flaky test as always failing when one execution produces multiple errors. |
| 🟡 MINOR | .github/actions/flaky-collect/action.yml:60 |
ci-reliability |
claude | Re-running a failed matrix job fails to upload fresh results because its artifact name is reused without overwrite. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:82 |
xml-parsing |
codex | XML-like text inside CDATA logs is interpreted as testcase failures or skips. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:137 |
input-validation |
codex | Spaces after commas can silently omit requested suites from a manual run. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:45 |
ci-reliability |
claude | Manual dispatches launched from the same branch cancel each other even when they test different refs or suites. |
| ⚪ NIT | .github/scripts/flaky-report.ts:148 |
error-handling |
claude | A test step skipped after setup or build failure is misleadingly reported as a timeout or cancellation. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:290— The new CLI e2e job depends on the Go sidecar and sources removed by pending work on next.
Stats
Claude findings: 5 · Codex findings: 5 · Confirmed: 8 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Adjudicated all 10 findings from both independent reviews; none overlap. Eight are confirmed, including the advisory next-branch conflict. Two remain uncertain because Vitest's actual failure serialization and GitHub's artifact collision behavior across rerun attempts could not be verified. The report's three existing tests pass; targeted inputs reproduced the duplicate-title merge, invalid-entity crash, and suite-whitespace omission.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/workflows/flaky-check.yml:250 |
workflow-compatibility |
codex | Manual dispatches targeting historical revisions that lack the newly introduced collector action cannot collect their results. |
| 🟠 MAJOR | .github/scripts/flaky-report.ts:136 |
correctness |
codex | Counting JUnit failure/error elements as failed executions and assuming all configured repeats ran can misclassify hook failures or executions containing multiple errors. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:236 |
test-coverage |
claude | The integration and focused suites omit packages/api's Node-runtime integration tests from JUnit reporting and focused repetition because forwarded arguments reach only the second command in its chained script. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:44 |
ci |
claude | Manual dispatches on the same workflow ref cancel each other even when they request different test refs, suites, or filters. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:61 |
error-handling |
claude | An out-of-range numeric XML entity crashes report generation before any report or PR comment is produced. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:126 |
correctness |
codex | Distinct tests with identical file and display titles are merged and can be falsely reported as flaky. |
| 🟡 MINOR | .github/actions/flaky-collect/action.yml:59 |
error-handling |
codex | Rerunning failed jobs can conflict with previously uploaded result artifacts because the collector reuses fixed names without overwrite. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:137 |
input-validation |
codex | Whitespace in a comma-separated suite list passes validation but silently omits affected suites from the matrices and expected results. |
| ⚪ NIT | .github/MAINTAINERS.md:107 |
documentation |
codex | The documented automatic label removal and PR comment behavior omits the fork-PR exception. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:290— The new E2E workflow depends on the Go sidecar and its source tree, conflicting with their pending removal on next.
Stats
Claude findings: 3 · Codex findings: 7 · Confirmed: 8 · Refuted: 0 · Uncertain: 2
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Flaky test check: ✅ no flaky testsCommit Each execution is a separate Vitest process; |
Run the @supabase/api node integration project with its own JUnit file, classify tests per run so multi-error executions do not inflate failures, keep same-title tests apart, load the collector from the workflow revision, and harden suite input, dispatch concurrency, artifact re-uploads, and entity decoding. Also fixes formatting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-e3bf03' into avallete/flaky-test-detection-ci-e3bf03
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Both independent reviews completed. Verified all 12 input findings and merged the duplicate next-impact advisories into 11 confirmed findings. The main risk is a successful check with zero executed tests; other issues hide missing reports or misclassify test results. The three existing tests pass, while targeted in-memory probes reproduce the aggregator defects. The next-branch conflict remains a minor advisory.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/scripts/flaky-report.ts:170 |
result-validation |
codex | An explicit filter matching no tests can produce a successful flaky check. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:111 |
correctness |
claude | A package that never produces a JUnit report in any run of a variant is omitted without a missing-report warning when another package reports a failing test. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:258 |
error-handling |
claude | A failed merge-base lookup is ignored and focused tests receive an empty --changed argument. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:153 |
correctness |
codex | Failure-element counts can misclassify a test with passing repeats as always failing. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:132 |
test-identity |
codex | Excluding skipped cases before numbering duplicate titles changes test identities between runs. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:138 |
test-identity |
codex | Generated duplicate-title suffixes can collide with literal test titles and merge distinct tests. |
| ⚪ NIT | .github/scripts/flaky-report.ts:220 |
security |
claude | Unescaped test titles and failure messages can introduce unintended GitHub mentions in the bot's PR comment. |
| ⚪ NIT | .github/workflows/flaky-check.yml:467 |
maintainability |
claude | Sticky-comment lookup duplicates the report marker and uses an early-closing head pipeline that can skip posting under pipefail. |
| ⚪ NIT | .github/MAINTAINERS.md:111 |
docs |
claude | The new flaky-check paragraph contains an unusually long line inconsistent with its surrounding wrapping. |
| ⚪ NIT | .github/scripts/flaky-report.ts:261 |
wording |
codex | Single-test reports render '1 tests'. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:315— The new e2e workflow depends on the Go sidecar that pending work on next removes.
Stats
Claude findings: 6 · Codex findings: 6 · Confirmed: 11 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Flag suites that ran no tests unless empty is legitimate (focused without a filter), stop focused runs when the merge base cannot be resolved, key same-title tests by original title and occurrence counted before skips, keep @ mentions out of the PR comment, read the comment marker from the report, and rename the always-failing category to what it measures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Both independent reviews were available. Verified all six findings and merged the overlapping repeat-classification findings into one, yielding five confirmed entries. The empty-run bug can produce a falsely clean report. Existing report tests pass, but targeted in-memory cases reproduce the reporting issues. The next-branch conflict remains advisory.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/scripts/flaky-report.ts:190 |
correctness |
codex | An individual run that executes no tests can leave the report green and count toward the advertised successful repetitions. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:158 |
correctness |
claude+codex | Comparing JUnit error-element counts with planned executions can misclassify focused repeats in both directions: once-per-run failures become flaky, while multiple errors per failed execution can hide passing executions and label a flaky test as consistently failing. |
| 🟡 MINOR | .github/scripts/flaky-report.ts:167 |
error-handling |
claude | A package that fails to produce JUnit in every run disappears from the report when another reported test also fails in those runs. |
| ⚪ NIT | .github/scripts/flaky-report.ts:251 |
formatting |
claude | File paths containing '<' or '@' display literal escape text because HTML escaping is applied before wrapping the path in a Markdown code span. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:319— The new E2E workflow depends on the Go sidecar that pending work on next removes.
Stats
Claude findings: 3 · Codex findings: 3 · Confirmed: 5 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Check each run, not only the suite-wide union, so one empty run cannot count as a clean repetition. Render file paths as literal code spans. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read Vitest's built-in JSON reporter instead of hand-parsed JUnit XML, and move planning, command selection, result collection, reporting, and the PR comment into a tested TypeScript CLI under .github/scripts/flaky-check. The workflow now only wires triggers, matrices, checkout, setup, and artifacts, and loads the CLI from its own revision so runs against older refs keep working. Files with a failing beforeAll or import now get a file-level entry so setup flakes surface. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Verified all seven input findings and merged the duplicate hook-failure reports into six entries: five confirmed and one refuted. Runtime mixing is refuted because the Node and Bun projects select disjoint files. The next-impact finding is confirmed as advisory. All 19 existing tests pass; read-only reproductions expose the classification, hook-reporting, and completeness gaps.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/scripts/flaky-check/report.ts:160 |
test-classification |
codex | Counting failure messages can classify a repeated flaky test as consistently failing. Multiple errors from one failed execution can reach the execution count even when other executions passed. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:79 |
error-reporting |
claude+codex | A failing suite-level beforeAll hook is omitted when another assertion in the same file fails and Vitest leaves the file message empty. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:169 |
result-completeness |
codex | Reports absent from every run are invisible to completeness checking; a single-run dispatch also accepts a partially missing report set when surviving results pass and the exit code is zero. |
| ⚪ NIT | .github/scripts/flaky-check/plan.test.ts:17 |
test-coverage |
claude | A change confined to flaky-check.yml does not trigger the CI test that checks its weekly cron against WEEKLY_CRON, allowing a schedule change to silently select the nightly plan. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:219— The new e2e flaky-check job depends on the Go CLI workspace that pending work on next removes, so e2e checks against that branch will fail during setup.
Refuted findings (kept for transparency, not posted as review comments)
.github/scripts/flaky-check/report.ts:137(runtime-identity): Node and Bun test identities can shift between runs, combining different runtimes and falsely reporting tests as flaky.
Refuted: commands.ts:54-55 selects the integration and bun-integration projects, whose configured file sets are disjoint. Because the aggregation key includes the file path, Bun cases cannot take a Node case's identity after an import failure. The reported synthetic example assumes a shared file that these projects never both execute.
Stats
Claude findings: 2 · Codex findings: 5 · Confirmed: 5 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Replace --repeats with one Vitest process per focused iteration so every execution has its own report and real outcome, and classify tests from those executions instead of failure-message counts. Record the reports each invocation should write (from turbo --dry=json, rejecting chained scripts) and flag missing ones without relying on other runs. Run the flaky-check tests when the workflow changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
The seven input findings merge into six: four confirmed and two uncertain. Reproductions confirm both reporting bugs; code inspection confirms missing namespace/port-reservation coverage and the advisory conflict with next removing the Go sidecar. The timeout risks lack runtime evidence. All 15 existing flaky-check tests pass. Citations have been corrected to actual file line numbers.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/scripts/flaky-check/report.ts:129 |
correctness |
codex | Aggregation merges independent stack scenarios and can classify a consistently failing scenario as flaky. Three lifecycle setup failures combined with three passing confinement reports become one flaky setup verdict at 3/6. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:181 |
error-handling |
codex | An isolated hook or import failure is additionally reported as 'ran no tests; check the filter', despite the aggregator recognizing its setup failure. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:225 |
test-coverage |
claude | Flaky-check omits the stack namespace and port-reservation E2E files that the normal integration workflow runs, leaving those scenarios without repeat coverage. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:86 |
ci-reliability |
claude | Combining unsharded integration packages on one runner may cause timeout or resource-contention failures that normal sharded CI avoids; focused jobs can repeat the combined workload five times within 60 minutes. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:136 |
ci-reliability |
claude | Dispatch permits up to 101 sequential focused executions while keeping a fixed 60-minute timeout, creating a potential timeout risk for broad selections. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:162— The E2E workflow requires the Go sidecar, fundamentally conflicting with next removing apps/cli-go and the supabase-go binary. This is advisory.
Stats
Claude findings: 4 · Codex findings: 3 · Confirmed: 4 · Refuted: 0 · Uncertain: 2
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Verified all six input findings and merged the duplicate Go-removal advisory into one entry. Four findings are confirmed; the integration load and timeout concern remains uncertain. All 16 existing tests passed. Additional probes reproduced success after an empty repetition and a crash on a malformed report.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:87 |
ci-reliability |
claude | Combining unsharded integration suites on one runner, and budgeting 60 minutes for five focused passes, may cause resource-induced failures or timeouts that do not occur under the normal CI layout. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:171 |
correctness |
codex | The checker does not enforce its documented retries-off behavior, so configured Vitest retries can conceal intermittent failures. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:187 |
correctness |
codex | A repeated job can report success when an earlier execution runs tests but a later execution runs none. |
| 🟡 MINOR | .github/scripts/flaky-check/cli.ts:163 |
error-handling |
codex | Malformed nested report entries can crash report generation instead of being classified as absent results. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:153— The E2E preparation requires apps/cli-go and conflicts with the pending removal of that package and the supabase-go binary on next.
Stats
Claude findings: 2 · Codex findings: 4 · Confirmed: 4 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…cutions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Both independent reviews were available. All four findings were verified: the next-branch compatibility advisory is confirmed, the focused-selection and API runtime-ambiguity claims are refuted, and the integration-load concern remains uncertain. No blocking defect was confirmed. All 16 planner and reporting tests passed.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:227 |
correctness |
claude | Running unsharded integration packages together on one runner may introduce contention or timeouts that make the flaky-test report noisier than normal CI. |
Next impact (advisory)
Possible conflicts with work on the next branch. Informational only; this does not affect merging.
- 🟡 MINOR
.github/workflows/flaky-check.yml:162— The new E2E workflow depends on the Go workspace and binary removed on next, requiring adaptation when integrated into that branch.
Refuted findings (kept for transparency, not posted as review comments)
.github/workflows/flaky-check.yml:133(correctness): The nested, unignored tooling checkout exposes package.json and Vitest configuration as untracked changes, forcing every focused job to run all unit and integration tests.
Refuted: The tooling directory is a nested Git repository, not an ordinary untracked directory. The pinned checkout action initializes its .git repository in git-source-provider.ts:108-131. A read-only reproduction using the existing nested pr checkout and the finding's Git command, git ls-files --other --exclude-standard, returned only pr/, without its package.json or vitest.config.ts. Consequently, the claimed individual untracked trigger files are not exposed to Vitest by this command, so this checkout does not establish the alleged unconditional full-suite selection..github/scripts/flaky-check/report.ts:154(reporting): Final API integration reports omit runtime identity, preventing maintainers from distinguishing Node failures from Bun failures.
Refuted: Although the source field is omitted, the configured API runtime projects use disjoint files. packages/api/vitest.config.ts:17-19 excludes bun-lifecycle.integration.test.ts from the Node integration project, while line 22 selects only that file for Bun. plan.ts:231-232 explicitly selects those projects. Verdicts preserve the file path at report.ts:156 and Markdown displays it at line 267, so the runtime remains identifiable. The synthetic identical-file Node/Bun example does not represent the checked-out configuration.
Stats
Claude findings: 2 · Codex findings: 2 · Confirmed: 1 · Refuted: 2 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…t-detection-ci-e3bf03
… prebuild Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Adjudicated all four findings from both completed reviews: three confirmed and one uncertain, with no duplicates. Both reporting defects were reproduced. All 16 existing flaky-check tests pass.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:96 |
ci-reliability |
claude | The focused-job timeout may be insufficient when broad changes cause all unit and integration tests to run five times. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:292 |
ci-reliability |
claude | The native lifecycle job runs the Docker-dependent port-reservation suite without restoring the Docker image cache, introducing avoidable registry dependence into flaky-test measurements. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:85 |
correctness |
codex | A beforeAll failure with an empty file message disappears from the report when a sibling assertion in the same file also fails. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:154 |
diagnostics |
codex | Published failure records discard stack-scenario identity, so failures from different matrix jobs can become indistinguishable. |
Stats
Claude findings: 2 · Codex findings: 2 · Confirmed: 3 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
|
/ai-review |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Verified all four findings from both independent reviews; none overlap. Confirmed the skipped-setup classification bug, dispatched focused runs' fixed develop base, and unnecessary Go-cache restoration. The integration timeout/contention concern remains uncertain without runtime evidence. All 16 existing tests passed; an in-memory reproduction confirmed the classification bug.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:85 |
ci-reliability |
claude | Integration runs unsharded in one 30-minute job, unlike normal CI's six separate jobs. Focused runs repeat both unit and integration suites five times within 60 minutes by default. This may cause timeouts or contention-induced failures that normal CI does not encounter. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:94 |
correctness |
codex | Files containing only skipped tests count as successful setup executions even when their hooks never ran, which can classify a consistently failing hook as flaky. |
| ⚪ NIT | .github/scripts/flaky-check/plan.ts:137 |
correctness |
claude | Dispatched focused runs without a file filter always compare against develop, including when testing next or a branch based on next, so their selection includes unrelated changes since divergence from develop. |
| ⚪ NIT | .github/workflows/flaky-check.yml:144 |
ci-efficiency |
claude | Jobs that do not build Go still restore the Go module and build caches because the setup invocation leaves go-cache enabled. |
Stats
Claude findings: 3 · Codex findings: 1 · Confirmed: 3 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…laky-check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Adjudicated all four findings from both reviews: three confirmed, with the integration-fidelity claim corrected to reflect sequential API execution, and one timeout prediction left uncertain. No duplicates. All 17 planner/report tests pass; a targeted reproduction confirms the masked process-failure issue.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:231 |
test-fidelity |
claude | The integration check combines unsharded stack and CLI integration tests on one runner, unlike CI's separate package/shard runners. This changes resource contention and can reduce the check's fidelity to CI. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:96 |
reliability |
claude | The default focused job's 60-minute timeout may be insufficient when its five executions each select the full unit and integration suites. |
| 🟡 MINOR | .github/scripts/flaky-check/report.ts:193 |
error-handling |
codex | A failing test in one invocation suppresses reporting of a separate invocation's nonzero process exit when all expected reports exist. |
| ⚪ NIT | .github/scripts/flaky-check/cli.ts:113 |
error-handling |
claude | CLI planning or configuration exceptions leave exit-code unset, causing the aggregate report to describe them as setup failure, timeout, or cancellation. |
Stats
Claude findings: 3 · Codex findings: 1 · Confirmed: 3 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…ng test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available. Verified all five Claude findings: one truncation nit is confirmed, one documentation nit is refuted, and three CI-reliability concerns remain uncertain. No critical or major defect was verified. All 17 flaky-check tests passed.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:96 |
ci-reliability |
claude | Focused jobs may time out when changed-file selection includes the full unit and integration suites, because the timeout grows by only six minutes per execution. The original finding also claims that delayed metadata outputs prevent partial per-test reporting. |
| 🟡 MINOR | .github/scripts/flaky-check/plan.ts:231 |
ci-reliability |
claude | Running the stack and CLI integration suites concurrently on one runner may introduce contention-only failures that do not reproduce in the ordinary CI package shards, reducing the flake report's usefulness. |
| 🟡 MINOR | .github/workflows/flaky-check.yml:134 |
ci-reliability |
claude | The unignored nested .flaky-tooling checkout may contribute package.json or Vitest configuration files to changed-file detection and unintentionally force focused runs to select every test. |
| ⚪ NIT | .github/scripts/flaky-check/report.ts:359 |
correctness |
claude | Comment truncation can split a Unicode surrogate pair or cut through a Markdown row or HTML mention-escape comment, damaging the rendered tail. |
Refuted findings (kept for transparency, not posted as review comments)
.github/workflows/flaky-check.yml:31(documentation): The filter description incorrectly names develop despite PR-base selection, and the repeat input description conflicts with the maintainer guide's five executions.
Refuted: The cited description belongs to manual-dispatch inputs, whose focused base is develop. PR events use their PR base separately. Four extra executions plus the initial execution equal the guide's five executions, so the descriptions agree. Describing merge-base selection as changes since develop is consistent with that dispatch behavior.
Stats
Claude findings: 5 · Codex findings: 0 · Confirmed: 1 · Refuted: 1 · Uncertain: 3
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
TL;DR: A new
Flaky checkworkflow runs one commit on many fresh runners with retries off and reports every test that both passed and failed. It runs nightly ondevelop, weekly for e2e, and on demand through therun-flaky-checkPR label, so PRs that fix flaky tests can show the fix holds.Why
Flaky tests surface one at a time in unrelated PRs and get retried away. There is no way to ask "is this commit stable?" or to show that a flaky-test fix actually holds.
stack-stress.ymlcovers two stack scenarios only.What changed
flaky-check.ymlruns:developcli-e2e) and stack whole-stack e2e ×3run-flaky-checklabel: unit + integration ×10 on the PR merge commit, plus afocusedsuite that repeats the tests the PR affects (vitest --changedagainst the merge base,--repeats=4, ×10 runs)--repeatsexecution counts separately.flaky-check-reportartifact (report.md,report.json), and one sticky PR comment. The report job fails when it finds anything..github/MAINTAINERS.mddocuments the triggers and thefocusedcaveats.Linked issue
None; maintainer change. The
run-flaky-checklabel already exists.🤖 Generated with Claude Code