Skip to content

fix(cli): detect nvm-windows global installs in update notices - #1801

Merged
mldangelo-oai merged 3 commits into
openai:mainfrom
WenJing95:fix/nvm-windows-update-command
Oct 20, 2026
Merged

mldangelo-oai merged 3 commits into
openai:mainfrom
WenJing95:fix/nvm-windows-update-command

Conversation

@WenJing95

@WenJing95 WenJing95 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Windows npm packages installed alongside Node, including nvm-windows installations, receive an update notice without -g. Running the suggested command installs a local package instead of updating the global CLI.

Changes

Recognize the node_modules directory beside the resolved Windows Node executable. This follows npm's global installation layout and handles custom nvm installation roots without depending on a directory named nvm or a version-string pattern.

Extend the existing command-selection table for default and custom Windows roots, mixed path separators and casing, local project installs, and the existing npx and standalone-release precedence. A real directory-link fixture verifies that the default executable path resolves to the same installation as the CLI module.

Testing

  • Reproduced the missing -g for a custom nvm root with the original patch, then passed the focused update-notice suite with the revised detection.
  • Reproduced the directory-link mismatch, then passed the regression after resolving the Windows executable path.
  • Passed package type checks and formatting checks.
  • Attempted both required full package test runs. Their build setup stopped before tests because this checkout lacks a generated native artifact (native/prebuilt/COPYRIGHT-library.html). Full platform coverage remains with CI.
  • Validation used deterministic Windows path fixtures on Linux; no manual nvm-windows installation was performed.

Risk and rollout

Changes only the printed update command. The new detection applies to Windows; existing package-manager selection and local-install behavior are preserved. No new public commands, flags, environment variables, or dependencies.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@github-actions github-actions Bot added the bug Something isn't working label Oct 20, 2026

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for identifying the missing -g. I reproduced it and checked the install layout against npm's documentation and configuration implementation. The issue is real: the suggested local install does not update the global CLI.

I replaced the directory-name regex with a comparison against the node_modules directory beside the resolved Windows Node executable. This also handles configurable nvm roots, using the existing path API and no subprocess or new setting. The existing table now covers default/custom roots, local installs, and npx/standalone precedence. A directory-link regression also verifies the nvm launch layout; resolving the executable path keeps it consistent with the CLI module path.

The regression fails with the original patch and passes with the revised detection. Focused tests, type checks, and formatting passed. I corrected the PR description to distinguish those fixtures from a manual Windows installation, which I did not perform.

@mldangelo-oai
mldangelo-oai merged commit 3c0cf4e into openai:main Oct 20, 2026
125 of 128 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants