From b2a0c92bd4d4da89d7b7523af41ec0a79fcd64b0 Mon Sep 17 00:00:00 2001 From: Naadir Jeewa Date: Sat, 10 Oct 2026 21:25:28 +0100 Subject: [PATCH 1/3] fix(omp): resolve host modules and tokenizer from plugin runtime --- packages/pi-plugin/package.json | 2 +- .../src/dreamer/pi-session-api.test.ts | 37 ++++++ .../pi-plugin/src/dreamer/pi-session-api.ts | 25 ++++- packages/pi-plugin/src/omp-host-modules.d.ts | 29 +++++ ...session-formatting-tokenizer-roots.test.ts | 106 ++++++++++++++++++ .../magic-context/read-session-formatting.ts | 48 ++++++-- 6 files changed, 234 insertions(+), 13 deletions(-) create mode 100644 packages/pi-plugin/src/omp-host-modules.d.ts create mode 100644 packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts diff --git a/packages/pi-plugin/package.json b/packages/pi-plugin/package.json index 9ef555b59b..7cff1197b4 100644 --- a/packages/pi-plugin/package.json +++ b/packages/pi-plugin/package.json @@ -29,7 +29,7 @@ ], "//": "The build intentionally bundles typebox (no --external typebox) and keeps it a regular dependency. OMP's extension loader rewrites every bare `typebox` import to its own omptype-based shim, whose schemas are callable objects rather than JSON Schema; they fail structuredClone during tool registration, so an external typebox import stops Magic Context loading on OMP. src/typebox-bundling.test.ts guards this.", "scripts": { - "build": "bun ../../scripts/check-bun.mjs && tsc -p ../retina-local-fs/tsconfig.build.json && bun ../../scripts/clean-dist-chunks.mjs dist index.js subagent-entry.js historian-calibration-extension.js transformers-web.js transformers-node-wasm.js && bun build ../plugin/src/features/magic-context/memory/transformers-web-entry.ts --outfile dist/transformers-web.js --target browser --format esm --external onnxruntime-web && bun ../plugin/scripts/build-transformers-node-wasm.ts dist && bun build src/index.ts src/subagent-entry.ts src/historian-calibration-extension.ts ../plugin/src/features/magic-context/memory/embedding-worker.ts ../plugin/src/features/magic-context/migration-worker.ts ../plugin/src/hooks/magic-context/auto-search-worker.ts --outdir dist --entry-naming '[name].[ext]' --target node --format esm --splitting --external @earendil-works/pi-coding-agent --external @earendil-works/pi-tui --external onnxruntime-node --external onnxruntime-web --external sharp --external node:sqlite", + "build": "bun ../../scripts/check-bun.mjs && tsc -p ../retina-local-fs/tsconfig.build.json && bun ../../scripts/clean-dist-chunks.mjs dist index.js subagent-entry.js historian-calibration-extension.js transformers-web.js transformers-node-wasm.js && bun build ../plugin/src/features/magic-context/memory/transformers-web-entry.ts --outfile dist/transformers-web.js --target browser --format esm --external onnxruntime-web && bun ../plugin/scripts/build-transformers-node-wasm.ts dist && bun build src/index.ts src/subagent-entry.ts src/historian-calibration-extension.ts ../plugin/src/features/magic-context/memory/embedding-worker.ts ../plugin/src/features/magic-context/migration-worker.ts ../plugin/src/hooks/magic-context/auto-search-worker.ts --outdir dist --entry-naming '[name].[ext]' --target node --format esm --splitting --external @earendil-works/pi-coding-agent --external @oh-my-pi/pi-coding-agent --external @earendil-works/pi-tui --external onnxruntime-node --external onnxruntime-web --external sharp --external node:sqlite", "build:e2e-argv": "bun ../../scripts/clean-dist-chunks.mjs ../../tests/docker/.generated subagent-runner-e2e.mjs && bun build src/subagent-runner.ts --outdir ../../tests/docker/.generated --entry-naming 'subagent-runner-e2e.mjs' --splitting --target node --format esm --external node:sqlite && node --input-type=module -e 'const { __test } = await import(\"../../tests/docker/.generated/subagent-runner-e2e.mjs\"); if (typeof __test.buildArgs !== \"function\" || typeof __test.resolvePiInvocation !== \"function\") throw new Error(\"argv renderer exports missing\"); console.log(\"Node argv renderer: 2 exports loaded\")'", "typecheck": "tsc -p ../retina-local-fs/tsconfig.build.json && tsc --noEmit", "test": "bun ../../scripts/check-bun.mjs && bun install --frozen-lockfile && BUN_JSC_useOMGJIT=0 bun test --parallel=4 --timeout 30000", diff --git a/packages/pi-plugin/src/dreamer/pi-session-api.test.ts b/packages/pi-plugin/src/dreamer/pi-session-api.test.ts index b26e744d06..fab561861d 100644 --- a/packages/pi-plugin/src/dreamer/pi-session-api.test.ts +++ b/packages/pi-plugin/src/dreamer/pi-session-api.test.ts @@ -184,6 +184,43 @@ describe("loadDefaultPiSessionApi", () => { expect(await api.listSessions()).toEqual(["omp-bare-import"]); }, 30000); + it("loads the legacy-scope package through the first bare-import fallback", async () => { + // Same harness as the OMP loader above, for the other host scope: each + // bare loader must actually resolve a package through node_modules and + // hand back the real session API, not merely be present in the list. + // The unique marker is what proves WHICH copy was imported -- a loader + // that silently resolved the repo's own devDependency instead of the + // fixture would return "9.9.9" and fail here. + const dir = createTestTempDir("pi-bare-import-").dir; + writeFixturePackage( + join(dir, "node_modules", "@earendil-works", "pi-coding-agent"), + { + manifest: { + name: PI_SPEC, + version: "0.84.1", + exports: { ".": { import: "./index.js" } }, + }, + files: { "index.js": fixtureModule("pi-bare-import") }, + }, + ); + const resolverCopy = join(dir, "src", "pi-session-api.ts"); + mkdirSync(dirname(resolverCopy), { recursive: true }); + copyFileSync( + fileURLToPath(new URL("./pi-session-api.ts", import.meta.url)), + resolverCopy, + ); + + const resolver = (await import( + pathToFileURL(resolverCopy).href + )) as typeof import("./pi-session-api"); + const bareLoader = resolver.defaultLoaders.find( + (loader) => loader.name === "Bare import", + ); + if (!bareLoader) throw new Error("bare-import loader missing"); + const api = await resolver.loadDefaultPiSessionApi([bareLoader]); + expect(await api.listSessions()).toEqual(["pi-bare-import"]); + }, 30000); + it("resolves through a bin-shim symlink when argv[1] is the shim path", async () => { const dir = createTestTempDir("pi-symlink-test-").dir; const pkgRoot = join( diff --git a/packages/pi-plugin/src/dreamer/pi-session-api.ts b/packages/pi-plugin/src/dreamer/pi-session-api.ts index c2fa4c4c7e..da5aea1634 100644 --- a/packages/pi-plugin/src/dreamer/pi-session-api.ts +++ b/packages/pi-plugin/src/dreamer/pi-session-api.ts @@ -295,12 +295,33 @@ export const defaultLoaders: ModuleLoader[] = [ }, }, { + // A static import cannot work here: this package is a peer of whichever + // host loaded the extension, and it is absent in the others, so the three + // loaders must probe in order and only the winner is ever evaluated. + // + // The specifier MUST stay a string literal, not the PI_CODING_AGENT_MODULE + // constant. OMP resolves host packages for legacy extensions by rewriting + // the extension SOURCE TEXT, and it only records a reference when the + // specifier parses as a StringLiteral (@oh-my-pi/pi-coding-agent + // src/extensibility/plugins/legacy-pi-compat.ts + // `collectExtensionSpecifierReferences` -> `record("import", node.source)`). + // A variable specifier is invisible to that pass, and inside the shipped + // `omp` binary (a `bun build --compile` executable whose own resolver does + // not reach an on-disk node_modules) the import then fails outright with + // "Cannot find package '@earendil-works/pi-coding-agent' imported from + // ~/.omp/plugins/node_modules/@cortexkit/pi-magic-context/dist/index.js", + // which took down the [session-projects] backfill and the dreamer + // retrospective. Bun keeps both literals runtime-resolved: they are + // `--external` in packages/pi-plugin/package.json `build`. name: "Bare import", - load: async () => await import(/* @vite-ignore */ PI_CODING_AGENT_MODULE), + load: async () => + await import(/* @vite-ignore */ "@earendil-works/pi-coding-agent"), }, { + // Same contract in OMP's canonical scope. name: "Bare import (OMP)", - load: async () => await import(/* @vite-ignore */ OMP_CODING_AGENT_MODULE), + load: async () => + await import(/* @vite-ignore */ "@oh-my-pi/pi-coding-agent"), }, ]; diff --git a/packages/pi-plugin/src/omp-host-modules.d.ts b/packages/pi-plugin/src/omp-host-modules.d.ts new file mode 100644 index 0000000000..ec8f477d96 --- /dev/null +++ b/packages/pi-plugin/src/omp-host-modules.d.ts @@ -0,0 +1,29 @@ +/** + * Ambient declaration for OMP's canonical coding-agent package. + * + * `dreamer/pi-session-api.ts` imports this specifier as a STRING LITERAL so the + * OMP legacy-extension loader can rewrite it (that loader only records + * references whose specifier parses as a StringLiteral), and the build keeps it + * `--external` so it is resolved at runtime rather than bundled. It is + * deliberately NOT a dependency: the shipped `omp` binary is a + * `bun build --compile` executable whose host modules live inside the + * executable, and adding a real devDependency here would pin a second, + * divergent copy of the host API -- the exact drift this resolver exists to + * avoid (see the header of dreamer/pi-session-api.ts). TypeScript can still + * only accept the literal import against a declaration, so it is declared here. + * + * The members are the ones the resolver probes, and the ones the running host + * actually exposes: loading the extension under `omp` and awaiting the bare + * import yields `SessionManager=function parseSessionEntries=function + * loadEntriesFromFile=function`. Same role as OMP's own + * `src/extensibility/plugins/legacy-pi-virtual-modules.d.ts`. + */ +declare module "@oh-my-pi/pi-coding-agent" { + export const SessionManager: { + listAll(sessionDir?: string): unknown[] | Promise; + }; + export function loadEntriesFromFile( + filePath: string, + ): unknown[] | Promise; + export function parseSessionEntries(content: string): unknown[]; +} diff --git a/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts new file mode 100644 index 0000000000..20de24ce75 --- /dev/null +++ b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts @@ -0,0 +1,106 @@ +import { expect, test } from "bun:test"; +import { existsSync, mkdirSync, realpathSync } from "node:fs"; +import { homedir } from "node:os"; +import { dirname, join, resolve, sep } from "node:path"; +import { fileURLToPath } from "node:url"; +import { + getTokenizerNativeMemoryStats, + preloadTokenizer, + tokenizerPackageRoots, +} from "./read-session-formatting"; + +/** + * Runtime coverage for where the tokenizer is allowed to come from. + * + * A compiled host (OMP ships as a `bun build --compile` executable) launches + * the extension with `process.argv[1]` holding a user CLI argument rather than + * a module path, and the plugin's own install tree is the only place + * ai-tokenizer is guaranteed to live, because the plugin declares it. That is + * precisely the state in which Magic Context logged "ai-tokenizer is + * unavailable; using approximate character-based token counts" on a host whose + * ~/.omp/plugins/node_modules/ai-tokenizer was installed and working: every + * probe root was derived from cwd or argv[1], and neither reaches the plugin. + * + * The tests therefore run against the real loader with cwd and argv[1] both + * pointing away from any node_modules tree, so the only candidates that can + * succeed are the extension's own ancestor chain and the host plugin root. + */ +const NEUTRAL_CWD = join(homedir(), ".cache", "magic-context-tests", "tokenizer-roots-cwd"); + +function moduleAncestors(): string[] { + const ownDir = dirname(fileURLToPath(new URL(import.meta.url))); + const ancestors: string[] = []; + let dir = ownDir; + while (true) { + ancestors.push(join(dir, "node_modules", "ai-tokenizer")); + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; + } + return ancestors; +} + +test("tokenizer probe roots include the extension's own install tree", () => { + // The compiled-host condition: neither the launch cwd nor argv[1] reaches a + // node_modules tree, so the only chain that can still locate the declared + // dependency is the tree the plugin was loaded from. Without it every + // candidate is a dead end and the loader reports "ai-tokenizer was not + // found under the project, runtime, or OpenCode cache node_modules roots". + mkdirSync(NEUTRAL_CWD, { recursive: true }); + const originalCwd = process.cwd(); + const originalArgv1 = process.argv[1]; + try { + process.chdir(NEUTRAL_CWD); + process.argv[1] = "-p"; // what the compiled host actually holds + const roots = tokenizerPackageRoots().map((root) => resolve(root)); + for (const root of roots) { + expect(root.endsWith(join("node_modules", "ai-tokenizer"))).toBe(true); + } + for (const ancestor of moduleAncestors().map((root) => resolve(root))) { + expect(roots).toContain(ancestor); + } + // A linked install (`omp plugin install `) loads the module from + // the linked source while its hoisted dependencies live in OMP's own + // plugin tree, so that root has to be probed too. + expect(roots).toContain( + resolve(join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer")), + ); + } finally { + process.chdir(originalCwd); + process.argv[1] = originalArgv1; + } +}); + +test("the tokenizer loads with cwd and argv[1] pointing at no node_modules tree", async () => { + mkdirSync(NEUTRAL_CWD, { recursive: true }); + const originalCwd = process.cwd(); + const originalArgv1 = process.argv[1]; + try { + // Precondition: the neutral cwd brings nothing to the table, so a pass + // here can only come from a tree the plugin itself owns or the host + // plugin root it is installed under. + expect(existsSync(join(NEUTRAL_CWD, "node_modules", "ai-tokenizer"))).toBe(false); + process.chdir(NEUTRAL_CWD); + process.argv[1] = "-p"; // what the compiled host actually holds + + expect(await preloadTokenizer()).toBe(true); + const stats = getTokenizerNativeMemoryStats(); + expect(stats.loaded).toBe(true); + expect(stats.tablePath).not.toBeNull(); + // bun's isolated install symlinks node_modules/ai-tokenizer into + // node_modules/.bun/..., and the loader realpaths what it imports, so + // the comparison has to be made on resolved directories. + const sanctioned = [ + ...moduleAncestors(), + join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer"), + ] + .filter((root) => existsSync(root)) + .map((root) => realpathSync(root)); + expect(sanctioned.some((root) => stats.tablePath?.startsWith(root + sep))).toBe(true); + // The launch cwd and argv[1] contributed nothing: neither is under it. + expect(stats.tablePath?.startsWith(NEUTRAL_CWD + sep)).toBe(false); + } finally { + process.chdir(originalCwd); + process.argv[1] = originalArgv1; + } +}, 30_000); diff --git a/packages/plugin/src/hooks/magic-context/read-session-formatting.ts b/packages/plugin/src/hooks/magic-context/read-session-formatting.ts index 27a29f7ec2..1d1d46faa3 100644 --- a/packages/plugin/src/hooks/magic-context/read-session-formatting.ts +++ b/packages/plugin/src/hooks/magic-context/read-session-formatting.ts @@ -2,7 +2,7 @@ import { existsSync, readFileSync, realpathSync, statSync } from "node:fs"; import { createRequire } from "node:module"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; -import { pathToFileURL } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; import { COMMIT_VERB_PATTERN, createCommitHashExtractPattern } from "../../shared/commit-detection"; import { OMO_INTERNAL_INITIATOR_MARKER } from "../../shared/internal-initiator-marker"; import { log } from "../../shared/logger"; @@ -154,10 +154,19 @@ let tokenizerWarningSent = false; let tokenizerEncodingPath: string | undefined; let tokenizerSerializedTableBytes: number | null | undefined; -function tokenizerPackageRoots(): string[] { +/** Candidate `ai-tokenizer` package directories, in probe order. Exported for + * read-session-formatting-tokenizer-roots.test.ts, which pins the compiled-host + * case where only the extension's own install tree or the host plugin root can + * supply the dependency. */ +export function tokenizerPackageRoots(): string[] { const cwd = process.cwd(); const openCodeCache = join(process.env.XDG_CACHE_HOME ?? join(homedir(), ".cache"), "opencode"); - const roots = [cwd, openCodeCache]; + // OMP's plugin tree, where a linked install (`omp plugin install `) + // leaves the package's hoisted dependencies while the module itself loads + // from the linked source directory. Same class of host-specific root as the + // OpenCode cache above. + const ompPlugins = join(homedir(), ".omp", "plugins"); + const roots = [cwd, openCodeCache, ompPlugins]; const candidates: string[] = []; for (const root of roots) { for (const packageDir of TOKENIZER_PACKAGE_DIRS) { @@ -170,13 +179,32 @@ function tokenizerPackageRoots(): string[] { candidates.push(join(root, "node_modules", "ai-tokenizer")); } - let ancestor = process.argv[1] ? dirname(resolve(process.argv[1])) : cwd; - while (true) { - candidates.push(join(ancestor, "node_modules", "ai-tokenizer")); - const parent = dirname(ancestor); - if (parent === ancestor) break; - ancestor = parent; - } + const pushAncestors = (startDir: string): void => { + let ancestor = startDir; + while (true) { + candidates.push(join(ancestor, "node_modules", "ai-tokenizer")); + const parent = dirname(ancestor); + if (parent === ancestor) break; + ancestor = parent; + } + }; + + pushAncestors(process.argv[1] ? dirname(resolve(process.argv[1])) : cwd); + + // The tree the extension was actually loaded from is the authoritative + // root: ai-tokenizer is a declared dependency of this package, so its + // node_modules chain is where a correct install keeps it. The argv walk + // above cannot reach it under a compiled host binary -- OMP ships as a + // `bun build --compile` executable whose process.argv[1] is a user CLI + // argument rather than the module path -- which is exactly when this + // probe runs, so the run fell back to approximate character counts while + // ~/.omp/plugins/node_modules/ai-tokenizer sat unused on disk. + // The host appends a `?mtime=` cache-bust to the loaded module's identity; + // fileURLToPath drops the query, and dirname is applied to the FILE path so + // no segment is lost to a trailing slash. + const ownDir = dirname(fileURLToPath(new URL(import.meta.url))); + pushAncestors(ownDir); + return [...new Set(candidates)]; } From 5985ff0c6525d2784bdbe3f7f800862f2d8a0083 Mon Sep 17 00:00:00 2001 From: Naadir Jeewa Date: Sat, 10 Oct 2026 22:12:49 +0100 Subject: [PATCH 2/3] fix(tokenizer): prove fallback loading and prefer plugin dependencies --- ...session-formatting-tokenizer-roots.test.ts | 288 +++++++++++++++--- .../magic-context/read-session-formatting.ts | 29 +- 2 files changed, 270 insertions(+), 47 deletions(-) diff --git a/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts index 20de24ce75..33e01bf970 100644 --- a/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts +++ b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts @@ -1,13 +1,10 @@ import { expect, test } from "bun:test"; -import { existsSync, mkdirSync, realpathSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { existsSync, cpSync, mkdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve, sep } from "node:path"; -import { fileURLToPath } from "node:url"; -import { - getTokenizerNativeMemoryStats, - preloadTokenizer, - tokenizerPackageRoots, -} from "./read-session-formatting"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { tokenizerPackageRoots } from "./read-session-formatting"; /** * Runtime coverage for where the tokenizer is allowed to come from. @@ -21,11 +18,23 @@ import { * ~/.omp/plugins/node_modules/ai-tokenizer was installed and working: every * probe root was derived from cwd or argv[1], and neither reaches the plugin. * - * The tests therefore run against the real loader with cwd and argv[1] both - * pointing away from any node_modules tree, so the only candidates that can - * succeed are the extension's own ancestor chain and the host plugin root. + * `preloadTokenizer()` tries the synchronous `createRequire(import.meta.url)` + * loader first and only falls back to `tokenizerPackageRoots()` -> + * `loadTokenizerFromInstalledPackage()` when that first loader throws. So an + * in-process test that imports this real module can never exercise the + * fallback: ai-tokenizer is a declared dev dependency of the package, so the + * primary loader resolves it from the repo tree regardless of cwd/argv[1]. + * The loader tests therefore run a COPY of the module inside an isolated + * fixture whose entire ancestor `node_modules` chain carries no ai-tokenizer, + * which forces the initial loader to throw and proves the fallback binds the + * sanctioned plugin-tree copy. */ const NEUTRAL_CWD = join(homedir(), ".cache", "magic-context-tests", "tokenizer-roots-cwd"); +const TESTS_ROOT = join(homedir(), ".cache", "magic-context-tests"); + +const SRC_HOOK_DIR = dirname(fileURLToPath(new URL("./read-session-formatting.ts", import.meta.url))); +const SRC_HOOK = join(SRC_HOOK_DIR, "read-session-formatting.ts"); +const SRC_SHARED = join(SRC_HOOK_DIR, "..", "..", "shared"); function moduleAncestors(): string[] { const ownDir = dirname(fileURLToPath(new URL(import.meta.url))); @@ -40,6 +49,137 @@ function moduleAncestors(): string[] { return ancestors; } +/** + * A planted `ai-tokenizer` whose `encode` returns an array of length + * `text.length + marker`, so a token count identifies exactly which copy the + * loader bound (the real ai-tokenizer never produces those lengths, and two + * planted copies carry different markers). + */ +function plantAiTokenizer(packageDir: string, marker: number): void { + mkdirSync(join(packageDir, "encoding"), { recursive: true }); + writeFileSync( + join(packageDir, "package.json"), + JSON.stringify({ + name: "ai-tokenizer", + version: `0.0.0-planted-${marker}`, + type: "module", + exports: { + ".": { import: "./index.js" }, + "./encoding/claude": { import: "./encoding/claude.js" }, + }, + }), + ); + writeFileSync( + join(packageDir, "index.js"), + `export class Tokenizer {\n` + + ` constructor(encoding) { this.encoding = encoding; }\n` + + ` encode(text, _allowedSpecial) { return new Array(text.length + ${marker}).fill(1); }\n` + + `}\n` + + `export default Tokenizer;\n`, + ); + writeFileSync( + join(packageDir, "encoding", "claude.js"), + `const claude = { marker: ${marker}, specialTokens: { "": 100256 } };\n` + + `export default claude;\n` + + `export { claude };\n`, + ); +} + +interface LoaderResult { + ok?: boolean; + loaded?: boolean; + tablePath?: string | null; + count?: number | null; + encErr?: string | null; + error?: string; +} + +/** + * Run `preloadTokenizer()` for a copied module inside an isolated fixture + * (fresh `$HOME`, neutral cwd, controllable `argv[1]`) so the synchronous + * primary loader is forced to fail and only `tokenizerPackageRoots()` can + * satisfy the load. Returns the loader's reported identity. + */ +function runIsolatedLoader(opts: { + fakeHome: string; + moduleDir: string; + cwd: string; + argv1: string; + sample: string; +}): LoaderResult { + const modulePath = join(opts.moduleDir, "src", "hooks", "magic-context", "read-session-formatting.ts"); + const moduleUrl = pathToFileURL(modulePath).href; + // The fixture module lives at a runtime-generated absolute path, so this + // import inside the child cannot be a static import (test-loading boundary). + const script = + `(async () => {\n` + + ` process.argv[1] = ${JSON.stringify(opts.argv1)};\n` + + ` process.chdir(${JSON.stringify(opts.cwd)});\n` + + ` const mod = await import(${JSON.stringify(moduleUrl)}); // fixture path is runtime-generated\n` + + ` const ok = await mod.preloadTokenizer();\n` + + ` const stats = mod.getTokenizerNativeMemoryStats();\n` + + ` let count = null; let encErr = null;\n` + + ` try { count = mod.estimateTokens(${JSON.stringify(opts.sample)}); } ` + + ` catch (e) { encErr = String((e && e.stack) || e); }\n` + + ` process.stdout.write(JSON.stringify({ ok, loaded: stats.loaded, tablePath: stats.tablePath, count, encErr }));\n` + + `})().catch((e) => process.stdout.write(JSON.stringify({ error: String((e && e.stack) || e) })));\n`; + + // `--no-install` (plus fresh BUN_INSTALL / XDG_CACHE_HOME) stops Bun's + // auto-install from fetching ai-tokenizer to satisfy the bare specifier in + // the primary `createRequire(import.meta.url)` loader. Without it the + // copied module's ancestor chain is empty, Bun downloads ai-tokenizer on + // demand, the primary loader succeeds, and the fallback stays unexercised -- + // the opposite of what this fixture proves. With it the bare resolve throws + // exactly as it does on the offline compiled host, forcing path B. + const child = spawnSync(process.execPath, ["--no-install", "-e", script], { + cwd: opts.cwd, + env: { + ...process.env, + HOME: opts.fakeHome, + BUN_INSTALL: opts.fakeHome, + XDG_CACHE_HOME: join(opts.fakeHome, ".cache"), + }, + encoding: "utf8", + windowsHide: true, + }); + if (child.error) { + throw new Error(`failed to spawn loader fixture: ${String(child.error)}`); + } + const out = (child.stdout ?? "").trim(); + if (child.status !== 0 || out === "") { + throw new Error( + `loader fixture exited ${String(child.status)}: stdout=${JSON.stringify(out)} stderr=${JSON.stringify(child.stderr)}`, + ); + } + return JSON.parse(out) as LoaderResult; +} + +/** + * Copy the module plus its shared runtime closure into an isolated tree under + * `base`, asserting nothing under that tree already carries ai-tokenizer (which + * would let the primary loader succeed and leave the fallback unexercised). + */ +function buildLoaderFixture(base: string): { moduleDir: string } { + const moduleDir = join(base, "mod"); + const hookDir = join(moduleDir, "src", "hooks", "magic-context"); + mkdirSync(hookDir, { recursive: true }); + cpSync(SRC_HOOK, join(hookDir, "read-session-formatting.ts")); + // Copy the shared directory (minus test files); the loader's import closure + // stays inside it, and bun only compiles files the copied module actually + // imports, so unreachable shared modules (and their own tree dependencies) + // are never loaded here. + cpSync(SRC_SHARED, join(moduleDir, "src", "shared"), { + recursive: true, + filter: (source) => !/(\.test\.ts|\.test-support\.ts)$/.test(source), + }); + for (const dir of [moduleDir, join(moduleDir, "src"), base]) { + expect(existsSync(join(dir, "node_modules"))).toBe(false); + } + return { moduleDir }; +} + +const SAMPLE_TEXT = "Coverage check: naïve café 日本語 const x = 1; // 3.14159"; + test("tokenizer probe roots include the extension's own install tree", () => { // The compiled-host condition: neither the launch cwd nor argv[1] reaches a // node_modules tree, so the only chain that can still locate the declared @@ -62,45 +202,113 @@ test("tokenizer probe roots include the extension's own install tree", () => { // A linked install (`omp plugin install `) loads the module from // the linked source while its hoisted dependencies live in OMP's own // plugin tree, so that root has to be probed too. - expect(roots).toContain( - resolve(join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer")), - ); + const hostPlugins = resolve(join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer")); + expect(roots).toContain(hostPlugins); + // Precedence contract: `findTokenizerImportPaths` binds the first + // resolvable candidate, so the host-wide ~/.omp/plugins copy -- a + // long-lived global tree that may hold a stale or foreign ai-tokenizer -- + // must be probed strictly AFTER every one of the plugin's own-tree + // ancestors, keeping the declared version authoritative. + const hostIndex = roots.indexOf(hostPlugins); + expect(hostIndex).toBeGreaterThan(-1); + for (const ancestor of moduleAncestors().map((root) => resolve(root))) { + expect(roots.indexOf(ancestor)).toBeLessThan(hostIndex); + } } finally { process.chdir(originalCwd); process.argv[1] = originalArgv1; } }); -test("the tokenizer loads with cwd and argv[1] pointing at no node_modules tree", async () => { +test("the fallback loads from the plugin tree when the primary loader fails", () => { + // Forces the initial `loadTokenizer()` (createRequire from import.meta.url) + // to throw, because the copied module's ancestor chain carries no + // ai-tokenizer. The only sanctioned copy is planted under the host plugin + // root, so a pass proves the `tokenizerPackageRoots()` fallback ran and + // bound it. mkdirSync(NEUTRAL_CWD, { recursive: true }); - const originalCwd = process.cwd(); - const originalArgv1 = process.argv[1]; + const base = join(TESTS_ROOT, `tokenizer-fallback-${process.pid}-${Date.now()}`); + const fakeHome = join(base, "home"); + const neutral = join(base, "neutral"); + mkdirSync(neutral, { recursive: true }); try { - // Precondition: the neutral cwd brings nothing to the table, so a pass - // here can only come from a tree the plugin itself owns or the host - // plugin root it is installed under. - expect(existsSync(join(NEUTRAL_CWD, "node_modules", "ai-tokenizer"))).toBe(false); - process.chdir(NEUTRAL_CWD); - process.argv[1] = "-p"; // what the compiled host actually holds + const { moduleDir } = buildLoaderFixture(base); + const hostPkg = join(fakeHome, ".omp", "plugins", "node_modules", "ai-tokenizer"); + // Precondition: the neutral cwd and the fake home bring nothing to the + // table before this copy is planted, so a pass can only come from it. + expect(existsSync(join(neutral, "node_modules", "ai-tokenizer"))).toBe(false); + expect(existsSync(hostPkg)).toBe(false); + plantAiTokenizer(hostPkg, 991); - expect(await preloadTokenizer()).toBe(true); - const stats = getTokenizerNativeMemoryStats(); - expect(stats.loaded).toBe(true); - expect(stats.tablePath).not.toBeNull(); - // bun's isolated install symlinks node_modules/ai-tokenizer into - // node_modules/.bun/..., and the loader realpaths what it imports, so - // the comparison has to be made on resolved directories. - const sanctioned = [ - ...moduleAncestors(), - join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer"), - ] - .filter((root) => existsSync(root)) - .map((root) => realpathSync(root)); - expect(sanctioned.some((root) => stats.tablePath?.startsWith(root + sep))).toBe(true); - // The launch cwd and argv[1] contributed nothing: neither is under it. - expect(stats.tablePath?.startsWith(NEUTRAL_CWD + sep)).toBe(false); + const result = runIsolatedLoader({ + fakeHome, + moduleDir, + cwd: neutral, + argv1: "-p", // what the compiled host actually holds + sample: SAMPLE_TEXT, + }); + + expect(result.error).toBeUndefined(); + expect(result.ok).toBe(true); + expect(result.loaded).toBe(true); + expect(result.tablePath).not.toBeNull(); + expect(result.encErr).toBeNull(); + // The fallback bound the planted host plugin-tree copy: its table lives + // under that package, and its token count carries the planted marker -- + // neither the real repo dependency nor the launch cwd could produce it. + const hostPkgReal = realpathSync(hostPkg); + expect(result.tablePath?.startsWith(hostPkgReal + sep)).toBe(true); + expect(result.tablePath?.startsWith(resolve(neutral) + sep)).toBe(false); + expect(result.tablePath?.startsWith(resolve(moduleDir) + sep)).toBe(false); + expect(result.count).toBe(SAMPLE_TEXT.length + 991); } finally { - process.chdir(originalCwd); - process.argv[1] = originalArgv1; + rmSync(base, { recursive: true, force: true }); + } +}, 30_000); + +test("the plugin's own tree outranks the host-wide ~/.omp/plugins copy", () => { + // Regression for the probe-order precedence fix: two sanctioned copies exist, + // the plugin's own install tree and the host-wide ~/.omp/plugins global. + // Because the initial loader fails for the copied module, the result is + // decided purely by `tokenizerPackageRoots()` probe order feeding + // `findTokenizerImportPaths` (first resolvable candidate wins). The own-tree + // copy must bind; if ~/.omp/plugins were probed before the own-tree ancestors + // the stale host copy would win instead. + mkdirSync(NEUTRAL_CWD, { recursive: true }); + const base = join(TESTS_ROOT, `tokenizer-precedence-${process.pid}-${Date.now()}`); + const fakeHome = join(base, "home"); + const neutral = join(base, "neutral"); + mkdirSync(neutral, { recursive: true }); + try { + const { moduleDir } = buildLoaderFixture(base); + const hostPkg = join(fakeHome, ".omp", "plugins", "node_modules", "ai-tokenizer"); + // The own-tree copy is reached by pushAncestors(argv[1]): argv[1] points + // into /launcher/deep, so /launcher/node_modules is an + // ancestor probed before the host-wide tree. + const ownTreePkg = join(base, "launcher", "node_modules", "ai-tokenizer"); + expect(existsSync(hostPkg)).toBe(false); + expect(existsSync(ownTreePkg)).toBe(false); + plantAiTokenizer(hostPkg, 555); // stale host copy -- must NOT win + plantAiTokenizer(ownTreePkg, 33); // the authoritative copy + + const result = runIsolatedLoader({ + fakeHome, + moduleDir, + cwd: neutral, + argv1: join(base, "launcher", "deep", "bin.ts"), + sample: SAMPLE_TEXT, + }); + + expect(result.error).toBeUndefined(); + expect(result.ok).toBe(true); + expect(result.loaded).toBe(true); + expect(result.tablePath).not.toBeNull(); + expect(result.encErr).toBeNull(); + // The authoritative own-tree copy (marker 33) won, not the host copy (555). + expect(result.count).toBe(SAMPLE_TEXT.length + 33); + expect(result.tablePath?.startsWith(realpathSync(ownTreePkg) + sep)).toBe(true); + expect(result.tablePath?.startsWith(realpathSync(hostPkg) + sep)).toBe(false); + } finally { + rmSync(base, { recursive: true, force: true }); } }, 30_000); diff --git a/packages/plugin/src/hooks/magic-context/read-session-formatting.ts b/packages/plugin/src/hooks/magic-context/read-session-formatting.ts index 1d1d46faa3..8e890ba45d 100644 --- a/packages/plugin/src/hooks/magic-context/read-session-formatting.ts +++ b/packages/plugin/src/hooks/magic-context/read-session-formatting.ts @@ -161,14 +161,10 @@ let tokenizerSerializedTableBytes: number | null | undefined; export function tokenizerPackageRoots(): string[] { const cwd = process.cwd(); const openCodeCache = join(process.env.XDG_CACHE_HOME ?? join(homedir(), ".cache"), "opencode"); - // OMP's plugin tree, where a linked install (`omp plugin install `) - // leaves the package's hoisted dependencies while the module itself loads - // from the linked source directory. Same class of host-specific root as the - // OpenCode cache above. - const ompPlugins = join(homedir(), ".omp", "plugins"); - const roots = [cwd, openCodeCache, ompPlugins]; const candidates: string[] = []; - for (const root of roots) { + // Probe a root for both a dependency nested under the plugin package and a + // copy hoisted directly into the root's own node_modules. + const probeRoot = (root: string): void => { for (const packageDir of TOKENIZER_PACKAGE_DIRS) { // Prefer a dependency nested under the plugin over a conflicting // version hoisted by the host application. @@ -177,6 +173,13 @@ export function tokenizerPackageRoots(): string[] { ); } candidates.push(join(root, "node_modules", "ai-tokenizer")); + }; + // `findTokenizerImportPaths` binds the first candidate whose package.json + // resolves, so probe order IS the precedence contract: the launch root and + // the OpenCode cache are host-specific trees, then the plugin's own install + // tree (below), and only LAST the host-wide OMP plugin tree. + for (const root of [cwd, openCodeCache]) { + probeRoot(root); } const pushAncestors = (startDir: string): void => { @@ -205,6 +208,18 @@ export function tokenizerPackageRoots(): string[] { const ownDir = dirname(fileURLToPath(new URL(import.meta.url))); pushAncestors(ownDir); + // OMP's plugin tree, where a linked install (`omp plugin install `) + // leaves the package's hoisted dependencies while the module itself loads + // from the linked source directory. Same class of host-specific root as the + // OpenCode cache above, but it MUST stay below the plugin's own install tree: + // `~/.omp/plugins/node_modules` is a long-lived host-wide tree, so a stale or + // plugin-unrelated `ai-tokenizer` hoisted there must never outrank the version + // this package declares in its own tree (probed just above) -- a wrong copy + // would silently change the vocabulary behind persisted per-message counts and + // budget/compartment decisions. It is still probed, last, so the linked-source + // case (where only the host tree carries the dependency) keeps working. + probeRoot(join(homedir(), ".omp", "plugins")); + return [...new Set(candidates)]; } From 49f86b588d67057aa0cb5fcceb46f2c82b18a64c Mon Sep 17 00:00:00 2001 From: Naadir Jeewa Date: Sun, 11 Oct 2026 00:58:26 +0100 Subject: [PATCH 3/3] test(tokenizer): copy recursive relative runtime import closure --- ...session-formatting-tokenizer-roots.test.ts | 113 ++++++++++++++---- 1 file changed, 93 insertions(+), 20 deletions(-) diff --git a/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts index 33e01bf970..9b52015e77 100644 --- a/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts +++ b/packages/plugin/src/hooks/magic-context/read-session-formatting-tokenizer-roots.test.ts @@ -1,8 +1,17 @@ import { expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { existsSync, cpSync, mkdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { + cpSync, + existsSync, + mkdirSync, + readFileSync, + realpathSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; import { homedir } from "node:os"; -import { dirname, join, resolve, sep } from "node:path"; +import { dirname, extname, join, relative, resolve, sep } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { tokenizerPackageRoots } from "./read-session-formatting"; @@ -32,9 +41,13 @@ import { tokenizerPackageRoots } from "./read-session-formatting"; const NEUTRAL_CWD = join(homedir(), ".cache", "magic-context-tests", "tokenizer-roots-cwd"); const TESTS_ROOT = join(homedir(), ".cache", "magic-context-tests"); -const SRC_HOOK_DIR = dirname(fileURLToPath(new URL("./read-session-formatting.ts", import.meta.url))); +const SRC_HOOK_DIR = dirname( + fileURLToPath(new URL("./read-session-formatting.ts", import.meta.url)), +); const SRC_HOOK = join(SRC_HOOK_DIR, "read-session-formatting.ts"); -const SRC_SHARED = join(SRC_HOOK_DIR, "..", "..", "shared"); +// The fixture tree mirrors `packages/plugin/src`, so every relative specifier +// inside the copied closure resolves against the same layout as the original. +const SRC_PKG = join(SRC_HOOK_DIR, "..", ".."); function moduleAncestors(): string[] { const ownDir = dirname(fileURLToPath(new URL(import.meta.url))); @@ -107,7 +120,13 @@ function runIsolatedLoader(opts: { argv1: string; sample: string; }): LoaderResult { - const modulePath = join(opts.moduleDir, "src", "hooks", "magic-context", "read-session-formatting.ts"); + const modulePath = join( + opts.moduleDir, + "src", + "hooks", + "magic-context", + "read-session-formatting.ts", + ); const moduleUrl = pathToFileURL(modulePath).href; // The fixture module lives at a runtime-generated absolute path, so this // import inside the child cannot be a static import (test-loading boundary). @@ -155,23 +174,75 @@ function runIsolatedLoader(opts: { } /** - * Copy the module plus its shared runtime closure into an isolated tree under - * `base`, asserting nothing under that tree already carries ai-tokenizer (which - * would let the primary loader succeed and leave the fallback unexercised). + * Resolution candidates for a relative specifier, kept identical to the + * maintained pack-graph walker (`scripts/tui-pack-graph.ts`) so the fixture's + * closure and the packaged-graph closure agree on what an import means. + */ +const SOURCE_EXTENSIONS = [".ts", ".tsx", ".js", ".jsx", ".mjs", ".json"]; + +/** + * The transitive closure of literal relative imports of `entry`, discovered + * the way `scripts/tui-pack-graph.ts` walks the packed graph: per-file + * `Bun.Transpiler.scanImports` (type-only imports are erased, lazy and + * `import()` branches are followed) resolved through `SOURCE_EXTENSIONS` and + * `index.*` candidates. Bare specifiers (`ai-tokenizer`) are skipped by + * design: the fixture must carry no `node_modules` subtree, and the isolated + * loader proves the fallback exactly because the bare primary resolve throws. + */ +function runtimeImportClosure(entry: string): string[] { + const visited = new Set(); + const pending = [entry]; + while (pending.length > 0) { + const file = pending.pop(); + if (!file || visited.has(file)) continue; + visited.add(file); + const ext = extname(file); + if (ext === ".json") continue; + const loader = ext === ".tsx" || ext === ".jsx" ? "tsx" : "ts"; + const imports = new Bun.Transpiler({ loader }).scanImports(readFileSync(file, "utf8")); + for (const { path: specifier } of imports) { + if (!specifier.startsWith(".")) continue; + const base = resolve(dirname(file), specifier); + const candidates = [ + base, + ...SOURCE_EXTENSIONS.map((extension) => base + extension), + ...SOURCE_EXTENSIONS.map((extension) => resolve(base, `index${extension}`)), + ]; + const target = candidates.find( + (candidate) => existsSync(candidate) && statSync(candidate).isFile(), + ); + if (!target) { + throw new Error( + `loader fixture: ${relative(SRC_PKG, file)} imports missing ${specifier}`, + ); + } + pending.push(target); + } + } + return [...visited]; +} + +/** + * Copy the module plus its full recursive relative-import closure into an + * isolated tree under `base`, mirroring the `packages/plugin/src` layout, and + * assert nothing under that tree already carries ai-tokenizer (which would let + * the primary loader succeed and leave the fallback unexercised). + * + * The closure is walked from the module itself instead of copying a fixed + * folder list: `read-session-formatting.ts` reaches outside `shared/` (its + * sibling `./token-count-exact`), and a hand-maintained folder copy goes + * silently stale the next time the module gains an import elsewhere — the + * isolated child then dies on the missing file rather than on the bare + * ai-tokenizer resolve, and the fallback is never exercised. Test files cannot + * leak in: runtime modules never import them, so no filter is needed. */ function buildLoaderFixture(base: string): { moduleDir: string } { const moduleDir = join(base, "mod"); - const hookDir = join(moduleDir, "src", "hooks", "magic-context"); - mkdirSync(hookDir, { recursive: true }); - cpSync(SRC_HOOK, join(hookDir, "read-session-formatting.ts")); - // Copy the shared directory (minus test files); the loader's import closure - // stays inside it, and bun only compiles files the copied module actually - // imports, so unreachable shared modules (and their own tree dependencies) - // are never loaded here. - cpSync(SRC_SHARED, join(moduleDir, "src", "shared"), { - recursive: true, - filter: (source) => !/(\.test\.ts|\.test-support\.ts)$/.test(source), - }); + for (const file of runtimeImportClosure(SRC_HOOK)) { + const dest = join(moduleDir, "src", relative(SRC_PKG, file)); + mkdirSync(dirname(dest), { recursive: true }); + cpSync(file, dest); + } for (const dir of [moduleDir, join(moduleDir, "src"), base]) { expect(existsSync(join(dir, "node_modules"))).toBe(false); } @@ -202,7 +273,9 @@ test("tokenizer probe roots include the extension's own install tree", () => { // A linked install (`omp plugin install `) loads the module from // the linked source while its hoisted dependencies live in OMP's own // plugin tree, so that root has to be probed too. - const hostPlugins = resolve(join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer")); + const hostPlugins = resolve( + join(homedir(), ".omp", "plugins", "node_modules", "ai-tokenizer"), + ); expect(roots).toContain(hostPlugins); // Precedence contract: `findTokenizerImportPaths` binds the first // resolvable candidate, so the host-wide ~/.omp/plugins copy -- a