Skip to content

fix(ci): retry the CI Required results download after a pause - #3298

Merged
freeqaz merged 2 commits into
mainfrom
fix-2241-artifact-retry
Oct 23, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2241-artifact-retry

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

GitHub sometimes refuses artifact downloads for a few minutes ("Egress is over the account limit"). actions/download-artifact gives up after its own retries, about 25 s, and the CI Required job then reports every lane it could not fetch as missing-lane-output, even though every lane passed. Two sightings on 10-23 within about two hours:

Each one needs a maintainer to rerun the job.

Change

In the CI Required job, the results download now continues on error. If it fails, the job waits 60 s, clears the partial download and tries again; if that also fails, it waits 240 s and tries a third time without continue-on-error, so a lasting outage still fails the job as before. The aggregate step is unchanged. On the normal path the extra steps are skipped. The retry steps run only after a download that the mode check already admitted failed, so they check only the cancellation guard and the previous outcome.

Every new step keeps the !cancelled() && !contains(needs.*.result, 'cancelled') guard that the existing CI Required test requires.

Verification

  • actionlint .github/workflows/ci.yml: clean.
  • node scripts/ci/run-tests.mjs audit: no issues.
  • node --test tests/integration/ci-impact.test.mjs at 2 CPUs: 66/66 pass.
  • Mutant: dropping the cancellation guard from one retry step fails "CI Required always reports and fails at once when a dependency was cancelled". The existing test covers the new steps, so no new test is added.
  • The failure itself (GitHub-side throttling) cannot be reproduced on demand; the retry relies on steps.<id>.outcome == 'failure' after a continue-on-error step, which is documented Actions behaviour.

Risks

Low. Workflow-only. A failed download now adds up to about 5 minutes of waiting before CI Required reports; each retry logs a notice. The last wait is 240 s because the manual rerun of the main run passed 8 minutes after its failure. Retries add a little more artifact egress while throttled.

