fix(ssh-manager): add sftp + dual-mode forwardOut passthroughs to fix ssh_upload, ssh_download, ssh_deploy, ssh_tunnel_create which all expect the ssh2 Client surface on the wrapper. Rename internal cached sftp handle to _sftpHandle so it doesnt shadow the new sftp() method. Expand regression test to cover sftp + both forwardOut styles (callback for tunnels, Promise for proxy jumps).

Author: hunchom

src/ssh-manager.js

@@ -12,7 +12,7 @@ class SSHManager {
     this.config = config;
     this.client = new Client();
     this.connected = false;
-    this.sftp = null;
+    this._sftpHandle = null; // cached SFTP subsystem; do not collide with sftp() passthrough
     this.cachedHomeDir = null;
     this.autoAcceptHostKey = config.autoAcceptHostKey || false;
     this.hostKeyVerification = config.hostKeyVerification !== false; // Default true
@@ -125,16 +125,22 @@ class SSHManager {
     });
   }
 
-  // Pass-through to ssh2 Client.exec so callers that hold an SSHManager can
-  // use streaming exec helpers (stream-exec.js, tail-tools.js) which expect
-  // a node-ssh2 Client interface.
+  // Pass-throughs to the underlying ssh2 Client. Modular tool handlers
+  // (transfer-tools, deploy-tools, stream-exec, tail-tools) expect the
+  // node-ssh2 Client surface (.exec, .sftp, .forwardOut), but
+  // getConnection() returns this SSHManager wrapper. Without these
+  // shims every exec/sftp call fails with "client.{exec,sftp} is not a function".
   exec(command, opts, cb) {
     if (typeof opts === 'function') { cb = opts; opts = undefined; }
     return opts !== undefined
       ? this.client.exec(command, opts, cb)
       : this.client.exec(command, cb);
   }
 
