refactor(natives): simplified native module loader and removed validation overhead
- Removed native binding validation function that checked required exports at load time. - Enhanced native module loader to check XDG_DATA_HOME environment variable before ~/.omp/natives fallback. - Removed dependency on @oh-my-pi/pi-utils from native module loader by inlining helper functions. - Migrated child process termination from waitForChildProcess utility to killTree native binding.
This commit is contained in:
can1357
@@ -1,6 +1,7 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Chunk read formatting: `anchorStyle` (full / kind / bare), `read.anchorstyle` setting, and `chunked` flag on file display mode
|
||||
@@ -25,6 +26,7 @@
|
||||
|
||||
### Removed
|
||||
|
||||
- Removed `waitForChildProcess` utility (child process termination now handled by native `killTree` from pi-natives)
|
||||
- `grep-chunk.md` (folded into unified grep template)
|
||||
- `startMacAppearanceObserver` export (use `MacAppearanceObserver.start()`)
|
||||
- `copyToClipboard` export from pi-natives
|
||||
|
||||
@@ -49,8 +49,8 @@ import {
|
||||
modelsAreEqual,
|
||||
parseRateLimitReason,
|
||||
} from "@oh-my-pi/pi-ai";
|
||||
import { MacOSPowerAssertion, type SearchDb } from "@oh-my-pi/pi-natives";
|
||||
import { abortableSleep, getAgentDbPath, isEnoent, logger } from "@oh-my-pi/pi-utils";
|
||||
import { killTree, MacOSPowerAssertion, type SearchDb } from "@oh-my-pi/pi-natives";
|
||||
import { abortableSleep, getAgentDbPath, isEnoent, logger, setNativeKillTree } from "@oh-my-pi/pi-utils";
|
||||
import type { AsyncJob, AsyncJobManager } from "../async";
|
||||
import type { Rule } from "../capability/rule";
|
||||
import { MODEL_ROLE_IDS, type ModelRegistry } from "../config/model-registry";
|
||||
@@ -535,6 +535,8 @@ export class AgentSession {
|
||||
}
|
||||
|
||||
constructor(config: AgentSessionConfig) {
|
||||
setNativeKillTree(killTree);
|
||||
|
||||
this.agent = config.agent;
|
||||
this.sessionManager = config.sessionManager;
|
||||
this.settings = config.settings;
|
||||
|
||||
@@ -1,88 +0,0 @@
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
|
||||
const EXIT_STDIO_GRACE_MS = 100;
|
||||
|
||||
/**
|
||||
* Wait for a child process to terminate without hanging on inherited stdio handles.
|
||||
*
|
||||
* Daemonized descendants can inherit the child's stdout/stderr pipe handles. In that
|
||||
* case the child emits `exit`, but `close` can hang forever even though the original
|
||||
* process is already gone. We wait briefly for stdio to end, then forcibly stop
|
||||
* tracking the inherited handles.
|
||||
*/
|
||||
export function waitForChildProcess(child: ChildProcess): Promise<number | null> {
|
||||
const { promise, resolve, reject } = Promise.withResolvers<number | null>();
|
||||
|
||||
let settled = false;
|
||||
let exited = false;
|
||||
let exitCode: number | null = null;
|
||||
let postExitTimer: NodeJS.Timeout | undefined;
|
||||
let stdoutEnded = child.stdout === null;
|
||||
let stderrEnded = child.stderr === null;
|
||||
|
||||
const cleanup = () => {
|
||||
if (postExitTimer) {
|
||||
clearTimeout(postExitTimer);
|
||||
postExitTimer = undefined;
|
||||
}
|
||||
child.removeListener("error", onError);
|
||||
child.removeListener("exit", onExit);
|
||||
child.removeListener("close", onClose);
|
||||
child.stdout?.removeListener("end", onStdoutEnd);
|
||||
child.stderr?.removeListener("end", onStderrEnd);
|
||||
};
|
||||
|
||||
const finalize = (code: number | null) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
cleanup();
|
||||
child.stdout?.destroy();
|
||||
child.stderr?.destroy();
|
||||
resolve(code);
|
||||
};
|
||||
|
||||
const maybeFinalizeAfterExit = () => {
|
||||
if (!exited || settled) return;
|
||||
if (stdoutEnded && stderrEnded) {
|
||||
finalize(exitCode);
|
||||
}
|
||||
};
|
||||
|
||||
const onStdoutEnd = () => {
|
||||
stdoutEnded = true;
|
||||
maybeFinalizeAfterExit();
|
||||
};
|
||||
|
||||
const onStderrEnd = () => {
|
||||
stderrEnded = true;
|
||||
maybeFinalizeAfterExit();
|
||||
};
|
||||
|
||||
const onError = (err: Error) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
cleanup();
|
||||
reject(err);
|
||||
};
|
||||
|
||||
const onExit = (code: number | null) => {
|
||||
exited = true;
|
||||
exitCode = code;
|
||||
maybeFinalizeAfterExit();
|
||||
if (!settled) {
|
||||
postExitTimer = setTimeout(() => finalize(code), EXIT_STDIO_GRACE_MS);
|
||||
}
|
||||
};
|
||||
|
||||
const onClose = (code: number | null) => {
|
||||
finalize(code);
|
||||
};
|
||||
|
||||
child.stdout?.once("end", onStdoutEnd);
|
||||
child.stderr?.once("end", onStderrEnd);
|
||||
child.once("error", onError);
|
||||
child.once("exit", onExit);
|
||||
child.once("close", onClose);
|
||||
|
||||
return promise;
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- Moved package entry point from `src/index.ts` to `native/index.js` — consumers must update imports to use the new native module path
|
||||
@@ -16,12 +17,16 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Updated native module loader to check `XDG_DATA_HOME` environment variable for native addon location before falling back to `~/.omp/natives`
|
||||
- Removed native binding validation function that checked for required exports at load time
|
||||
- Refactored build pipeline to use napi-rs generated bindings instead of hand-written TypeScript wrappers
|
||||
- Updated `build-native.ts` to generate runtime enum exports after native compilation
|
||||
- Updated `embed-native.ts` to output JavaScript instead of TypeScript for embedded addon metadata
|
||||
|
||||
### Removed
|
||||
|
||||
- Removed inline pi-utils helpers and dependency on `@oh-my-pi/pi-utils` from native module loader
|
||||
- Removed `logger.time()` wrapper calls from native module loading
|
||||
- Removed all TypeScript wrapper modules from `src/` directory (appearance, ast, chunk, clipboard, glob, grep, highlight, html, image, keys, projfs, ps, pty, shell, text, work)
|
||||
- Removed `src/bindings.ts` and `src/index.ts` entry points
|
||||
- Removed `src/search-db.ts` and `src/search-db-types.ts`
|
||||
|
||||
@@ -7,24 +7,17 @@ const fs = require("node:fs");
|
||||
const { createRequire } = require("node:module");
|
||||
const os = require("node:os");
|
||||
const path = require("node:path");
|
||||
// Inline the two pi-utils helpers this loader needs (getNativesDir, logger.time).
|
||||
// Pi-natives used to require('@oh-my-pi/pi-utils') at module top, but Bun's
|
||||
// static resolver rejects that import when pi-natives is installed via a
|
||||
// tarball where its workspace:* dep cannot be resolved next to this nested
|
||||
// CJS file. Inlining removes the hard dep.
|
||||
|
||||
function getNativesDir() {
|
||||
const xdgDataHome = process.env.XDG_DATA_HOME;
|
||||
if (xdgDataHome && fs.existsSync(path.join(xdgDataHome, "omp"))) {
|
||||
return path.join(xdgDataHome, "omp", "natives");
|
||||
}
|
||||
return path.join(os.homedir(), ".omp", "natives");
|
||||
}
|
||||
const logger = {
|
||||
time(_label, fn, ...args) {
|
||||
return fn(...args);
|
||||
},
|
||||
};
|
||||
const packageJson = require("../package.json");
|
||||
const { embeddedAddon } = require("./embedded-addon");
|
||||
|
||||
// NOTE: TypeScript types are omitted in JS version
|
||||
|
||||
const require_ = createRequire(__filename);
|
||||
const platformTag = `${process.platform}-${process.arch}`;
|
||||
const packageVersion = packageJson.version;
|
||||
@@ -189,7 +182,6 @@ function loadNative() {
|
||||
const bindings = logger.time(`native:loadNative:require:${path.basename(candidate)}`, () =>
|
||||
require_(candidate),
|
||||
);
|
||||
validateNative(bindings, candidate);
|
||||
if (process.env.PI_DEV) {
|
||||
console.log(`Loaded native addon from ${candidate}`);
|
||||
}
|
||||
@@ -234,66 +226,7 @@ function loadNative() {
|
||||
throw new Error(`Failed to load pi_natives native addon for ${addonLabel}.\n\nTried:\n${details}\n\n${helpMessage}`);
|
||||
}
|
||||
|
||||
function validateNative(bindings, source) {
|
||||
const missing = [];
|
||||
const checkFn = name => {
|
||||
if (typeof bindings[name] !== "function") {
|
||||
missing.push(name);
|
||||
}
|
||||
};
|
||||
checkFn("copyToClipboard");
|
||||
checkFn("readImageFromClipboard");
|
||||
checkFn("ChunkState");
|
||||
checkFn("formatAnchor");
|
||||
checkFn("glob");
|
||||
checkFn("fuzzyFind");
|
||||
checkFn("grep");
|
||||
checkFn("search");
|
||||
checkFn("hasMatch");
|
||||
checkFn("htmlToMarkdown");
|
||||
checkFn("highlightCode");
|
||||
checkFn("supportsLanguage");
|
||||
checkFn("getSupportedLanguages");
|
||||
checkFn("truncateToWidth");
|
||||
checkFn("sanitizeText");
|
||||
checkFn("wrapTextWithAnsi");
|
||||
checkFn("sliceWithWidth");
|
||||
checkFn("extractSegments");
|
||||
checkFn("matchesKittySequence");
|
||||
checkFn("executeShell");
|
||||
checkFn("PtySession");
|
||||
checkFn("SearchDb");
|
||||
checkFn("Shell");
|
||||
checkFn("parseKey");
|
||||
checkFn("matchesLegacySequence");
|
||||
checkFn("parseKittySequence");
|
||||
checkFn("matchesKey");
|
||||
checkFn("visibleWidth");
|
||||
checkFn("killTree");
|
||||
checkFn("listDescendants");
|
||||
checkFn("getWorkProfile");
|
||||
checkFn("invalidateFsScanCache");
|
||||
checkFn("astGrep");
|
||||
checkFn("astEdit");
|
||||
checkFn("detectMacOSAppearance");
|
||||
checkFn("MacAppearanceObserver");
|
||||
checkFn("MacOSPowerAssertion");
|
||||
checkFn("projfsOverlayProbe");
|
||||
checkFn("projfsOverlayStart");
|
||||
checkFn("projfsOverlayStop");
|
||||
if (missing.length) {
|
||||
throw new Error(
|
||||
`Native addon missing exports (${source}). Missing: ${missing.join(", ")}. ` +
|
||||
"Rebuild with `bun --cwd=packages/natives run build:native`.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = logger.time("native:loadNative", () => loadNative());
|
||||
|
||||
// Side effect: register native killTree with pi-utils.
|
||||
const { setNativeKillTree } = require("@oh-my-pi/pi-utils");
|
||||
setNativeKillTree(module.exports.killTree);
|
||||
module.exports = loadNative();
|
||||
|
||||
// --- generated const enum exports (do not edit) ---
|
||||
exports.AstMatchStrictness = {
|
||||
|
||||
Reference in New Issue
Block a user