Fix TYPESPEC_NPM_REGISTRY forwarding during install - #11694
Open
Vivek JM (vivekjm) wants to merge 1 commit into
Open
Fix TYPESPEC_NPM_REGISTRY forwarding during install#11694Vivek JM (vivekjm) wants to merge 1 commit into
Vivek JM (vivekjm) wants to merge 1 commit into
Conversation
Vivek JM (vivekjm)
requested review from
catalinaperalta,
iscai-msft,
Laurent Mazuel (lmazuel),
Mark Cowlishaw (markcowl) and
Timothee Guerin (timotheeguerin)
as code owners
August 15, 2026 15:48
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
added
the
compiler:core
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR forwards the TYPESPEC_NPM_REGISTRY override to the npm process spawned by tsp install, ensuring installs use the same registry as manifest resolution.
Changes:
- Add
getNpmRegistryEnvironment()to produce a child-processenvwithnpm_config_registryset fromTYPESPEC_NPM_REGISTRY. - Update installer to use the new environment builder when invoking the package manager.
- Add unit tests covering forwarding behavior and preservation of existing npm registry env vars.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/compiler/test/package-manager/npm-registry.test.ts | Adds tests validating registry env forwarding and non-interference when no override is set. |
| packages/compiler/src/package-manger/npm-registry.ts | Introduces getNpmRegistryEnvironment() to map TypeSpec registry override to npm’s env convention. |
| packages/compiler/src/install/install.ts | Uses getNpmRegistryEnvironment() when spawning the package manager to apply the override. |
| .chronus/changes/forward-typespec-npm-registry-2026-08-15.md | Adds changelog entry documenting the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+97
to
+112
| export function getNpmRegistryEnvironment(): Record<string, string | undefined> { | ||
| const environment = { ...process.env }; | ||
| if (process.env["TYPESPEC_NPM_REGISTRY"] === undefined) { | ||
| return environment; | ||
| } | ||
|
|
||
| // Environment variable names are case-insensitive on Windows. Remove any | ||
| // existing spelling so Node does not choose between duplicate keys. | ||
| for (const name of Object.keys(environment)) { | ||
| if (name.toLowerCase() === "npm_config_registry") { | ||
| delete environment[name]; | ||
| } | ||
| } | ||
| environment["npm_config_registry"] = getNpmRegistry(); | ||
| return environment; | ||
| } |
Comment on lines
+97
to
+98
| export function getNpmRegistryEnvironment(): Record<string, string | undefined> { | ||
| const environment = { ...process.env }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TYPESPEC_NPM_REGISTRYto the npm process launched bytsp installFixes #11688.
Root cause
TypeSpec already used
TYPESPEC_NPM_REGISTRYwhen it fetched package-manager metadata and archives. The laternpm installsubprocess inherited that TypeSpec-specific variable, but npm readsnpm_config_registryinstead, so project dependencies still came from npm's default configuration.Testing
pnpm setup:minpnpm --filter @typespec/compiler exec vitest run test/package-manager/npm-registry.test.ts— 4 tests passedpnpm --filter @typespec/compiler run buildpnpm exec prettier --check packages/compiler/src/install/install.ts packages/compiler/src/package-manger/npm-registry.ts packages/compiler/test/package-manager/npm-registry.test.ts .chronus/changes/forward-typespec-npm-registry-2026-08-15.mdpnpm --filter @typespec/compiler run lintgit diff --checkLocal setup note: I initially invoked the compiler tests before building the generated manifest and internal workspace packages, which produced bootstrap-related missing-module errors. After running the documented
pnpm setup:minstep, the focused tests and all checks above passed.