Skip to content

Gate the test matrix and retry setup-uv's flaky manifest fetch#3080

Merged
maxisbey merged 1 commit into
mainfrom
fix-uv-flake
Jul 9, 2026
Merged

Gate the test matrix and retry setup-uv's flaky manifest fetch#3080
maxisbey merged 1 commit into
mainfrom
fix-uv-flake

Conversation

@maxisbey

@maxisbey maxisbey commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Two paired CI fixes: make the required all-green check actually gate the test matrix, and stop a known setup-uv flake from becoming merge-blocking once it does.

Motivation and Context

The gate. The test matrix job in shared.yml has had job-level continue-on-error: true since #708 (May 2025). That flag makes the checks reusable workflow report success to all-green no matter what the matrix does, so the only required status check passes even when every test leg fails. Concrete example: in run 28935462867 the test (3.10, locked, ubuntu-latest) job failed, yet the run concluded success and all-green passed. Scanning the last 200 CI runs (8 days): 14 green runs had all 20 test legs failing. Failures are still visible as red ✗ on the PR checks list — which is why nothing broken has actually landed — but the gate itself is decorative, and RELEASE.md had to document the manual workaround ("a green workflow run does not mean the tests passed").

The flag made sense in #708 as insulation for the then-new, then-flaky Windows matrix (the old hang issues), but it silences every leg, not just Windows, and the Windows instability it guarded against was fixed (most recently #3070). Removing it needs a companion: job-level continue-on-error was also suppressing fail-fast cancellation, so fail-fast: false is added to keep today's run-all-legs behavior (identical compute; without it, the first failing leg would cancel the other 19 and destroy per-leg signal).

