fix(plugins): make SIGKILL escalation idempotent per child (#13092)

`once` only detaches when exit actually fires, so a plugin trapping SIGTERM accumulated one listener and one timer per hook timeout. Keying idempotence on the child via a `WeakSet` is right — a second SIGKILL timer would only re-signal a corpse.

---

Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the other 13 PRs of this batch — zero merge conflicts between them.

- `typecheck:core` clean
- complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline
- 71 focused assertions green across the 13 test files this batch adds or touches

⚠️ base-red inherited: #12732 — `Docs Gates (fast-path)`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098). None of them touch this diff.

Thanks @anhtahaylove — the root-cause write-up, the measured before/after numbers and the red-before-green proof on every one of these made the batch reviewable as a unit.

changelog.d/fixes/plugin-sigkill-listener-leak.md

@@ -0,0 +1 @@
+- fix(plugins): stop leaking an exit listener per plugin hook timeout, which triggered MaxListenersExceededWarning on plugins that ignore SIGTERM

src/lib/plugins/loader.ts

@@ -9,6 +9,7 @@
  */
 
 import { spawn } from "child_process";
+import type { ChildProcess } from "child_process";
 import { writeFile, readFile } from "fs/promises";
 import { rmSync } from "fs";
 import { join } from "path";
@@ -105,6 +106,37 @@ function forwardChildOutput(
  * against process exit — under `node --test --test-force-exit` the runner exits
  * before the promise settles, leaking one temp .mjs per plugin load.
  */
+/** Children already escalating to SIGKILL. Prevents re-arming a second timer + listener
+ *  for a child that is already being killed. */
+const escalating = new WeakSet<ChildProcess>();
+
+/**
+ * SIGTERM has already been sent; escalate to SIGKILL if the child ignores it.
+ *
+ * Must be idempotent per child. Every hook timeout hits this path, and a plugin that
+ * traps SIGTERM keeps taking calls, so re-arming would add one exit listener plus one
+ * killTimer closure per timeout — Node starts printing MaxListenersExceededWarning at 11.
+ * One pending kill per child is also all that is useful: SIGKILL cannot be ignored, so a
+ * second timer would only re-signal a corpse. (#12819)
+ */
+function escalateToSigkill(child: ChildProcess): void {
+  if (escalating.has(child)) return;
+  escalating.add(child);
+
+  const onExit = () => {
+    clearTimeout(killTimer);
+    escalating.delete(child);
+  };
+  const killTimer = setTimeout(() => {
+    child.removeListener("exit", onExit);
+    escalating.delete(child);
+    try {
+      child.kill("SIGKILL");
+    } catch {}
+  }, SIGKILL_GRACE_MS);
+  child.once("exit", onExit);
+}
+
 function removeHostScript(path: string): void {
   try {
     rmSync(path, { force: true });
@@ -293,12 +325,7 @@ export async function loadPlugin(
         }
         child.kill("SIGTERM");
         // Escalate to SIGKILL if plugin ignores SIGTERM
-        const killTimer = setTimeout(() => {
-          try {
-            child.kill("SIGKILL");
-          } catch {}
-        }, SIGKILL_GRACE_MS);
-        child.once("exit", () => clearTimeout(killTimer));
+        escalateToSigkill(child);
         reject(new Error(`Plugin hook '${hook}' timed out after ${timeout}ms`));
       }, timeout);
 
@@ -399,12 +426,7 @@ export async function loadPlugin(
   const cleanup = () => {
     child.kill("SIGTERM");
     // Escalate to SIGKILL after grace period
-    const killTimer = setTimeout(() => {
-      try {
-        child.kill("SIGKILL");
-      } catch {}
-    }, SIGKILL_GRACE_MS);
-    child.once("exit", () => clearTimeout(killTimer));
+    escalateToSigkill(child);
     removeHostScript(hostScriptPath);
     log.info("loader.cleanup", { name: manifest.name });
   };

tests/unit/plugins-sigkill-listener-leak-12819.test.ts

@@ -0,0 +1,100 @@
+// Regression test for #12819 — loadPlugin() leaked one "exit" listener per hook timeout.
+//
+// Root cause: on the SIGTERM→SIGKILL escalation path the loader attached a fresh
+// `child.once("exit", () => clearTimeout(killTimer))`. `once` only detaches when exit
+// actually FIRES, so a plugin that ignores SIGTERM leaves the listener (and its killTimer
+// closure) attached on every hook timeout. Node then prints MaxListenersExceededWarning
+// once 11 accumulate.
+//
+// The plugin below traps SIGTERM and keeps running, which is exactly the condition the
+// bug needs. We drive several hook timeouts and assert the listener count stays bounded.
+import { test, describe, after } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+
+const { loadPlugin } = await import("../../src/lib/plugins/loader.ts");
+
+const dirs: string[] = [];
+after(() => {
+  for (const d of dirs) rmSync(d, { recursive: true, force: true });
+});
+
+/** A plugin that ignores SIGTERM and never answers a hook, forcing the escalation path. */
+function writeStubbornPlugin(): string {
+  const dir = mkdtempSync(join(tmpdir(), "omniroute-plugin-12819-"));
+  dirs.push(dir);
+  const entry = join(dir, "index.mjs");
+  writeFileSync(
+    entry,
+    [
+      // Trap SIGTERM so the loader has to escalate to SIGKILL.
+      'process.on("SIGTERM", () => {});',
+      "export default {",
+      "  // Never resolves → every call hits the hook timeout.",
+      "  onRequest: () => new Promise(() => {}),",
+      "};",
+      "",
+    ].join("\n")
+  );
+  return entry;
+}
+
+describe("plugin loader SIGKILL escalation (#12819)", () => {
+  test("does not accumulate an exit listener per hook timeout", async () => {
+    const entryPoint = writeStubbornPlugin();
+    const loaded = await loadPlugin(
+      entryPoint,
+      {
+        name: "sigkill-listener-leak",
+        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: {},
+      } as never,
+      { hookTimeoutMs: 120 }
+    );
+
+    const onRequest = (
+      loaded.plugin as unknown as {
+        onRequest?: (ctx: unknown) => Promise<unknown>;
+      }
+    ).onRequest;
+    assert.ok(onRequest, "onRequest hook should be registered");
+
+    // `child` is private to the loader, so observe the leak the way a user does: Node
+    // itself emits MaxListenersExceededWarning once an emitter passes 10 listeners.
+    const warnings: string[] = [];
+    const onWarning = (w: Error) => {
+      if (w.name === "MaxListenersExceededWarning") warnings.push(w.message);
+    };
+    process.on("warning", onWarning);
+
+    try {
+      // 12 timeouts: comfortably past Node's default limit of 10, so the pre-fix code
+      // trips the warning while the fixed code stays flat.
+      for (let i = 0; i < 12; i++) {
+        await onRequest({ body: {} }).catch(() => undefined);
+      }
+      // Warnings are delivered on the next tick; let them land before asserting.
+      await new Promise((r) => setTimeout(r, 50));
+    } finally {
+      process.removeListener("warning", onWarning);
+    }
+
+    assert.deepEqual(
+      warnings,
+      [],
+      `hook timeouts must not accumulate exit listeners (#12819): ${warnings[0] ?? ""}`
+    );
+
+    loaded.cleanup?.();
+  });
+});