Fix duplicate user_input dispatch (github-app#4249)

The CLI sends two messages for every user-input prompt:
1. A `user_input.requested` session-event notification, intended
   for observers (UI streaming, telemetry, logging).
2. A `userInput.request` JSON-RPC method call, which is the
   actual ask-and-wait prompt that drives the handler.

The Rust SDK's session event loop was wiring up `HandlerEvent::UserInput`
dispatch on BOTH paths. Result: the consumer's `on_event` ran twice for
every prompt, which surfaced as duplicate `ask_user` / `exit_plan`
widgets in github-app's session UI (#4249).

Other SDKs (Node, Python, Go, .NET) only register the JSON-RPC
handler; the notification is purely observational. Aligning with
that convention.

Changes:

1. `session.rs`: drop the `SessionEventType::UserInputRequested`
   auto-dispatch arm. The CLI's follow-up `userInput.request`
   JSON-RPC call still drives the handler. Replaced with a
   short comment block explaining why the notification is
   intentionally a no-op (and citing #4249 so the next person
   tempted to add it back understands the trade).
2. `tests/session_test.rs`: replaced the
   `user_input_requested_event_dispatches_to_handler_and_responds`
   test (which asserted the broken behavior) with a regression
   test (`user_input_requested_notification_does_not_double_dispatch`)
   that:
     - sends a `user_input.requested` notification and asserts no
       wire response, no handler invocation;
     - then sends a `userInput.request` JSON-RPC call and asserts
       the handler fires exactly once.
   This locks the fix in place and makes a future re-introduction
   surface immediately as a test failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: tclem

rust/CHANGELOG.md

@@ -416,6 +416,14 @@ public surface.
   fail fast as before.
 
 ### Fixed
+- `Session::user_input` no longer double-dispatches when the CLI sends
+  both a `user_input.requested` notification (for observers) and a
+  `userInput.request` JSON-RPC call (the actual prompt) for the same
+  prompt. The notification path is now a no-op; the JSON-RPC path
+  remains authoritative. Matches Python / Go / .NET / Node SDK
+  behavior, all of which only register the JSON-RPC handler. Fixes
+  github/github-app#4249, where consumers saw duplicate `ask_user`
+  and `exit_plan` widgets on every prompt.
 - `SessionUi::elicitation` (and the `confirm` / `select` / `input`
   convenience helpers that delegate through it) now sends the user-supplied
   JSON Schema as `requestedSchema` on the wire, matching the

rust/src/session.rs

@@ -17,7 +17,7 @@ use crate::generated::api_types::{
 };
 use crate::generated::session_events::{
     CommandExecuteData, ElicitationRequestedData, ExternalToolRequestedData, SessionErrorData,
-    SessionEventType, UserInputRequestedData,
+    SessionEventType,
 };
 use crate::handler::{
     AutoModeSwitchResponse, ExitPlanModeResult, HandlerEvent, HandlerResponse, PermissionResult,
@@ -1306,47 +1306,12 @@ async fn handle_notification(
             });
         }
         SessionEventType::UserInputRequested => {
-            let user_input_data: UserInputRequestedData =
-                match serde_json::from_value(notification.event.data.clone()) {
-                    Ok(d) => d,
-                    Err(e) => {
-                        warn!(error = %e, "failed to deserialize user_input.requested");
-                        return;
-                    }
-                };
-            let client = client.clone();
-            let handler = handler.clone();
-            let sid = session_id.clone();
-            tokio::spawn(async move {
-                let response = handler
-                    .on_event(HandlerEvent::UserInput {
-                        session_id: sid.clone(),
-                        question: user_input_data.question,
-                        choices: (!user_input_data.choices.is_empty())
-                            .then_some(user_input_data.choices),
-                        allow_freeform: user_input_data.allow_freeform,
-                    })
-                    .await;
-                let result = match response {
-                    HandlerResponse::UserInput(Some(UserInputResponse {
-                        answer,
-                        was_freeform,
-                    })) => serde_json::json!({
-                        "sessionId": sid,
-                        "requestId": user_input_data.request_id,
-                        "answer": answer,
-                        "wasFreeform": was_freeform,
-                    }),
-                    _ => serde_json::json!({
-                        "sessionId": sid,
-                        "requestId": user_input_data.request_id,
-                        "noResponse": true,
-                    }),
-                };
-                let _ = client
-                    .call("session.respondToUserInput", Some(result))
-                    .await;
-            });
+            // Notification-only signal for observers (UI, telemetry).
+            // The CLI follows up with a `userInput.request` JSON-RPC call
+            // that drives `HandlerEvent::UserInput` dispatch — handling
+            // the notification here too would double-fire the handler
+            // and produce duplicate prompts on the consumer side. See
+            // github/github-app#4249.
         }
         SessionEventType::ElicitationRequested => {
             let Some(request_id) = extract_request_id(&notification.event.data) else {

rust/tests/session_test.rs

@@ -1386,32 +1386,38 @@ async fn user_input_request_dispatches_to_handler() {
 }
 
 #[tokio::test]
-async fn user_input_requested_event_dispatches_to_handler_and_responds() {
-    struct InputHandler;
+async fn user_input_requested_notification_does_not_double_dispatch() {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+    // Regression for github/github-app#4249. The CLI sends BOTH a
+    // `user_input.requested` notification (for observers) AND a
+    // `userInput.request` JSON-RPC call (the actual prompt) for every
+    // user-input prompt. Only the JSON-RPC path should reach the
+    // handler — dispatching from the notification too produced
+    // duplicate ask_user widgets on the consumer side.
+
+    struct CountingHandler {
+        invocations: Arc<AtomicUsize>,
+    }
     #[async_trait]
-    impl SessionHandler for InputHandler {
+    impl SessionHandler for CountingHandler {
         async fn on_event(&self, event: HandlerEvent) -> HandlerResponse {
-            match event {
-                HandlerEvent::UserInput {
-                    question,
-                    choices,
-                    allow_freeform,
-                    ..
-                } => {
-                    assert_eq!(question, "Allow shell access?");
-                    assert_eq!(choices, Some(vec!["Yes".to_string(), "No".to_string()]));
-                    assert_eq!(allow_freeform, Some(false));
-                    HandlerResponse::UserInput(Some(UserInputResponse {
-                        answer: "Yes".to_string(),
-                        was_freeform: false,
-                    }))
-                }
-                _ => HandlerResponse::Ok,
+            if let HandlerEvent::UserInput { .. } = event {
+                self.invocations.fetch_add(1, Ordering::SeqCst);
+                return HandlerResponse::UserInput(Some(UserInputResponse {
+                    answer: "ok".to_string(),
+                    was_freeform: true,
+                }));
             }
+            HandlerResponse::Ok
         }
     }
 
-    let (_session, mut server) = create_session_pair(Arc::new(InputHandler)).await;
+    let invocations = Arc::new(AtomicUsize::new(0));
+    let handler = Arc::new(CountingHandler {
+        invocations: invocations.clone(),
+    });
+    let (_session, mut server) = create_session_pair(handler).await;
+
     server
         .send_event(
             "user_input.requested",
@@ -1424,12 +1430,35 @@ async fn user_input_requested_event_dispatches_to_handler_and_responds() {
         )
         .await;
 
-    let request = timeout(TIMEOUT, server.read_request()).await.unwrap();
-    assert_eq!(request["method"], "session.respondToUserInput");
-    assert_eq!(request["params"]["sessionId"], server.session_id);
-    assert_eq!(request["params"]["requestId"], "ui-1");
-    assert_eq!(request["params"]["answer"], "Yes");
-    assert_eq!(request["params"]["wasFreeform"], false);
+    // Give the SDK a beat to (incorrectly) auto-dispatch if the
+    // regression returned. Nothing should arrive on the wire.
+    let respond_observed = timeout(Duration::from_millis(150), server.read_request()).await;
+    assert!(
+        respond_observed.is_err(),
+        "notification triggered unexpected wire activity: {respond_observed:?}",
+    );
+    assert_eq!(
+        invocations.load(Ordering::SeqCst),
+        0,
+        "notification path must not invoke the user-input handler",
+    );
+
+    // Now drive the JSON-RPC path and confirm the handler still runs once.
+    server
+        .send_request(
+            301,
+            "userInput.request",
+            serde_json::json!({
+                "sessionId": server.session_id,
+                "question": "Pick a color",
+                "allowFreeform": true,
+            }),
+        )
+        .await;
+    let response = timeout(TIMEOUT, server.read_response()).await.unwrap();
+    assert_eq!(response["id"], 301);
+    assert_eq!(response["result"]["answer"], "ok");
+    assert_eq!(invocations.load(Ordering::SeqCst), 1);
 }
 
 #[tokio::test]