Files
OmniRoute/tests/unit/plugins-loader.test.ts
MumuTW 5840d0961f fix(plugins): delete the generated host script synchronously so plugin loads stop leaking temp files (#8749)
* fix(plugins): delete the host script synchronously and stop two tests leaking child processes

Three related leaks in the plugin child-process lifecycle, found while tracing 27 node
processes on a developer machine.

1. loader.ts removed the generated omniroute-plugin-host-*.mjs with a fire-and-forget
   `rm(...).catch(() => {})`. That unlink loses the race against process exit: test:unit
   runs with --test-force-exit, which tears the process down before the promise settles,
   so every plugin load leaked one temp .mjs into TMPDIR. Measured at 6 files per
   full-suite run, 40 accumulated over a handful of local runs. rmSync closes the race;
   the throw stays swallowed because an exception raised from a child "exit" handler
   would take the server down, and a leftover temp script would not.

2. plugins-manager-lifecycle.test.ts "activates an installed plugin" called activate() --
   which spawns the plugin's child process -- but never deactivate(). deactivate() is the
   only path that reaches the loader's cleanup(), so the child outlived the test and its
   IPC channel kept the test process's event loop alive.

3. plugins-manager-restart-reload-7806.test.ts simulateRestart() deleted the entry from
   loadedPlugins without calling cleanup(), dropping the only handle that can kill child
   #1. The reload then spawned child #2, and the finally block's deactivate() could reach
   only child #2 -- one dangling child per test. A real restart takes the whole process
   tree down, so calling cleanup() here is both the faithful simulation and the fix.

Combined effect: a run without --test-force-exit deadlocks. The test process cannot exit
while its child holds the IPC channel open, and the child waits for messages that never
come. Observed as three plugin hosts alive for 4h51m under a runner that never finished.

Validation (Hard Rule #18, TDD): the new plugins-loader.test.ts case fails against the old
async unlink ("must delete the host script synchronously, not on a later tick") and passes
with rmSync. It redirects TMPDIR/TEMP/TMP to a private directory before counting, because
test:unit runs at --test-concurrency=20 and a concurrent file's host scripts would
otherwise land in the counted directory and flake the assertion.

After: 19/19 pass across the three files, 0 temp scripts created, 0 orphan processes.
tests/unit/build/** 334/334; typecheck:core and eslint clean.

* docs(changelog): add fragment for plugin host script sync delete
2026-07-27 17:25:38 -03:00

264 lines
8.6 KiB
TypeScript

import test from "node:test";
import assert from "node:assert/strict";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { readdirSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { loadPlugin, type LoadedPlugin } from "../../src/lib/plugins/loader.ts";
import type { Plugin, PluginContext, PluginResult } from "../../src/lib/plugins/index.ts";
// ── Type checks ──
test("LoadedPlugin interface has required fields", () => {
// Verify the type structure exists by checking the module exports
const mock: LoadedPlugin = {
name: "test",
manifest: {
name: "test",
version: "1.0.0",
license: "MIT",
main: "index.js",
source: "local",
tags: [],
requires: { permissions: [] },
hooks: { onRequest: false, onResponse: false, onError: false },
skills: [],
enabledByDefault: false,
configSchema: {},
},
plugin: { name: "test" },
cleanup: () => {},
};
assert.equal(mock.name, "test");
assert.equal(typeof mock.cleanup, "function");
});
test("Plugin interface supports lifecycle hooks", () => {
const plugin: Plugin = {
name: "test",
onRequest: async (_ctx: PluginContext): Promise<PluginResult | void> => {
return { blocked: false };
},
onResponse: async (_ctx: PluginContext, response: unknown) => response,
onError: async (_ctx: PluginContext, _error: Error) => null,
};
assert.equal(typeof plugin.onRequest, "function");
assert.equal(typeof plugin.onResponse, "function");
assert.equal(typeof plugin.onError, "function");
});
test("PluginContext has required fields", () => {
const ctx: PluginContext = {
requestId: "test-123",
body: { model: "gpt-4" },
model: "gpt-4",
provider: "openai",
metadata: {},
};
assert.equal(ctx.requestId, "test-123");
assert.equal(ctx.model, "gpt-4");
});
test("PluginResult supports blocking", () => {
const blocked: PluginResult = {
blocked: true,
response: { error: "denied" },
};
assert.ok(blocked.blocked);
assert.deepEqual(blocked.response, { error: "denied" });
});
test("PluginResult supports body modification", () => {
const modified: PluginResult = {
body: { model: "gpt-4-turbo" },
metadata: { plugin: "model-switcher" },
};
assert.equal(modified.body.model, "gpt-4-turbo");
assert.equal(modified.metadata?.plugin, "model-switcher");
});
test(
"loadPlugin runs hooks in an isolated child process over IPC",
{ timeout: 5_000 },
async (t) => {
const pluginDir = await mkdtemp(join(tmpdir(), "omniroute-plugin-loader-"));
const entryPoint = join(pluginDir, "index.mjs");
let loaded: LoadedPlugin | undefined;
t.after(async () => {
loaded?.cleanup();
await rm(pluginDir, { recursive: true, force: true });
});
await writeFile(
entryPoint,
`
export async function onRequest(ctx) {
return {
body: { ...ctx.body, touchedByPlugin: true },
metadata: { pluginHook: "onRequest" },
};
}
`,
"utf-8"
);
loaded = await loadPlugin(entryPoint, {
name: "ipc-test",
version: "1.0.0",
license: "MIT",
main: "index.mjs",
source: "local",
tags: [],
requires: { permissions: [] },
hooks: { onRequest: true, onResponse: false, onError: false },
skills: [],
enabledByDefault: false,
configSchema: {},
});
const result = await loaded.plugin.onRequest?.({
requestId: "test-request",
body: { model: "gpt-4" },
model: "gpt-4",
metadata: {},
});
assert.deepEqual(result, {
body: { model: "gpt-4", touchedByPlugin: true },
metadata: { pluginHook: "onRequest" },
});
}
);
// Regression (PR #3562, Hard Rule #18): the loader must build the plugin's
// lifecycle-hook methods (onInstall/onActivate/onDeactivate/onUninstall) when —
// and only when — the manifest declares them. manager.ts registers exactly these
// methods with emitHook; before #3562 the loader only wired onRequest/onResponse/
// onError, so the lifecycle hooks were declared-but-dead (manager registered
// `undefined` and the fire-and-forget emitHook never reached the plugin).
test(
"loadPlugin wires declared lifecycle hooks and skips undeclared ones",
{ timeout: 5_000 },
async (t) => {
const pluginDir = await mkdtemp(join(tmpdir(), "omniroute-plugin-lifecycle-"));
const entryPoint = join(pluginDir, "index.mjs");
let loaded: LoadedPlugin | undefined;
t.after(async () => {
loaded?.cleanup();
await rm(pluginDir, { recursive: true, force: true });
});
await writeFile(
entryPoint,
`
export async function onInstall(_payload) {}
export async function onActivate(_payload) {}
export async function onDeactivate(_payload) {}
export async function onUninstall(_payload) {}
`,
"utf-8"
);
loaded = await loadPlugin(entryPoint, {
name: "lifecycle-test",
version: "1.0.0",
license: "MIT",
main: "index.mjs",
source: "local",
tags: [],
requires: { permissions: [] },
hooks: {
onRequest: false,
onResponse: false,
onError: false,
onInstall: true,
onActivate: true,
onDeactivate: false, // declared in code but disabled in manifest — must NOT be wired
onUninstall: true,
},
skills: [],
enabledByDefault: false,
configSchema: {},
});
// Hooks enabled in the manifest must be wired as callable methods — this is
// exactly what manager.ts hands to registerHook(hookName, name, handler).
assert.equal(typeof loaded.plugin.onInstall, "function", "onInstall must be wired");
assert.equal(typeof loaded.plugin.onActivate, "function", "onActivate must be wired");
assert.equal(typeof loaded.plugin.onUninstall, "function", "onUninstall must be wired");
// A hook disabled in the manifest must NOT be wired, even if the plugin
// exports it (gated by the manifest flag).
assert.equal(loaded.plugin.onDeactivate, undefined, "disabled onDeactivate must not be wired");
// The wired method must bridge to the worker without throwing (fire-and-forget).
await loaded.plugin.onActivate?.({ name: "lifecycle-test", version: "1.0.0" });
}
);
// Regression (Hard Rule #18): the generated host script used to be deleted with a
// fire-and-forget `rm(...).catch()`. That unlink loses the race against process exit —
// under `npm run test:unit` (--test-force-exit) the runner tore the process down before
// the promise settled, so every plugin load leaked one omniroute-plugin-host-*.mjs into
// TMPDIR and they accumulated run over run. cleanup() must have removed it by return.
test(
"cleanup() removes the generated host script before returning",
{ timeout: 5_000 },
async (t) => {
const pluginDir = await mkdtemp(join(tmpdir(), "omniroute-plugin-loader-"));
const entryPoint = join(pluginDir, "index.mjs");
// Redirect the loader's tmpdir() so a concurrent test file's host scripts cannot
// land in the directory we count (test:unit runs at --test-concurrency=20).
// POSIX reads TMPDIR, Windows reads TEMP/TMP — set all three.
const hostScriptDir = await mkdtemp(join(tmpdir(), "omniroute-hostscript-"));
const tmpEnvKeys = ["TMPDIR", "TEMP", "TMP"] as const;
const savedEnv = tmpEnvKeys.map((k) => [k, process.env[k]] as const);
let loaded: LoadedPlugin | undefined;
t.after(async () => {
loaded?.cleanup();
for (const [key, value] of savedEnv) {
if (value === undefined) delete process.env[key];
else process.env[key] = value;
}
await rm(pluginDir, { recursive: true, force: true });
await rm(hostScriptDir, { recursive: true, force: true });
});
await writeFile(entryPoint, "export async function onRequest() { return {}; }\n", "utf-8");
for (const key of tmpEnvKeys) process.env[key] = hostScriptDir;
loaded = await loadPlugin(entryPoint, {
name: "host-script-cleanup-test",
version: "1.0.0",
license: "MIT",
main: "index.mjs",
source: "local",
tags: [],
requires: { permissions: [] },
hooks: { onRequest: true, onResponse: false, onError: false },
skills: [],
enabledByDefault: false,
configSchema: {},
});
assert.deepEqual(
readdirSync(hostScriptDir).filter((f) => f.startsWith("omniroute-plugin-host-")).length,
1,
"sanity: loadPlugin writes exactly one host script"
);
loaded.cleanup();
loaded = undefined;
assert.deepEqual(
readdirSync(hostScriptDir).filter((f) => f.startsWith("omniroute-plugin-host-")),
[],
"cleanup() must delete the host script synchronously, not on a later tick"
);
}
);