diff --git a/.gitignore b/.gitignore index fd1901cfd..3fa6fd9c0 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,6 @@ python/omp-rpc/src/omp_rpc.egg-info/ scripts/session-stats/Cargo.lock scripts/session-stats/edit-analysis.csv + +# parallel-agent worktrees +.wt/ diff --git a/packages/coding-agent/scripts/build-binary.ts b/packages/coding-agent/scripts/build-binary.ts index 9e6f1af28..fa5de7caf 100644 --- a/packages/coding-agent/scripts/build-binary.ts +++ b/packages/coding-agent/scripts/build-binary.ts @@ -34,7 +34,7 @@ async function main(): Promise { "build", "--compile", "--define", - "PI_COMPILED=true", + 'process.env.PI_COMPILED="true"', "--external", "mupdf", "--root", diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index 599cd2e8a..4fe062c46 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -320,7 +320,7 @@ export declare function copyToClipboard(text: string): void * * Uses ordinary encoding (no special-token handling), which is the right * choice for measuring user/model content rather than wire-protocol tokens. - * Defaults to `o200k_base`; pass `Cl100kBase` for older OpenAI models. + * Defaults to `o200k_base`; pass `Cl100kBase` for older `OpenAI` models. */ export declare function countTokens(input: string | Array, encoding?: Encoding | undefined | null): number diff --git a/packages/natives/native/index.js b/packages/natives/native/index.js index 3703ad5f6..7ff2492d7 100644 --- a/packages/natives/native/index.js +++ b/packages/natives/native/index.js @@ -16,7 +16,7 @@ function getNativesDir() { return path.join(os.homedir(), ".omp", "natives"); } const packageJson = require("../package.json"); -let embeddedAddon = null; +const { detectCompiledBinary, getAddonFilenames, resolveLoaderCandidates } = require("./loader-state"); const require_ = createRequire(__filename); const platformTag = `${process.platform}-${process.arch}`; @@ -28,19 +28,22 @@ const userDataDir = process.platform === "win32" ? path.join(process.env.LOCALAPPDATA || path.join(os.homedir(), "AppData", "Local"), "omp") : path.join(os.homedir(), ".local", "bin"); -const isCompiledBinary = - process.env.PI_COMPILED || - __filename.includes("$bunfs") || - __filename.includes("~BUN") || - __filename.includes("%7EBUN"); -if (isCompiledBinary) { - try { - ({ embeddedAddon } = require("./embedded-addon")); - } catch { - embeddedAddon = null; - } +// Eagerly load the embedded-addon manifest. In dev/non-compiled mode this resolves to the +// `embed:native --reset` stub which exports `embeddedAddon: null`. In a Bun standalone +// binary the manifest is regenerated by `embed:native` to a populated object and bundled +// into the binary; its presence is the authoritative compiled-mode signal (issue #823). +let embeddedAddon = null; +try { + ({ embeddedAddon } = require("./embedded-addon")); +} catch { + embeddedAddon = null; } +const isCompiledBinary = detectCompiledBinary({ + embeddedAddon, + env: process.env, + importMetaUrl: typeof import.meta === "object" && import.meta ? import.meta.url : null, +}); const SUPPORTED_PLATFORMS = ["linux-x64", "linux-arm64", "darwin-x64", "darwin-arm64", "win32-x64"]; function getVariantOverride() { @@ -92,32 +95,19 @@ function resolveCpuVariant(override) { return detectAvx2Support() ? "modern" : "baseline"; } -function getAddonFilenames(tag, variant) { - const defaultFilename = `pi_natives.${tag}.node`; - if (process.arch !== "x64" || !variant) return [defaultFilename]; - const baselineFilename = `pi_natives.${tag}-baseline.node`; - const modernFilename = `pi_natives.${tag}-modern.node`; - if (variant === "modern") { - return [modernFilename, baselineFilename, defaultFilename]; - } - return [baselineFilename, defaultFilename]; -} - const variantOverride = getVariantOverride(); const selectedVariant = resolveCpuVariant(variantOverride); -const addonFilenames = getAddonFilenames(platformTag, selectedVariant); +const addonFilenames = getAddonFilenames({ tag: platformTag, arch: process.arch, variant: selectedVariant }); const addonLabel = selectedVariant ? `${platformTag} (${selectedVariant})` : platformTag; -const baseReleaseCandidates = addonFilenames.flatMap(filename => [ - path.join(nativeDir, filename), - path.join(execDir, filename), -]); -const compiledCandidates = addonFilenames.flatMap(filename => [ - path.join(versionedDir, filename), - path.join(userDataDir, filename), -]); -const releaseCandidates = isCompiledBinary ? [...compiledCandidates, ...baseReleaseCandidates] : baseReleaseCandidates; -const dedupedCandidates = [...new Set(releaseCandidates)]; +const dedupedCandidates = resolveLoaderCandidates({ + addonFilenames, + isCompiledBinary, + nativeDir, + execDir, + versionedDir, + userDataDir, +}); function runCommand(command, args) { // removed logger.time diff --git a/packages/natives/native/loader-state.d.ts b/packages/natives/native/loader-state.d.ts new file mode 100644 index 000000000..3e08402be --- /dev/null +++ b/packages/natives/native/loader-state.d.ts @@ -0,0 +1,38 @@ +export interface EmbeddedAddonFile { + variant: "modern" | "baseline" | "default"; + filename: string; + filePath: string; +} + +export interface EmbeddedAddon { + platformTag: string; + version: string; + files: EmbeddedAddonFile[]; +} + +export interface DetectCompiledBinaryInput { + embeddedAddon: EmbeddedAddon | null | undefined; + env: Record; + importMetaUrl: string | null | undefined; +} + +export function detectCompiledBinary(input: DetectCompiledBinaryInput): boolean; + +export interface GetAddonFilenamesInput { + tag: string; + arch: string; + variant: "modern" | "baseline" | null | undefined; +} + +export function getAddonFilenames(input: GetAddonFilenamesInput): string[]; + +export interface ResolveLoaderCandidatesInput { + addonFilenames: string[]; + isCompiledBinary: boolean; + nativeDir: string; + execDir: string; + versionedDir: string; + userDataDir: string; +} + +export function resolveLoaderCandidates(input: ResolveLoaderCandidatesInput): string[]; diff --git a/packages/natives/native/loader-state.js b/packages/natives/native/loader-state.js new file mode 100644 index 000000000..e56c42efc --- /dev/null +++ b/packages/natives/native/loader-state.js @@ -0,0 +1,83 @@ +"use strict"; + +/** + * Pure helpers used by `./index.js` to decide whether the loader is running + * inside a Bun-compiled standalone binary, and to compute the ordered list of + * candidate paths the loader probes for `pi_natives.-*.node`. + * + * Kept as a separate CommonJS module so the logic can be unit-tested without + * triggering the side-effectful `loadNative()` call in `index.js`. + * + * Background (issue #823): `bun build --compile --define PI_COMPILED=true` + * substitutes the bare identifier `PI_COMPILED`, NOT `process.env.PI_COMPILED`, + * so a runtime read of the env var returns `undefined`. Bun also retains the + * original build-host absolute path in `__filename` for required CJS modules — + * only `import.meta.url` is rewritten to the bunfs URL. Both legacy signals are + * therefore false in the shipped binary, which is why the embedded-addon + * presence (true iff the build pipeline ran `embed:native`, false in the + * post-build `--reset` stub) is the authoritative compiled-mode signal. + */ + +const path = require("node:path"); + +/** + * @param {{ + * embeddedAddon: { platformTag: string; version: string; files: unknown[] } | null | undefined; + * env: Record; + * importMetaUrl: string | null | undefined; + * }} input + * @returns {boolean} + */ +function detectCompiledBinary({ embeddedAddon, env, importMetaUrl }) { + if (embeddedAddon) return true; + if (env && env.PI_COMPILED) return true; + if (typeof importMetaUrl === "string") { + if (importMetaUrl.includes("$bunfs")) return true; + if (importMetaUrl.includes("~BUN")) return true; + if (importMetaUrl.includes("%7EBUN")) return true; + } + return false; +} + +/** + * @param {{ tag: string; arch: string; variant: "modern" | "baseline" | null | undefined }} input + * @returns {string[]} + */ +function getAddonFilenames({ tag, arch, variant }) { + const defaultFilename = `pi_natives.${tag}.node`; + if (arch !== "x64" || !variant) return [defaultFilename]; + const baselineFilename = `pi_natives.${tag}-baseline.node`; + const modernFilename = `pi_natives.${tag}-modern.node`; + if (variant === "modern") { + return [modernFilename, baselineFilename, defaultFilename]; + } + return [baselineFilename, defaultFilename]; +} + +/** + * @param {{ + * addonFilenames: string[]; + * isCompiledBinary: boolean; + * nativeDir: string; + * execDir: string; + * versionedDir: string; + * userDataDir: string; + * }} input + * @returns {string[]} + */ +function resolveLoaderCandidates({ addonFilenames, isCompiledBinary, nativeDir, execDir, versionedDir, userDataDir }) { + const baseReleaseCandidates = addonFilenames.flatMap(filename => [ + path.join(nativeDir, filename), + path.join(execDir, filename), + ]); + const compiledCandidates = addonFilenames.flatMap(filename => [ + path.join(versionedDir, filename), + path.join(userDataDir, filename), + ]); + const releaseCandidates = isCompiledBinary + ? [...compiledCandidates, ...baseReleaseCandidates] + : baseReleaseCandidates; + return [...new Set(releaseCandidates)]; +} + +module.exports = { detectCompiledBinary, getAddonFilenames, resolveLoaderCandidates }; diff --git a/packages/natives/scripts/build-native.ts b/packages/natives/scripts/build-native.ts index 933aba62d..c3eac3358 100644 --- a/packages/natives/scripts/build-native.ts +++ b/packages/natives/scripts/build-native.ts @@ -129,32 +129,6 @@ async function installBinary(src: string, dest: string): Promise { } } } -async function patchGeneratedIndexLoader(): Promise { - const indexPath = path.join(nativeDir, "index.js"); - let content = await Bun.file(indexPath).text(); - const embeddedLoadPatch = "let embeddedAddon = null;\n"; - if (!content.includes(embeddedLoadPatch)) { - content = content.replace(/const \{ embeddedAddon \} = require\("\.\/embedded-addon"\);\n/, embeddedLoadPatch); - } - const lazyLoadPatch = [ - "if (isCompiledBinary) {", - "\ttry {", - '\t\t({ embeddedAddon } = require("./embedded-addon"));', - "\t} catch {", - "\t\tembeddedAddon = null;", - "\t}", - "}", - "", - ].join("\n"); - if (!content.includes(lazyLoadPatch)) { - content = content.replace( - /(const isCompiledBinary =[\s\S]*?__filename\.includes\("%7EBUN"\);\n)/, - `$1\n${lazyLoadPatch}`, - ); - } - await Bun.write(indexPath, content); -} - async function resolveBuiltAddonPath(outputDir: string, canonicalFilename: string): Promise { // napi-rs 3.x emits `${binaryName}.${platformArchABI}.node` where // platformArchABI is e.g. `darwin-x64`, `linux-x64-gnu`, `win32-x64-msvc`, @@ -308,7 +282,6 @@ try { await installGeneratedBindings(buildOutputDir); await generateEnumExports(); - await patchGeneratedIndexLoader(); console.log("Build complete."); } finally { diff --git a/packages/natives/test/issue-823-repro.test.ts b/packages/natives/test/issue-823-repro.test.ts new file mode 100644 index 000000000..ced5eaff4 --- /dev/null +++ b/packages/natives/test/issue-823-repro.test.ts @@ -0,0 +1,131 @@ +/** + * Regression for https://github.com/can1357/oh-my-pi/issues/823. + * + * On WSL (and any host where the user moves the standalone binary away from the + * build-time native artifacts), the compiled `omp` binary fails to load + * `pi_natives.linux-x64-*.node`. Root cause: the loader's `isCompiledBinary` + * detection in `packages/natives/native/index.js` relies on + * - `process.env.PI_COMPILED` — never set, because `bun build --compile + * --define PI_COMPILED=true` substitutes the bare identifier, not + * property accesses on `process.env`. + * - `__filename.includes("$bunfs"|"~BUN"|"%7EBUN")` — Bun's compiled + * binaries keep the original build-host absolute path in `__filename` + * (only `import.meta.url` is rewritten to the bunfs URL). + * + * Both signals are false at runtime, so the loader skips the embedded-addon + * extraction path and only tries `nativeDir` (the dev machine's checkout) and + * `execDir`. On WSL with `~/.local/bin/omp` and no sibling `.node` file, this + * fails with the error reported in the issue. + * + * The fix is to make the loader's compiled-binary detection authoritative on + * the embedded-addon module presence (the embedded-addon stub exports `null` + * outside of `--compile`, and is regenerated to a populated object during the + * standalone build), and to expose the candidate-path computation as a pure + * helper so it can be tested host-agnostically. + */ + +import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import { detectCompiledBinary, getAddonFilenames, resolveLoaderCandidates } from "../native/loader-state"; + +describe("issue 823: standalone-binary native loader path resolution", () => { + it("detects compiled-binary mode from embedded-addon presence when env and url markers are absent", () => { + // Mirrors what a Bun standalone binary actually sees on linux-x64 / WSL: + // - `process.env.PI_COMPILED` is undefined (the build flag does not substitute property accesses). + // - `import.meta.url` does point at `$bunfs` for the entrypoint, but a CJS native loader + // that lives in a required module historically read `__filename`, which is NOT rewritten. + // The embedded-addon module is the authoritative compiled-mode signal: it is `null` in + // development (the stub) and a populated object in the standalone build (after + // `embed:native` runs), and is bundled into the binary by `bun build --compile`. + expect( + detectCompiledBinary({ + embeddedAddon: { + platformTag: "linux-x64", + version: "14.5.2", + files: [ + { + variant: "modern", + filename: "pi_natives.linux-x64-modern.node", + filePath: "/$bunfs/root/packages/natives/native/pi_natives.linux-x64-modern.node", + }, + ], + }, + env: {}, + importMetaUrl: "/home/u/build-host/packages/natives/native/index.js", + }), + ).toBe(true); + + // Without an embedded-addon and without env/url markers, we are NOT compiled. + expect( + detectCompiledBinary({ + embeddedAddon: null, + env: {}, + importMetaUrl: "/home/u/dev/packages/natives/native/index.js", + }), + ).toBe(false); + + // Env override (e.g. user-set PI_COMPILED=1) still wins. + expect( + detectCompiledBinary({ + embeddedAddon: null, + env: { PI_COMPILED: "1" }, + importMetaUrl: "/anywhere", + }), + ).toBe(true); + + // `import.meta.url` bunfs marker still wins when present. + expect( + detectCompiledBinary({ + embeddedAddon: null, + env: {}, + importMetaUrl: "file:///$bunfs/root/cli", + }), + ).toBe(true); + }); + + it("places embedded-extracted candidates ahead of build-host candidates for linux-x64 standalone", () => { + const versionedDir = "/home/u/.omp/natives/14.5.2"; + const userDataDir = "/home/u/.local/bin"; + const nativeDir = "/build-host/packages/natives/native"; + const execDir = "/home/u/.local/bin"; + const candidates = resolveLoaderCandidates({ + addonFilenames: getAddonFilenames({ tag: "linux-x64", arch: "x64", variant: "modern" }), + isCompiledBinary: true, + nativeDir, + execDir, + versionedDir, + userDataDir, + }); + + const versionedModern = path.join(versionedDir, "pi_natives.linux-x64-modern.node"); + const versionedBaseline = path.join(versionedDir, "pi_natives.linux-x64-baseline.node"); + const userDataModern = path.join(userDataDir, "pi_natives.linux-x64-modern.node"); + const buildHostModern = path.join(nativeDir, "pi_natives.linux-x64-modern.node"); + + // Versioned cache and user-data dir candidates must exist for compiled binaries — + // these are where the embedded-addon extraction lands (~/.omp/natives/) and where + // `omp update` writes the standalone binary on linux (~/.local/bin). + expect(candidates).toContain(versionedModern); + expect(candidates).toContain(versionedBaseline); + expect(candidates).toContain(userDataModern); + + // Order matters: embedded-extracted destinations must be probed before the + // (potentially-missing) build-host nativeDir path that survives in __dirname. + expect(candidates.indexOf(versionedModern)).toBeLessThan(candidates.indexOf(buildHostModern)); + }); + + it("does not probe user-data candidates when running outside a standalone binary", () => { + const versionedDir = "/home/u/.omp/natives/14.5.2"; + const userDataDir = "/home/u/.local/bin"; + const candidates = resolveLoaderCandidates({ + addonFilenames: getAddonFilenames({ tag: "linux-x64", arch: "x64", variant: "baseline" }), + isCompiledBinary: false, + nativeDir: "/repo/packages/natives/native", + execDir: "/usr/bin", + versionedDir, + userDataDir, + }); + expect(candidates).not.toContain(path.join(versionedDir, "pi_natives.linux-x64-baseline.node")); + expect(candidates).not.toContain(path.join(userDataDir, "pi_natives.linux-x64-baseline.node")); + }); +}); diff --git a/scripts/ci-release-build-binaries.ts b/scripts/ci-release-build-binaries.ts index b2c51861d..77853af2e 100644 --- a/scripts/ci-release-build-binaries.ts +++ b/scripts/ci-release-build-binaries.ts @@ -106,7 +106,7 @@ async function buildBinary(target: BinaryTarget): Promise { console.log(`Building ${target.outfile}...`); await embedNative(target); if (isDryRun) { - console.log(`DRY RUN bun build --compile --no-compile-autoload-bunfig --no-compile-autoload-dotenv --define PI_COMPILED=true --root . --external mupdf --target=${target.target} ${entrypoint} --outfile ${target.outfile}`); + console.log(`DRY RUN bun build --compile --no-compile-autoload-bunfig --no-compile-autoload-dotenv --define process.env.PI_COMPILED="true" --root . --external mupdf --target=${target.target} ${entrypoint} --outfile ${target.outfile}`); return; } @@ -121,7 +121,7 @@ async function buildBinary(target: BinaryTarget): Promise { "--no-compile-autoload-bunfig", "--no-compile-autoload-dotenv", "--define", - "PI_COMPILED=true", + 'process.env.PI_COMPILED="true"', "--root", ".", "--external",