+  sftp(cb) {
+    return this.client.sftp(cb);
+  }
+
   async execCommand(command, options = {}) {
     if (!this.connected) {
       throw new Error('Not connected to SSH server');
@@ -284,15 +290,15 @@ class SSHManager {
   }
 
   async getSFTP() {
-    if (this.sftp) return this.sftp;
+    if (this._sftpHandle) return this._sftpHandle;
 
     return new Promise((resolve, reject) => {
       this.client.sftp((err, sftp) => {
         if (err) {
           reject(err);
           return;
         }
-        this.sftp = sftp;
+        this._sftpHandle = sftp;
         resolve(sftp);
       });
     });
@@ -458,19 +464,28 @@ class SSHManager {
   }
 
   dispose() {
-    if (this.sftp) {
-      this.sftp.end();
-      this.sftp = null;
+    if (this._sftpHandle) {
+      this._sftpHandle.end();
+      this._sftpHandle = null;
     }
     if (this.client) {
       this.client.end();
       this.connected = false;
     }
   }
 
-  async forwardOut(srcAddr, srcPort, dstAddr, dstPort) {
+  // Dual-mode: callback-style (matches ssh2 Client surface used by
+  // tunnel-tools.js) AND Promise-style (used by index.js for proxy jumps).
+  // Detect by presence of a 5th function arg.
+  forwardOut(srcAddr, srcPort, dstAddr, dstPort, cb) {
+    if (typeof cb === 'function') {
+      if (!this.connected) {
+        return cb(new Error('Not connected to SSH server'));
+      }
+      return this.client.forwardOut(srcAddr, srcPort, dstAddr, dstPort, cb);
+    }
     if (!this.connected) {
-      throw new Error('Not connected to SSH server');
+      return Promise.reject(new Error('Not connected to SSH server'));
     }
     return new Promise((resolve, reject) => {
       this.client.forwardOut(srcAddr, srcPort, dstAddr, dstPort, (err, stream) => {

tests/test-ssh-manager-exec-passthrough.js

@@ -1,11 +1,9 @@
 // Regression test: the modular tool handlers (exec, db, deploy, monitoring,
-// systemctl, journalctl, cat, tail, transfer, etc.) all pass the SSHManager
-// instance returned by getConnection() into stream-exec.js's streamExecCommand,
-// which then calls `client.exec(...)`. SSHManager wraps an ssh2 Client; if
-// .exec isn't exposed as a passthrough, every tool fails at runtime with
-// "client.exec is not a function" while every unit test still passes.
-//
-// This test asserts the passthrough exists and forwards arguments correctly.
+// systemctl, journalctl, cat, tail, transfer, tunnels, etc.) all expect the
+// node-ssh2 Client surface (.exec, .sftp, .forwardOut), but getConnection()
+// returns an SSHManager wrapper. Without passthroughs every call fails at
+// runtime with "client.{exec,sftp,forwardOut} is not a function" or hangs
+// silently — while every existing unit test passes (they mock SSH).
 
 import SSHManager from '../src/ssh-manager.js';
 
@@ -17,28 +15,62 @@ function assert(cond, msg) {
   else { console.log(`  [FAIL] ${msg}`); failed++; }
 }
 
-// 1. The method exists
 const mgr = new SSHManager({ host: 'x', user: 'x' });
+mgr.connected = true;
+
+// --- exec passthrough ---
 assert(typeof mgr.exec === 'function', 'SSHManager.exec is a function');
 
-// 2. It forwards (cmd, cb) to the underlying ssh2 client
 let captured = null;
 mgr.client = {
-  exec(cmd, optsOrCb, maybeCb) {
-    captured = { cmd, optsOrCb, maybeCb };
-  },
+  exec(cmd, optsOrCb, maybeCb) { captured = { cmd, optsOrCb, maybeCb }; },
+  sftp(cb) { captured = { sftpCb: cb }; },
+  forwardOut(srcA, srcP, dstA, dstP, cb) { captured = { srcA, srcP, dstA, dstP, cb }; },
 };
+
 mgr.exec('uname -a', () => {});
-assert(captured?.cmd === 'uname -a', 'forwards command string');
-assert(typeof captured?.optsOrCb === 'function', 'forwards callback as 2nd arg when no options given');
-assert(captured?.maybeCb === undefined, 'no 3rd arg when no options given');
+assert(captured?.cmd === 'uname -a', 'exec forwards command string');
+assert(typeof captured?.optsOrCb === 'function', 'exec forwards callback as 2nd arg without options');
+assert(captured?.maybeCb === undefined, 'exec no 3rd arg without options');
 
-// 3. It forwards (cmd, opts, cb) when options are provided
 captured = null;
 mgr.exec('echo hi', { pty: true }, () => {});
-assert(captured?.cmd === 'echo hi', 'forwards command with options');
-assert(captured?.optsOrCb?.pty === true, 'forwards options object');
-assert(typeof captured?.maybeCb === 'function', 'forwards callback as 3rd arg when options given');
+assert(captured?.cmd === 'echo hi', 'exec forwards command with options');
+assert(captured?.optsOrCb?.pty === true, 'exec forwards options object');
+assert(typeof captured?.maybeCb === 'function', 'exec forwards callback as 3rd arg with options');
+
+// --- sftp passthrough ---
+assert(typeof mgr.sftp === 'function', 'SSHManager.sftp is a function');
+captured = null;
+const sftpCb = (err, sftp) => {};
+mgr.sftp(sftpCb);
+assert(captured?.sftpCb === sftpCb, 'sftp forwards callback to underlying client');
+
+// --- forwardOut callback-style (used by tunnel-tools.js) ---
+captured = null;
+const fwdCb = () => {};
+mgr.forwardOut('127.0.0.1', 1234, 'remote.host', 22, fwdCb);
+assert(captured?.cb === fwdCb, 'forwardOut callback-style passes cb to underlying client');
+assert(captured?.srcA === '127.0.0.1' && captured?.dstP === 22, 'forwardOut callback-style passes src/dst args');
+
+// --- forwardOut Promise-style (used by index.js for proxy jumps) ---
+let resolvedStream = null;
+mgr.client.forwardOut = (srcA, srcP, dstA, dstP, cb) => cb(null, { tag: 'mockStream' });
+const promise = mgr.forwardOut('127.0.0.1', 0, 'jump.host', 22);
+assert(promise && typeof promise.then === 'function', 'forwardOut Promise-style returns a Promise');
+await promise.then(s => { resolvedStream = s; });
+assert(resolvedStream?.tag === 'mockStream', 'forwardOut Promise-style resolves with the stream');
+
+// --- error paths ---
+mgr.connected = false;
+const fwdRejection = mgr.forwardOut('127.0.0.1', 0, 'jump.host', 22);
+let rejected = false;
+await fwdRejection.catch(() => { rejected = true; });
+assert(rejected, 'forwardOut Promise-style rejects when not connected');
+
+let cbErr = null;
+mgr.forwardOut('127.0.0.1', 0, 'h', 22, e => { cbErr = e; });
+assert(cbErr instanceof Error, 'forwardOut callback-style invokes cb with error when not connected');
 
 console.log(`\n${passed} passed, ${failed} failed`);
 process.exit(failed === 0 ? 0 : 1);