Skip to content

fix: save dependency review evidence for large updates - #2487

Merged
groupthinking merged 4 commits into
mainfrom
codex/fix-dependency-review-evidence
Oct 29, 2026
Merged

groupthinking merged 4 commits into
mainfrom
codex/fix-dependency-review-evidence

Conversation

@groupthinking

@groupthinking groupthinking commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

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 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: 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.

Copilot AI balanced review requested due to automatic review settings October 28, 2026 08:31
@vercel

vercel Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
v0-uvai Ready Ready Preview, v0 Oct 28, 2026 3:34pm UTC

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: The number of snapshots compared for the base SHA (3) and the head SHA (1) do not match. You may see unexpected removals in the diff.
Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

  • apps/backend/requirements.txt
  • docs/design/enterprise-demo/package.json
  • docs/guides/requirements.txt
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-code-assistant/package.json
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-communication-hub/package.json
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-creative-studio/package.json
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-data-analysis/package.json
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-knowledge-management/package.json
  • docs/knowledge_prototypes/mcp-servers/legacy_proxies/server-workflow-automation/package.json
  • docs/knowledge_prototypes/mcp-servers/mcp-alexnet/requirements.txt
  • docs/knowledge_prototypes/mcp-servers/mcp_youtube-0.2.0/uv.lock
  • docs/knowledge_prototypes/universal-automation-service/monitoring/package.json
  • docs/knowledge_prototypes/universal-automation-service/requirements.txt
  • requirements.txt

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The evidence recorder ignores dependency-graph snapshot warnings, allowing successful API responses with incomplete dependency data to be recorded as complete evidence.

1 open issue
What changed in this PR

Addresses #2485 by moving dependency-review evidence collection from large Actions outputs to GitHub’s compare API, while retaining the existing vulnerability and license policy.

Changes:

  • Fetches dependency data for exact base and head commits without workflow-template expansion.
  • Saves the raw response alongside review metadata, change count, and SHA-256 digest.
  • Updates artifact uploads and tests for large responses and evidence integrity.
File Description
tests/​unit/​test_dependency_review_workflow.py Tests large-response preservation, metadata, digest, and workflow wiring.
scripts/​ci/​record_dependency_review.py Adds API-based dependency evidence recording.
.github/​workflows/​dependency-review.yml Runs the recorder and uploads the evidence directory.

Review effort: Balanced

Comment thread scripts/ci/record_dependency_review.py Resolved
Comment thread scripts/ci/record_dependency_review.py Resolved
@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR Validation

⚠️ Large PR detected (519 lines changed)

@groupthinking
groupthinking merged commit e010c9f into main Oct 29, 2026
37 checks passed
@groupthinking
groupthinking deleted the codex/fix-dependency-review-evidence branch October 29, 2026 05:41
@linear-code

linear-code Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

GRV-858

@groupthinking
groupthinking restored the codex/fix-dependency-review-evidence branch October 29, 2026 05:42
groupthinking added a commit that referenced this pull request Oct 29, 2026
## Current status — approved exact-head repair

