refactor(updates): fetch version from registry instead of npm CLI to support multiple managers (#446)
## Background Addressing feedback from https://github.com/openai/codex/pull/333#discussion_r2050893224, this PR adds support for Bun alongside npm, pnpm while keeping the code simple. ## Summary The update‑check flow is refactored to use a direct registry lookup (`fast-npm-meta` + `semver`) instead of shelling out to `npm outdated`, and adds a lightweight installer‑detection mechanism that: 1. Checks if the invoked script lives under a known global‑bin directory (npm, pnpm, or bun) 2. If not, falls back to local detection via `getUserAgent()` (the `package‑manager‑detector` library) ## What’s Changed - **Registry‑based version check** - Replace `execFile("npm", ["outdated"])` with `getLatestVersion()` and `semver.gt()` - **Multi‑manager support** - New `renderUpdateCommand` handles update commands for `npm`, `pnpm`, and `bun`. - Detect global installer first via `detectInstallerByPath()` - Fallback to local detection via `getUserAgent()` - **Module cleanup** - Extract `detectInstallerByPath` into `utils/package-manager-detector.ts` - Remove legacy `checkOutdated`, `getNPMCommandPath`, and child‑process JSON parsing - **Flow improvements in `checkForUpdates`** 1. Short‑circuit by `UPDATE_CHECK_FREQUENCY` 3. Fetch & compare versions 4. Persist new timestamp immediately 5. Render & display styled box only when an update exists - **Maintain simplicity** - All multi‑manager logic lives in one small helper and a concise lookup rather than a complex adapter hierarchy - Core `checkForUpdates` remains a single, easy‑to‑follow async function - **Dependencies added** - `fast-npm-meta`, `semver`, `package-manager-detector`, `@types/semver` ## Considerations If we decide to drop the interactive update‑message (`npm install -g @openai/codex`) rendering altogether, we could remove most of the installer‑detection code and dependencies, which would simplify the codebase further but result in a less friendly UX. ## Preview * npm  * bun  ## Simple Flow Chart ```mermaid flowchart TD A(Start) --> B[Read state] B --> C{Recent check?} C -- Yes --> Z[End] C -- No --> D[Fetch latest version] D --> E[Save check time] E --> F{Version data OK?} F -- No --> Z F -- Yes --> G{Update available?} G -- No --> Z G -- Yes --> H{Global install?} H -- Yes --> I[Select global manager] H -- No --> K{Local install?} K -- No --> Z K -- Yes --> L[Select local manager] I & L --> M[Render update message] M --> N[Format with boxen] N --> O[Print update] O --> Z ```
This commit is contained in:
66
codex-cli/tests/package-manager-detector.test.ts
Normal file
66
codex-cli/tests/package-manager-detector.test.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
import { describe, it, expect, beforeEach, vi, afterEach } from "vitest";
|
||||
import which from "which";
|
||||
import { detectInstallerByPath } from "../src/utils/package-manager-detector";
|
||||
import { execFileSync } from "node:child_process";
|
||||
|
||||
vi.mock("which", () => ({
|
||||
default: { sync: vi.fn() },
|
||||
}));
|
||||
vi.mock("node:child_process", () => ({ execFileSync: vi.fn() }));
|
||||
|
||||
describe("detectInstallerByPath()", () => {
|
||||
const originalArgv = process.argv;
|
||||
const fakeBinDirs = {
|
||||
// `npm prefix -g` returns the global “prefix” (we’ll add `/bin` when detecting)
|
||||
npm: "/usr/local",
|
||||
pnpm: "/home/user/.local/share/pnpm/bin",
|
||||
bun: "/Users/test/.bun/bin",
|
||||
} as const;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
// Pretend each manager binary is on PATH:
|
||||
vi.mocked(which.sync).mockImplementation(() => "/fake/path");
|
||||
|
||||
vi.mocked(execFileSync).mockImplementation(
|
||||
(
|
||||
cmd: string,
|
||||
_args: ReadonlyArray<string> = [],
|
||||
_options: unknown,
|
||||
): string => {
|
||||
return fakeBinDirs[cmd as keyof typeof fakeBinDirs];
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore the real argv so tests don’t leak
|
||||
process.argv = originalArgv;
|
||||
});
|
||||
|
||||
it.each(Object.entries(fakeBinDirs))(
|
||||
"detects %s when invoked from its global-bin",
|
||||
async (manager, binDir) => {
|
||||
// Simulate the shim living under that binDir
|
||||
process.argv =
|
||||
manager === "npm"
|
||||
? [process.argv[0]!, `${binDir}/bin/my-cli`]
|
||||
: [process.argv[0]!, `${binDir}/my-cli`];
|
||||
const detected = await detectInstallerByPath();
|
||||
expect(detected).toBe(manager);
|
||||
},
|
||||
);
|
||||
|
||||
it("returns undefined if argv[1] is missing", async () => {
|
||||
process.argv = [process.argv[0]!];
|
||||
expect(await detectInstallerByPath()).toBeUndefined();
|
||||
expect(execFileSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns undefined if shim isn't in any manager's bin", async () => {
|
||||
// stub execFileSync to some other dirs
|
||||
vi.mocked(execFileSync).mockImplementation(() => "/some/other/dir");
|
||||
process.argv = [process.argv[0]!, "/home/user/.node_modules/.bin/my-cli"];
|
||||
expect(await detectInstallerByPath()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user