Skip to content

fix: codex update targets correct npm prefix on multi-prefix systems - #132

Merged
bash0816 merged 1 commit into
mainfrom
fix/codex-update-npm-prefix
Jul 20, 2026
Merged

fix: codex update targets correct npm prefix on multi-prefix systems#132
bash0816 merged 1 commit into
mainfrom
fix/codex-update-npm-prefix

Conversation

@bash0816

Copy link
Copy Markdown
Owner

Summary

  • Adds explicit --prefix flag to npm install -g in installTarget(), derived from packageDir
  • Fixes update on machines with multiple npm global prefixes where the wrong prefix was silently updated instead of the running installation
  • Includes fixture-based regression tests (prefixA/prefixB isolation via fake npm binary)
  • Version bump and npm publish deferred to next upstream release per project policy

Test plan

  • CI (lint.yml, etc.) passes
  • Manual verification: npm pack → install → verify update targets correct prefix (not needed for code review, test fixtures cover scenarios)

🤖 Generated with Claude Code

Adds explicit --prefix flag to npm install -g in installTarget(), derived from
packageDir. Fixes update on machines with multiple npm global prefixes where the
wrong prefix was silently updated instead of the running installation. Includes
fixture-based regression tests (prefixA/prefixB isolation via fake npm binary).
Version bump and npm publish deferred to next upstream release per project policy.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bash0816
bash0816 merged commit f4ddc26 into main Jul 20, 2026
3 checks passed
@bash0816
bash0816 deleted the fix/codex-update-npm-prefix branch July 20, 2026 07:59

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bef7635665

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const latestAuditedVersion = manifest.latest_audited_version;
const testVersion = '0.144.4'; // Older version for testing

test('installTarget with prefixA uses correct --prefix', async (t) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Run the new updater tests in CI

In the checked .github/workflows/npm-package.yml workflow, the syntax-check step explicitly runs only sync-public-release-from-manifest.test.js and magi-node-launcher.test.js, and this package has no test script that discovers suites automatically. Consequently, this newly added prefix regression suite is never executed on pull requests or pushes, so regressions in the behavior it covers can pass CI; add node --test lib/check-updates.test.js to that workflow.

Useful? React with 👍 / 👎.

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