Repository navigation
fix: let Renovate link node_modules when updating lockfiles - #10311
Conversation
| "dependencyDashboardApproval": true, | ||
| "minimumReleaseAge": "3 days", | ||
| "postUpdateOptions": ["yarnDedupeHighest"], | ||
| "skipInstalls": false, |
There was a problem hiding this comment.
Can't we just disable the post-install for Renovate? There's no reason to run it.
Bit hacky, but this is how we do it in the extension:
There was a problem hiding this comment.
Where are we doing this yarn run postinstall then? 🤔
There was a problem hiding this comment.
we're not, yarn is. it's a root lifecycle script so yarn runs it itself after the Link step, that's where the yarn run ... <scriptName> in the error comes from.
and it never gets to the script body anyway. swapped it for a bare node -e "console.log('hi')" and same failure.
There was a problem hiding this comment.
So with yarn install --mode=update-lockfile, Yarn still runs postinstall, even though it isn't able to? That seems confusing.
There was a problem hiding this comment.
nah you were right to doubt it, i had it wrong. yarn doesn't run postinstall in that mode at all.
it's the lavamoat allow-scripts plugin in .yarnrc.yml. its afterAllInstalled hook shells out to yarn run allow-scripts, which needs the node_modules state file update-lockfile never writes. dropping the postinstall changes nothing, dropping the plugin makes it pass.
same fix though, skip-build runs the Link step. PR description updated.
There was a problem hiding this comment.
Right, that makes more sense.
Maybe we should add an environment variable to the allow-scripts plugin (or detect the install mode, if that's possible) so we can skip it when it's not needed instead?
I can approve this for now, but maybe look into that as follow-up.
There was a problem hiding this comment.
Maybe we should add an environment variable to the allow-scripts plugin (or detect the install mode, if that's possible) so we can skip it when it's not needed instead?
I had the same problem when using Renovate on my Prettier plugin, and this is the same idea I landed on 😄. I forgot to mention it, and haven't implemented it yet. Very easy to do though.
## Explanation After MetaMask#10311 the lockfile update works and Renovate pushes a branch, but still no pull request. It aborts one step later: ``` INFO: Branch created (commitSha 8aadb64...) DEBUG: Updating renovate/stability-days status check state to green POST https://api.github.com/repos/MetaMask/core/statuses/8aadb644... = 403 GitHub failure: Resource not accessible by integration x-accepted-github-permissions: statuses=write DEBUG: Caught error setting branch status - aborting Error: integration-unauthorized INFO: Repository has changed during renovation - aborting ``` Renovate posts a commit status on every branch it pushes, and our token grant has no `statuses` scope, so the 403 ends the whole repository run before `ensurePr`. The closing "Repository has changed" line is just how that abort surfaces, nothing had actually changed. The alternative was disabling the checks with `statusCheckNames`, but `renovate/artifacts` and `renovate/config-validation` are how a partially failed lockfile update or a bad `renovate.json` show up on the pull request instead of only in a debug log, so they are worth keeping. > [!IMPORTANT] > Depends on consensys-vertical-apps/token-exchange-service#182, which grants the permission. Merging this first makes the token exchange reject the request and the job fail outright, so it stays a draft until that lands. ## References Follows MetaMask#10311. Part of WPC-1161. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Workflow-only permission scope change for the Renovate bot token; no application runtime impact, but the job fails until the token exchange service allows `statuses: write`. > > **Overview** > Renovate was failing after pushing branches because the exchanged GitHub token could not set commit statuses (`403`, `statuses=write` required), which aborted the run before pull requests were created. > > This adds **`statuses: write`** to the permissions requested in the Renovate workflow’s `get-token` step so Renovate can mark checks such as `renovate/stability-days` and keep **`renovate/artifacts`** / **`renovate/config-validation`** visible on PRs. > > **Note:** The token exchange service must grant the same permission (see PR dependency); merging before that service change will cause token requests to fail. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 481f6b7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Explanation MetaMask#10316 broke the Renovate job outright. Asking the token exchange for `statuses: write` now fails before Renovate even starts: ``` ##[error]Token exchange failed: 500 Internal Server Error {"status":"error","message":"Failed to create installation token from GitHub"} ``` That is GitHub refusing to mint the token, not the policy rejecting it. consensys-vertical-apps/token-exchange-service#182 allows the permission, but the `metamask-ci` App installation does not hold Commit statuses, and a policy can only narrow what the App already has. Granting it means an org level App permission change that every installation has to accept, which is a lot of process for four informational checks. So this reverts the permission request and stops Renovate posting the statuses at all. With every entry in `statusCheckNames` set to null, Renovate skips the POST rather than 403ing on it ([`status-checks.ts:91`](https://github.com/renovatebot/renovate/blob/main/lib/workers/repository/update/branch/status-checks.ts#L91)), which was what aborted the run before the pull request got created. What we lose is the checks appearing on the pull request. Artifact and config errors still show up in the run log and on the dependency dashboard. `minimumReleaseAge` is unaffected, it is enforced internally whether or not the status is posted. ## References Reverts MetaMask#10316. Follows MetaMask#10311. Part of WPC-1161. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > CI-only changes to Renovate permissions and config with no runtime or application code impact. > > **Overview** > Fixes the Renovate workflow failing at token exchange by **removing `statuses: write`** from the MetaMask token-exchange permissions in `.github/workflows/renovate.yml`. The `metamask-ci` app installation does not have commit-status scope, so requesting it caused token minting to fail before Renovate could run. > > **Disables Renovate’s GitHub commit statuses** by adding `statusCheckNames` in `renovate.json` with `artifactError`, `configValidation`, `mergeConfidence`, and `minimumReleaseAge` set to `null`, so Renovate skips posting those checks instead of erroring. Dependency updates and internal rules like `minimumReleaseAge` still apply; only the PR-visible status checks are dropped. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e5288f2. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Explanation
Approving an update on the Dependency Dashboard (#10300) produced no pull request. Renovate built the branch, hit a
lock file error, and reportedNo files to commit. Underneath that:Renovate runs
yarn install --mode=update-lockfile, which skips the Link step, sonode_modulesand its install state file are never written. Our.yarnrc.ymlloads LavaMoat'splugin-allow-scripts, whoseafterAllInstalledhook runsyarn run allow-scripts.yarn runneeds that state file to resolve the binary, so it exits non zero, and Renovate reads that as a lock file error and throws the updated lockfile away.Isolated it by running
yarn install --mode=update-lockfilecold onmainwith each piece removed in turn:.yarnrc.ymlpluginpostinstallThe third row also shows Yarn does not run root lifecycle scripts in this mode at all, so the root
postinstallis not involved.Setting
skipInstalls: falsemakes Renovate useyarn install --mode=skip-buildinstead (source): the Link step runs, so the state file exists andallow-scriptssucceeds, while dependency build scripts stay skipped. Cold run onmain, exit 0 in 26 seconds.This also reverts #10303. The
! Corepack is about to downloadline is a notice on stderr, not a blocking prompt, and was never the cause.References
Reverts #10303. Follows #10298. Part of WPC-1161.
Checklist