build: validate dependency updates through CFS - #1096
build: validate dependency updates through CFS#1096wenyt (wenytang-ms) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The CFS path filter and npm registry handling need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Azure DevOps PR validation through CFS for npm and Maven/Tycho dependency updates, with isolated caches, while removing scheduled npm Dependabot updates.
Changes:
- Adds path-filtered CFS validation jobs.
- Configures isolated npm and Maven caches.
- Restricts existing CI behavior for PR builds.
- Removes npm Dependabot scheduling.
File summaries
| File | Summary | Review findings |
|---|---|---|
.github/dependabot.yml |
Removes scheduled npm updates. | None. |
.azure-pipelines/ci.yml |
Adds CFS validation and dependency path filters. | Moderate (3 votes): misses the OSGi manifest dependency path. Moderate (2 votes): lockfile URLs may bypass CFS and use npmjs directly. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Retain npm Dependabot updates or provide a guaranteed replacement updater.
Review details
Suppressed comments (1)
.github/dependabot.yml:8
- Removing the npm entry leaves Dependabot with no in-repository mechanism to create npm dependency-update PRs. The new CFS job only validates existing
package.json/package-lock.jsonchanges; it does not discover new versions or open update PRs, so npm dependencies will stop receiving automated updates unless a separate CFS updater is guaranteed. Please retain this entry or wire the replacement updater into this change.
- package-ecosystem: "github-actions"
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The critical credential exposure in the PR-triggered job must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Fix the npm authentication environment variable casing before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Two critical credential-isolation issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Address credential exposure and preserve or replace automated npm dependency updates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.azure-pipelines/ci.yml:76
- Deleting the npm
package-ecosystementry disables Dependabot's version-update PRs, while this new job only validates dependencies on an already-existing PR (npm ci/the Maven build) and does not create updates. Unless an external CFS updater is provisioned, npm dependencies will stop receiving automated update PRs; retain the Dependabot entry or add/document that replacement mechanism before removing it.
- script: npm ci --ignore-scripts --prefer-online --cache "$(Agent.TempDirectory)/npm-cache"
displayName: Validate npm dependencies from CFS
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
Summary
Validation
.azure-pipelines/ci.ymlwithjs-yamlcfs-settings.xmlgit diff --checknpm run compilecurrently reports the existingminimatch/globtype incompatibility also present outside this YAML-only change.