From a39fb59bd1dec010431d026cad57af88061e38e0 Mon Sep 17 00:00:00 2001
From: Nicky Leach <nicky@paperclip.ing>
Date: Mon, 9 Nov 2026 16:27:08 -0800
Subject: [PATCH] Join in-flight PTY close before releasing login ownership

Input delivery failure terminalizes the route before its worker close finishes. Retain one close promise so the runner disposal and every later close await the existing acknowledgement instead of allowing allocation cleanup to race process stop.

Validated all 91 plugin worker manager tests, including a held close acknowledgement after input failure with concurrent disposal calls, and the server TypeScript check.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 .../__tests__/plugin-worker-manager.test.ts   | 26 +++++++++++++++
 server/src/services/plugin-worker-manager.ts  | 33 +++++++++++--------
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/server/src/__tests__/plugin-worker-manager.test.ts b/server/src/__tests__/plugin-worker-manager.test.ts
index 55424e9f9b3..fbc6bda25ec 100644
--- a/server/src/__tests__/plugin-worker-manager.test.ts
+++ b/server/src/__tests__/plugin-worker-manager.test.ts
@@ -1368,6 +1368,32 @@ describe("plugin worker manager setup-token pty route gate", () => {
     } finally { await handle.stop().catch(() => undefined); }
   });
 
+  it("joins an in-flight close after input failure before releasing the login", async () => {
+    let releaseClose!: () => void;
+    const heldClose = new Promise<void>(resolve => { releaseClose = resolve; });
+    let closeCalls = 0;
+    const handle = makeLoginPtyHandle({ hostHandlers: { "companies.get": async (params: { stage: string }) => {
+      if (params.stage === "loginPtyClose") { closeCalls++; await heldClose; }
+      return { id: "company-1" };
+    } } });
+    try {
+      await handle.start();
+      const session = await handle.openLoginPtySession(ptyOpenInput({ rejectInput: true, probeLifecycle: true }));
+      session.write("browser-code");
+      await expect(session.wait()).resolves.toEqual({ exitCode: null });
+      await vi.waitFor(() => expect(closeCalls).toBe(1));
+      let completed = 0;
+      const first = session.close().then(() => { completed++; });
+      const second = session.close().then(() => { completed++; });
+      await new Promise(resolve => setImmediate(resolve));
+      expect(completed).toBe(0);
+      releaseClose();
+      await Promise.all([first, second]);
+      expect(completed).toBe(2);
+      expect(closeCalls).toBe(1);
+    } finally { releaseClose(); await handle.stop().catch(() => undefined); }
+  });
+
   it("terminalizes the route when the cumulative output passes the per-route bound", async () => {
     const handle = makeLoginPtyHandle({
       loginPtyLimits: { maxTotalChars: 10 },
diff --git a/server/src/services/plugin-worker-manager.ts b/server/src/services/plugin-worker-manager.ts
index 459f676148e..38ed65d8a53 100644
--- a/server/src/services/plugin-worker-manager.ts
+++ b/server/src/services/plugin-worker-manager.ts
@@ -1567,6 +1567,7 @@ export function createPluginWorkerHandle(
   interface LoginPtyRoute {
     companyId: string;
     hostRouteId: string;
+    closePromise: Promise<void> | null;
     state: LoginPtyRouteState;
     workerSessionId: string | null;
     listener: ((chunk: string) => void) | null;
@@ -1649,6 +1650,9 @@ export function createPluginWorkerHandle(
   // every other route this worker still holds settles through the worker-exit
   // path that follows.
   async function terminalizeLoginPtyRoute(route: LoginPtyRoute): Promise<void> {
+    // A notification may already have started close. Every later disposer must
+    // join that acknowledgement before its owner can release the allocation.
+    if (route.closePromise) return route.closePromise;
     if (route.terminalized) return;
     route.terminalized = true;
     route.state = "closed";
@@ -1662,20 +1666,22 @@ export function createPluginWorkerHandle(
       loginPtyRoutesByWorkerSessionId.delete(route.workerSessionId);
     }
     releaseLoginPtyRouteSlot(route);
-    // A terminalized route reports a null exit code, which the runner treats as a
-    // failure.
+    route.closePromise = (async () => {
+      const confirmed = await closeLoginPtyTerminal(route.hostRouteId, route.companyId);
+      if (!confirmed) {
+        // The worker did not acknowledge the close, so the host cannot prove the
+        // terminal is gone. Retire the worker before any reuse.
+        log.error(
+          { pluginId },
+          "login pseudo-terminal close not acknowledged; retiring worker",
+        );
+        retireWorkerProcess();
+      }
+    })();
+    // Report failure promptly, but retain the shared close barrier so the
+    // runner's following dispose waits for confirmed stop (or worker retirement).
     settleRouteWait(route, { exitCode: null });
-    const confirmed = await closeLoginPtyTerminal(route.hostRouteId, route.companyId);
-    if (!confirmed) {
-      // The worker did not acknowledge the close, so the host cannot prove the
-      // terminal is gone. Fail closed: retire the worker before any reuse. This
-      // also settles and clears every other route the worker still holds.
-      log.error(
-        { pluginId },
-        "login pseudo-terminal close not acknowledged; retiring worker",
-      );
-      retireWorkerProcess();
-    }
+    return route.closePromise;
   }
 
   // Resolve one login pseudo-terminal notification to its route. Return null
@@ -1924,6 +1930,7 @@ export function createPluginWorkerHandle(
     const route: LoginPtyRoute = {
       companyId: input.companyId,
       hostRouteId,
+      closePromise: null,
       state: "reserved",
       workerSessionId: null,
       listener: null,