ci(workflows): catch & auto-fix Dependabot pnpm.overrides drift#253
Open
miguelpeixe wants to merge 5 commits into
Open
ci(workflows): catch & auto-fix Dependabot pnpm.overrides drift#253miguelpeixe wants to merge 5 commits into
miguelpeixe wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens CI against pnpm override drift (especially from lockfile-only Dependabot PRs) by ensuring dependency installs run when root dependency files change, and by automatically syncing package.json’s pnpm.overrides to match pnpm-lock.yaml on Dependabot PRs.
Changes:
- Update the CI change detector to treat edits to
pnpm-lock.yaml, rootpackage.json, orpnpm-workspace.yamlas requiring theInstall depsjob. - Add a new
pull_request_targetworkflow that, for Dependabot PRs touchingpnpm-lock.yaml, updates only existingpnpm.overrideskeys inpackage.jsonto match the lockfile and pushes the fix commit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/workflows/dependabot-sync-overrides.yml | New workflow to auto-sync existing pnpm.overrides entries in package.json with pnpm-lock.yaml on Dependabot PRs and push a fix commit. |
| .github/workflows/ci.yml | Ensure Install deps runs when root dependency files change, preventing lockfile/config mismatches from merging undetected. |
Without `install` in ci-ok's needs, a `--frozen-lockfile` failure on a root-deps-only PR (empty js/php matrices) is masked by the skipped matrix jobs and ci-ok passes green -- silently defeating the path-trigger safety net added earlier in this branch. Also: drop `2>/dev/null` from the new diff so a bad BASE_REF fails loudly instead of silently flipping ROOT_DEPS_CHANGED to false, and collapse the any=true/false if/else into one echo against the already-true/false var.
Address review findings on the Dependabot sync workflow:
- Add per-PR concurrency group (cancel-in-progress) so racing events
on the same PR don't push and fail non-fast-forward.
- Gate on pull_request.user.login (stable PR author) instead of
github.actor (per-event pusher) so a human pushing a fix to a
Dependabot PR still triggers the sync.
- Belt-and-braces loop-break: skip when HEAD already is the sync's
own commit subject (the actor change above removed the incidental
loop-break the original gate relied on).
- Fetch base.sha before diffing so a post-event Dependabot rebase
doesn't make the diff error out with `fatal: bad revision`.
- Use grep -Fqx so the `.` in `pnpm-lock.yaml` isn't treated as a
regex metachar (matches ci.yml's escaped form).
- Python: coerce both sides of the compare/assign with str() so a
future numeric-looking override (`foo: 8`, `foo: 5.3`) doesn't end
up written as an unquoted JSON number.
- Python: pass ensure_ascii=False so non-ASCII in package.json isn't
rewritten as \uXXXX, producing massive unrelated diff noise.
- Python: defensive `(pkg.get('pnpm') or {})` for `"pnpm": null`.
- Python: warn (don't silently no-op) when package.json has override
keys missing from the lockfile -- `--frozen-lockfile` still rejects
that mismatch and the sync can't fix it (removal is a human change).
- Explicit `git push origin HEAD:$HEAD_REF` so the push target is
unambiguous if checkout left no upstream.
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.
All Submissions:
Changes proposed in this Pull Request:
Follow-up to #252. Adds two independent layers of defense against the lockfile/
package.jsonpnpm.overridesdrift that brokemainin run 27210804365:1. CI safety net —
.github/workflows/ci.ymlThe
changed-packagesdetector currently skipsInstall depswhen no source package is affected. That misses a class of PRs that only modify root dependency files (the Dependabot lockfile-only case, but also any manual root edit). The detector now additionally checksgit difffor changes topnpm-lock.yaml, rootpackage.json, orpnpm-workspace.yaml; if any are touched,any=truesoInstall depsruns. The JS / PHPCS / PHPUnit matrix jobs continue to skip cleanly because their matrices stay empty.2. Dependabot auto-fix —
.github/workflows/dependabot-sync-overrides.yml(new)Runs on every Dependabot PR via
pull_request_target. When the PR diff touchespnpm-lock.yaml, it compares the lockfile'soverrides:block againstpackage.json'spnpm.overridesand, for any key already present inpackage.jsonwhose value differs, updatespackage.jsonto match. Commits and pushes via the sameGH_TOKENPAT pattern used bydependabot-branch-auto-update.yml, so CI re-fires against the synced state and the existing auto-merge can complete.Safety:
github.actor == 'dependabot[bot]'.pull_request_targetruns the workflow definition frommain, not from the PR, so a hostile PR cannot modify the workflow that holds the PAT.package.json.pnpm.overrides; it never adds or removes entries. So a hostile lockfile diff cannot inject arbitrary overrides intopackage.json.dependabot[bot]so the existingauto-merge.yml(which checks PR author) continues to treat the PR as auto-mergeable. The repo does not dismiss stale reviews, so the prior approval survives the sync commit.How to test the changes in this Pull Request:
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml')); yaml.safe_load(open('.github/workflows/dependabot-sync-overrides.yml'))").package.jsonmismatch, pushes a follow-up commit, CI re-runs and passes, auto-merge completes.pnpm-lock.yaml(e.g. add then immediately revert a whitespace change) and confirmInstall depsruns in CI on that PR rather than being skipped.Other information:
What this does not cover:
package.json— by design we don't touch new keys. If we ever start tracking a new transitive override, a human still adds it topackage.jsonfirst.