GitHub sometimes refuses artifact downloads for a few minutes ("Egress is
over the account limit"). actions/download-artifact gives up after about
25 s, so CI Required reported every lane it could not fetch as
missing-lane-output although all lanes had passed: run 39914661162 (#3281)
and main run 39931258427 (4e9f593) both needed a manual rerun of the
failed job.

Download twice more, after 60 s and then 120 s, before the aggregate step
runs; clear the partial download before each retry. The happy path is
unchanged: the retry steps run only when the previous download failed.
@freeqaz
freeqaz requested a review from a team as a code owner October 23, 2026 11:03
@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

… last one

Review follow-up: the manual rerun of main run 39931258427 passed 8 min
after the failure, so wait 240 s instead of 120 s before the final
attempt. Name the download and wait steps, log a notice before each
retry, and drop the mode check from the retry conditions (a retry runs
only after a download that the mode check already admitted failed).
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR retries failed CI result downloads after 60-second and 240-second pauses, clearing partial downloads before each retry.

Example: GitHub temporarily refuses the CI Required artifact download.

  • Before: CI Required fails immediately after the action exhausts its retries and reports missing lane results.
  • After: CI Required makes two additional delayed attempts, then aggregates downloaded results or remains failed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Focused and source-correct, with exact-head normal-path CI evidence but no observed execution of the new recovery path.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; exact-head hosted CI proves successful initial download and aggregation with retries skipped, while delayed recovery remains unobserved.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Maintainers must rerun CI Required when transient artifact-download failures obscure completed lane results.
Reason: The reported failure sequence is corroborated, and the collaborator-authored change records the bounded latency tradeoff without adding product configuration or changing platform contracts.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it addresses confirmed CI artifact-download failures, preserves failure enforcement, and introduces no identified correctness defect.

Priority: P2
Reviewed head: 90d6249201defa2105f236620c2b4d31282f2bd8

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The supplied evidence does not exercise recovery after a failed download or exhaustion of all three attempts; the existing cancellation test checks workflow structure rather than executing retries.
Agent review details

How this fits together

The CI Required workflow job downloads lane-result artifacts and passes them to the existing aggregator, which verifies job success, result completeness, and source identity.

flowchart TB
  A[Completed CI lanes] --> B[Download result artifacts]
  B -->|success| E[Aggregate and validate results]
  B -->|failure| C[Wait 60 seconds and retry]
  C -->|success| E
  C -->|failure| D[Wait 240 seconds and retry]
  D --> E
  E --> F[CI Required conclusion]
Loading

Technical review

Best possible solution:

Keep the bounded retries in the existing CI Required job while preserving cancellation handling and strict aggregation.

Do we have a high-confidence way to reproduce the issue?

The current-main workflow immediately reaches strict aggregation after a failed download, and GitHub job records corroborate that failure sequence; transient GitHub throttling cannot be triggered deterministically from the supplied evidence.

Is this the best way to solve the issue?

Retrying the existing pinned action is a focused response to an external transient failure, and the final non-tolerated attempt prevents a persistent outage from becoming a passing check.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against a4146f549835.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Pinned introduction and current-main comparison: The introduced diff changes only the existing artifact-download sequence; current main has one download attempt, while the PR retains its action pin, artifact pattern, destination, and aggregator. (.github/workflows/ci.yml:587, 90d6249201de)
  • Reported failures independently corroborated: Jobs 120290283823 and 120342543478 failed at artifact download and aggregation; the latter run's second attempt succeeded at downloading artifacts. Detailed logs redirected to a proxy-blocked Azure host, so the precise throttling message remains supported by the PR report rather than independently inspected logs. (4e9f5935d5fa)
  • Exact-head normal path passes: CI Required job 120387440616 passed at the reviewed head: the initial download and aggregation succeeded, and both retry sequences were skipped. (.github/workflows/ci.yml:587, 90d6249201de)
  • Original cancellation intent preserved: ci(required): finish cancelled runs sooner #1360 explains why conditional steps check both workflow cancellation and cancelled dependencies; every added retry step retains those guards. (.github/workflows/ci.yml:597, ee78dd19283b)
  • Aggregation remains strict: The unchanged aggregator rejects missing results, unsuccessful jobs, failed lanes, invalid evidence, and mismatched source SHAs; the final download attempt does not continue on error. (scripts/ci/run-tests.mjs:1612, 90d6249201de)
  • Policy and discussion reviewed: Read the complete root AGENTS.md; no nested .github AGENTS.md, maintainer-notes directory, or pr-simplify skill exists. The full PR body and both discussion comments contain no additional recovery trace. The maintainer-authored body explicitly accepts the bounded delay and extra artifact requests. Some historical diff blobs were unavailable locally; commit metadata, blame, and linked PR descriptions established the relevant intent. (AGENTS.md:1, 90d6249201de)

Likely related people:

  • freeqaz: Raw commit ee78dd1 adds .github/workflows/ci.yml:432 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ee78dd19283b; files: .github/workflows/ci.yml)
  • kevinlin-openai: Raw commit d353d52 adds .github/workflows/ci.yml:129 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: d353d527feb7; files: .github/workflows/ci.yml)

Labels

Label changes:

  • add P2: Confirmed artifact-download failures require manual CI reruns, with no demonstrated product-runtime outage.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: Confirmed artifact-download failures require manual CI reruns, with no demonstrated product-runtime outage.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Capture a hosted run showing the real download action recover after failure and a persistent failure remain red.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 23, 2026, 7:25 AM ET / 11:25 UTC.

@freeqaz
freeqaz merged commit 643ac52 into main Oct 23, 2026
79 of 81 checks passed
@freeqaz
freeqaz deleted the fix-2241-artifact-retry branch October 23, 2026 11:29
freeqaz added a commit that referenced this pull request Oct 23, 2026
…3304)

Moves the results-download retry from #3298 (download, wait 60 s, retry, wait 240 s, retry) into a shared composite action, .github/actions/download-results. CI Required in ci.yml and the full-integration aggregate job both use it, so a temporary artifact egress refusal no longer fails the full integration aggregate on the first try.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant