Skip to content

fix: isolate determine-changes npm install from consuming repo's package.json - #194

Closed
madhur310 wants to merge 1 commit into
mainfrom
ms/fix-determine-changes-npm-workspaces
Closed

madhur310 wants to merge 1 commit into
mainfrom
ms/fix-determine-changes-npm-workspaces

Conversation

@madhur310

Copy link
Copy Markdown
Contributor

Summary

  • determine-changes (in vscode-publish-extensions.yml) runs npm install --no-save simple-git chalk semver zod tsx directly at the consuming repo's root, so npm's arborist parses that repo's own package.json/workspace members as part of resolving the install, even though those packages have nothing to do with the repo's own dependency graph.
  • A consuming repo using pnpm's workspace:* protocol (which plain npm's arborist doesn't understand) fails there with EUNSUPPORTEDPROTOCOL, before change detection ever runs — which skips every downstream job (package, bump-versions, Create GitHub Releases, publish).
  • Hit this live: forcedotcom/salesforcedx-vscode's nightly pipeline has been failing since #8396 merged (switched several internal deps to workspace:*) — see run 38001111461.

Fix

Install into an isolated mktemp -d prefix instead of the repo root, and point NODE_PATH at it when running the downloaded script — fully independent of the consuming repo's dependency graph regardless of its package manager or specifier syntax.

Verification

Reproduced and verified against forcedotcom/salesforcedx-vscode@c8d281a9e (the exact commit that triggered this in CI), in a disposable worktree:

  • Old command (npm install --no-save simple-git@3.27.0 ...): reproduces EUNSUPPORTEDPROTOCOL exactly as seen in CI.
  • New command (isolated prefix + NODE_PATH): runs change detection to completion, producing correct selected-extensions/version-bumps outputs.

Test plan

  • CI run on this PR itself exercises any workflow-syntax checks this repo has
  • Once merged, re-dispatch nightly.yml on forcedotcom/salesforcedx-vscode (currently pinned to @main) to confirm determine-changes succeeds end-to-end

…age.json

The determine-changes job's ad-hoc `npm install --no-save simple-git chalk
semver zod tsx` runs at the consuming repo's root, so npm's arborist parses
that repo's own package.json/workspace members as part of resolving the
install - unrelated to what's being installed. A repo using pnpm's
`workspace:*` protocol (which plain npm doesn't understand) fails there with
EUNSUPPORTEDPROTOCOL before change detection ever runs, breaking every
downstream job (package, bump-versions, create-releases, publish).

Install into an isolated `mktemp -d` prefix instead, and point NODE_PATH at
it when running the downloaded script - fully independent of the consuming
repo's dependency graph regardless of its package manager or specifier
syntax. Verified against forcedotcom/salesforcedx-vscode@c8d281a9e (the
exact commit that triggered this in CI): old command reproduces
EUNSUPPORTEDPROTOCOL there; new command runs change detection to completion.
@madhur310

Copy link
Copy Markdown
Contributor Author

Superseded by #195, which was merged to main while this was in review — same root cause (npm install --no-save at the repo root chokes on workspace:* before change detection runs), independently discovered via a different consuming repo. #195's approach is slightly cleaner (co-locates the downloaded script with its isolated deps so Node's normal node_modules resolution just works, no NODE_PATH needed, plus it cleans up its temp dir). Closing in favor of that.

@madhur310 madhur310 closed this Oct 12, 2026
@madhur310
madhur310 deleted the ms/fix-determine-changes-npm-workspaces branch October 12, 2026 16:23
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.

1 participant