fix(natives): made variant detection robust in worker threads

`pi_natives` failed 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 (`scripts/host-detect.ts`) succeeded
in the parent shell, so `loadNative()` resolved `variant=baseline` and
searched a file list that excluded the `pi_natives.darwin-x64-modern.node`
the build had actually produced.

Two compounding root causes in `packages/natives/native/loader-state.js`:

- `runCommand` only used `child_process.spawnSync`, which is the path
  observed to fail under Bun's worker shim on darwin. The build-time
  detector uses `Bun.spawnSync` and works fine.
- The darwin branch looked up `sysctl` via PATH. Login shells supply
  `/usr/sbin` so the build picks it up, but worker/embedded spawn
  contexts can ship without it.

Fix:

- `runCommand` now prefers `Bun.spawnSync` (matches the build-time
  detector) and falls back to `child_process.spawnSync` for non-Bun
  embeds.
- The darwin branch tries `/usr/sbin/sysctl` before bare `sysctl`.
- New private env key `__PI_NATIVE_VARIANT_CACHE`: once any context
  resolves the variant (the main thread does first), it is written
  there. Bun workers and child subprocesses inherit `process.env` at
  spawn, so they read the cache and skip detection — sidestepping the
  worker-context spawn flakiness end-to-end.
- New exported pure helper `selectCpuVariant({ arch, override, env,
  detectAvx2 })` codifies the override > cache > detect order and
  returns the cache write hints so the helper itself stays
  side-effect-free.

Regression test `packages/natives/test/issue-3238-repro.test.ts` pins
every branch of the resolution order, including the file-list shape
that surfaced the bug.

Fixes #3238
This commit is contained in:
roboomp
2026-06-22 07:16:51 +00:00
parent 320261fca9
commit 4b2c3d524c
4 changed files with 264 additions and 8 deletions
+4
View File
@@ -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
+17
View File
@@ -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<string, string | undefined>;
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<string, unknown>;
+90 -8
View File
@@ -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<string, string | undefined>;
* 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) {
@@ -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");
});
});