From 2be82f00999569e2e2d462a9f3eab57bd36d371e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 26 Sep 2026 01:38:48 +0000 Subject: [PATCH] fix(cli): use onnxruntime-node installed beside the CLI when it is the pinned version (#4507) * fix(cli): use onnxruntime-node installed beside the CLI when it is the pinned version An optional package (onnxruntime-node, @google/genai) installed next to the CLI at the pinned version is now loaded from there, before the ~/.cache/hyperframes/optional copy and without an npm install. Any other version is ignored and the cache path runs as before. doctor reports that copy's version. * refactor(cli): name the beside-the-CLI lookup instead of commenting it * fix(cli): a bad optional package copy beside the CLI no longer breaks doctor The lookup reads the first copy Node would load and treats an unreadable manifest as not the pinned copy, so doctor and the loader fall back to the cache instead of throwing or checking a different copy. * fix(cli): let Node pick the optional package copy before checking its version The beside-the-CLI lookup now asks require.resolve which copy it would load and checks that copy's manifest, so an empty folder or a same-named file earlier on the path can no longer make the check and the loader disagree. * test(cli): cover a symlinked optional package copy beside the CLI * fix(cli): find a symlinked optional package when Node preserves symlinks Under --preserve-symlinks require.resolve returns the symlink path while the candidate folder was compared by its real path, so a pinned copy Node loads was rejected. Both sides are now real paths. A test runs the lookup in a Node subprocess with --preserve-symlinks. --- packages/cli/src/commands/doctor.test.ts | 7 +- packages/cli/src/commands/doctor.ts | 8 +- .../cli/src/utils/optionalPackages.test.ts | 155 +++++++++++++++++- packages/cli/src/utils/optionalPackages.ts | 28 +++- 4 files changed, 192 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/commands/doctor.test.ts b/packages/cli/src/commands/doctor.test.ts index 9a5361c31a..9de1951c07 100644 --- a/packages/cli/src/commands/doctor.test.ts +++ b/packages/cli/src/commands/doctor.test.ts @@ -1,6 +1,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { pathToFileURL } from "node:url"; import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { optionalPackageDir } from "../utils/optionalPackages.js"; import { @@ -287,7 +288,8 @@ describe("checkOptionalPackage", () => { it("reports a package that has not been installed yet as ok, installing on first use", () => { const cacheDir = mkdtempSync(join(tmpdir(), "hf-doctor-optional-")); dirs.push(cacheDir); - expect(checkOptionalPackage("onnxruntime-node", cacheDir)).toEqual({ + const cliUrl = pathToFileURL(join(cacheDir, "cli.js")).href; + expect(checkOptionalPackage("onnxruntime-node", cacheDir, cliUrl)).toEqual({ ok: true, detail: "Not installed (installs on first use)", }); @@ -302,7 +304,8 @@ describe("checkOptionalPackage", () => { ); mkdirSync(pkgDir, { recursive: true }); writeFileSync(join(pkgDir, "package.json"), JSON.stringify({ version: "1.52.0" })); - expect(checkOptionalPackage("@google/genai", cacheDir)).toEqual({ + const cliUrl = pathToFileURL(join(cacheDir, "cli.js")).href; + expect(checkOptionalPackage("@google/genai", cacheDir, cliUrl)).toEqual({ ok: true, detail: "1.52.0 installed", }); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index ad86d0a1fa..6d3114f823 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -287,8 +287,12 @@ function checkLocalMusic(): CheckResult { } /** Not a failure when missing: the package installs itself the first time a feature needs it. */ -export function checkOptionalPackage(name: OptionalPackage, cacheDir?: string): CheckResult { - const version = installedOptionalPackageVersion(name, cacheDir); +export function checkOptionalPackage( + name: OptionalPackage, + cacheDir?: string, + cliUrl?: string, +): CheckResult { + const version = installedOptionalPackageVersion(name, cacheDir, cliUrl); return { ok: true, detail: version ? `${version} installed` : "Not installed (installs on first use)", diff --git a/packages/cli/src/utils/optionalPackages.test.ts b/packages/cli/src/utils/optionalPackages.test.ts index a9780f9793..3761301298 100644 --- a/packages/cli/src/utils/optionalPackages.test.ts +++ b/packages/cli/src/utils/optionalPackages.test.ts @@ -1,11 +1,23 @@ import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { buildSync } from "esbuild"; import { describe, expect, it, vi } from "vitest"; import { OPTIONAL_PACKAGES, install, + installedOptionalPackageVersion, + loadBesideCli, loadOptionalPackage, optionalPackageDir, type OptionalPackageDeps, @@ -19,6 +31,7 @@ function fakeDeps(overrides: Partial = {}) { }); const deps: OptionalPackageDeps = { cacheDir: "/cache", + loadBesideCli: () => null, loadInstalled: (dir) => installed.get(dir) ?? null, install, log, @@ -44,6 +57,15 @@ describe("loadOptionalPackage", () => { expect(log.mock.calls).toEqual([["installing onnxruntime-node for background removal, once"]]); }); + it("uses the pinned copy installed beside the CLI, without installing", async () => { + const beside = { beside: "module" }; + const { deps, install, log } = fakeDeps({ loadBesideCli: () => beside }); + + expect(await loadOptionalPackage("onnxruntime-node", "on-device search", deps)).toBe(beside); + expect(install).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + it("names the manual command when the install fails", async () => { const { deps } = fakeDeps({ install: async () => { @@ -96,6 +118,137 @@ describe("loadOptionalPackage", () => { }); }); +describe("a copy installed beside the CLI", () => { + // node_modules/hyperframes/dist/cli.js with onnxruntime-node installed next to hyperframes. + function layout(version: string) { + const root = mkdtempSync(join(tmpdir(), "hf-beside-")); + const pkg = join(root, "node_modules", "onnxruntime-node"); + mkdirSync(pkg, { recursive: true }); + writeFileSync(join(pkg, "package.json"), JSON.stringify({ version, main: "index.js" })); + writeFileSync(join(pkg, "index.js"), `module.exports = { copy: "beside ${version}" };`); + const cliUrl = pathToFileURL(join(root, "node_modules", "hyperframes", "dist", "cli.js")).href; + return { root, cliUrl }; + } + + it("loads it and reports its version when it is the pinned version", () => { + const pin = OPTIONAL_PACKAGES["onnxruntime-node"]; + const { root, cliUrl } = layout(pin); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toEqual({ copy: `beside ${pin}` }); + expect(installedOptionalPackageVersion("onnxruntime-node", "/no-cache", cliUrl)).toBe(pin); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("ignores it when its manifest is unreadable, instead of throwing", () => { + const { root, cliUrl } = layout(OPTIONAL_PACKAGES["onnxruntime-node"]); + writeFileSync(join(root, "node_modules", "onnxruntime-node", "package.json"), "{ version: 1"); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toBeNull(); + expect(installedOptionalPackageVersion("onnxruntime-node", "/no-cache", cliUrl)).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("checks the copy require would load first, not a later pinned one", () => { + const { root, cliUrl } = layout(OPTIONAL_PACKAGES["onnxruntime-node"]); + const nearer = join(root, "node_modules", "hyperframes", "node_modules", "onnxruntime-node"); + mkdirSync(nearer, { recursive: true }); + writeFileSync(join(nearer, "index.js"), `module.exports = { copy: "unversioned" };`); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("loads a symlinked copy, as bun and pnpm lay packages out", () => { + const pin = OPTIONAL_PACKAGES["onnxruntime-node"]; + const { root, cliUrl } = layout(pin); + const linked = join(root, "node_modules", "onnxruntime-node"); + const store = join(root, "store", "onnxruntime-node"); + mkdirSync(join(root, "store"), { recursive: true }); + renameSync(linked, store); + symlinkSync(store, linked, "dir"); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toEqual({ copy: `beside ${pin}` }); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("loads a symlinked copy when Node preserves symlinks", () => { + const pin = OPTIONAL_PACKAGES["onnxruntime-node"]; + const { root, cliUrl } = layout(pin); + const linked = join(root, "node_modules", "onnxruntime-node"); + const store = join(root, "store", "onnxruntime-node"); + mkdirSync(join(root, "store"), { recursive: true }); + renameSync(linked, store); + symlinkSync(store, linked, "dir"); + // Bundled first: a TS loader cannot itself load under --preserve-symlinks in a bun store. + const bundle = join(root, "optionalPackages.mjs"); + buildSync({ + entryPoints: [fileURLToPath(new URL("./optionalPackages.ts", import.meta.url))], + bundle: true, + platform: "node", + format: "esm", + outfile: bundle, + }); + const script = `const m = await import(${JSON.stringify(pathToFileURL(bundle).href)}); +const url = ${JSON.stringify(cliUrl)}; +console.log(JSON.stringify([m.loadBesideCli("onnxruntime-node", url), + m.installedOptionalPackageVersion("onnxruntime-node", "/no-cache", url)]));`; + try { + const child = spawnSync( + process.execPath, + ["--preserve-symlinks", "--input-type=module", "-e", script], + { encoding: "utf-8" }, + ); + expect(child.stderr).toBe(""); + expect(JSON.parse(child.stdout)).toEqual([{ copy: `beside ${pin}` }, pin]); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("looks past an empty folder that require skips", () => { + const pin = OPTIONAL_PACKAGES["onnxruntime-node"]; + const { root, cliUrl } = layout(pin); + mkdirSync(join(root, "node_modules", "hyperframes", "node_modules", "onnxruntime-node"), { + recursive: true, + }); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toEqual({ copy: `beside ${pin}` }); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("ignores a file require would load before the pinned folder", () => { + const { root, cliUrl } = layout(OPTIONAL_PACKAGES["onnxruntime-node"]); + const nearer = join(root, "node_modules", "hyperframes", "node_modules"); + mkdirSync(nearer, { recursive: true }); + writeFileSync(join(nearer, "onnxruntime-node.js"), `module.exports = { copy: "file" };`); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("ignores it at any other version, so the cache install is used", () => { + const { root, cliUrl } = layout("1.0.0"); + try { + expect(loadBesideCli("onnxruntime-node", cliUrl)).toBeNull(); + expect(installedOptionalPackageVersion("onnxruntime-node", "/no-cache", cliUrl)).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +}); + describe("install", () => { const name = "@google/genai"; const version = "1.0.0"; diff --git a/packages/cli/src/utils/optionalPackages.ts b/packages/cli/src/utils/optionalPackages.ts index 1da73e3733..bea22ff4d2 100644 --- a/packages/cli/src/utils/optionalPackages.ts +++ b/packages/cli/src/utils/optionalPackages.ts @@ -5,13 +5,14 @@ import { mkdirSync, readdirSync, readFileSync, + realpathSync, renameSync, rmSync, writeFileSync, } from "node:fs"; import { createRequire } from "node:module"; import { homedir } from "node:os"; -import { basename, dirname, join } from "node:path"; +import { basename, dirname, join, sep } from "node:path"; import { buildNpmCommand } from "./npxCommand.js"; /** Module type of each optional package; the keys are the only names the loader accepts. */ @@ -32,6 +33,7 @@ const CACHE_DIR = join(homedir(), ".cache", "hyperframes", "optional"); export interface OptionalPackageDeps { cacheDir: string; + loadBesideCli(name: OptionalPackage): unknown | null; /** The package's exports when already installed in `dir`, else null. */ loadInstalled(dir: string, name: string): unknown | null; /** Install `name@version` into `dir`; rejects with npm's output on failure. */ @@ -48,7 +50,9 @@ export function optionalPackageDir(name: OptionalPackage, cacheDir = CACHE_DIR): export function installedOptionalPackageVersion( name: OptionalPackage, cacheDir = CACHE_DIR, + cliUrl = import.meta.url, ): string | null { + if (pinnedCopyBesideCli(name, cliUrl)) return OPTIONAL_PACKAGES[name]; const dir = optionalPackageDir(name, cacheDir); if (!isInstalled(dir, name)) return null; return (JSON.parse(readFileSync(manifestPath(dir, name), "utf-8")) as { version: string }) @@ -64,6 +68,8 @@ export async function loadOptionalPackage( feature: string, deps: OptionalPackageDeps = defaultDeps, ): Promise { + const beside = deps.loadBesideCli(name); + if (beside !== null) return beside as OptionalPackageModules[N]; const dir = optionalPackageDir(name, deps.cacheDir); const installed = deps.loadInstalled(dir, name); if (installed !== null) return installed as OptionalPackageModules[N]; @@ -104,6 +110,25 @@ function loadInstalled(dir: string, name: string): unknown | null { return createRequire(join(dir, "package.json"))(name); } +function pinnedCopyBesideCli(name: OptionalPackage, cliUrl: string): boolean { + const req = createRequire(cliUrl); + try { + const entry = realpathSync(req.resolve(name)); + const copy = (req.resolve.paths(name) ?? []) + .map((dir) => join(dir, name)) + .find((dir) => existsSync(dir) && entry.startsWith(realpathSync(dir) + sep)); + if (!copy) return false; + const manifest = readFileSync(join(copy, "package.json"), "utf-8"); + return (JSON.parse(manifest) as { version?: string }).version === OPTIONAL_PACKAGES[name]; + } catch { + return false; + } +} + +export function loadBesideCli(name: OptionalPackage, cliUrl = import.meta.url): unknown | null { + return pinnedCopyBesideCli(name, cliUrl) ? createRequire(cliUrl)(name) : null; +} + function runNpm(args: string[]): Promise { const npm = buildNpmCommand(args); const child = spawn(npm.command, npm.args, { stdio: ["ignore", "pipe", "pipe"] }); @@ -186,6 +211,7 @@ export async function install( const defaultDeps: OptionalPackageDeps = { cacheDir: CACHE_DIR, + loadBesideCli: (name) => loadBesideCli(name), loadInstalled, install, log: (line) => console.error(line),