fix(nrepl): stop the harness freezing when an eval errors

CHANGELOG.md

@@ -6,6 +6,21 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Fixed
+- The nREPL plugin no longer hangs the harness when an eval errors. A failed
+  eval used to be treated as a dead socket: `nrepl-eval` reconnected and
+  re-ran the *entire* eval on any error. For a timeout that doubled the wait
+  (a 3 s timeout took 6 s) and, when the server had gone away, turned the
+  retry into an unbounded clone handshake — a blocking socket read the Janet
+  interrupt cannot reach, which froze dirge until its own 30 s worker
+  deadline. Now only genuine transport failures (broken pipe / closed socket
+  / connection reset) trigger the reconnect-and-retry; a timeout drops the
+  connection instead (the timed-out eval keeps running server-side, so its
+  late reply would desync the next eval), and any other eval error just
+  propagates with the connection intact. The clone handshake is bounded by
+  `nrepl-connect-timeout` (10 s), so a server that accepts but never answers
+  fails fast instead of wedging.
+
 ## [0.26.0] - 2026-10-26
 
 ### Added

plugins/nrepl/00-state.janet

@@ -105,6 +105,7 @@
 (var nrepl-port nil)
 (var nrepl-connected false)
 (var nrepl-eval-timeout 120)  # per-eval timeout in seconds
+(var nrepl-connect-timeout 10)  # seconds to await the clone handshake
 (var nrepl-current-eval-id nil)  # active eval id for interrupt
 (var nrepl-rbuf @"")      # bytes read but not yet decoded (see below)
 
@@ -154,7 +155,16 @@
   # Fresh socket → drop any leftover bytes from a previous session.
   (set nrepl-rbuf @"")
   (nrepl-send-msg conn @{"op" "clone" "id" "dirge-clone"})
-  (def clone-resp (nrepl-read-msg conn))
+  # Bounded: an endpoint that accepts but never answers must fail, not
+  # wedge the (uninterruptible) C read. The bound is deliberately
+  # independent of `nrepl-eval-timeout` — that one may be minutes for a
+  # long computation, which would be an unacceptable connect hang.
+  (def clone-resp
+    (try
+      (nrepl-read-msg conn nrepl-connect-timeout)
+      ([err]
+        (try (:close conn) ([_] nil))
+        (error (string "nREPL clone handshake failed: " err)))))
   (def session (get clone-resp "new-session"))
   (set nrepl-conn conn)
   (set nrepl-session session)
@@ -203,6 +213,38 @@
     (set nrepl-rbuf @""))
   (connect-nrepl-inner nrepl-host nrepl-port))
 
+(defn nrepl-connection-error? [err]
+  "True only for errors that mean the SOCKET is gone and a reconnect +
+  retry can help. An eval error (including the per-eval timeout) is not
+  one: retrying it repeats work the user is already waiting on, and if
+  the server died mid-eval the retry's clone handshake blocks in a C
+  socket read that no interrupt can reach — freezing dirge until the
+  host's own deadline. So match on transport-failure text only."
+  (def s (string/ascii-lower (string err)))
+  (def needles
+    ["connection closed by server" "broken pipe" "connection reset"
+     "connection aborted" "eof" "not connected"])
+  (var hit false)
+  (each needle needles
+    (if (string/find needle s)
+      (set hit true)))
+  hit)
+
+(defn nrepl-timeout-error? [err]
+  "True when an eval failed on the per-eval timeout (either our own
+  \"timed out after Ns\" or the Janet read timeout). The connection has
+  to be dropped in that case: the eval keeps running server-side and its
+  late reply would desync the next eval. Matches on timeout wording only,
+  so an ordinary eval error (compile error, thrown exception) leaves the
+  connection intact."
+  (def s (string/ascii-lower (string err)))
+  (def needles ["timeout" "timed out"])
+  (var hit false)
+  (each needle needles
+    (if (string/find needle s)
+      (set hit true)))
+  hit)
+
 # ── paren repair ─────────────────────────────────────────────────
 #
 # LLMs frequently emit Clojure code with unbalanced delimiters.
