cider-jack-in throws `Wrong type argument: stringp, nil` when invoked from TRAMP ssh-buffer

[vroide]

Expected behavior

In a tramp buffer connected to my rasperry pi via ssh, cider-jack-in creates a remote repl and connects to it.

Actual behaviour

cider-jack-in creates a remote repl and throws on connect:
"error in process filter: Wrong type argument: stringp, nil"

Steps

find-file /ssh:****@pi.local:~/hello/deps.edn [containing only "{}"]
M-x cider-jack-in

[nREPL] Starting server via clojure -Sdeps \{\:deps\ \{nrepl/nrepl\ \{\:mvn/version\ \"1.0.0\"\}\ cider/cider-nrepl\ \{\:mvn/version\ \"0.40.0\"\}\}\ \:aliases\ \{\:cider/nrepl\ \{\:main-opts\ \[\"-m\"\ \"nrepl.cmdline\"\ \"--middleware\"\ \"\[cider.nrepl/cider-middleware\]\"\]\}\}\} -M:cider/nrepl
[nREPL] server started on 40699
...
[nREPL] Falling back to SSH tunneled connection ...
[nREPL] Establishing SSH tunneled connection to pi.local:40699 ...
error in process filter: Wrong type argument: stringp, nil

Environment & Version Information

GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37, cairo version 1.16.0)
CIDER 1.8.3-snapshot, nREPL 1.0.0
Clojure 1.11.1, Java 17.0.8

Likely Reason & Attempted Solutions

The problem is that nrepl--ssh-tunnel-connect tries to string-match on (buffer-file-name) - which is nil, because the current execution context is the nrepl buffer:

;; (current-buffer) in edebug
#<buffer *nrepl-server hello:localhost*<13>>

Is this expected / did something change since this feature was introduced (#3264)?

To confirm, I changed this in nrepl--ssh-tunnel-connect:

(defun nrepl--ssh-tunnel-connect (host port)
  "Connect to a remote machine identified by HOST and PORT through SSH tunnel."
  (message "[nREPL] Establishing SSH tunneled connection to %s:%s ..." host port)
- (let* ((current-buf (buffer-file-name))
+ (let* ((current-buf (buffer-file-name (window-buffer (selected-window))))
         (tramp-file-regexp "/ssh:\\(.+@\\)?\\(.+?\\)\\(:\\|#\\).+")
         (remote-dir (cond
                      ;; If current buffer is a TRAMP buffer and its host is
                      ;; the same as HOST, reuse its connection parameters for
                      ;; SSH tunnel.
                      ((and (string-match tramp-file-regexp current-buf)
                            (string= host (match-string 2 current-buf))) current-buf)
                            ...

... which works, but only if one doesn't switch windows while cider is booting up.

A less flaky approach would be to pass down the buffer from which cider-jack-in was invoked.
This is what I'm doing now, but it's a lot of change in a lot of places and feels wrong:
master...adrech:cider:fix-ssh-jack-in

Originally posted by @adrech in #3303 (comment)

[vemv]

Thanks much for the accurate report and analysis.

We may use your branch, perhaps simplifying it by using a global variable or dynamic scope?

(But passing args is also OK, given that we intend to support concurrent repls)

I'll look into it.

[vroide]

I thought about using a global at first, but wasn't sure how to handle situations were multiple connections are started in succession and might not finish in order.
This is my first deeper look into cider and my emacs-fu is quite basic... I'm intrigued how this would be solved with dynamic scope? Haven't made use of it yet.

Edit: Best case scenario would be not needing any of this. From #3264 I'd assume that it worked before and the bug is coming from something else that changed the current-buffer to the nrepl one in that context.

[vemv]

Checking out the issue, your branch, etc carefully, I'd say that your branch is the best approach as-is.

A PR that was verified to work would be most welcome.

Please make sure that the linting is passing in advance (use our Makefile for that).

Thanks - V

[vroide]

Alright, will do. I'll look into how to write proper tests for this tomorrow.
Are there contributor guidelines that I missed somehow [apart from the linting], or should I just follow the conventions I find in the source?

[vemv]

We don't have a lot of integration tests so adding the changes would suffice.

Simply indicate how to QA this, preferrably using https://github.com/clojure-emacs/cider/tree/master/dev/tramp-sample-project which has a readme / makefile.