fix(rust): respond when request handlers panic

rust/src/session.rs

@@ -1,8 +1,10 @@
 use std::collections::HashMap;
+use std::panic::AssertUnwindSafe;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
+use futures_util::FutureExt;
 use parking_lot::Mutex as ParkingLotMutex;
 use serde_json::Value;
 use tokio::sync::oneshot;
@@ -314,9 +316,8 @@ impl Session {
     ///
     /// Cooperative: signals shutdown via the session's [`CancellationToken`]
     /// and awaits the loop's natural exit rather than aborting the task.
-    /// Any in-flight handler (permission callback, tool call, elicitation
-    /// response) completes before the loop exits, so the CLI never sees a
-    /// half-handled request. See RFD-400 review finding #3.
+    /// Request handlers spawned by the loop are independent tasks and may
+    /// finish after this method returns, matching the other language SDKs.
     pub async fn stop_event_loop(&self) {
         self.shutdown.cancel();
         let handle = self.event_loop.lock().take();
@@ -633,12 +634,8 @@ impl Session {
 impl Drop for Session {
     fn drop(&mut self) {
         // Cooperative shutdown: cancel the event loop's token to signal
-        // exit between iterations. The loop will see the cancellation on
-        // its next select poll and break cleanly without interrupting an
-        // in-flight handler. We do NOT abort the JoinHandle — that would
-        // land at any await point in the loop body, potentially leaving
-        // the CLI with an unanswered request id. RFD-400 review finding
-        // #3.
+        // exit between iterations. Spawned request handlers own their cloned
+        // dispatch state and may finish after the parent loop exits.
         //
         // The handle itself is left in `event_loop` to be reaped by the
         // tokio runtime when it next polls; we intentionally don't await
@@ -1481,6 +1478,8 @@ fn spawn_event_loop(
                         let canvas_handler = canvas_handler.clone();
                         let session_fs_provider = session_fs_provider.clone();
                         let bearer_token_providers = bearer_token_providers.clone();
+                        let request_id = request.id;
+                        let request_method = request.method.clone();
                         tokio::spawn(
                             async move {
                                 let ctx = RequestDispatchContext {
@@ -1492,7 +1491,24 @@ fn spawn_event_loop(
                                     session_fs_provider: session_fs_provider.as_ref(),
                                     bearer_token_providers: &bearer_token_providers,
                                 };
-                                handle_request(&session_id, ctx, request).await;
+                                if AssertUnwindSafe(handle_request(&session_id, ctx, request))
+                                    .catch_unwind()
+                                    .await
+                                    .is_err()
+                                {
+                                    warn!(
+                                        request_id,
+                                        method = request_method,
+                                        "session request handler panicked"
+                                    );
+                                    let _ = send_error_response(
+                                        &client,
+                                        request_id,
+                                        error_codes::INTERNAL_ERROR,
+                                        "request handler panicked",
+                                    )
+                                    .await;
+                                }
                             }
                             .instrument(span),
                         );

rust/tests/session_test.rs

@@ -2671,36 +2671,41 @@ async fn send_and_wait_drop_clears_waiter() {
     assert_eq!(result.unwrap(), "msg-after-abort");
 }
 
-/// Cancel-safety regression: `Session::stop_event_loop` must NOT abort
-/// the event-loop task mid-handler. An in-flight handler (here a slow
-/// `userInput.request` callback) must run to completion before the loop
-/// exits — the CLI receives the response on the wire before the session
-/// tears down.
-///
-/// Closes RFD-400 review finding #3.
+/// Spawned request handlers own enough state to finish even after the
+/// parent session event loop stops.
 #[tokio::test]
-async fn stop_event_loop_completes_in_flight_handler() {
-    struct SlowHandler;
+async fn spawned_handler_can_finish_after_stop_event_loop() {
+    struct ControlledHandler {
+        started: Arc<tokio::sync::Notify>,
+        release: Arc<tokio::sync::Notify>,
+    }
+
     #[async_trait]
-    impl UserInputHandler for SlowHandler {
+    impl UserInputHandler for ControlledHandler {
         async fn handle(
             &self,
             _session_id: SessionId,
             _question: String,
             _choices: Option<Vec<String>>,
             _allow_freeform: Option<bool>,
         ) -> Option<UserInputResponse> {
-            tokio::time::sleep(Duration::from_millis(150)).await;
+            self.started.notify_one();
+            self.release.notified().await;
             Some(UserInputResponse {
                 answer: "completed".to_string(),
                 was_freeform: false,
             })
         }
     }
 
+    let started = Arc::new(tokio::sync::Notify::new());
+    let release = Arc::new(tokio::sync::Notify::new());
+    let handler = Arc::new(ControlledHandler {
+        started: started.clone(),
+        release: release.clone(),
+    });
     let (session, mut server) =
-        create_session_pair_with_config(|cfg| cfg.with_user_input_handler(Arc::new(SlowHandler)))
-            .await;
+        create_session_pair_with_config(move |cfg| cfg.with_user_input_handler(handler)).await;
     let session = Arc::new(session);
 
     server
@@ -2716,40 +2721,27 @@ async fn stop_event_loop_completes_in_flight_handler() {
         )
         .await;
 
-    // Give the loop a moment to dispatch into the handler.
-    tokio::time::sleep(Duration::from_millis(20)).await;
+    timeout(TIMEOUT, started.notified()).await.unwrap();
 
-    // Now request shutdown. The loop is parked in handle_request awaiting
-    // the slow handler. `notify_one()` buffers the signal until the loop
-    // re-enters its select, which can only happen after the handler
-    // returns and the response is sent on the wire.
     let stop_handle = tokio::spawn({
         let session = session.clone();
         async move { session.stop_event_loop().await }
     });
 
-    // Verify the handler's response lands on the wire BEFORE the loop
-    // exits — i.e. stop_event_loop did not abort mid-handler.
+    // The parent loop does not wait for independently spawned handlers.
+    timeout(TIMEOUT, stop_handle).await.unwrap().unwrap();
+
+    // The handler still owns its dispatch context and can answer afterward.
+    release.notify_one();
     let response = timeout(Duration::from_secs(2), server.read_response())
         .await
         .unwrap();
     assert_eq!(response["id"], 900);
     assert_eq!(response["result"]["answer"], "completed");
-
-    // stop_event_loop completes after the handler returns and the loop
-    // observes the buffered shutdown signal on its next select iteration.
-    timeout(Duration::from_secs(2), stop_handle)
-        .await
-        .unwrap()
-        .unwrap();
 }
 
-/// Cancel-safety regression: dropping a Session does NOT abort the event
-/// loop mid-handler. The loop sees the buffered shutdown signal on its
-/// next select iteration and exits cleanly. This is the Drop equivalent
-/// of stop_event_loop_completes_in_flight_handler; closes RFD-400 review
-/// finding #3 for the implicit-drop path that used to call
-/// `JoinHandle::abort()`.
+/// Dropping a Session does not abort an independently spawned request
+/// handler; the handler may finish after the parent loop exits.
 #[tokio::test]
 async fn drop_session_does_not_abort_handler() {
     use std::sync::atomic::{AtomicBool, Ordering};
@@ -2810,6 +2802,47 @@ async fn drop_session_does_not_abort_handler() {
     );
 }
 
+#[tokio::test]
+async fn panicking_request_handler_returns_internal_error() {
+    struct PanickingHandler;
+
+    #[async_trait]
+    impl UserInputHandler for PanickingHandler {
+        async fn handle(
+            &self,
+            _session_id: SessionId,
+            _question: String,
+            _choices: Option<Vec<String>>,
+            _allow_freeform: Option<bool>,
+        ) -> Option<UserInputResponse> {
+            panic!("handler exploded");
+        }
+    }
+
+    let (_session, mut server) = create_session_pair_with_config(|cfg| {
+        cfg.with_user_input_handler(Arc::new(PanickingHandler))
+    })
+    .await;
+
+    server
+        .send_request(
+            902,
+            "userInput.request",
+            serde_json::json!({
+                "sessionId": server.session_id,
+                "question": "panic-test",
+                "choices": null,
+                "allowFreeform": true,
+            }),
+        )
+        .await;
+
+    let response = timeout(TIMEOUT, server.read_response()).await.unwrap();
+    assert_eq!(response["id"], 902);
+    assert_eq!(response["error"]["code"], -32603);
+    assert_eq!(response["error"]["message"], "request handler panicked");
+}
+
 /// `Session::cancellation_token()` returns a child token that fires when
 /// the session shuts down. Lets external tasks bind their lifetime to the
 /// session via `tokio::select!` without taking a strong reference to the