Fix media workflow npm lockfile assumptions - #15
Conversation
Co-authored-by: myself-aas <129354542+myself-aas@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Summary by QodoRemove npm lockfile assumptions from media verification
AI Description
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Lockfile drift passes CI
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
||
| - name: Install Dependencies | ||
| run: npm ci | ||
| run: npm install |
There was a problem hiding this comment.
1. Lockfile drift passes ci 🐞 Bug ☼ Reliability
Replacing npm ci with npm install allows this job to silently reconcile an out-of-date lockfile instead of failing, so dependency changes can pass CI while the committed installation remains unusable through the repository's documented npm ci setup. This weakens reproducibility and can defer dependency failures until development or deployment.
Agent Prompt
## Issue description
The media workflow uses `npm install`, which can update or reconcile lockfile state rather than rejecting drift between `package.json` and the committed lockfile.
## Issue Context
The repository documents `npm ci` as its installation method, and its dependency declarations contain version ranges. CI should install exactly the committed dependency graph and fail when the manifest and lockfile disagree.
## Fix Focus Areas
- .github/workflows/media-verify.yml[22-23]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: 'npm' |
There was a problem hiding this comment.
2. Dependency cache unnecessarily removed 🐞 Bug ➹ Performance
Removing the setup-node npm cache eliminates reusable package-data caching on every push and pull-request run, increasing installation latency and exposure to registry/network failures. The modified repository has a lockfile available for generating the cache key, so the removed cache configuration remains applicable.
Agent Prompt
## Issue description
The workflow removes setup-node's npm cache even though the repository has a lockfile that can key the cache.
## Issue Context
The media workflow executes for pushes and pull requests and installs the project's full dependency graph. setup-node supports caching npm's global package data using the lockfile hash.
## Fix Focus Areas
- .github/workflows/media-verify.yml[17-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🟡 Changes recommended
The repo contains a package-lock.json, so the workflow should keep npm ci (and npm caching) to preserve reproducibility and avoid unnecessary CI slowdowns.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the Media Verification GitHub Actions workflow’s Node dependency installation strategy. This is intended to avoid assumptions around npm lockfiles in CI, impacting how dependencies are installed and cached for the media:check script.
Changes:
- Removed
actions/setup-nodenpm caching configuration. - Switched dependency installation from
npm citonpm install.
File summaries
| File | Description |
|---|---|
| .github/workflows/media-verify.yml | Adjusts Node setup caching and dependency installation command for the media verification job. |
Review details
- Files reviewed: 1/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| with: | ||
| node-version: 20 | ||
| cache: 'npm' | ||
|
|
||
| - name: Install Dependencies | ||
| run: npm ci | ||
| run: npm install |
No description provided.