Skip to content

fix(vscode): Fix VS Code extension CFS npm install - #9522

Merged
Brian Lam (lambrianmsft) merged 1 commit into
Azure:mainfrom
lambrianmsft:lambrian-microsoft-pipeline-cfs-network-isolation
Aug 11, 2026
Merged

fix(vscode): Fix VS Code extension CFS npm install#9522
Brian Lam (lambrianmsft) merged 1 commit into
Azure:mainfrom
lambrianmsft:lambrian-microsoft-pipeline-cfs-network-isolation

Conversation

@lambrianmsft

@lambrianmsft Brian Lam (lambrianmsft) commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Follow-up to the CFS npm registry migration for SR21 / 1ES Network Isolation. The root pnpm install uses the authenticated CFS npmrc, but the VS Code extension build runs a nested npm install from apps/vs-code-designer/dist. This change routes that nested install through a helper that passes --userconfig from NPM_CONFIG_USERCONFIG when the pipeline provides the authenticated npmrc, avoiding fallback to unauthenticated/public npm during CFSClean builds.

Impact of Change

  • Users: No user-facing product changes.
  • Developers: VS Code extension package scripts now call scripts/install-dist-dependencies.js instead of inline cd dist && npm install.
  • System: CFS-authenticated npm configuration is explicitly applied to the nested extension dependency install in CI/package builds.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: pnpm exec biome check --write apps\vs-code-designer\scripts\install-dist-dependencies.js apps\vs-code-designer\package.json; helper simulation with temporary non-secret NPM_CONFIG_USERCONFIG; pnpm --filter vscode-designer build:extension reached and completed nested npm install. The build still logs pre-existing unrelated typecheck diagnostics, including validateNodeJsIsLatest.ts, but exits successfully.

Contributors

N/A

Screenshots/Videos

N/A

Copilot AI balanced review requested due to automatic review settings August 11, 2026 02:08
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(vscode): Fix VS Code extension CFS npm install
  • Issue: None — uses a valid fix: prefix with a (vscode) scope and is descriptive.
  • Recommendation: No change needed.

Commit Type

  • Exactly one type selected: fix - Bug fix.
  • Matches the title prefix and the nature of the change (routing nested npm install through an authenticated helper).

Risk Level

  • Body selects Medium and the risk:medium label is present — they agree. This matches the advised estimate: the change affects VS Code extension distribution build/packaging (apps/vs-code-designer) and CI npm authentication, which is Medium per the rubric. No user-facing product behavior or security/auth-of-shipped-product change, so not High.

What & Why

  • Current: Clear explanation of the CFS npm registry migration follow-up and why the nested npm install needed to route through the helper for authenticated npmrc.
  • Issue: None.
  • Recommendation: No change needed.

Impact of Change

  • All three audiences addressed (Users, Developers, System).
  • Recommendation:
    • Users: Accurately noted as no user-facing changes.
    • Developers: Correctly notes scripts now call install-dist-dependencies.js.
    • System: Correctly notes CFS-authenticated npm config applied to nested install.

Test Plan

  • No unit/E2E tests, but this is a build/packaging tooling change (package scripts + a CI install helper) with no shipped product code. Manual testing is checked with an adequate explanation (biome check, helper simulation with NPM_CONFIG_USERCONFIG, and a full build:extension run reaching/completing the nested npm install). This satisfies CHECK TESTS.

⚠️ Contributors

  • Marked N/A. If any PMs, designers, or reviewers contributed, please credit them. Non-blocking.

Screenshots/Videos

  • No UI files touched (libs/designer-ui/src, libs/designer/src, apps/vs-code-react/src UI components). Not required; N/A is fine.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level Keep Medium (matches advised estimate)
What & Why No change needed
Impact of Change No change needed
Test Plan Manual testing justified for build tooling
Contributors ⚠️ Credit contributors if any
Screenshots/Videos Not applicable

All required checks pass. This PR is compliant and ready to merge. 🎉


Powered by: Copilot CLI (claude-opus-4.8) | Last updated: Tue, 11 Aug 2026 02:39:35 GMT

@lambrianmsft Brian Lam (lambrianmsft) added risk:low Low risk change with minimal impact and removed needs-pr-update labels Aug 11, 2026
@lambrianmsft Brian Lam (lambrianmsft) changed the title Fix VS Code extension CFS npm install fix(vscode): Fix VS Code extension CFS npm install Aug 11, 2026

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.

Pull request overview

Routes VS Code extension dependency installation through the authenticated CFS npm configuration and resolves Node.js warning-flow type errors.

Changes:

  • Adds a helper for installing dist dependencies with NPM_CONFIG_USERCONFIG.
  • Updates extension build and packaging scripts to use the helper.
  • Refactors Node.js warning handling and telemetry formatting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
validateNodeJsIsLatest.ts Adjusts telemetry values and warning handling.
install-dist-dependencies.js Adds authenticated nested npm installation.
package.json Uses the new installation helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lambrianmsft
Brian Lam (lambrianmsft) enabled auto-merge (squash) August 11, 2026 02:12
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@lambrianmsft
Brian Lam (lambrianmsft) force-pushed the lambrian-microsoft-pipeline-cfs-network-isolation branch from dd7a0de to e9d56c1 Compare August 11, 2026 02:25
@lambrianmsft Brian Lam (lambrianmsft) added risk:medium Medium risk change with potential impact and removed risk:low Low risk change with minimal impact needs-pr-update labels Aug 11, 2026
@lambrianmsft
Brian Lam (lambrianmsft) merged commit ae46e2f into Azure:main Aug 11, 2026
70 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants