From 425809cf71a8017f04f6dc5e795c57bfd9c11c2a Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 07:55:47 +0000 Subject: [PATCH 1/7] fix(embeddings): resolve onnxruntime-common under pnpm-strict / pnpm dlx (#307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@huggingface/transformers` does a bare `import 'onnxruntime-common'` from its shipped `dist/transformers.node.mjs`, but never declares onnxruntime-common in its own `dependencies`. npm's flat node_modules (and pnpm with hoisting) place it on transformers' resolution path by accident; pnpm's isolated store only links a package's declared deps into its scope, so under pnpm-strict / `pnpm dlx` / `pnpx` the import dies with ERR_MODULE_NOT_FOUND before `analyze --embeddings` can run. Declaring onnxruntime-common in gitnexus' own deps (#2074) does not fix this under pnpm: Node resolves the bare specifier from transformers' module scope, not ours, and overrides/resolutions can only re-version an existing edge, never add the missing one (verified against a real `hoist=false` install — the declaration only changes which version wins the hoist, never whether the import resolves). Fix: install a synchronous, in-thread ESM resolution hook (`module.registerHooks`) right before the lazy transformers import that redirects `onnxruntime-common` to the copy gitnexus depends on — but only when the default resolver fails. On npm / hoisted layouts the default resolver succeeds first and the hook never fires, so working setups are unchanged. The hook only intercepts the exact `onnxruntime-common` specifier on failure, so it can never mask an unrelated resolution error; onnxruntime-node's native binding still loads normally from transformers' own scope. `registerHooks` (sync, in-thread, single inline closure) is preferred over the older `module.register` (async, off-thread, now deprecated — DEP0205, removed in Node 26): the redirect is a one-line conditional that needs no worker thread, no separate hook module, and no `data` marshalling. It is available on Node >= 22.15; on older runtimes the helper is a graceful no-op (the gitnexus engines floor is >= 22.0.0, and the import still resolves on hoisted layouts there). Chosen over bundling transformers (the build is tsc-only, and transformers carries native onnxruntime-node + WASM onnxruntime-web assets that bundle poorly). Installation is idempotent, best-effort, and lazy — only on the local-embedding path, so it never affects analysis, the parse workers, or HTTP embedding mode. Validated end-to-end: the compiled resolver fixes a real pnpm `hoist=false` transformers install (ERR_MODULE_NOT_FOUND -> resolved). The separate `@ladybugdb/core` native-binary path under pure `pnpm dlx` is unchanged (#1967 handles that gracefully). Refs #307, #2069 Co-Authored-By: Claude Opus 4.8 (1M context) --- gitnexus/src/core/embeddings/embedder.ts | 4 + .../embeddings/onnxruntime-common-resolver.ts | 98 +++++++++++++++++ gitnexus/src/mcp/core/embedder.ts | 4 + .../unit/onnxruntime-common-resolver.test.ts | 103 ++++++++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts create mode 100644 gitnexus/test/unit/onnxruntime-common-resolver.test.ts diff --git a/gitnexus/src/core/embeddings/embedder.ts b/gitnexus/src/core/embeddings/embedder.ts index d873f3a0a3..3ec5f08e16 100644 --- a/gitnexus/src/core/embeddings/embedder.ts +++ b/gitnexus/src/core/embeddings/embedder.ts @@ -28,6 +28,7 @@ import { isHttpMode, getHttpDimensions, httpEmbed } from './http-client.js'; import { resolveEmbeddingConfig } from './config.js'; import { applyHfEnvOverrides, isHfDownloadFailure, withHfDownloadRetry } from './hf-env.js'; import { getLocalEmbeddingRuntimeBlocker } from './runtime-support.js'; +import { ensureOnnxRuntimeCommonResolvable } from './onnxruntime-common-resolver.js'; import { logger } from '../logger.js'; /** @@ -179,6 +180,9 @@ export const initEmbedder = async ( try { // Lazy-load transformers.js only after the runtime guard has passed, so // unsupported platforms never reach the native ONNX import (#1515). + // Under pnpm-strict / `pnpm dlx`, transformers' phantom `onnxruntime-common` + // import is unresolvable; register the fallback resolver first (#307). + ensureOnnxRuntimeCommonResolvable(); const { pipeline, env } = await import('@huggingface/transformers'); // Configure transformers.js environment diff --git a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts new file mode 100644 index 0000000000..c92c48d08e --- /dev/null +++ b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts @@ -0,0 +1,98 @@ +/** + * Make `@huggingface/transformers`' phantom `onnxruntime-common` import + * resolvable under strict package-manager layouts (#307, #2069). + * + * ## Why + * transformers' shipped `dist/transformers.node.mjs` does a bare + * `import 'onnxruntime-common'`, but transformers' `package.json` never declares + * onnxruntime-common (it lists onnxruntime-node / onnxruntime-web / sharp). With + * npm's flat `node_modules` — or pnpm with hoisting — the package is hoisted to + * a directory on transformers' resolution path and the import resolves by + * accident. Under pnpm's isolated store (and therefore `pnpm dlx` / `pnpx`), a + * package only sees its *declared* deps, so the import dies with + * `ERR_MODULE_NOT_FOUND` before `analyze --embeddings` can run. + * + * Declaring onnxruntime-common in gitnexus' own dependencies (#2074) does NOT + * fix this under pnpm: Node resolves the bare specifier from *transformers'* + * module scope, not ours, and overrides/resolutions can only re-version an + * existing edge, never add the missing one. + * + * ## What this does + * Install a synchronous, in-thread ESM resolution hook (`module.registerHooks`, + * Node >= 22.15) that redirects `onnxruntime-common` to the copy gitnexus itself + * depends on — but only when the default resolver fails. onnxruntime-common is a + * direct gitnexus dependency (a stable, pure-JS package whose `Tensor` surface + * is unchanged across 1.24–1.26), version-aligned with the onnxruntime-node + * transformers loads, so it is always resolvable from gitnexus' own scope + * regardless of how the consumer's package manager linked things. On working + * layouts the default resolver succeeds first and the hook never fires, so + * behaviour is unchanged. + * + * `registerHooks` (synchronous, in-thread) is preferred over the older + * `module.register` (async, off-thread, now deprecated — DEP0205, removed in + * Node 26): the redirect is a one-line conditional that needs no worker thread, + * no separate hook module, and no `data` marshalling. + * + * ## Safety + * Best-effort and idempotent. On Node < 22.15 `registerHooks` is absent and this + * is a graceful no-op (embeddings then resolve onnxruntime-common exactly as + * before — fine on hoisted layouts). Any failure is swallowed. The hook is + * installed lazily, only on the local-embedding code path, so it never affects + * analysis, the parse workers, or HTTP embedding mode; and it only ever touches + * the exact `onnxruntime-common` specifier on failure, so it cannot mask an + * unrelated resolution error. + */ +import { registerHooks, createRequire } from 'node:module'; +import { pathToFileURL } from 'node:url'; +import { logger } from '../logger.js'; + +let attempted = false; + +/** + * Idempotently install the onnxruntime-common resolution fallback. Call once + * immediately before the dynamic `import('@huggingface/transformers')` on the + * local-embedding path. + */ +export const ensureOnnxRuntimeCommonResolvable = (): void => { + if (attempted) return; + // Mark attempted up-front: a failed attempt must not retry on every + // initEmbedder() call, and the hook is process-global — once is enough. + attempted = true; + + try { + // Node < 22.15 (the gitnexus engines floor is >= 22.0.0): no synchronous + // hooks API. Degrade gracefully — the import still works on hoisted layouts. + if (typeof registerHooks !== 'function') return; + + const require = createRequire(import.meta.url); + // Resolve from gitnexus' own scope — onnxruntime-common is a direct + // dependency, so this succeeds under npm and pnpm-strict alike. + const redirectUrl = pathToFileURL(require.resolve('onnxruntime-common')).href; + + registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier !== 'onnxruntime-common') return nextResolve(specifier, context); + // Honour a real, package-manager-provided copy when one is on the path + // (npm / hoisted pnpm); only substitute ours when resolution fails. + try { + return nextResolve(specifier, context); + } catch { + return { url: redirectUrl, shortCircuit: true }; + } + }, + }); + logger.debug({ redirectUrl }, 'Installed onnxruntime-common resolution fallback (#307)'); + } catch (err) { + // Never block embeddings on the fallback. On layouts where the package + // manager already resolves onnxruntime-common this is unnecessary anyway. + logger.debug( + { err: err instanceof Error ? err.message : String(err) }, + 'onnxruntime-common resolution fallback not installed', + ); + } +}; + +/** Test-only: reset the one-shot guard so installation can be re-exercised. */ +export const __resetOnnxRuntimeCommonResolverForTests = (): void => { + attempted = false; +}; diff --git a/gitnexus/src/mcp/core/embedder.ts b/gitnexus/src/mcp/core/embedder.ts index 451d12c1f8..4dd73f317a 100644 --- a/gitnexus/src/mcp/core/embedder.ts +++ b/gitnexus/src/mcp/core/embedder.ts @@ -22,6 +22,7 @@ import { withHfDownloadRetry, } from '../../core/embeddings/hf-env.js'; import { getLocalEmbeddingRuntimeBlocker } from '../../core/embeddings/runtime-support.js'; +import { ensureOnnxRuntimeCommonResolvable } from '../../core/embeddings/onnxruntime-common-resolver.js'; import { silenceStdout, restoreStdout, realStderrWrite } from '../../core/lbug/pool-adapter.js'; import { logger } from '../../core/logger.js'; @@ -65,6 +66,9 @@ export const initEmbedder = async (): Promise => { try { // Lazy-load transformers.js only after the runtime guard has passed, so // unsupported platforms never reach the native ONNX import (#1515). + // Under pnpm-strict / `pnpm dlx`, transformers' phantom `onnxruntime-common` + // import is unresolvable; register the fallback resolver first (#307). + ensureOnnxRuntimeCommonResolvable(); const { pipeline, env } = await import('@huggingface/transformers'); env.allowLocalModels = false; diff --git a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts new file mode 100644 index 0000000000..cef511dd9d --- /dev/null +++ b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts @@ -0,0 +1,103 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; + +/** + * Tests for the #307 pnpm-strict / `pnpm dlx` fix: a synchronous in-thread ESM + * resolution hook (`module.registerHooks`) that redirects @huggingface/ + * transformers' phantom `onnxruntime-common` import to gitnexus' own copy. + * + * Each test mocks `node:module` with a chosen `registerHooks` (a spy, or + * `undefined` to simulate Node < 22.15) so we can assert one-shot installation, + * graceful degradation, and the redirect/passthrough/rethrow logic of the + * resolve closure — without mutating the real process loader. + */ + +const RESOLVER = '../../src/core/embeddings/onnxruntime-common-resolver.js'; + +/** (Re)load the resolver with a chosen `registerHooks` mocked into node:module. */ +async function loadResolver(registerHooks: unknown) { + vi.resetModules(); + vi.doMock('node:module', async (importOriginal) => { + const orig = await importOriginal(); + return { ...orig, registerHooks }; + }); + const mod = await import(RESOLVER); + mod.__resetOnnxRuntimeCommonResolverForTests(); + return mod; +} + +const ctx = { conditions: [], importAttributes: {} } as never; +const moduleNotFound = (): Error => { + const e = new Error("Cannot find package 'onnxruntime-common'") as Error & { code: string }; + e.code = 'ERR_MODULE_NOT_FOUND'; + return e; +}; + +afterEach(() => { + vi.doUnmock('node:module'); +}); + +describe('ensureOnnxRuntimeCommonResolvable — installation', () => { + it('installs the resolve hook exactly once (idempotent)', async () => { + const spy = vi.fn(); + const mod = await loadResolver(spy); + + mod.ensureOnnxRuntimeCommonResolvable(); + mod.ensureOnnxRuntimeCommonResolvable(); // second call is a no-op + + expect(spy).toHaveBeenCalledTimes(1); + expect(typeof spy.mock.calls[0][0].resolve).toBe('function'); + }); + + it('no-ops gracefully when registerHooks is unavailable (Node < 22.15)', async () => { + const mod = await loadResolver(undefined); + // Must not throw even though there is no synchronous-hooks API to call. + expect(() => mod.ensureOnnxRuntimeCommonResolvable()).not.toThrow(); + }); +}); + +describe('ensureOnnxRuntimeCommonResolvable — resolve hook behaviour', () => { + /** Install the fallback and return the resolve closure handed to registerHooks. */ + async function captureResolve() { + const spy = vi.fn(); + const mod = await loadResolver(spy); + mod.ensureOnnxRuntimeCommonResolvable(); + return spy.mock.calls[0][0].resolve as ( + s: string, + c: never, + n: (s: string, c: never) => unknown, + ) => unknown; + } + + it('passes a successful default resolution through unchanged (no-op on hoisted layouts)', async () => { + const resolve = await captureResolve(); + const real = { url: 'file:///real/onnxruntime-common/index.js', shortCircuit: true }; + const next = vi.fn(() => real); + + const res = resolve('onnxruntime-common', ctx, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(res).toBe(real); // the real resolution, NOT a redirect + }); + + it('redirects onnxruntime-common to the gitnexus copy when default resolution fails', async () => { + const resolve = await captureResolve(); + const next = vi.fn(() => { + throw moduleNotFound(); + }); + + const res = resolve('onnxruntime-common', ctx, next) as { url: string; shortCircuit: boolean }; + + expect(res.shortCircuit).toBe(true); + expect(res.url).toMatch(/^file:\/\/.*onnxruntime-common/); // gitnexus' own copy + }); + + it('never masks an unrelated resolution failure (other specifiers rethrow)', async () => { + const resolve = await captureResolve(); + const err = moduleNotFound(); + const next = vi.fn(() => { + throw err; + }); + + expect(() => resolve('some-other-package', ctx, next)).toThrow(err); + }); +}); From bacd9a37b6658edf014166ab74095bd51c308c6c Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:33:25 +0000 Subject: [PATCH 2/7] fix(embeddings): version-match the onnxruntime-common redirect target (#307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prefer the onnxruntime-common that onnxruntime-node (the native binding transformers actually loads) depends on, so the redirected copy is version- matched to that binding even under `pnpm dlx` — where gitnexus' npm-style `overrides` block does not apply, because it is honoured only from a root manifest and gitnexus is a transitive dependency there. The walk resolves transformers' main entry (not its `exports`-blocked package.json) -> onnxruntime-node -> its onnxruntime-common, and falls back to gitnexus' own direct dependency when the chain can't be walked. Also corrects the doc comment that claimed the gitnexus copy was already "version-aligned". Addresses a PR #2139 tri-review finding (P2). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../embeddings/onnxruntime-common-resolver.ts | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts index c92c48d08e..223674952e 100644 --- a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts +++ b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts @@ -19,12 +19,16 @@ * * ## What this does * Install a synchronous, in-thread ESM resolution hook (`module.registerHooks`, - * Node >= 22.15) that redirects `onnxruntime-common` to the copy gitnexus itself - * depends on — but only when the default resolver fails. onnxruntime-common is a - * direct gitnexus dependency (a stable, pure-JS package whose `Tensor` surface - * is unchanged across 1.24–1.26), version-aligned with the onnxruntime-node - * transformers loads, so it is always resolvable from gitnexus' own scope - * regardless of how the consumer's package manager linked things. On working + * Node >= 22.15) that redirects `onnxruntime-common` to a copy gitnexus can + * resolve — but only when the default resolver fails. The redirect target is + * preferentially the `onnxruntime-common` that `onnxruntime-node` (the native + * binding transformers actually loads) itself depends on, so the redirected copy + * is version-matched to that binding even under `pnpm dlx` — where gitnexus' + * npm-style `overrides` block does NOT apply, because it is honoured only from a + * root manifest and gitnexus is a transitive dependency there. It falls back to + * gitnexus' own direct `onnxruntime-common` dependency when that chain can't be + * walked. onnxruntime-common is a stable, pure-JS package whose `Tensor` surface + * is unchanged across 1.24–1.26, so either target is API-compatible. On working * layouts the default resolver succeeds first and the hook never fires, so * behaviour is unchanged. * @@ -48,6 +52,30 @@ import { logger } from '../logger.js'; let attempted = false; +/** + * Compute the file: URL the hook redirects `onnxruntime-common` to. + * + * Prefer the copy `onnxruntime-node` (the native binding transformers loads) + * depends on, so the redirected module is version-matched to the binding even + * under `pnpm dlx`, where transformers keeps its own pinned onnxruntime-node. + * The walk resolves transformers' MAIN entry — NOT `@huggingface/transformers/ + * package.json`, which transformers' `exports` map blocks + * (`ERR_PACKAGE_PATH_NOT_EXPORTED`) — then onnxruntime-node, then its + * onnxruntime-common. Falls back to gitnexus' own direct dependency (always + * resolvable from our scope) when any step fails. + */ +const resolveOnnxRuntimeCommonUrl = (): string => { + const require = createRequire(import.meta.url); + try { + const transformersMain = require.resolve('@huggingface/transformers'); + const ortNodePkg = createRequire(transformersMain).resolve('onnxruntime-node/package.json'); + const common = createRequire(ortNodePkg).resolve('onnxruntime-common'); + return pathToFileURL(common).href; + } catch { + return pathToFileURL(require.resolve('onnxruntime-common')).href; + } +}; + /** * Idempotently install the onnxruntime-common resolution fallback. Call once * immediately before the dynamic `import('@huggingface/transformers')` on the @@ -64,10 +92,7 @@ export const ensureOnnxRuntimeCommonResolvable = (): void => { // hooks API. Degrade gracefully — the import still works on hoisted layouts. if (typeof registerHooks !== 'function') return; - const require = createRequire(import.meta.url); - // Resolve from gitnexus' own scope — onnxruntime-common is a direct - // dependency, so this succeeds under npm and pnpm-strict alike. - const redirectUrl = pathToFileURL(require.resolve('onnxruntime-common')).href; + const redirectUrl = resolveOnnxRuntimeCommonUrl(); registerHooks({ resolve(specifier, context, nextResolve) { From 4c06492df0fb36d2a26688c823a0fe8b04327247 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:34:47 +0000 Subject: [PATCH 3/7] fix(embeddings): narrow the onnxruntime-common resolve fallback to absence errors (#307) The resolve closure's `catch` swallowed every error from `nextResolve` and redirected, which would silently paper over a genuinely present-but-broken onnxruntime-common install. Only substitute gitnexus' copy when the specifier is actually absent (ERR_MODULE_NOT_FOUND, or ERR_PACKAGE_PATH_NOT_EXPORTED for an exports-broken copy); rethrow anything else. Adds a test that an unrelated error code rethrows. Addresses a PR #2139 tri-review finding (P3). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/embeddings/onnxruntime-common-resolver.ts | 14 +++++++++++--- .../test/unit/onnxruntime-common-resolver.test.ts | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts index 223674952e..89c08cf565 100644 --- a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts +++ b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts @@ -98,11 +98,19 @@ export const ensureOnnxRuntimeCommonResolvable = (): void => { resolve(specifier, context, nextResolve) { if (specifier !== 'onnxruntime-common') return nextResolve(specifier, context); // Honour a real, package-manager-provided copy when one is on the path - // (npm / hoisted pnpm); only substitute ours when resolution fails. + // (npm / hoisted pnpm); only substitute ours when the specifier is + // genuinely absent. try { return nextResolve(specifier, context); - } catch { - return { url: redirectUrl, shortCircuit: true }; + } catch (err) { + // The phantom import surfaces as ERR_MODULE_NOT_FOUND (or, for a + // present-but-exports-broken copy, ERR_PACKAGE_PATH_NOT_EXPORTED). + // Rethrow anything else so a genuinely broken install is not masked. + const code = (err as { code?: string } | null | undefined)?.code; + if (code === 'ERR_MODULE_NOT_FOUND' || code === 'ERR_PACKAGE_PATH_NOT_EXPORTED') { + return { url: redirectUrl, shortCircuit: true }; + } + throw err; } }, }); diff --git a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts index cef511dd9d..17c5b11107 100644 --- a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts +++ b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts @@ -100,4 +100,18 @@ describe('ensureOnnxRuntimeCommonResolvable — resolve hook behaviour', () => { expect(() => resolve('some-other-package', ctx, next)).toThrow(err); }); + + it('rethrows when onnxruntime-common fails for a non-absence reason', async () => { + const resolve = await captureResolve(); + // A present-but-otherwise-broken resolution (not a missing package) must + // surface, not be silently papered over with gitnexus' copy. + const err = Object.assign(new Error('bad specifier'), { + code: 'ERR_INVALID_MODULE_SPECIFIER', + }); + const next = vi.fn(() => { + throw err; + }); + + expect(() => resolve('onnxruntime-common', ctx, next)).toThrow(err); + }); }); From 20cb251a691f99d48b5e5dbceda2f0fadc58a2fb Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:35:39 +0000 Subject: [PATCH 4/7] test(embeddings): cover the onnxruntime-common resolver best-effort swallow path (#307) The outer try/catch in ensureOnnxRuntimeCommonResolvable() was untested. A throwing registerHooks spy drives it; the call must not throw (initEmbedder does not guard the return, so a throw would break `analyze --embeddings`). The vitest quirk that surfaced an earlier attempt applies to throwing mock factories, not a throwing spy implementation, so this is testable cleanly. Addresses a PR #2139 tri-review finding (P2). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/unit/onnxruntime-common-resolver.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts index 17c5b11107..ade186e04c 100644 --- a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts +++ b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts @@ -53,6 +53,17 @@ describe('ensureOnnxRuntimeCommonResolvable — installation', () => { // Must not throw even though there is no synchronous-hooks API to call. expect(() => mod.ensureOnnxRuntimeCommonResolvable()).not.toThrow(); }); + + it('is best-effort: swallows a registerHooks() failure instead of throwing into the embedder', async () => { + const mod = await loadResolver( + vi.fn(() => { + throw new Error('hook-install-failed'); + }), + ); + // The call site (initEmbedder) does not guard the return; a throw here would + // break `analyze --embeddings`. The outer try/catch must absorb it. + expect(() => mod.ensureOnnxRuntimeCommonResolvable()).not.toThrow(); + }); }); describe('ensureOnnxRuntimeCommonResolvable — resolve hook behaviour', () => { From 3a3b004337ed1e179ecccbfa0d1b8f18965d071e Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:36:17 +0000 Subject: [PATCH 5/7] test(embeddings): tighten the onnxruntime-common redirect-URL assertion (#307) `/^file:\/\/.*onnxruntime-common/` matched the substring anywhere, so a lookalike path (e.g. `/x/onnxruntime-common-fake/`) would pass. Require an actual `/node_modules/onnxruntime-common/...js` segment so the assertion proves the redirect resolves to the real package, not just a string match. Addresses a PR #2139 tri-review finding (P3). Co-Authored-By: Claude Opus 4.8 (1M context) --- gitnexus/test/unit/onnxruntime-common-resolver.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts index ade186e04c..74468c3333 100644 --- a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts +++ b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts @@ -99,7 +99,9 @@ describe('ensureOnnxRuntimeCommonResolvable — resolve hook behaviour', () => { const res = resolve('onnxruntime-common', ctx, next) as { url: string; shortCircuit: boolean }; expect(res.shortCircuit).toBe(true); - expect(res.url).toMatch(/^file:\/\/.*onnxruntime-common/); // gitnexus' own copy + // The real resolved onnxruntime-common in node_modules (require.resolve runs + // for real here) — not just any path containing the substring. + expect(res.url).toMatch(/^file:\/\/.*\/node_modules\/onnxruntime-common\/.*\.js$/); }); it('never masks an unrelated resolution failure (other specifiers rethrow)', async () => { From 6aa1ce2edad1ad73b589291190f9e8f89e762eab Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:37:09 +0000 Subject: [PATCH 6/7] refactor(embeddings): drop the no-op __resetOnnxRuntimeCommonResolverForTests seam (#307) The test helper reloads the resolver via vi.resetModules() + a fresh import(), which already re-initialises the module-level one-shot `attempted` flag to false. The __reset export it then called was therefore a no-op. Remove the test-only export and its call; isolation now rests solely on vi.resetModules(). Addresses a PR #2139 tri-review finding (P3). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/embeddings/onnxruntime-common-resolver.ts | 5 ----- gitnexus/test/unit/onnxruntime-common-resolver.test.ts | 10 ++++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts index 89c08cf565..e4b9699c45 100644 --- a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts +++ b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts @@ -124,8 +124,3 @@ export const ensureOnnxRuntimeCommonResolvable = (): void => { ); } }; - -/** Test-only: reset the one-shot guard so installation can be re-exercised. */ -export const __resetOnnxRuntimeCommonResolverForTests = (): void => { - attempted = false; -}; diff --git a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts index 74468c3333..801cb001ee 100644 --- a/gitnexus/test/unit/onnxruntime-common-resolver.test.ts +++ b/gitnexus/test/unit/onnxruntime-common-resolver.test.ts @@ -13,16 +13,18 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; const RESOLVER = '../../src/core/embeddings/onnxruntime-common-resolver.js'; -/** (Re)load the resolver with a chosen `registerHooks` mocked into node:module. */ +/** + * (Re)load the resolver with a chosen `registerHooks` mocked into node:module. + * `vi.resetModules()` + the fresh `import()` re-initialises the module-level + * one-shot guard, so each test gets a pristine resolver with no shared state. + */ async function loadResolver(registerHooks: unknown) { vi.resetModules(); vi.doMock('node:module', async (importOriginal) => { const orig = await importOriginal(); return { ...orig, registerHooks }; }); - const mod = await import(RESOLVER); - mod.__resetOnnxRuntimeCommonResolverForTests(); - return mod; + return import(RESOLVER); } const ctx = { conditions: [], importAttributes: {} } as never; From 720e99c12b70644ce27a579f7c51588b7c930295 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 10 Jun 2026 10:37:52 +0000 Subject: [PATCH 7/7] docs(embeddings): correct the onnxruntime-common resolver isolation comment (#307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc comment claimed the hook "never affects other tools' resolution". Once installed, `module.registerHooks` is process-global and its resolve closure runs for every subsequent resolution — it passes them all through untouched and only substitutes the exact `onnxruntime-common` specifier on genuine absence, at a cost of one string comparison. Also note `registerHooks` is @experimental and requires Node >= 22.15 (graceful no-op below that). Comment-only. Addresses a PR #2139 tri-review finding (P3). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../embeddings/onnxruntime-common-resolver.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts index e4b9699c45..fbb4f40823 100644 --- a/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts +++ b/gitnexus/src/core/embeddings/onnxruntime-common-resolver.ts @@ -38,13 +38,20 @@ * no separate hook module, and no `data` marshalling. * * ## Safety - * Best-effort and idempotent. On Node < 22.15 `registerHooks` is absent and this - * is a graceful no-op (embeddings then resolve onnxruntime-common exactly as - * before — fine on hoisted layouts). Any failure is swallowed. The hook is - * installed lazily, only on the local-embedding code path, so it never affects - * analysis, the parse workers, or HTTP embedding mode; and it only ever touches - * the exact `onnxruntime-common` specifier on failure, so it cannot mask an - * unrelated resolution error. + * Best-effort and idempotent. The hook is installed lazily, only on the + * local-embedding code path (after parsing), so it is never registered during + * analysis, in the parse workers, or in HTTP embedding mode. Once installed it + * is process-global: its resolve closure runs for every subsequent module + * resolution, but it passes all of them through untouched and only substitutes a + * result for the exact `onnxruntime-common` specifier when that specifier is + * genuinely absent — so it cannot mask an unrelated resolution error, and the + * per-resolution cost is a single string comparison. + * + * `module.registerHooks` is marked `@experimental` and requires Node >= 22.15 + * (the gitnexus engines floor is >= 22.0.0). On older runtimes it is absent and + * this is a graceful no-op: embeddings then resolve onnxruntime-common exactly + * as before — fine on hoisted layouts. Any failure during installation is + * swallowed. */ import { registerHooks, createRequire } from 'node:module'; import { pathToFileURL } from 'node:url';