The flake. With the gate re-armed, a known setup-uv flake would become merge-blocking: the action fetches its version manifest from raw.githubusercontent.com in a single request with a hardcoded 5-second timeout and no retry (astral-sh/setup-uv#869; the timeout was introduced by astral-sh/setup-uv#883, first shipped in v8.2.0 — the version pinned here). The fetch happens even with an exact version: pin because the download URL only exists in the manifest, and it is unfixed through v8.3.2 (the fetch code is byte-identical), so bumping the pin doesn't help. It hit this repo 3 times in the last ~19 days (1, 2, 3) — roughly 0.06% of jobs, which across the 23 gating setup-uv executions per run would flake-block a run every couple of days.

Each setup-uv step in shared.yml therefore becomes a pair: the first attempt has step-level continue-on-error: true, and an identical retry step runs on steps.setup-uv.outcome == 'failure' (outcome is the pre-continue-on-error result; failure() would never fire here). The retry step has no continue-on-error, so deterministic errors — bad version, real checksum mismatch — fail both attempts and the job goes red; nothing is masked. Same pattern as e.g. apache/shenyu's setup-java retry. The steps are duplicated inline on purpose; a composite action felt like more machinery than a temporary workaround deserves — these steps should be deleted once upstream adds a retry.

Notes:

  • A flake absorbed by the retry no longer shows a red ✗ on the PR — look for a non-skipped "Install uv (retry)" step in the run. The failed first attempt also leaves a harmless "Error retrieving cache key from state." warning from its post step; expected, not a cache bug.
  • The setup-uv steps in conformance.yml, deploy-docs.yml, docs-preview.yml, and publish-pypi.yml stay single-shot on purpose: they don't feed all-green, no flakes were observed there (300 conformance runs scanned), and the release/preview workflows are better left churn-free here. The pattern is trivially copyable if one of them starts flaking.
  • Side effect of the re-armed gate: release-build in publish-pypi.yml has needs: [checks], so releases become genuinely test-gated too, and the RELEASE.md caveat about checking individual jobs is now stale (removed here).

How Has This Been Tested?

  • actionlint, zizmor, and the repo pre-commit hooks pass with no findings (zizmor output identical to the current file).
  • The retry wiring (continue-on-error + outcome semantics) was exercised with act: retry fires on first-attempt failure, is skipped on success, and a double failure fails the job.
  • Live validation on this PR (temporary commits, since dropped): one commit pointed the first setup-uv attempt at an unreachable manifest-file URL to force the flake — the retry fired on all 20 legs and the jobs stayed green; one commit added a deliberately failing test — all-green went red, confirming the re-armed gate blocks.

Breaking Changes

None for SDK users. For contributors: PRs with failing tests now fail all-green instead of showing a green run with red legs.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The upstream root cause is fixable in setup-uv itself (a retry in its fetch wrapper, or skipping the manifest for exact version pins); happy to take that to astral-sh/setup-uv#869 separately. When upstream ships a retry, the duplicated steps here should be deleted — the inline comments say so.

AI Disclaimer

The test job's job-level continue-on-error (from #708) made the required
all-green check pass even when test legs failed. Remove it, adding
fail-fast: false to keep the run-all-legs behavior it was incidentally
providing.

With the gate live, setup-uv's single-attempt manifest fetch (hard 5s
timeout, astral-sh/setup-uv#869, unfixed through v8.3.2) would become
merge-blocking, so give each setup-uv step in shared.yml one retry gated
on the step outcome; deterministic install errors still fail both
attempts. Update RELEASE.md accordingly.

No-Verification-Needed: CI workflow config with no local runtime surface; exercised via live CI runs on the PR
@maxisbey maxisbey marked this pull request as ready for review July 9, 2026 10:20

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/test_tempgate.py">

<violation number="1">
P0: CI will fail on every run because this new pytest test unconditionally raises `AssertionError`. This looks like the temporary gate-validation probe from the rollout notes; it should be removed (or changed to a real assertion) before merge so the restored all-green gate can pass legitimate PRs.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found — the retry wiring and gate semantics all check out — but this re-arms the repo's required merge gate and makes releases test-blocked, which is a policy change a maintainer should sign off on rather than a bot.

Extended reasoning...

Overview

This PR touches only CI configuration (.github/workflows/shared.yml) and release docs (RELEASE.md). It removes the job-level continue-on-error: true from the test matrix (adding fail-fast: false to preserve run-all-legs behavior), and wraps each setup-uv step in shared.yml in a two-attempt retry pair using step-level continue-on-error plus an outcome == 'failure' guard on the retry.

Verification

I independently verified the key claims: all-green in main.yml depends on the checks reusable workflow, so removing the job-level continue-on-error does re-arm the gate as described; release-build in publish-pypi.yml has needs: [checks], so the RELEASE.md edit is accurate. The retry wiring uses steps.setup-uv.outcome (the pre-continue-on-error result), which is the correct expression — failure() would never fire here. The retry step has no continue-on-error, so deterministic failures still fail the job. The four duplicated retry blocks are byte-identical to each other and pin the same action SHA as before. The local branch carries TEMP validation commits, but the PR diff confirms they were dropped — only the two clean files are in the PR.

Security risks

None. The action SHA pins are unchanged, persist-credentials: false is retained on all checkouts, and no new permissions or secrets are introduced. The retry pattern cannot mask deterministic failures (e.g., a checksum mismatch fails both attempts).

Level of scrutiny

The YAML mechanics are correct and were validated live by the author (forced-flake and forced-failure commits). What keeps me from approving is not correctness but policy: this changes what blocks merges for every contributor and makes releases genuinely test-gated, and it trades red-✗ visibility of setup-uv flakes for silent absorption by the retry step. The original continue-on-error was a deliberate maintainer decision (#708); reversing it should be a deliberate maintainer decision too.

Other factors

No bugs were found by the bug hunting system, and I found none either. The PR description is unusually thorough with live validation evidence. A maintainer review here should be quick — the question is simply whether the team wants the gate re-armed now.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the retry wiring (step-level continue-on-error + outcome check) and the gate change and found no bugs, but re-arming the required all-green check changes merge-blocking and release-gating behavior for the whole repo, so a maintainer should sign off on the policy shift.

Extended reasoning...

Overview

This PR touches only .github/workflows/shared.yml and RELEASE.md. It removes the job-level continue-on-error: true from the test matrix (adding fail-fast: false to preserve run-all-legs behavior), duplicates each setup-uv step into an attempt/retry pair guarded by steps.setup-uv.outcome == 'failure', and updates the now-stale RELEASE.md caveat about green runs not implying passing tests.

Correctness

The GitHub Actions semantics are used correctly: outcome reflects the pre-continue-on-error result (so the retry actually fires; conclusion or failure() would not work here), the retry step has no continue-on-error so deterministic failures still fail the job, and id: setup-uv is scoped per job so the reuse across four jobs is fine. fail-fast: false correctly replaces the fail-fast suppression that the job-level flag was implicitly providing. The author reports validation via actionlint, zizmor, act, and live forced-failure commits on this PR, which matches what the diff would predict.

Security risks

None introduced. Action pins remain SHA-pinned at the same commit, persist-credentials: false is unchanged, no new permissions or secrets are involved. If anything the change strengthens integrity by making the required check actually gate merges and (via publish-pypi.yml's needs: [checks]) releases.

Level of scrutiny

The mechanics are simple and I found no defects, but this is a repo-wide process change: the required all-green check goes from decorative to genuinely merge-blocking, and releases become test-gated. That is a deliberate policy decision affecting every contributor and the release pipeline, which a maintainer should consciously accept rather than have auto-approved — hence deferral despite finding no bugs.

Other factors

No prior reviews exist on this PR and the bug hunting system found no issues. The duplicated inline steps (vs. a composite action) are an explicit, documented tradeoff for a temporary workaround, with removal instructions in the comments.

@maxisbey maxisbey merged commit 148278e into main Jul 9, 2026
63 checks passed
@maxisbey maxisbey deleted the fix-uv-flake branch July 9, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant