From 001a3d1204eb3d9b927a5d9d229f23f5a667ae5f Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 14 Jun 2026 11:57:56 +0000 Subject: [PATCH] fix(natives): cleaned stale native cache versions Removed stale per-version native cache directories after successful addon loads so updates do not leave old ~/.omp/natives/ trees behind. Added a focused loader regression test for keeping the current version directory and non-directory files while removing stale version folders. Fixes #2560 --- packages/natives/CHANGELOG.md | 4 ++ packages/natives/native/loader-state.d.ts | 7 +++ packages/natives/native/loader-state.js | 45 +++++++++++++++++-- packages/natives/test/windows-staging.test.ts | 25 ++++++++++- 4 files changed, 77 insertions(+), 4 deletions(-) diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 8d330796e..e679d86af 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed native addon loading leaving stale `~/.omp/natives/` cache directories behind after updates; successful loads now remove older version directories best-effort. + ## [15.12.6] - 2026-06-14 ### Fixed diff --git a/packages/natives/native/loader-state.d.ts b/packages/natives/native/loader-state.d.ts index 232ef27b5..32e3267cc 100644 --- a/packages/natives/native/loader-state.d.ts +++ b/packages/natives/native/loader-state.d.ts @@ -55,6 +55,13 @@ export interface ResolveLoaderCandidatesInput { export function resolveLoaderCandidates(input: ResolveLoaderCandidatesInput): string[]; +export interface CleanupStaleNativeVersionsInput { + nativesDir: string; + currentVersion: string; +} + +export function cleanupStaleNativeVersions(input: CleanupStaleNativeVersionsInput): string[]; + export interface ExtractEmbeddedAddonArchiveInput { archivePath: string; files: EmbeddedAddonFile[]; diff --git a/packages/natives/native/loader-state.js b/packages/natives/native/loader-state.js index 179ff5293..d5531b81b 100644 --- a/packages/natives/native/loader-state.js +++ b/packages/natives/native/loader-state.js @@ -177,6 +177,37 @@ export function resolveLoaderCandidates({ } // ========================================================================= + +/** + * Remove version-pinned native cache directories older than the loaded package. + * Best-effort by design: permission errors and concurrent processes must not + * abort startup after the native addon has already loaded successfully. + * + * @param {{ nativesDir: string; currentVersion: string }} input + * @returns {string[]} + */ +export function cleanupStaleNativeVersions({ nativesDir, currentVersion }) { + const removed = []; + let entries; + try { + entries = fs.readdirSync(nativesDir, { withFileTypes: true }); + } catch { + return removed; + } + + for (const entry of entries) { + if (!entry.isDirectory() || entry.name === currentVersion) continue; + const targetPath = path.join(nativesDir, entry.name); + try { + fs.rmSync(targetPath, { recursive: true, force: true }); + removed.push(targetPath); + } catch { + // Stale caches are opportunistic cleanup only. + } + } + return removed; +} + // Side-effectful loader. Everything below runs only when `loadNative()` is // called from `native/index.js` — tests that only import the pure helpers // above pay nothing for variant detection, subprocess spawns, or fs probes. @@ -485,6 +516,13 @@ function validateLoadedBindings(ctx, bindings, candidate) { ); } +function finishSuccessfulLoad(ctx, bindings) { + cleanupStaleNativeVersions({ nativesDir: ctx.nativesDir, currentVersion: ctx.packageVersion }); + startupMarker("native:loadNative:done"); + return bindings; +} + + function buildHelpMessage(ctx) { if (ctx.isCompiledBinary) { const expectedPaths = ctx.addonFilenames.map(filename => ` ${path.join(ctx.versionedDir, filename)}`).join("\n"); @@ -518,7 +556,8 @@ function initLoaderContext() { const packageVersion = packageJson.version; const nativeDir = path.join(import.meta.dir, "..", "native"); const execDir = path.dirname(process.execPath); - const versionedDir = path.join(getNativesDir(), packageVersion); + const nativesDir = getNativesDir(); + const versionedDir = path.join(nativesDir, packageVersion); const userDataDir = process.platform === "win32" ? path.join(process.env.LOCALAPPDATA || path.join(os.homedir(), "AppData", "Local"), "omp") @@ -576,6 +615,7 @@ function initLoaderContext() { candidates, versionSentinelExport, isWorkspaceLoad, + nativesDir, }; } @@ -595,8 +635,7 @@ export function loadNative() { startupMarker(`native:require:${path.basename(candidate)}`); const bindings = require_(candidate); validateLoadedBindings(ctx, bindings, candidate); - startupMarker("native:loadNative:done"); - return bindings; + return finishSuccessfulLoad(ctx, bindings); } catch (err) { const message = err instanceof Error ? err.message : String(err); errors.push(`${candidate}: ${message}`); diff --git a/packages/natives/test/windows-staging.test.ts b/packages/natives/test/windows-staging.test.ts index ece93c79c..6d019a8f6 100644 --- a/packages/natives/test/windows-staging.test.ts +++ b/packages/natives/test/windows-staging.test.ts @@ -20,8 +20,15 @@ * build`) and on non-Windows so the regular path is unchanged. */ import { describe, expect, it } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; import * as path from "node:path"; -import { getAddonFilenames, resolveLoaderCandidates, shouldStageNodeModulesAddon } from "../native/loader-state.js"; +import { + cleanupStaleNativeVersions, + getAddonFilenames, + resolveLoaderCandidates, + shouldStageNodeModulesAddon, +} from "../native/loader-state.js"; import packageJson from "../package.json" with { type: "json" }; const winNodeModulesNativeDir = "C:\\Users\\Admin\\node_modules\\@oh-my-pi\\pi-natives\\native"; @@ -126,6 +133,22 @@ describe("windows native addon staging", () => { expect(candidates).not.toContain(versionedBaseline); expect(candidates).toContain(nodeModulesBaseline); }); + + it("removes stale version directories after the current native version loads", async () => { + const nativesDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-natives-cache-")); + try { + await fs.mkdir(path.join(nativesDir, "15.10.11")); + await fs.mkdir(path.join(nativesDir, packageJson.version)); + await Bun.write(path.join(nativesDir, "README.txt"), "not a version directory"); + + const removed = cleanupStaleNativeVersions({ nativesDir, currentVersion: packageJson.version }); + + expect(removed.map(filePath => path.basename(filePath))).toEqual(["15.10.11"]); + expect((await fs.readdir(nativesDir)).sort()).toEqual(["README.txt", packageJson.version].sort()); + } finally { + await fs.rm(nativesDir, { recursive: true, force: true }); + } + }); }); describe("pi-natives version sentinel", () => {