From e1cfdb5e48a9d9fcdd3699585f35156077d72634 Mon Sep 17 00:00:00 2001
From: anhtahaylove <37265396+anhtahaylove@users.noreply.github.com>
Date: Fri, 11 Sep 2026 23:30:44 +0700
Subject: [PATCH] fix(acp): release listeners, timers and sessions on every
 sendPrompt outcome (#13096)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`acpManager` being a module-level singleton is what turns this from a per-call leak into unbounded growth — the `MaxListenersExceededWarning` at 11 is the visible symptom. Routing every outcome through one `settle()` is the right shape, and deleting the session on the child's own exit fixes the map growth that `getActiveSessions()`'s `alive` filter was hiding.


I reformatted the changelog fragment to the `changelog.d` convention (`- **fix(scope):** …`) before merging — `check:changelog-integrity` rejects a fragment that does not start with a markdown bullet, which is the same gate your #13158 was about. Wording is yours, unchanged in substance.

---

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.
---
 .../13095-acp-sendprompt-listener-leak.md     |   1 +
 src/lib/acp/manager.ts                        |  34 +++---
 .../acp-manager-sendprompt-leak-13095.test.ts | 101 ++++++++++++++++++
 3 files changed, 123 insertions(+), 13 deletions(-)
 create mode 100644 changelog.d/fixes/13095-acp-sendprompt-listener-leak.md
 create mode 100644 tests/unit/acp-manager-sendprompt-leak-13095.test.ts

diff --git a/changelog.d/fixes/13095-acp-sendprompt-listener-leak.md b/changelog.d/fixes/13095-acp-sendprompt-listener-leak.md
new file mode 100644
index 0000000000..56473cc8c2
--- /dev/null
+++ b/changelog.d/fixes/13095-acp-sendprompt-listener-leak.md
@@ -0,0 +1 @@
+- **fix(acp):** release the `stdout`/`exit` listeners and the idle timer that a `sendPrompt` timeout used to leave attached to the `acpManager` singleton, and drop sessions that exited on their own from the session map instead of keeping them forever.
diff --git a/src/lib/acp/manager.ts b/src/lib/acp/manager.ts
index 85bc05e720..85725fd5fc 100644
--- a/src/lib/acp/manager.ts
+++ b/src/lib/acp/manager.ts
@@ -90,6 +90,10 @@ export class AcpManager extends EventEmitter {
 
     child.on("exit", (code, signal) => {
       session.alive = false;
+      // Only kill() used to remove entries, so any agent that exited on its own
+      // stayed in the map forever. getActiveSessions() filters on `alive`, which
+      // hid the growth from callers.
+      this.sessions.delete(sessionId);
       this.emit("exit", { sessionId, code, signal });
     });
 
@@ -129,31 +133,35 @@ export class AcpManager extends EventEmitter {
 
     // Wait for response (collect until process goes idle or timeout)
     return new Promise((resolve, reject) => {
-      const timer = setTimeout(() => {
-        reject(new Error(`ACP timeout after ${timeoutMs}ms`));
-      }, timeoutMs);
+      let idleTimer: ReturnType<typeof setTimeout> | undefined;
 
-      let idleTimer: ReturnType<typeof setTimeout>;
+      // Every outcome -- idle, exit, or timeout -- has to release the same
+      // resources. `acpManager` is a module-level singleton, so a branch that
+      // skips this leaks a listener per call for the lifetime of the process.
+      const settle = (finish: () => void) => {
+        clearTimeout(timer);
+        clearTimeout(idleTimer);
+        this.removeListener("stdout", onData);
+        this.removeListener("exit", onExit);
+        finish();
+      };
+
+      const timer = setTimeout(() => {
+        settle(() => reject(new Error(`ACP timeout after ${timeoutMs}ms`)));
+      }, timeoutMs);
 
       const onData = ({ sessionId: sid }: { sessionId: string }) => {
         if (sid !== sessionId) return;
         // Reset idle timer on new data
         clearTimeout(idleTimer);
         idleTimer = setTimeout(() => {
-          clearTimeout(timer);
-          this.removeListener("stdout", onData);
-          this.removeListener("exit", onExit);
-          resolve(session.stdoutBuffer);
+          settle(() => resolve(session.stdoutBuffer));
         }, 2000); // 2s idle = response complete
       };
 
       const onExit = ({ sessionId: sid }: { sessionId: string }) => {
         if (sid !== sessionId) return;
-        clearTimeout(timer);
-        clearTimeout(idleTimer);
-        this.removeListener("stdout", onData);
-        this.removeListener("exit", onExit);
-        resolve(session.stdoutBuffer);
+        settle(() => resolve(session.stdoutBuffer));
       };
 
       this.on("stdout", onData);
diff --git a/tests/unit/acp-manager-sendprompt-leak-13095.test.ts b/tests/unit/acp-manager-sendprompt-leak-13095.test.ts
new file mode 100644
index 0000000000..d3b5dce291
--- /dev/null
+++ b/tests/unit/acp-manager-sendprompt-leak-13095.test.ts
@@ -0,0 +1,101 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { AcpManager } = await import("../../src/lib/acp/manager.ts");
+const { setCustomAgents } = await import("../../src/lib/acp/registry.ts");
+
+// A registered agent whose binary is just node running a script that stays quiet,
+// so sendPrompt() reliably hits its timeout instead of resolving on data/exit.
+const AGENT_ID = "acp-leak-probe";
+setCustomAgents([
+  {
+    id: AGENT_ID,
+    name: "ACP leak probe",
+    binary: process.execPath,
+    description: "test-only agent",
+  },
+]);
+
+function spawnIdleSession(manager) {
+  // Keeps stdin open and never writes to stdout: the prompt can only time out.
+  return manager.spawn(AGENT_ID, process.execPath, [
+    "-e",
+    "process.stdin.resume(); setTimeout(() => {}, 60_000);",
+  ]);
+}
+
+test("sendPrompt timeout does not leak listeners on the manager (#13095)", async () => {
+  const manager = new AcpManager();
+  const session = spawnIdleSession(manager);
+
+  try {
+    const before = {
+      stdout: manager.listenerCount("stdout"),
+      exit: manager.listenerCount("exit"),
+    };
+
+    // Each of these must reject on the timeout path.
+    for (let i = 0; i < 12; i++) {
+      await assert.rejects(
+        () => manager.sendPrompt(session.id, "ping", 15),
+        /ACP timeout after 15ms/,
+        `attempt ${i + 1} should time out`
+      );
+    }
+
+    // The timeout branch has to tear down both listeners it registered. Before the
+    // fix these grew by one per timed-out prompt and were never released, which
+    // matters because `acpManager` is a module-level singleton.
+    assert.equal(
+      manager.listenerCount("stdout"),
+      before.stdout,
+      "stdout listeners must return to the pre-prompt count"
+    );
+    assert.equal(
+      manager.listenerCount("exit"),
+      before.exit,
+      "exit listeners must return to the pre-prompt count"
+    );
+  } finally {
+    manager.killAll();
+  }
+});
+
+test("sendPrompt timeout clears its idle timer so the process can settle (#13095)", async () => {
+  const manager = new AcpManager();
+  const session = spawnIdleSession(manager);
+
+  try {
+    await assert.rejects(
+      () => manager.sendPrompt(session.id, "ping", 15),
+      /ACP timeout after 15ms/
+    );
+
+    // A leaked idle timer keeps a 2s handle (and the captured session) alive after
+    // the promise already rejected. Nothing should be pending on the manager.
+    assert.equal(manager.listenerCount("stdout"), 0);
+    assert.equal(manager.listenerCount("exit"), 0);
+  } finally {
+    manager.killAll();
+  }
+});
+
+test("exited sessions are removed from the session map (#13095)", async () => {
+  const manager = new AcpManager();
+  // Exits immediately on its own; nothing calls kill() for it.
+  const session = manager.spawn(AGENT_ID, process.execPath, ["-e", "process.exit(0)"]);
+
+  await new Promise((resolve) => {
+    manager.on("exit", ({ sessionId }) => {
+      if (sessionId === session.id) resolve();
+    });
+  });
+  // Let the exit handler finish its bookkeeping.
+  await new Promise((resolve) => setTimeout(resolve, 50));
+
+  assert.equal(
+    manager.getSession(session.id),
+    undefined,
+    "a session that exited on its own must not stay in the map"
+  );
+});