fix(pty): free Rust session on kill() to stop FD leak (#974)

src/lib/pty.test.ts

@@ -0,0 +1,70 @@
+// Regression test for #974 — VMarkPty.kill() leaked the Rust session map entry
+// because _cleanup() removes the pty:exit listener BEFORE pty_kill, so the
+// exit handler that calls pty_close never runs. kill() must call pty_close
+// itself (after pty_kill), and the setup-time guard path must too.
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const { invokeMock, listenMock } = vi.hoisted(() => ({
+  invokeMock: vi.fn(),
+  listenMock: vi.fn(),
+}));
+
+vi.mock("@tauri-apps/api/core", () => ({
+  invoke: (...args: unknown[]) => invokeMock(...args),
+}));
+vi.mock("@tauri-apps/api/event", () => ({
+  listen: (...args: unknown[]) => listenMock(...args),
+}));
+vi.mock("@/utils/debug", () => ({ ptyWarn: vi.fn(), terminalLog: vi.fn() }));
+
+// The global test setup (src/test/setup.ts) mocks the whole @/lib/pty module
+// for Terminal component tests. This suite exercises the REAL implementation,
+// so unmock it here (the Tauri core/event mocks above still apply).
+vi.unmock("@/lib/pty");
+
+import { spawn } from "@/lib/pty";
+
+const PID = 4242;
+
+beforeEach(() => {
+  invokeMock.mockReset();
+  listenMock.mockReset();
+  invokeMock.mockImplementation((cmd: string) =>
+    cmd === "pty_spawn" ? Promise.resolve(PID) : Promise.resolve(undefined),
+  );
+  // listen() resolves to an unlisten fn
+  listenMock.mockResolvedValue(() => {});
+});
+
+describe("VMarkPty.kill() — #974 session leak", () => {
+  it("calls pty_close after pty_kill so the Rust session is freed", async () => {
+    const pty = spawn("bash", []);
+    // Wait for setup to finish (reader started = ready).
+    await vi.waitFor(() => {
+      expect(invokeMock).toHaveBeenCalledWith("pty_start", { pid: PID });
+    });
+
+    pty.kill();
+
+    await vi.waitFor(() => {
+      expect(invokeMock).toHaveBeenCalledWith("pty_close", { pid: PID });
+    });
+    expect(invokeMock).toHaveBeenCalledWith("pty_kill", { pid: PID });
+    // Exactly once on the normal path (the eagerly-removed exit listener can't
+    // double-close).
+    const closeCalls = invokeMock.mock.calls.filter((c) => c[0] === "pty_close");
+    expect(closeCalls).toHaveLength(1);
+  });
+
+  it("frees the session even when kill() races setup (setup-guard path)", async () => {
+    const pty = spawn("bash", []);
+    pty.kill(); // before _ready resolves → hits the setup guard
+
+    await vi.waitFor(() => {
+      expect(invokeMock).toHaveBeenCalledWith("pty_close", { pid: PID });
+    });
+    expect(invokeMock).toHaveBeenCalledWith("pty_kill", { pid: PID });
+    // Guard aborts before phase 3 — the reader never starts.
+    expect(invokeMock).not.toHaveBeenCalledWith("pty_start", { pid: PID });
+  });
+});

src/lib/pty.ts

@@ -16,7 +16,10 @@
  *     Uint8Array.
  *   - `kill()` eagerly cleans up event listeners and guards against mid-setup
  *     races via a `_destroyed` flag.
- *   - `pty_close` is called after exit to free the Rust-side session (FDs).
+ *   - `pty_close` frees the Rust-side session (FDs/channels/child handle). It
+ *     runs from the exit handler on natural exit; but because `kill()` (and the
+ *     mid-setup guard) tear down the exit listener first, those paths call
+ *     `pty_close` directly so the session is never leaked (#974).
  *
  * @coordinates-with src-tauri/src/pty.rs — Rust backend commands and events
  * @coordinates-with components/Terminal/spawnPty.ts — consumes this wrapper
@@ -162,6 +165,11 @@ class VMarkPty implements IPty {
       await invoke("pty_kill", { pid: this._pid }).catch((err) => {
         terminalLog("pty_kill (setup guard) failed:", errorMessage(err));
       });
+      // Listeners were torn down, so the exit handler won't free the session
+      // (#974). Close it explicitly here too.
+      await invoke("pty_close", { pid: this._pid }).catch((err) => {
+        terminalLog("pty_close (setup guard) failed:", errorMessage(err));
+      });
       return;
     }
 
@@ -197,10 +205,19 @@ class VMarkPty implements IPty {
   kill(): void {
     this._destroyed = true;
     this._cleanup();
+    // _cleanup() removed the pty:exit listener, so the natural exit handler
+    // that calls pty_close never runs (#974). Close the Rust session
+    // explicitly after pty_kill — in finally so a failed kill still frees the
+    // session map entry (FDs, channels, child handle).
     this._ready
       .then(() => invoke("pty_kill", { pid: this._pid }))
       .catch((err) => {
         terminalLog("pty_kill failed:", errorMessage(err));
+      })
+      .finally(() => {
+        invoke("pty_close", { pid: this._pid }).catch((err) => {
+          terminalLog("pty_close failed:", errorMessage(err));
+        });
       });
   }