Head `85f1815ddb7ec121680d067d165e18eb40d90eda` targets `main` at merge
commit `e010c9f3cec5ae72be16814d0841578ea9f8ded2`, after
[#2487](#2487) merged at
exact head `148cbf55fdce412255d2a61780459143521fa4c4`. 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
scoped `uv.lock` classification, 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](#2501) test
passed on an exact four-file dependency update. This has **not** yet
produced a passing dependency comparison against the current `main` or
cleared the original review queue.

The exact-head [dependency review run
40760369080](https://github.com/groupthinking/EventRelay/actions/runs/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 `main` snapshot and a refreshed real
dependency PR pass the full comparison. #2487 merged at the approved
head, and GitHub automatically retargeted this draft to `main` at
`e010c9f3cec5ae72be16814d0841578ea9f8ded2`. 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

- Remove focused one-ecosystem snapshot staging from
`.github/workflows/dependency-review.yml`. Submit the same full-tree
component-detection scan on `main` pushes and same-repository dependency
PR heads at the exact commit SHA and ref. Keep the verified upstream
component-detection v0.2.0 pin from #2397 and the existing severity,
license and advisory policy.
- Treat Dependabot configuration changes as configuration, and compare a
PR head to its merge base when detecting new dependency inputs. A stale
branch cannot turn a change made only on `main` into a dependency update
in that PR. Real changed manifests and locks remain subject to review.
- Include **every distinct name/version** from the tracked npm
lockfiles, including nested second versions that component detection can
omit. Group duplicate installed paths with runtime scope if any path is
runtime. Submit those npm manifests and the scoped root `uv.lock`
manifest in **one** manual snapshot after the full-tree scan. The scoped
uv manifest treats packages reached only through `dev` or `docs` as
development. The evidence recorder rejects missing, extra,
wrong-manifest, wrong-version or wrong-scope rows against the exact
changed lockfiles.
- Increase the bounded per-file Python `PipReport` scan 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.
- Add regression coverage for workflow selection, stale branches, npm
nested versions and duplicate scope, combined one-request submission,
scoped uv reachability, and exact evidence validation.

## Changed paths

- `.github/workflows/dependency-review.yml`
- `scripts/ci/record_dependency_review.py`
- `scripts/ci/submit_npm_lock_snapshot.py`
- `scripts/ci/submit_uv_lock_snapshot.py`
- `tests/unit/test_dependency_review_workflow.py`
- `tests/unit/test_submit_npm_lock_snapshot.py`
- `tests/unit/test_submit_uv_lock_snapshot.py`

## Validation

- Focused workflow, recorder, npm and uv snapshot suite: `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 check` and
`ruff format --check` on those affected Python files, plus `git diff
--check`, passed.
- Controlled uv
[#2499](#2499) used the
exact `uv.lock` delta from original #2486 on matching off-main branches.
Final [hosted run 40738274345, attempt
2](https://github.com/groupthinking/EventRelay/actions/runs/40738274345)
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-internal` had unknown
license metadata. The first attempt raced the base snapshot; the
unchanged rerun passed. #2499 was closed unmerged.
- Earlier controlled
[#2500](#2500) used
original #2355's exact two npm lockfile changes and exposed a **10/12
coverage gap** in component detection: nested development
`brace-expansion` 2.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; #2500 was closed unmerged.
- Final controlled
[#2501](#2501) used the
exact four-file #2355 patch between base
`eb6e556ef9eb8df80250613291c73abfb59b9f0e` and head
`050f211b9715918f52c488a912d3ae797e76d68e`. [Base run
40759330706](https://github.com/groupthinking/EventRelay/actions/runs/40759330706)
submitted the full-tree and combined lock snapshots; [head run
40759346059](https://github.com/groupthinking/EventRelay/actions/runs/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 nested `brace-expansion` row, correct
scopes, zero added vulnerabilities, and added licenses MIT or
BSD-3-Clause. GitHub's comparison with the review action's `per_page=5`
request is also warning free. #2501 was closed unmerged; its temporary
refs were deleted.
- The final #2501 success followed two diagnosed issues: an unchanged
Python requirements file generated 83 false removals after a 120-second
fallback, and the pinned review action's five-item API page returned a
**0-base/3-head** warning when full-tree, uv and npm were submitted
separately (six snapshots across both sides). The same comparison with a
larger page was warning free and had the correct 12 rows. Combining uv
and npm manifests leaves two submissions per side and yielded the
passing hosted result. Earlier #2493–#2496 diagnostics documented the
initial main snapshot mismatch, the upstream manifest mapping update and
scope validation; those results remain historical evidence, not a
main-based pass.

## Remaining gates

- [x] #2487 merged at its approved head; this draft automatically
retargeted to `main`, and its unchanged seven-file diff was reviewed.
- [x] Complete npm lock coverage and correct uv scope demonstrated in
controlled hosted canaries, with the five-item review comparison warning
free on #2501.
- [x] Exact-head #2492 code-only dependency check and other hosted
checks passed or skipped before the base retarget (27 successful, 3
skipped). This does not prove a main-based dependency update comparison.
- [x] Hayden approved marking exact head
`85f1815ddb7ec121680d067d165e18eb40d90eda` ready and merging #2492 after
review of the main snapshot uncertainty.
- [ ] After the merge, confirm the new `main` full-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.
- [ ] Resolve any outstanding review findings and merge decisions for
the original PRs independently of this shared repair.

This branch was successfully deployed

1 active deployment
Preview — 148cbf55 Deployed Oct 28, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix dependency review evidence failure on large pull requests

2 participants