Skip to content

fix: Add retry mechanism for .nvmrc and yarn.lock downloads#63

Open
Mrtenz wants to merge 7 commits into
mainfrom
mrtenz/retry-download
Open

fix: Add retry mechanism for .nvmrc and yarn.lock downloads#63
Mrtenz wants to merge 7 commits into
mainfrom
mrtenz/retry-download

Conversation

@Mrtenz
Copy link
Copy Markdown
Member

@Mrtenz Mrtenz commented Mar 17, 2026


Note

Medium Risk
Touches the CI environment bootstrapping path; retries are low risk, but uninstalling global Yarn and changing download behavior could affect workflows that implicitly relied on a preinstalled Yarn or on immediate download failure semantics.

Overview
Improves resiliency of the composite action by wrapping the .nvmrc and yarn.lock download steps with MetaMask/action-retry-command@v1, reducing flaky failures when fetching from raw.githubusercontent.com.

Also adds a setup step to npm uninstall -g yarn before corepack enable, ensuring the action uses the Corepack-managed Yarn rather than any globally installed Yarn when setup is not skipped.

Reviewed by Cursor Bugbot for commit d907392. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread action.yml

- name: Remove globally installed Yarn
if: ${{ steps.try-skip-setup.outputs.cache-hit != 'true' }}
run: npm uninstall -g yarn
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Had to add this to fix compatibility with Windows, not sure why Yarn inside MetaMask/action-retry-command breaks otherwise. PATH issue?

Comment thread action.yml

- name: Synthesize our node version from inputs.node-version and .nvmrc
id: compute-node-version
run: |
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Diff is a bit hard to read, but essentially the only change here is wrapping everything in MetaMask/action-retry-command.

@Mrtenz Mrtenz marked this pull request as ready for review March 17, 2026 13:40
@Mrtenz Mrtenz requested a review from a team as a code owner March 17, 2026 13:40
@Mrtenz Mrtenz requested a review from HowardBraham March 17, 2026 13:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves resilience of the composite GitHub Action by reducing failures from transient network errors when determining Node/Yarn state during CI setup.

Changes:

  • Wrap .nvmrc download + Node version computation in MetaMask/action-retry-command.
  • Wrap yarn.lock download (used for cache-based setup skipping) in MetaMask/action-retry-command.
  • Add a step to uninstall globally-installed Yarn before corepack enable on the non-cache path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread action.yml

- name: Remove globally installed Yarn
if: ${{ steps.try-skip-setup.outputs.cache-hit != 'true' }}
run: npm uninstall -g yarn
Copy link
Copy Markdown
Contributor

@HowardBraham HowardBraham left a comment

Choose a reason for hiding this comment

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

This change is failing on metamask-extension, run here

I think we may need to update node to 24, and perhaps we should also update yarn to newest? Just make sure it works on your repos too.

Why npm uninstall -g yarn is necessary is confusing me. Could be solved with version updates?

@HowardBraham HowardBraham force-pushed the mrtenz/retry-download branch from 2b8760c to d907392 Compare April 20, 2026 06:40
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d907392. Configure here.

Comment thread action.yml

echo "node-version=$NODE_VERSION" >> "$GITHUB_OUTPUT"
shell: bash
echo "node-version=$NODE_VERSION" >> "$GITHUB_OUTPUT"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Step output lost through composite action wrapping

High Severity

The compute-node-version step was changed from run: to uses: MetaMask/action-retry-command@v1. The command inside writes node-version to $GITHUB_OUTPUT, but in GitHub Actions, outputs set within a composite action's internal steps are not automatically propagated to the calling step. Unless action-retry-command explicitly declares and forwards the node-version output (which a generic retry action almost certainly does not), steps.compute-node-version.outputs.node-version will be empty everywhere it's referenced — breaking setup-node, all cache keys, and the action's own computed-node-version output.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d907392. Configure here.

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.

4 participants