diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index f885ab6d2..041478391 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `pi_natives` failing to load in Bun worker threads on macOS x64 when the host built only the `modern` (AVX2) variant. The runtime detector's `child_process.spawnSync("sysctl", …)` returned null from the worker even though the build-time detector succeeded in the parent, so `loadNative()` resolved `variant=baseline` and searched a file list that excluded the on-disk `pi_natives.darwin-x64-modern.node`. Resolution now prefers `Bun.spawnSync`, tries `/usr/sbin/sysctl` before bare `sysctl`, and caches the first context's verdict via a private env key so child workers and subprocesses inherit it instead of re-detecting ([#3238](https://github.com/can1357/oh-my-pi/issues/3238)). + ## [16.1.14] - 2026-06-22 ### Fixed diff --git a/packages/natives/native/loader-state.d.ts b/packages/natives/native/loader-state.d.ts index 32e3267cc..73e119ee1 100644 --- a/packages/natives/native/loader-state.d.ts +++ b/packages/natives/native/loader-state.d.ts @@ -69,4 +69,21 @@ export interface ExtractEmbeddedAddonArchiveInput { } export function extractEmbeddedAddonArchive(input: ExtractEmbeddedAddonArchiveInput): string[]; + +export interface SelectCpuVariantInput { + arch: string; + override: "modern" | "baseline" | null | undefined; + env: Record; + detectAvx2: () => boolean; +} + +export interface SelectCpuVariantResult { + variant: "modern" | "baseline" | null; + source: "non-x64" | "override" | "cache" | "detect"; + cacheEnvKey?: string; + cacheEnvValue?: string; +} + +export function selectCpuVariant(input: SelectCpuVariantInput): SelectCpuVariantResult; + export function loadNative(): Record; diff --git a/packages/natives/native/loader-state.js b/packages/natives/native/loader-state.js index 3d550e0d3..486bed688 100644 --- a/packages/natives/native/loader-state.js +++ b/packages/natives/native/loader-state.js @@ -213,7 +213,33 @@ export function cleanupStaleNativeVersions({ nativesDir, currentVersion }) { // above pay nothing for variant detection, subprocess spawns, or fs probes. // ========================================================================= +/** + * Hidden env key for the resolved x64 variant. Once any context (main thread, + * worker, subprocess) finishes variant detection, the result is written here + * so every Bun worker and child process spawned afterwards inherits the same + * verdict and skips re-detection. See `selectCpuVariant` for the lookup order. + */ +const VARIANT_CACHE_ENV_KEY = "__PI_NATIVE_VARIANT_CACHE"; + +/** + * Spawn `command` with `args` and capture stdout. Prefers `Bun.spawnSync` + * because Bun's `child_process.spawnSync` shim has been observed to return + * non-zero / null in worker threads on macOS even when the same binary works + * fine from the parent — the failure mode behind issue #3238, where the worker + * silently falls back to the "baseline" variant. Falls back to the Node shim + * for non-Bun embeds. + */ function runCommand(command, args) { + if (typeof Bun !== "undefined" && typeof Bun.spawnSync === "function") { + try { + const result = Bun.spawnSync([command, ...args], { stdout: "pipe", stderr: "pipe" }); + if (result.exitCode === 0) { + return result.stdout.toString("utf-8").trim(); + } + } catch { + // fall through to childProcess + } + } try { const result = childProcess.spawnSync(command, args, { encoding: "utf-8" }); if (result.error) return null; @@ -246,12 +272,15 @@ function detectAvx2Support() { } if (process.platform === "darwin") { - const leaf7 = runCommand("sysctl", ["-n", "machdep.cpu.leaf7_features"]); - if (leaf7 && /\bAVX2\b/i.test(leaf7)) { - return true; + // Try the absolute path before bare `sysctl`: PATH may not include + // `/usr/sbin` in worker/embedded spawn contexts (issue #3238). + for (const sysctlBin of ["/usr/sbin/sysctl", "sysctl"]) { + const leaf7 = runCommand(sysctlBin, ["-n", "machdep.cpu.leaf7_features"]); + if (leaf7 && /\bAVX2\b/i.test(leaf7)) return true; + const features = runCommand(sysctlBin, ["-n", "machdep.cpu.features"]); + if (features && /\bAVX2\b/i.test(features)) return true; } - const features = runCommand("sysctl", ["-n", "machdep.cpu.features"]); - return Boolean(features && /\bAVX2\b/i.test(features)); + return false; } if (process.platform === "win32") { @@ -267,10 +296,63 @@ function detectAvx2Support() { return false; } +/** + * Pure variant-selection helper, exposed for unit tests. Resolution order: + * + * 1. `override` (user-facing `PI_NATIVE_VARIANT` env var). Always wins. + * 2. The private `__PI_NATIVE_VARIANT_CACHE` env var, populated by the first + * context that detected at runtime. Lets child workers / subprocesses + * inherit the main thread's verdict instead of re-spawning `sysctl` etc. + * from a worker context where the spawn may fail (issue #3238). + * 3. `detectAvx2()` — the slow path, called at most once per process. + * + * Non-x64 architectures return `{ variant: null }` and never set the cache. + * When detection runs, the result is surfaced as `cacheEnvKey`/`cacheEnvValue` + * so the caller can write `process.env` (the pure helper itself stays + * side-effect-free, which keeps it easy to test). + * + * @param {{ + * arch: string; + * override: "modern" | "baseline" | null | undefined; + * env: Record; + * detectAvx2: () => boolean; + * }} input + * @returns {{ + * variant: "modern" | "baseline" | null; + * source: "non-x64" | "override" | "cache" | "detect"; + * cacheEnvKey?: string; + * cacheEnvValue?: string; + * }} + */ +export function selectCpuVariant({ arch, override, env, detectAvx2 }) { + if (arch !== "x64") return { variant: null, source: "non-x64" }; + if (override === "modern" || override === "baseline") { + return { variant: override, source: "override" }; + } + const cached = env[VARIANT_CACHE_ENV_KEY]; + if (cached === "modern" || cached === "baseline") { + return { variant: cached, source: "cache" }; + } + const variant = detectAvx2() ? "modern" : "baseline"; + return { + variant, + source: "detect", + cacheEnvKey: VARIANT_CACHE_ENV_KEY, + cacheEnvValue: variant, + }; +} + function resolveCpuVariant(override) { - if (process.arch !== "x64") return null; - if (override) return override; - return detectAvx2Support() ? "modern" : "baseline"; + const result = selectCpuVariant({ + arch: process.arch, + override, + env: process.env, + detectAvx2: detectAvx2Support, + }); + if (result.cacheEnvKey) { + process.env[result.cacheEnvKey] = result.cacheEnvValue; + } + return result.variant; } function selectEmbeddedAddonFile(selectedVariant) { diff --git a/packages/natives/test/issue-3238-repro.test.ts b/packages/natives/test/issue-3238-repro.test.ts new file mode 100644 index 000000000..35f057987 --- /dev/null +++ b/packages/natives/test/issue-3238-repro.test.ts @@ -0,0 +1,153 @@ +/** + * Regression for https://github.com/can1357/oh-my-pi/issues/3238. + * + * On macOS x64 (Intel), `omp stats` builds only the `modern` + * (`pi_natives.darwin-x64-modern.node`) variant when the host has AVX2, + * because `scripts/host-detect.ts` uses `Bun.spawnSync("sysctl", …)` from a + * normal shell context and correctly resolves AVX2 → modern. + * + * The runtime loader then re-detects from inside Bun worker threads. There + * the old detector hit two failure modes at once: + * - `child_process.spawnSync` returned non-zero/null on darwin under Bun's + * worker shim while `Bun.spawnSync` would have worked. + * - It looked up `sysctl` via PATH, which can lack `/usr/sbin` in + * non-shell-derived spawn contexts. + * + * `detectAvx2Support` returned `false`, `resolveCpuVariant` selected + * `baseline`, and `getAddonFilenames("baseline")` searched only + * `pi_natives.darwin-x64-baseline.node` + `pi_natives.darwin-x64.node` — + * neither of which exists on a modern-only on-disk build. The main thread, + * which ran the same detector before spawning the worker, picked `modern` + * fine; only the worker failed. + * + * The contract pinned here: + * 1. Once any context resolves the variant (the main thread does first), + * it is cached via a private env key. Bun workers and child + * subprocesses inherit `process.env` at spawn, so they read the cache + * and skip detection entirely — sidestepping the worker-context spawn + * flakiness. + * 2. The user-facing `PI_NATIVE_VARIANT` override always wins, including + * over a stale cache value. + * 3. Non-x64 architectures still return `null` and never poison the cache. + * 4. The `darwin-x64` candidate list always carries `modern` ahead of + * `baseline` when the resolved variant is `modern`, so the failing + * "baseline-only file list" from the report cannot reappear under that + * verdict. + */ +import { describe, expect, it } from "bun:test"; +import { getAddonFilenames, selectCpuVariant } from "../native/loader-state.js"; + +const VARIANT_CACHE_ENV_KEY = "__PI_NATIVE_VARIANT_CACHE"; + +describe("issue 3238: variant resolution across worker contexts", () => { + it("returns the cached variant from env without re-detecting", () => { + let detectorCalls = 0; + const result = selectCpuVariant({ + arch: "x64", + override: null, + env: { [VARIANT_CACHE_ENV_KEY]: "modern" }, + detectAvx2: () => { + detectorCalls += 1; + return false; + }, + }); + expect(result.variant).toBe("modern"); + expect(result.source).toBe("cache"); + expect(detectorCalls).toBe(0); + expect(result.cacheEnvKey).toBeUndefined(); + expect(result.cacheEnvValue).toBeUndefined(); + }); + + it("surfaces a fresh detection so the caller can cache it for workers", () => { + const result = selectCpuVariant({ + arch: "x64", + override: null, + env: {}, + detectAvx2: () => true, + }); + expect(result.variant).toBe("modern"); + expect(result.source).toBe("detect"); + // The caller must persist these so spawned Bun workers and child + // subprocesses (which inherit process.env at spawn time) read the + // resolved variant instead of re-running `sysctl` from contexts where + // the spawn is unreliable. + expect(result.cacheEnvKey).toBe(VARIANT_CACHE_ENV_KEY); + expect(result.cacheEnvValue).toBe("modern"); + }); + + it("falls through to baseline when detection fails, still emitting a cache hint", () => { + const result = selectCpuVariant({ + arch: "x64", + override: null, + env: {}, + detectAvx2: () => false, + }); + expect(result.variant).toBe("baseline"); + expect(result.source).toBe("detect"); + expect(result.cacheEnvKey).toBe(VARIANT_CACHE_ENV_KEY); + expect(result.cacheEnvValue).toBe("baseline"); + }); + + it("honors PI_NATIVE_VARIANT override above both cache and detection", () => { + let detectorCalls = 0; + const result = selectCpuVariant({ + arch: "x64", + override: "baseline", + env: { [VARIANT_CACHE_ENV_KEY]: "modern" }, + detectAvx2: () => { + detectorCalls += 1; + return true; + }, + }); + expect(result.variant).toBe("baseline"); + expect(result.source).toBe("override"); + expect(detectorCalls).toBe(0); + // Override path must NOT poison the cache: the user may toggle it + // per-invocation, and child processes should still re-evaluate the + // override env var themselves. + expect(result.cacheEnvKey).toBeUndefined(); + }); + + it("ignores garbage values in PI_NATIVE_VARIANT and in the cache", () => { + const result = selectCpuVariant({ + arch: "x64", + override: "garbage" as unknown as "modern", + env: { [VARIANT_CACHE_ENV_KEY]: "also-garbage" }, + detectAvx2: () => true, + }); + expect(result.variant).toBe("modern"); + expect(result.source).toBe("detect"); + }); + + it("returns variant=null for non-x64 architectures and never emits a cache entry", () => { + for (const arch of ["arm64", "ia32", "ppc64"]) { + const result = selectCpuVariant({ + arch, + override: null, + env: {}, + detectAvx2: () => true, + }); + expect(result.variant).toBeNull(); + expect(result.source).toBe("non-x64"); + expect(result.cacheEnvKey).toBeUndefined(); + } + }); + + it("places modern ahead of baseline when the resolved verdict is modern (issue #3238 root file-search list)", () => { + // The bug surfaced because the worker resolved variant=baseline and + // then searched only [baseline, default]. With the cache populated by + // the main thread, the worker resolves modern and the candidate list + // regains the modern filename — the same on-disk artifact the build + // just produced. + const variant = selectCpuVariant({ + arch: "x64", + override: null, + env: { [VARIANT_CACHE_ENV_KEY]: "modern" }, + detectAvx2: () => false, // simulate the failing worker detector + }).variant; + expect(variant).toBe("modern"); + const filenames = getAddonFilenames({ tag: "darwin-x64", arch: "x64", variant }); + expect(filenames[0]).toBe("pi_natives.darwin-x64-modern.node"); + expect(filenames).toContain("pi_natives.darwin-x64-baseline.node"); + }); +});