ci: retry Chocolatey installs and drop the yq dependency on Windows - #8954
Open
myleshorton wants to merge 5 commits into
Open
ci: retry Chocolatey installs and drop the yq dependency on Windows#8954myleshorton wants to merge 5 commits into
myleshorton wants to merge 5 commits into
Conversation
Two consecutive release builds (v9.1.18-beta, v9.1.19-beta) failed on community.chocolatey.org gateway timeouts, and each failure then cascaded: release-finalize's cleanup deleted the draft release, and 'gh run rerun --failed' never re-runs the successful release-create job, so re-run uploads died with 'release not found' until the draft was recreated by hand. - scripts/ci/choco-retry.sh: choco install with 5 attempts and linear backoff; 1641/3010 (reboot-required) count as success. Used for mingw (three workflows) and innosetup. - build-windows: pass flutter-version parsed from the pinned version file instead of flutter-version-file — with the file input, flutter-action unconditionally runs 'choco install yq' on Windows, which is the exact step that 504'd both times. The direct input skips it. - release.yml: 'Ensure release exists' guard in upload-release-artifacts recreates the draft (mirroring release-create) when a failed run's cleanup deleted it, making 'rerun --failed' self-sufficient. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesWindows CI installation reliability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WindowsWorkflow
participant choco_retry_sh
participant Chocolatey
WindowsWorkflow->>choco_retry_sh: pass MinGW or Inno Setup package
choco_retry_sh->>Chocolatey: run choco install
Chocolatey-->>choco_retry_sh: return installation status
choco_retry_sh->>Chocolatey: retry failed installation with increasing delays
choco_retry_sh-->>WindowsWorkflow: return final status
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the release CI pipeline against two recurring failure modes in Windows/release workflows: transient Chocolatey 504s and re-run failures after a draft release is deleted during cleanup.
Changes:
- Add a
choco installretry wrapper with backoff and apply it to MinGW/Inno Setup installs on Windows workflows. - Avoid
subosito/flutter-action’s Windowschoco install yqpath by parsing the pinned Flutter version and passing it viaflutter-version. - Make
upload-release-artifactsself-healing on re-runs by recreating the draft release if it was deleted by failure cleanup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/ci/choco-retry.sh | Adds a retry/backoff wrapper for Chocolatey installs (treats reboot-required codes as success). |
| .github/workflows/release.yml | Ensures the draft release exists before uploading artifacts on reruns. |
| .github/workflows/build-windows.yml | Uses choco-retry.sh for installs and passes pinned Flutter version directly to avoid yq install. |
| .github/workflows/build-windows-service.yml | Switches MinGW install to choco-retry.sh under bash. |
| .github/workflows/build-windows-dll.yml | Switches MinGW install to choco-retry.sh under bash. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per PR review: if 'gh release view' fails transiently with the release actually present, the recreate dies on 'already exists' — re-check existence when create fails and continue if the release is there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-windows.yml:
- Around line 104-117: Update the “Read pinned Flutter version” step to require
exactly one matching flutter entry, trim its parsed value, and validate it
against the permitted Flutter version format before writing version to
GITHUB_OUTPUT. Reject values containing comments, trailing whitespace, malformed
versions, or duplicate matches, while preserving the existing error-and-exit
behavior.
In @.github/workflows/release.yml:
- Around line 693-696: Update the release-existence checks around gh release
view in the release workflow to query isDraft and isPrerelease, and only reuse
the tag when the draft status matches the expected recreate state and prerelease
status matches BUILD_TYPE. Reject mismatched or finalized releases instead of
proceeding, applying the same validation to both tag-check paths.
In `@scripts/ci/choco-retry.sh`:
- Around line 10-16: Update the retry loop around choco install to execute it
through PowerShell and capture the native full-width exit code via
$LASTEXITCODE, then retain the existing success handling for 0, 1641, and 3010
so reboot-required installs do not retry.
- Around line 13-16: Update the exit-code case in the Chocolatey retry script to
treat 1605 and 1614 as successful alongside 0, 1641, and 3010. Preserve the
existing comment and success handling, adding the two documented codes unless
the targeted packages are explicitly unable to return them.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 523f6892-2eb8-48c5-b869-c3be1f1c90a6
📒 Files selected for processing (5)
.github/workflows/build-windows-dll.yml.github/workflows/build-windows-service.yml.github/workflows/build-windows.yml.github/workflows/release.ymlscripts/ci/choco-retry.sh
Rework of the re-run fix per PR discussion: rather than recovering from
the draft's deletion (the 'Ensure release exists' guard, now removed),
stop deleting it. Deletion on failure is what deadlocked 'gh run rerun
--failed' — release-create never re-runs, so re-run uploads found no
release (v9.1.18-beta, v9.1.19-beta).
- Delete draft release: now only for test runs and nightlies, whose
drafts are throwaway by design.
- New 'Mark draft release failed': a failed tag release keeps its draft
with warning notes ('do not publish, re-run to continue'); a re-run's
notes update overwrites the warning before finalize publishes. exit 1
keeps finalize red on failure instead of green-after-cleanup.
- Drop the cleanup_on_failure input and CLEANUP_ON_FAILURE env: the
opt-out was unreachable for tag pushes (inputs are empty there, so the
expression was always true) and is moot now.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Reworked per discussion: instead of recovering from the draft's deletion (the "Ensure release exists" guard), the failure-path deletion is removed at the source — a failed tag release now keeps its draft with "do not publish, re-run to continue" notes, and only test runs and nightlies delete theirs. The guard (and the create-race handling it needed) is gone; the cleanup_on_failure input, which was unreachable on tag pushes anyway, is removed. Diagram and description updated. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/release.yml:1100
- This step prints a draft-release URL and claims the draft was kept even when
gh release viewfails and you log "no draft release found". That can be misleading (and links to a 404) if the release is genuinely missing or if theviewcall fails transiently. Consider only printing the draft URL / 'kept for re-run' message when the release is confirmed to exist.
else
echo "no draft release found to mark"
fi
echo "Build failed or cancelled - draft release kept for re-run"
echo "Draft: https://github.com/getlantern/lantern/releases/tag/$RELEASE_TAG"
scripts/ci/choco-retry.sh:25
- On failure after all retries, the script always exits with status 1, which loses the underlying Chocolatey exit code and makes failures harder to diagnose. Consider exiting with the last observed choco exit code instead (it will still fail the step).
echo "choco install $* failed after 5 attempts" >&2
exit 1
.github/workflows/release.yml:1037
- PR description says
upload-release-artifactsgained an "Ensure release exists" guard that recreates the draft on re-runs, but this workflow change appears to solve the rerun deadlock by keeping the draft on failure instead (removingcleanup_on_failureand narrowing deletion to test runs/nightlies). Please update the PR description to match the implemented approach, or add the missing guard if you still want automatic draft recreation when the release is absent.
# Only test runs and nightlies delete their draft — those drafts are
# throwaway by design. A failed real release deliberately KEEPS its
# draft (see "Mark draft release failed"): deleting it here deadlocked
# every `gh run rerun --failed`, because release-create does not re-run
# and the re-run uploads then found no release to upload to
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Line 1091: Update the release existence check around gh release view to query
and validate both isDraft and isPrerelease, requiring the release state to match
BUILD_TYPE before editing its notes. Fail closed for stale, published, or
mismatched releases so the warning is never written to an unsafe release.
- Around line 1074-1081: The failure-marker condition in the release workflow
must also detect failure of the “Publish release on success” step, not only
dependency-job results. Update the surrounding publication/failure-marker logic
to run with always() and distinguish successful publication from failed
publication, ensuring a draft retained after gh release edit "$RELEASE_TAG"
--draft=false receives the “do not publish” warning while preserving the
existing test-run and nightly exclusions.
- Around line 1092-1094: Update the release-finalize flow around gh release edit
so it restores the final release notes before changing the release from draft to
published. Preserve or regenerate the notes produced by
upload-release-artifacts, ensuring a later failed-job rerun cannot publish the
draft warning text; keep the existing --draft=false publication step afterward.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1233dd2e-515a-414f-a8cc-be0261cdbd9c
📒 Files selected for processing (1)
.github/workflows/release.yml
Per PR discussion the failed-build and cleanup-on-failure behavior stays as-is on main; this PR now only hardens the Windows toolchain installs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
myleshorton
changed the title
… review - bash on Windows truncates exit codes to 8 bits, so matching choco's MSI reboot codes (1641/3010) by value could never work; choco's --ignore-package-exit-codes makes it exit 0 unless it itself detected a failure, which also covers 1605/1614. - The flutter version parse now requires exactly one match and a version-shaped result before writing GITHUB_OUTPUT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/build-windows.yml:117
- The current
sedpattern can accidentally include trailing YAML comments/whitespace in the extracted version (e.g.,flutter: 3.41.0 # pinned), which would then be passed verbatim to flutter-action and likely fail. Tighten the capture to stop before whitespace or#, and consider enablingpipefailsince this step now uses a pipeline.
run: |
matches=$(sed -n 's/^[[:space:]]*flutter:[[:space:]]*"\{0,1\}\([0-9][^"]*\)"\{0,1\}[[:space:]]*$/\1/p' .github/flutter-version.yaml)
if [ "$(printf '%s\n' "$matches" | grep -c .)" -ne 1 ]; then
scripts/ci/choco-retry.sh:25
- On failure, this script always exits 1, which loses the underlying Chocolatey exit code and makes CI failures harder to diagnose. Consider exiting with the last
choco installexit code and include it in the final error message.
done
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/ci/choco-retry.sh:16
--ignore-package-exit-codeswithout an explicit list ignores all package exit codes, which can mask genuine installer failures (not just the reboot-required 1641/3010 cases mentioned in the comment). Restrict the ignored codes to 1641/3010 and align the script with the repo’s usualset -euo pipefailstrict mode.
# --ignore-package-exit-codes: MSI success-with-reboot codes (1641/3010) would
# otherwise surface as nonzero — and 8-bit-truncated by bash on Windows, so
# unmatchable by value — and read as failures. With the flag, choco exits 0
# unless it itself detected a failure.
set -u
.github/workflows/build-windows.yml:113
- In GitHub Actions,
shell: bashruns with-e/pipefail; when there are zero matches,grep -c .exits 1 and can terminate the step before your custom "expected exactly one" error message runs. Add|| trueso the count check works even when the count is 0.
if [ "$(printf '%s\n' "$matches" | grep -c .)" -ne 1 ]; then
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Hardens the Windows toolchain installs that failed two consecutive release builds (v9.1.18-beta on 2026-07-26, v9.1.19-beta on 2026-08-05) on
community.chocolatey.orggateway timeouts:scripts/ci/choco-retry.sh:choco installwith 5 attempts and linear backoff (exit codes 1641/3010 — reboot-required — count as success). Applied tomingw(build-windows, build-windows-service, build-windows-dll) andinnosetup.build-windowsnow passesflutter-version(parsed from the pinned.github/flutter-version.yaml) instead offlutter-version-file: with the file input,subosito/flutter-actionunconditionally runschoco install yqon Windows — the exact step that 504'd in both failed releases (action source). The direct version input skips that step entirely; the version stays pinned in the same file.Deliberately not in scope (discussed and dropped): changes to
release.yml's failed-build / cleanup-on-failure behavior. A failed run still deletes its draft release; the recovery for a re-run remains recreating the draft by hand beforegh run rerun --failed.Verification
choco-retry.shis shellcheck-clean.🤖 Generated with Claude Code
Summary by CodeRabbit