Repository navigation
ci(danger): re-check package.json through the api before failing - #2202
Conversation
JSONDiffForFile throws when the workflow's shallow checkout lacks the base commit, and the catch-all treated that as a dependency change, so scripts-only edits failed the lockfile guard (#2182). Read both versions through the GitHub API and compare the dependency keys; warn and skip only when that also fails.
|
Failed to generate code suggestions for PR |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Fixes the lockfile guard failing scripts-only PRs on shallow/rebased checkouts by falling back to depFieldsChangedViaApi, which re-reads the base and head package.json through the GitHub API and compares only the dependency fields when the local JSONDiffForFile throws. When both the local diff and the API read fail, checkLockfileGuard now warns and skips that file instead of unconditionally failing.
Worth a look
- Lockfile guard now skips enforcement when both local and API diffing fail —
dangerfile.ts:137· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 20 functions depend on the 20 functions this change touches.
Health — grade A; no new coupling hotspots.
Verification — 20 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 20 function(s) in the blast radius were not formally verified this run
|
1 participant
Root cause
The lockfile guard in
dangerfile.tswas already keyed on dependency fields viadanger.git.JSONDiffForFile. The false positive on #2178 came from its catch-all: indanger cithat call diffs with local git, and the reusable workflow checks out withfetch-depth: 50, so on a heavily rebased PR the base commit is missing, the call throws, and the catch setdepFieldChanged = true. Reproduced the other way round:danger pr(API-backed diff) on #2178 passes with the unchanged file.The issue text said the Dangerfile lives in the org repo; it lives here. Org PR LucasSantana-Dev/.github#16 only added a comment to the workflow and needs no follow-up.
Change
package.jsonthroughdanger.github.utils.fileContentsand compare the sameDEP_FIELDS.warnand skip that file instead of failing.Verification (local
danger pr, API path forced by making the local diff throw)scriptsonlyoverrideschanged, lockfile untouchedCloses #2182
Summary by cubic
Fixes #2182: the lockfile guard in
dangerfile.tsno longer fails on scripts-only edits when the local diff is unavailable. Previously, any local diff failure was treated as a dependency change; now the guard re-reads base and headpackage.jsonvia the API and compares the same dependency fields. Only if that API check also fails does it warn and skip the file instead of failing.Written for commit 7db8844. Summary will update on new commits.