Repository navigation
fix: match dependency snapshots on update pull requests - #2492
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
This was referenced Oct 28, 2026
4 tasks done
Contributor
🔍 PR Validation |
groupthinking
added a commit
that referenced
this pull request
Oct 29, 2026
## Summary - Fetch the dependency diff from GitHub's compare API after the existing review action, avoiding GitHub's template-object limit on large action outputs. - Save the raw response and a separate record with the exact base/head commits, review outcome, count, and SHA-256 digest. Missing, malformed, snapshot-warned, or graph-inconsistent API responses fail before evidence is saved. For changed npm lockfiles, verify returned package names, versions, and scopes against the exact base/head lockfile revisions. - Treat a change only to the dependency review workflow as a code change with no dependency graph delta. CI tests still validate the workflow. Actual manifest and lockfile changes still run dependency review. - Keep the existing vulnerability and license policy unchanged. Closes #2485 ## Paths changed - `.github/workflows/dependency-review.yml` - `scripts/ci/record_dependency_review.py` - `tests/unit/test_dependency_review_workflow.py` ## Validation - Focused `tests/unit/test_dependency_review_workflow.py` suite — **19 passed** on the final head, including warned empty/partial responses, a warning-free empty response, graph drift rejection, and a real Git rename regression. - `uvx ruff check scripts/ci/record_dependency_review.py tests/unit/test_dependency_review_workflow.py` — passed. - `git diff --check` — passed. ## Hosted validation and limits The current head is `148cbf55fdce412255d2a61780459143521fa4c4`. Its hosted checks are being verified on that exact head; see the live check list for final status. The dependency-review job passes through its no-delta path because this PR changes no dependency manifest or lockfile. Its hosted run tests the code-only path, while a real dependency PR must still validate the evidence recorder against a warning-free comparison. The Vercel Agent Review found that Git rename detection could omit an old manifest path; `--no-renames` and a real Git rename regression test address that finding. The review thread is resolved. An earlier [hosted artifact on superseded head `80fdf2b`](https://github.com/groupthinking/EventRelay/actions/runs/40657379137) contained a raw 276-entry comparison whose bytes matched its recorded digest. GitHub also warned that the base had 3 dependency snapshots and the head had 1. That comparison may have unexpected removals and is **not complete evidence** under the new guard. The prior local trial against #2479 saved 1,390 raw changes before snapshot warnings were checked; it likewise cannot establish a complete comparison. ## Live recorder check and graph scope limitation At the current code head, the local recorder rejects the live, warning-free #2495 comparison **without writing evidence**, because its removed root fast-uri 3.1.7 entry is labeled `runtime` while the exact base lockfile marks it `dev: true`. It also rejects current `main` versus #2452 before writing evidence because GitHub reports a 3-base/1-head snapshot warning. In contrast, it accepts controlled off-main [#2496](#2496): the v0.2.0 comparison has only the intended root fast-uri 3.1.7 removal and 3.1.9 addition, both `development`, and the saved raw bytes match the recorded SHA-256. #2496 was closed unmerged. The older v0.1.6 component-detection action caches packages by PURL and can apply one development flag across manifests. The v0.2.0 action used on #2496 maps each manifest's scope separately; that version is proposed in stacked draft #2492. The new recorder validation reduces the risk of accepting a wrong npm lockfile scope or a change attributed to an unchanged manifest. It does **not** establish complete snapshot parity or accurate non-npm package metadata for real updates against current `main`. Inspect the full comparison, including manifests, packages, scopes, vulnerabilities, and licenses, before a real dependency PR verdict. A fresh compare for #2452 still reports the base 3/head 1 snapshot mismatch. Once this guard is on `main`, that update will require complete matching snapshots before its dependency evidence can pass. The previously approved per-file Python scan timeout and declared-dependency fallback are unchanged. Python fallback can omit transitive dependencies when a requirements scan times out.
groupthinking
marked this pull request as ready for review
October 29, 2026 06:14
Contributor
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Name-only dependency traversal misclassifies dev-only package versions and their children as runtime dependencies in split-version locks, and the recorder accepts these incorrect scopes.
1 open issue
What changed in this PR
Repairs EventRelay’s dependency-review pipeline by aligning base/head snapshot submissions and validating comparison evidence against exact lockfile contents. The controlled off-main checks support the repair; verification against a refreshed main remains outstanding under #2490.
Changes:
- Replace focused scans with matching full-tree scans and one combined npm/uv lock snapshot.
- Validate complete lockfile changes, including nested npm versions and dependency scopes.
- Add regression coverage and increase bounded scan and snapshot-retry windows.
| File | Description |
|---|---|
tests/unit/test_submit_uv_lock_snapshot.py |
Tests uv scope classification, submission, and evidence validation. |
tests/unit/test_submit_npm_lock_snapshot.py |
Tests nested versions, duplicate scopes, and combined submission. |
tests/unit/test_dependency_review_workflow.py |
Tests workflow selection, stale branches, and evidence completeness. |
scripts/ci/submit_uv_lock_snapshot.py |
Derives uv dependency scopes from root reachability. |
scripts/ci/submit_npm_lock_snapshot.py |
Submits complete npm manifests and scoped uv data together. |
scripts/ci/record_dependency_review.py |
Checks reported changes against exact npm and uv lock contents. |
.github/workflows/dependency-review.yml |
Aligns snapshot paths, updates detection, and extends time limits. |
Review effort: Balanced
Comment on lines
+32
to
+36
| if (name, extras) in visited: | ||
| continue | ||
| visited.add((name, extras)) | ||
| reachable.add(name) | ||
| for package in packages_by_name.get(name, []): |
This branch was successfully deployed
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.
Current status — approved exact-head repair
Head
85f1815ddb7ec121680d067d165e18eb40d90edatargetsmainat merge commite010c9f3cec5ae72be16814d0841578ea9f8ded2, after #2487 merged at exact head148cbf55fdce412255d2a61780459143521fa4c4. The verified merge commit has the same tree as that head, so this PR's seven-file diff is unchanged. It adds complete npm lockfile coverage, preserves the scopeduv.lockclassification, and combines both lock types into one manual snapshot so GitHub's pinned review action can compare the full base and head graphs. The controlled #2501 test passed on an exact four-file dependency update. This has not yet produced a passing dependency comparison against the currentmainor cleared the original review queue.The exact-head dependency review run 40760369080 succeeded through its code-only no-delta path: the full-tree and combined lock submissions were correctly skipped because this PR changes workflow code and tests, not dependency inputs. Before the automatic base retarget, all 30 hosted checks at this same code head finished: 27 successful, 3 skipped, zero failed or pending. The retarget did not run a new dependency comparison. A green no-delta check does not substitute for the controlled #2501 dependency test or a postmerge real PR test. A description-only edit may rerun metadata checks.
Canonical issue
Fixes #2505 on merge under the repository's issue-link rule. Parent #2490 stays open until a repaired
mainsnapshot and a refreshed real dependency PR pass the full comparison. #2487 merged at the approved head, and GitHub automatically retargeted this draft tomainate010c9f3cec5ae72be16814d0841578ea9f8ded2. The head and seven-file diff remain unchanged. Hayden approved marking this exact head ready and merging it. Main-based dependency comparison remains a required postmerge verification.Changes
.github/workflows/dependency-review.yml. Submit the same full-tree component-detection scan onmainpushes and same-repository dependency PR heads at the exact commit SHA and ref. Keep the verified upstream component-detection v0.2.0 pin from fix: bump advanced-security/component-detection-dependency-submission-action from 0.1.6 to 0.2.0 #2397 and the existing severity, license and advisory policy.maininto a dependency update in that PR. Real changed manifests and locks remain subject to review.uv.lockmanifest in one manual snapshot after the full-tree scan. The scoped uv manifest treats packages reached only throughdevordocsas development. The evidence recorder rejects missing, extra, wrong-manifest, wrong-version or wrong-scope rows against the exact changed lockfiles.PipReportscan from 120 to 240 seconds and the job window to 30 minutes after an observed timeout produced unrelated requirements-file removals. A timeout still falls back to declared dependencies and can omit transitive packages; the guard rejects unrelated graph drift. Keep the 600-second snapshot-warning retry and warning rejection.Changed paths
.github/workflows/dependency-review.ymlscripts/ci/record_dependency_review.pyscripts/ci/submit_npm_lock_snapshot.pyscripts/ci/submit_uv_lock_snapshot.pytests/unit/test_dependency_review_workflow.pytests/unit/test_submit_npm_lock_snapshot.pytests/unit/test_submit_uv_lock_snapshot.pyValidation
uv run --no-project --with pytest --with pyyaml python -m pytest -q -o addopts='' tests/unit/test_submit_npm_lock_snapshot.py tests/unit/test_submit_uv_lock_snapshot.py tests/unit/test_dependency_review_workflow.py— 31 passed in 3.10s at the source head.uv run --no-project --with ruff ruff checkandruff format --checkon those affected Python files, plusgit diff --check, passed.uv.lockdelta from original fix: bump the uv group across 1 directory with 16 updates #2486 on matching off-main branches. Final hosted run 40738274345, attempt 2 passed policy, recorder and upload with 65 exact rows (34 additions, 31 removals), correct package names, versions, manifests and development scopes, and no reported vulnerabilities.ddtrace-internalhad unknown license metadata. The first attempt raced the base snapshot; the unchanged rerun passed. test: verify exact uv lock delta under repaired dependency review #2499 was closed unmerged.brace-expansion2.1.4→2.1.7 was missing. Independent local npm audits of both lockfiles found no new advisory and fewer findings after the update (fetch 18→14; SDK 10→9). That green hosted check was insufficient; test: verify exact #2355 multi-lock dependency review #2500 was closed unmerged.eb6e556ef9eb8df80250613291c73abfb59b9f0eand head050f211b9715918f52c488a912d3ae797e76d68e. Base run 40759330706 submitted the full-tree and combined lock snapshots; head run 40759346059 passed policy, recorder and upload. Its artifact and independent GitHub comparison contain all 12 exact lock changes, only from the two changed npm locks, with the nestedbrace-expansionrow, correct scopes, zero added vulnerabilities, and added licenses MIT or BSD-3-Clause. GitHub's comparison with the review action'sper_page=5request is also warning free. test: verify complete nested npm lock dependency review #2501 was closed unmerged; its temporary refs were deleted.Remaining gates
main, and its unchanged seven-file diff was reviewed.85f1815ddb7ec121680d067d165e18eb40d90edaready and merging fix: match dependency snapshots on update pull requests #2492 after review of the main snapshot uncertainty.mainfull-tree and combined lock submission, refresh one original manifest-changing PR onto that main, and require a warning-free hosted policy verdict plus complete artifact matching its actual dependency files. Then refresh and recheck the remaining original dependency PRs. Do not claim the original 21 dependency failures are resolved until this passes.