@@ -337,11 +379,26 @@
   (var result
     (try (nrepl-eval-inner repaired)
          ([err]
-          # A broken-pipe / closed error means the socket went stale
-          # (e.g. the nREPL server was restarted). Reconnect once and
-          # retry; if the server is genuinely down, this re-raises.
-          (nrepl-reconnect)
-          (nrepl-eval-inner repaired))))
+          (cond
+            # A transport failure (broken pipe / closed socket) means the
+            # socket went stale (e.g. the nREPL server was restarted).
+            # Reconnect once and retry then.
+            (nrepl-connection-error? err)
+            (do
+              (nrepl-reconnect)
+              (nrepl-eval-inner repaired))
+            # A TIMEOUT is not retried: the slow eval would just run
+            # again (doubling the wait) and, if the server is gone, the
+            # retry's clone handshake wedges dirge. Worse, the timed-out
+            # eval is still running server-side, so its late reply would
+            # desync the next eval — drop the connection to start clean.
+            (nrepl-timeout-error? err)
+            (do
+              (nrepl-disconnect)
+              (error err))
+            # Any other eval error (compile error, thrown exception)
+            # leaves the connection perfectly usable — propagate it.
+            (error err)))))
   (def result-table @{:result (get result "result")
                        :out (get result "out")
                        :err (get result "err")

plugins/nrepl/README.md

@@ -33,6 +33,15 @@ attempt. Strings and line comments are skipped.
 `/nrepl-timeout`). An `interrupt` op is sent when the limit is exceeded.
 `/nrepl-interrupt` is available for manual cancellation.
 
+A timed-out eval is *not* retried. The eval keeps running server-side after
+the timeout, so its late reply would desync every later eval; instead the
+connection is dropped, and the next `nrepl_eval` reconnects cleanly. Only a
+genuine transport failure (broken pipe / closed socket) triggers the
+reconnect-and-retry path — an ordinary eval error (compile error, thrown
+exception) just propagates with the connection intact. The connect handshake
+is also bounded (`nrepl-connect-timeout`, 10 s) so a server that accepts the
+TCP connection but never answers fails fast instead of hanging the harness.
+
 ## Installation
 
 Copy into one of two auto-discovered directories:

src/plugin/mod_tests.rs

@@ -3943,3 +3943,76 @@ fn top_dyns_offset_is_stable() {
         "janet_dyn must read back what janet_setdyn wrote"
     );
 }
+
+/// An eval that times out must NOT be treated as a dead connection.
+/// `nrepl-eval`'s retry path reconnects and re-runs the whole eval on any
+/// error; for a *timeout* that just repeats the slow work (observed: a 3 s
+/// timeout took 6 s) and, when the server has gone away, turns a clean
+/// timeout into an uninterruptible handshake that freezes dirge. So only
+/// genuine transport failures may be retried.
+#[cfg(feature = "plugin")]
+#[test]
+fn nrepl_timeout_is_not_retried_as_a_connection_error() {
+    let mut mgr = nrepl_state_env();
+    for msg in [
+        "nREPL eval timed out after 3s",
+        "timeout",
+        "Runtime VM error: timeout",
+    ] {
+        assert_eq!(
+            mgr.eval(&format!("(nrepl-connection-error? \"{msg}\")"))
+                .unwrap(),
+            "false",
+            "{msg:?} must not be retried"
+        );
+    }
+    for msg in [
+        "nREPL connection closed by server",
+        "broken pipe",
+        "Connection reset by peer",
+    ] {
+        assert_eq!(
+            mgr.eval(&format!("(nrepl-connection-error? \"{msg}\")"))
+                .unwrap(),
+            "true",
+            "{msg:?} must be retried"
+        );
+    }
+}
+
+/// `nrepl-timeout-error?` gates the OTHER half of the timeout handling:
+/// after a timeout the eval is still running server-side and its eventual
+/// reply would desync every later eval, so `nrepl-eval` must drop the
+/// connection rather than reuse the session. Only timeout text may do that;
+/// an ordinary eval error (a compile error, a thrown exception) leaves the
+/// connection perfectly usable and must NOT be torn down.
+#[cfg(feature = "plugin")]
+#[test]
+fn nrepl_timeout_detection_is_precise() {
+    let mut mgr = nrepl_state_env();
+    for msg in [
+        "nREPL eval timed out after 3s",
+        "timeout",
+        "Runtime VM error: timeout",
+        "Execution error ... timeout",
+    ] {
+        assert_eq!(
+            mgr.eval(&format!("(nrepl-timeout-error? \"{msg}\")"))
+                .unwrap(),
+            "true",
+            "{msg:?} must be detected as a timeout"
+        );
+    }
+    for msg in [
+        "Execution error (ExceptionInfo) at user/eval (REPL:1). boom",
+        "Syntax error compiling at (REPL:1:1). Unable to resolve symbol",
+        "nREPL connection closed by server",
+    ] {
+        assert_eq!(
+            mgr.eval(&format!("(nrepl-timeout-error? \"{msg}\")"))
+                .unwrap(),
+            "false",
+            "{msg:?} is not a timeout and must not drop the connection"
+        );
+    }
+}