diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 41d09b1ee..6118df8ba 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -59,6 +59,7 @@ - Fixed the native build script failing to locate the `@napi-rs/cli` `napi` binary on Windows because the `PATH` lookup joined entries with a Unix `:` separator instead of the platform delimiter (`path.delimiter`). - Fixed a Windows regression where an abnormal `omp` exit or bash cancellation could `TerminateProcess` unrelated `pwsh.exe` / `powershell.exe` sessions (including other Cursor terminal tabs). `SpawnRegistry` stored only the raw pid of each brush-spawned child and re-opened it via `Process::from_pid` at cancellation time; between those two moments Windows could recycle a freed pid onto an unrelated PowerShell, and `signal_tree` then walked the wrong subtree via Toolhelp. The observer now pins a stable `Process` handle at spawn time — on Windows the open handle keeps the pid slot reserved, on Linux the pidfd carries identity, on macOS the `(pid, start_time)` triple detects impersonation — so cancellation can only reach children this run actually launched. The registry sweeps exited entries once the recorded set crosses a small threshold so a long bash loop of short external commands cannot pin one owned OS handle per historical spawn. ([#4605](https://github.com/can1357/oh-my-pi/issues/4605)) +- Fixed the pi-natives version sentinel emitting "reinstall to re-sync" when a long-lived process survives an in-place upgrade: the loader now detects that the resident addon exposes a *prior* release's sentinel and reports "omp was upgraded while this session was running — restart to pick up the new version (disk is already consistent)" instead of misdiagnosing it as a stale on-disk file ([#4812](https://github.com/can1357/oh-my-pi/issues/4812)). ## [16.3.6] - 2026-07-04 diff --git a/packages/natives/native/loader-state.d.ts b/packages/natives/native/loader-state.d.ts index 73e119ee1..d6fe483be 100644 --- a/packages/natives/native/loader-state.d.ts +++ b/packages/natives/native/loader-state.d.ts @@ -86,4 +86,16 @@ export interface SelectCpuVariantResult { export function selectCpuVariant(input: SelectCpuVariantInput): SelectCpuVariantResult; +export interface ValidateLoadedBindingsContext { + isWorkspaceLoad: boolean; + packageVersion: string; + versionSentinelExport: string; +} + +export function validateLoadedBindings( + ctx: ValidateLoadedBindingsContext, + bindings: Record, + candidate: string, +): void; + export function loadNative(): Record; diff --git a/packages/natives/native/loader-state.js b/packages/natives/native/loader-state.js index 486bed688..44033a61e 100644 --- a/packages/natives/native/loader-state.js +++ b/packages/natives/native/loader-state.js @@ -583,7 +583,7 @@ function maybeStageNodeModulesAddon(ctx, errors) { return stagedPath; } -function validateLoadedBindings(ctx, bindings, candidate) { +export function validateLoadedBindings(ctx, bindings, candidate) { // In workspace dev (running out of `packages/natives/native/` rather than a // `node_modules` install or a compiled bundle) the local `.node` only gains // the renamed sentinel after `bun --cwd=packages/natives run build`. Skip @@ -591,6 +591,30 @@ function validateLoadedBindings(ctx, bindings, candidate) { // completes; install and compiled-binary paths still validate. if (ctx.isWorkspaceLoad) return; if (typeof bindings[ctx.versionSentinelExport] === "function") return; + + // The expected sentinel is missing. Distinguish two failure modes by the + // sentinel the bindings DO carry: + // - disk stale: the `.node` on disk predates this loader (its own build); + // reinstalling re-syncs the file. + // - process stale: an in-place upgrade landed a new release on disk while + // this process still holds the previous addon generation resident in the + // dynamic-loader's native-module cache. `require` returns those old + // exports, which carry the PRIOR sentinel — disk is already consistent, + // so reinstall is a no-op and only restarting the process re-syncs. + const residentSentinel = Object.keys(bindings).find( + key => key !== ctx.versionSentinelExport && /^__piNativesV[A-Za-z0-9_]+$/.test(key), + ); + if (residentSentinel) { + const residentVersion = residentSentinel.slice("__piNativesV".length).replace(/_/g, "."); + throw new Error( + `Loaded ${candidate}, which exposes the @oh-my-pi/pi-natives@${residentVersion} version ` + + `sentinel \`${residentSentinel}\` but not the @${ctx.packageVersion} sentinel ` + + `\`${ctx.versionSentinelExport}\` this loader expects. omp was upgraded to ` + + `${ctx.packageVersion} while this session was running; the ${residentVersion} addon is ` + + "still resident in this process. Disk is already consistent — restart omp to pick up " + + `${ctx.packageVersion} (reinstalling changes nothing).`, + ); + } throw new Error( `Loaded ${candidate} but it does not expose the @oh-my-pi/pi-natives@${ctx.packageVersion} ` + `version sentinel \`${ctx.versionSentinelExport}\`. The .node file on disk is from a different ` + diff --git a/packages/natives/test/issue-4812-repro.test.ts b/packages/natives/test/issue-4812-repro.test.ts new file mode 100644 index 000000000..2d18924d0 --- /dev/null +++ b/packages/natives/test/issue-4812-repro.test.ts @@ -0,0 +1,71 @@ +/** + * Repro for https://github.com/can1357/oh-my-pi/issues/4812 + * + * A long-lived omp session that survives an in-place `bun install -g` upgrade + * keeps the previous pi-natives NAPI addon resident in the process. A tab + * worker spawned afterwards runs the freshly-installed JS loader, which expects + * the new sentinel (e.g. `__piNativesV16_3_11`), but `require` returns the + * resident old exports carrying the PRIOR sentinel (`__piNativesV16_3_10`). + * + * The contract this test pins down: `validateLoadedBindings` distinguishes a + * process-stale mix (disk consistent — restart to re-sync) from a genuinely + * disk-stale addon (reinstall to re-sync), and never tells the operator to + * reinstall when the bindings already carry a versioned sentinel. + */ +import { describe, expect, it } from "bun:test"; +import { validateLoadedBindings } from "../native/loader-state.js"; + +const candidate = "/home/u/.bun/install/global/node_modules/@oh-my-pi/pi-natives-linux-x64/pi_natives.linux-x64.node"; + +function ctxFor(version: string) { + return { + isWorkspaceLoad: false, + packageVersion: version, + versionSentinelExport: `__piNativesV${version.replace(/[^A-Za-z0-9]/g, "_")}`, + }; +} + +describe("issue 4812: pi-natives sentinel process-stale diagnosis", () => { + it("accepts bindings that expose the expected sentinel", () => { + const ctx = ctxFor("16.3.11"); + expect(() => + validateLoadedBindings(ctx, { __piNativesV16_3_11: () => {}, grep: () => {} }, candidate), + ).not.toThrow(); + }); + + it("reports a mid-session upgrade (restart) when bindings carry an older sentinel", () => { + const ctx = ctxFor("16.3.11"); + const resident = { __piNativesV16_3_10: () => {}, grep: () => {} }; + let message = ""; + try { + validateLoadedBindings(ctx, resident, candidate); + } catch (err) { + message = err instanceof Error ? err.message : String(err); + } + expect(message).toContain("16.3.10"); + expect(message).toContain("restart omp"); + expect(message).toContain("Disk is already consistent"); + // The disk-stale advice must NOT appear for a process-stale mix. + expect(message).not.toContain("reinstall to re-sync"); + expect(message).not.toContain("from a different release than this loader"); + }); + + it("still reports disk-stale (reinstall) when no versioned sentinel is present", () => { + const ctx = ctxFor("16.3.11"); + const stale = { grep: () => {}, astGrep: () => {} }; + let message = ""; + try { + validateLoadedBindings(ctx, stale, candidate); + } catch (err) { + message = err instanceof Error ? err.message : String(err); + } + expect(message).toContain("from a different release than this loader"); + expect(message).toContain("reinstall to re-sync"); + expect(message).not.toContain("restart omp"); + }); + + it("skips validation entirely in workspace dev", () => { + const ctx = { ...ctxFor("16.3.11"), isWorkspaceLoad: true }; + expect(() => validateLoadedBindings(ctx, { grep: () => {} }, candidate)).not.toThrow(); + }); +});