Skip to content

build: focus Dependabot on security updates - #1759

Merged
wenyt (wenytang-ms) merged 8 commits into
mainfrom
build/validate-dependency-prs-with-cfs
Sep 10, 2026
Merged

build: focus Dependabot on security updates#1759
wenyt (wenytang-ms) merged 8 commits into
mainfrom
build/validate-dependency-prs-with-cfs

Conversation

@wenytang-ms

@wenytang-ms wenyt (wenytang-ms) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove the npm entry from .github/dependabot.yml so routine npm version-update pull requests are no longer scheduled
  • keep the grouped weekly GitHub Actions updates and their seven-day cooldown
  • run pull request validation only through a deterministic CFS-backed npm restore
  • disable npm lifecycle scripts, audit, and funding requests in the PR validation job
  • skip extension build, VSIX packaging, and artifact publication for pull requests
  • preserve the existing full build and packaging behavior for non-PR runs, including commits to main
  • trigger validation when npm dependency inputs or the CFS pipeline configuration changes
  • stabilize the go-to-super-implementation AutoTest plan by avoiding an LLM assertion on a transient macOS Loading... hover screenshot; the following link click and Base.java editor assertion remain authoritative
  • correct the single-file AutoTest editing step so the new statement is inserted inside main rather than after its closing brace
  • keep Dependabot alerts and security-update pull requests controlled by the repository security settings

Why

The repository restores npm packages through the Central Feed Service, which can withhold newly published upstream versions for about seven days. A scheduled npm Dependabot update can therefore open before the selected version is available from CFS and fail the ADO build even when a cooldown is configured.

CFS availability should be enforced by a dedicated PR validation path instead of delaying routine npm updates. GitHub Actions updates do not restore packages through the ADO npm feed, so their existing grouped weekly updates remain enabled.

The purpose of the ADO check is to verify that the exact lockfile dependencies can be restored from CFS, not to execute package code or package the extension. PR runs therefore use npm ci --ignore-scripts --no-audit --no-fund and stop after the authenticated restore.

Scope

All ADO npm restores in this repository use the shared CFS templates and the vscjava Azure Artifacts feed. The PR trigger covers package.json, package-lock.json, the CI pipeline, and both shared npm CFS templates. Unrelated pull requests do not consume the CFS validation pipeline.

CFS validation is configured through the Azure DevOps pipeline VSCode-Java-Pack-CI-Build.

Validation

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The pipeline pins the self repository to refs/heads/main, which can cause PR validation to run against main instead of the PR’s dependency changes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR changes the repo’s dependency update and CI strategy to avoid routine Dependabot bumps that can fail due to Central Feed Service (CFS) ingestion lag, by shifting PR validation to a minimal CFS-backed npm restore path while keeping full packaging for non-PR builds.

Changes:

  • Remove the scheduled Dependabot version-update configuration so routine npm/GitHub Actions update PRs are no longer created on a schedule.
  • Add a PR-triggered Azure Pipelines path-filtered validation job that authenticates to CFS and runs an npm restore, while skipping VSIX packaging/artifact publication for PR runs.
  • Preserve the existing full build/package behavior for non-PR runs (e.g., main).
File summaries
File Description
.github/dependabot.yml Removes scheduled Dependabot version-update configuration to rely on security settings instead.
.azure-pipelines/ci.yml Adds PR-triggered, CFS-authenticated npm restore validation and gates the full build job to non-PR runs.
Review details
  • Files reviewed: 2/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.

Comment thread .azure-pipelines/ci.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The PR-only CFS validation uses npm install, which can mutate/regen the lockfile and reduce determinism for validating the committed dependency inputs.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.azure-pipelines/ci.yml:58

  • The PR validation job uses npm install, which can update package-lock.json and resolve newer versions than what’s committed. That can make the CFS validation pass even when the PR forgot to update the lockfile (or when you want to validate the exact locked versions). Using npm ci makes the restore deterministic and fails if package.json and the lockfile are out of sync.
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes CI trigger scoping and job execution paths in Azure Pipelines, which is operationally impactful and should be validated via real PR/non-PR pipeline runs.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test-plans/java-go-to-super-implementation.yaml:68

  • With skipLlmVerify: true, this step no longer has an LLM-backed assertion, but the verify: text still claims the hover popup already “contains a clickable 'Go to super implementation' link”. Since the link is now effectively validated by the next clickHoverAction step, update this verify: text so it doesn’t imply coverage that is intentionally skipped here.
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

PR validation currently runs npm ci (scripts enabled) on PRs and the PR description doesn’t mention the included AutoTest plan change, so security/scope alignment should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test-plans/java-go-to-super-implementation.yaml:68

  • This PR updates an AutoTest plan (java-go-to-super-implementation.yaml), but the PR description is entirely about Dependabot and Azure Pipelines behavior. Please either describe this test plan change in the PR summary or split it into a separate PR so the intent and review scope stay aligned.
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .azure-pipelines/ci.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes align with the stated goal (CFS-only deterministic PR validation + reduced Dependabot noise), and the only feedback is a non-blocking test-plan verification nuance.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test-plans/java-go-to-super-implementation.yaml:73

  • skipLlmVerify: true means this hover step no longer performs any assertion at all (it has no deterministic verifier like verifyEditor/verifyProblems). That can make failures harder to diagnose because the first explicit check becomes the following clickHoverAction step. Since the updated verify text no longer asserts that the link is visible, consider re-enabling the (non-deterministic) LLM verify here so the plan still at least asserts that a hover popup opened.
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@wenytang-ms
wenyt (wenytang-ms) merged commit efa46a8 into main Sep 10, 2026
63 checks passed
@wenytang-ms
wenyt (wenytang-ms) deleted the build/validate-dependency-prs-with-cfs branch September 10, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants