fix(transfer): deferred-dispose soft-cancelled isolated SFTP (#4134)

Codex P2 on 88b4890: softDispose dropped cancelled isolated download
channels from the pool without ending them, leaking remote subsystems.
Schedule sftp.end() after a ~2s quiet settle instead of abandoning forever
or ending mid-flight on the shared SSH transport.

Aligned cheap fix for isolated fastPut cancel: after force-settle, defer
end of the disposable channel the same way. Stopping ssh2 fastXfer
internal scheduling still needs invasive hooks — left as documented
residual; deferred end after quiet is the minimal leak fix.

Tests cover mid-flight isolated upload/download cancel (no end while
held) and eventual deferred end after settle.

electron/bridges/transferBridge.cjs

@@ -1512,6 +1512,10 @@ async function inspectLocalPromotionTarget(targetPath) {
 // ~5 Hz is smooth enough for bars; do not stack another 500ms in the store.
 const PROGRESS_THROTTLE_MS = 200;
 const ISOLATED_DOWNLOAD_IDLE_TTL_MS = 5000;
+// Quiet settle before ending a soft-disposed isolated subsystem after cancel.
+// Matches forceFinish / CLOSE timeout grace so mid-flight READs/WRITEs can drain
+// before sftp.end() (#4134 / Codex P2 deferred dispose).
+const ISOLATED_CHANNEL_DEFERRED_DISPOSE_MS = 2000;
 
 // Speed calculation uses strict sliding-window average:
 // speed = bytes_delta_in_window / time_delta_in_window
@@ -1810,6 +1814,8 @@ function takePendingCancel(transferId) {
 const admittedActiveByResource = new Map();
 let admittedTransferLimit = 2;
 const isolatedDownloadChannelPools = new WeakMap();
+/** @type {WeakMap<object, NodeJS.Timeout>} deferred sftp.end() after softDispose cancel */
+const deferredIsolatedChannelDisposeTimers = new WeakMap();
 // One concurrent 64-READ fanout download per SFTP session (#1507) — covers both
 // isolated channels and shared/sudo browse READs. Extra downloads wait for the
 // session slot instead of degrading to serial createReadStream (#2719 / #2449).
@@ -2338,14 +2344,43 @@ function scheduleIdleIsolatedDownloadChannel(client, sftp) {
   sftp?.once?.("error", onIdleError);
 }
 
+function clearDeferredIsolatedChannelDispose(sftp) {
+  const timer = deferredIsolatedChannelDisposeTimers.get(sftp);
+  if (!timer) return;
+  clearTimeout(timer);
+  deferredIsolatedChannelDisposeTimers.delete(sftp);
+}
+
+/**
+ * End an isolated SFTP subsystem after a quiet settle window. Used when cancel
+ * must not call sftp.end() mid-flight on a transport shared with the terminal
+ * (#4134), but the channel must not be abandoned forever (Codex P2).
+ */
+function scheduleDeferredIsolatedChannelDispose(sftp) {
+  if (!sftp) return;
+  clearDeferredIsolatedChannelDispose(sftp);
+  const timer = setTimeout(() => {
+    deferredIsolatedChannelDisposeTimers.delete(sftp);
+    try { sftp.end?.(); } catch { /* ignore */ }
+  }, ISOLATED_CHANNEL_DEFERRED_DISPOSE_MS);
+  if (typeof timer.unref === "function") timer.unref();
+  deferredIsolatedChannelDisposeTimers.set(sftp, timer);
+}
+
 function releaseIsolatedDownloadChannel(client, sftp, options = {}) {
   const { dispose = false, softDispose = false } = options;
   const pool = isolatedDownloadChannelPools.get(client);
-  // softDispose: drop the channel without sftp.end(). Abrupt end mid-transfer
+  // softDispose: do not sftp.end() at the abort instant. Abrupt end mid-transfer
   // on a transport shared with the terminal can kill the whole SSH (#4134).
+  // Drop from the pool immediately, then schedule a deferred end after quiet
+  // settle so remote handles/subsystems are not leaked forever (Codex P2).
   if (!pool) {
-    if (dispose && !softDispose) {
-      try { sftp?.end?.(); } catch { }
+    if (dispose) {
+      if (softDispose) scheduleDeferredIsolatedChannelDispose(sftp);
+      else {
+        clearDeferredIsolatedChannelDispose(sftp);
+        try { sftp?.end?.(); } catch { }
+      }
     }
     return;
   }
@@ -2355,12 +2390,16 @@ function releaseIsolatedDownloadChannel(client, sftp, options = {}) {
   clearIdleIsolatedDownloadErrorHandler(pool, sftp);
 
   if (dispose) {
-    if (!softDispose) {
+    if (softDispose) {
+      scheduleDeferredIsolatedChannelDispose(sftp);
+    } else {
+      clearDeferredIsolatedChannelDispose(sftp);
       try { sftp?.end?.(); } catch { }
     }
     return;
   }
 
+  clearDeferredIsolatedChannelDispose(sftp);
   pool.idle.push(sftp);
   scheduleIdleIsolatedDownloadChannel(client, sftp);
 }
@@ -2651,10 +2690,18 @@ async function uploadViaFastPut(localPath, remotePath, sftp, fileSize, transfer,
         try { sftp.removeListener("error", onFastSftpError); } catch { /* ignore */ }
         onFastSftpError = null;
       }
-      // End disposable channels only after a quiet settle — never on cancel.
-      // Abrupt end mid-transfer can tear down the shared SSH transport (#4134).
-      if (disposeChannel && !transfer.cancelled) {
-        try { sftp.end(); } catch { /* ignore */ }
+      // End disposable channels after settle. On cancel, defer the end so we
+      // never tear the shared SSH transport mid-flight (#4134) but still close
+      // the subsystem once quiet (Codex P2 deferred dispose). Stopping ssh2
+      // fastXfer's internal scheduling itself would need invasive hooks; ending
+      // after the force-settle window is the aligned minimal fix.
+      if (disposeChannel) {
+        if (transfer.cancelled) {
+          scheduleDeferredIsolatedChannelDispose(sftp);
+        } else {
+          clearDeferredIsolatedChannelDispose(sftp);
+          try { sftp.end(); } catch { /* ignore */ }
+        }
       }
 
       if (transfer.cancelled) reject(new Error("Transfer cancelled"));
@@ -2992,10 +3039,17 @@ async function uploadFileOnChannel(
       fastPutOk = true;
     } catch (err) {
       // Gate-wait / fastPut failure: end the reopened isolated channel before
-      // nulling — except on cancel. Ending mid-fastPut on the shared transport
-      // can drop the whole SSH session (#4134). Non-cancel failures still end.
-      if (!transfer.cancelled && isolated && typeof isolated.end === "function") {
-        try { isolated.end(); } catch { /* ignore */ }
+      // nulling. On cancel, defer the end — never mid-flight on the shared
+      // transport (#4134) — so the subsystem is not leaked (Codex P2).
+      // uploadViaFastPut may already have scheduled the same deferred dispose;
+      // rescheduling is idempotent (resets the quiet timer).
+      if (isolated) {
+        if (transfer.cancelled) {
+          scheduleDeferredIsolatedChannelDispose(isolated);
+        } else if (typeof isolated.end === "function") {
+          clearDeferredIsolatedChannelDispose(isolated);
+          try { isolated.end(); } catch { /* ignore */ }
+        }
       }
       isolated = null;
       // Restore pause capability for subsequent pause-aware strategies.

electron/bridges/transferBridge.test.cjs

@@ -5520,6 +5520,138 @@ test("large isolated upload cancel does not end the shared SSH client (#4134)",
   assert.equal(browseEndCalls, 0);
   assert.equal(sender.sent.some((entry) => entry.channel === "netcatty:transfer:cancelled"), true);
   assert.equal(sender.sent.some((entry) => entry.channel === "netcatty:transfer:error"), false);
+  // Concurrent isolated cancel ends the disposable channel only after WRITE drain
+  // (finally), so it must not stay abandoned forever.
+  assert.ok(
+    await waitUntil(() => isolatedEndCalls >= 1, 3000),
+    "isolated upload channel must end after cancel drain/settle",
+  );
+  assert.equal(isolatedEndWhileHeld, 0);
+});
+
+test("large isolated download cancel softDispose deferred-ends the channel (#4134)", async (t) => {
+  // softDispose must not sftp.end() while READs are held, and must not abandon
+  // the isolated subsystem forever — deferred dispose closes it after quiet.
+  const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "netcatty-4134-isolated-dl-cancel-"));
+  t.after(async () => fs.promises.rm(tempDir, { recursive: true, force: true }));
+  // Large enough for a full READ fanout window so softDispose is exercised
+  // while many READs are held (same shape as the #4134 isolated upload test).
+  const payload = Buffer.alloc(SMALL_UPLOAD_SHARED_MAX_BYTES + TRANSFER_CHUNK_SIZE, 77);
+  const targetPath = path.join(tempDir, "download.bin");
+
+  let sshClientEndCalls = 0;
+  let isolatedEndWhileHeld = 0;
+  let isolatedEndCalls = 0;
+  let browseEndCalls = 0;
+  let holdReads = true;
+  const pendingReads = [];
+
+  const isolatedSftp = createFastSftp({
+    open(_remotePath, _flags, callback) {
+      callback(null, Buffer.from("isolated-read-handle"));
+    },
+    read(_handle, buffer, offset, length, position, callback) {
+      const complete = () => {
+        const end = Math.min(position + length, payload.length);
+        const slice = payload.subarray(position, end);
+        slice.copy(buffer, offset);
+        callback(null, slice.length, buffer, position);
+      };
+      if (holdReads) {
+        pendingReads.push(complete);
+        return;
+      }
+      setImmediate(complete);
+    },
+    close(_handle, callback) {
+      callback(null);
+    },
+    end() {
+      isolatedEndCalls += 1;
+      if (holdReads && pendingReads.length > 0) {
+        isolatedEndWhileHeld += 1;
+      }
+    },
+  });
+
+  const browseSftp = createFastSftp({
+    readdir(_remotePath, callback) {
+      if (browseEndCalls > 0 || sshClientEndCalls > 0) {
+        callback(new Error("browse session ended"));
+        return;
+      }
+      callback(null, [{ filename: "keep" }]);
+    },
+    end() {
+      browseEndCalls += 1;
+    },
+  });
+
+  const sshClient = {
+    end() {
+      sshClientEndCalls += 1;
+    },
+    sftp(callback) {
+      callback(null, isolatedSftp);
+    },
+  };
+
+  const client = {
+    sftp: browseSftp,
+    client: sshClient,
+    stat() {
+      return Promise.resolve({ size: payload.length });
+    },
+  };
+  transferBridge.init({ sftpClients: new Map([["source", client]]) });
+
+  const sender = createSender();
+  const transferId = "isolated-download-cancel-4134";
+  const running = transferBridge.startTransfer({ sender }, {
+    transferId,
+    sourcePath: "/tmp/isolated-cancel-4134.bin",
+    targetPath,
+    sourceType: "sftp",
+    targetType: "local",
+    sourceSftpId: "source",
+    totalBytes: payload.length,
+    resumable: true,
+    skipAdmission: true,
+  });
+
+  const ready = await waitUntil(
+    () => pendingReads.length >= DOWNLOAD_TRANSFER_CONCURRENCY,
+    2000,
+  );
+  assert.equal(ready, true, "expected isolated READ fanout to be in flight");
+  assert.equal(isolatedEndWhileHeld, 0);
+  assert.equal(sshClientEndCalls, 0);
+
+  await transferBridge.cancelTransfer(null, { transferId });
+  await new Promise((resolve) => setImmediate(resolve));
+  assert.equal(
+    isolatedEndWhileHeld,
+    0,
+    "cancel must not sftp.end() the isolated download channel while READs are held",
+  );
+  assert.equal(sshClientEndCalls, 0, "cancel must not end the shared SSH client");
+  assert.equal(browseEndCalls, 0, "cancel must not end the browse SFTP channel");
+
+  // Force-settle / abort may release the transfer before READs complete; still
+  // softDispose must not have ended mid-flight. Release held READs, then wait
+  // for deferred dispose (~2s) to end the abandoned channel.
+  holdReads = false;
+  for (const complete of pendingReads.splice(0)) complete();
+  const result = await running;
+  assert.equal(result.cancelled, true, result);
+  assert.equal(sshClientEndCalls, 0);
+  assert.equal(browseEndCalls, 0);
+  assert.equal(sender.sent.some((entry) => entry.channel === "netcatty:transfer:cancelled"), true);
+  assert.ok(
+    await waitUntil(() => isolatedEndCalls >= 1, 3500),
+    "softDisposed isolated download channel must deferred-end after quiet settle",
+  );
+  assert.equal(isolatedEndWhileHeld, 0);
 });
 
 test("small shared upload pause leaves the browse session usable", async (t) => {
@@ -6389,6 +6521,8 @@ test("fastPut fallback cancel settles while prior isolated write OPEN gate is un
   let firstOpenStarted = false;
   let firstIsolated = null;
   let fastPutCalls = 0;
+  let reopenedEnded = 0;
+  let reopenedEndAtCancel = 0;
   const sharedSftp = createFastSftp({
     lstat(remotePath, callback) {
       const key = String(remotePath);
@@ -6454,7 +6588,9 @@ test("fastPut fallback cancel settles while prior isolated write OPEN gate is un
             fastPutCalls += 1;
             cb(null);
           },
-          end() {},
+          end() {
+            reopenedEnded += 1;
+          },
         }));
       },
     },
@@ -6500,10 +6636,20 @@ test("fastPut fallback cancel settles while prior isolated write OPEN gate is un
 
   assert.match(result.error || "", /cancel/i);
   assert.equal(fastPutCalls, 0, "cancel must settle before fastPut runs on unresolved gate");
-  // #4134: cancel must settle without requiring sftp.end() on the reopened
-  // channel — ending mid-flight on the shared SSH transport can drop the
-  // whole session. Hang-freedom + cancelled event are the contract.
+  reopenedEndAtCancel = reopenedEnded;
+  // #4134: cancel must settle without requiring sftp.end() at the abort instant.
+  assert.equal(
+    reopenedEndAtCancel,
+    0,
+    "cancel during gate wait must not end the reopened channel mid-flight",
+  );
   assert.equal(sender.sent.some((entry) => entry.channel === "netcatty:transfer:cancelled"), true);
+  // Codex P2 deferred dispose: after the quiet settle window the disposable
+  // channel must still be ended so it is not permanently abandoned.
+  assert.ok(
+    await waitUntil(() => reopenedEnded >= 1, 3500),
+    "reopened isolated channel must deferred-end after cancel settle",
+  );
 });
 
 test("fastPut fallback fails closed when prior isolated write OPEN gate never settles", async (t) => {