Skip to content

Commit bb78252

Browse files
Fix ask_user starving the Rust SDK per-session event loop
Each session runs one `tokio::select!` loop in `rust/src/session.rs`. Tool-call, permission, and elicitation callbacks arrive as notifications and are `tokio::spawn`ed, so they run concurrently. But `ask_user` arrives as a JSON-RPC request (`userInput.request`) and was handled by `handle_request(...).await` inline in the select loop. While the user's answer was pending (host backstop timeout: 5 min) the loop was parked and could not drain the next notification, starving a sibling tool call co-emitted in the same turn (e.g. `set_session_title` + `ask_user` — github/copilot-experiences#12540) and freezing the UI. Dispatch the `userInput.request` arm the same way as the other interactive handlers: parse the params up front (keeping the inline INVALID_PARAMS error for a missing `question`) then spawn a child task that runs `UserInputHandler::handle` and sends the JSON-RPC response, mirroring the permission-request spawn pattern. The loop now keeps draining while `ask_user` is pending. Add an e2e regression test (`ask_user` category) where the model emits `set_marker` and `ask_user` in one turn; the user-input handler waits for the sibling tool to fire before answering and asserts it observed the tool while its own request was still pending. The test fails on the inline-await code and passes once the handler is spawned. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86146d70-7b81-497c-9d80-cd8dd8cf8a41
1 parent 3cbf4e7 commit bb78252

3 files changed

Lines changed: 223 additions & 31 deletions

File tree

rust/src/session.rs

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,37 +2263,57 @@ async fn handle_request(
22632263
.and_then(|p| p.get("allowFreeform"))
22642264
.and_then(|v| v.as_bool());
22652265

2266-
let handler_start = Instant::now();
2267-
let response = if let Some(user_input_handler) = handlers.user_input.as_ref() {
2268-
user_input_handler
2269-
.handle(sid.clone(), question, choices, allow_freeform)
2270-
.await
2271-
} else {
2272-
None
2273-
};
2274-
tracing::debug!(
2275-
elapsed_ms = handler_start.elapsed().as_millis(),
2266+
// Spawn the handler instead of awaiting it inline: `userInput.request`
2267+
// can stay pending for the full input backstop (minutes). Awaiting it
2268+
// inside the session's `tokio::select!` loop would park the loop and
2269+
// starve sibling tool-call/permission/elicitation notifications
2270+
// co-emitted in the same turn. Spawning mirrors how the
2271+
// notification-based interactive handlers are dispatched (see
2272+
// `handle_notification`) and keeps the loop draining concurrently.
2273+
let client = client.clone();
2274+
let user_input_handler = handlers.user_input.clone();
2275+
let request_id = request.id;
2276+
let span = tracing::error_span!(
2277+
"user_input_request_handler",
22762278
session_id = %sid,
2277-
"UserInputHandler::handle dispatch"
2279+
request_id = request_id
22782280
);
2281+
tokio::spawn(
2282+
async move {
2283+
let handler_start = Instant::now();
2284+
let response = if let Some(user_input_handler) = user_input_handler.as_ref() {
2285+
user_input_handler
2286+
.handle(sid.clone(), question, choices, allow_freeform)
2287+
.await
2288+
} else {
2289+
None
2290+
};
2291+
tracing::debug!(
2292+
elapsed_ms = handler_start.elapsed().as_millis(),
2293+
session_id = %sid,
2294+
"UserInputHandler::handle dispatch"
2295+
);
22792296

2280-
let rpc_result = match response {
2281-
Some(UserInputResponse {
2282-
answer,
2283-
was_freeform,
2284-
}) => serde_json::json!({
2285-
"answer": answer,
2286-
"wasFreeform": was_freeform,
2287-
}),
2288-
None => serde_json::json!({ "noResponse": true }),
2289-
};
2290-
let rpc_response = JsonRpcResponse {
2291-
jsonrpc: "2.0".to_string(),
2292-
id: request.id,
2293-
result: Some(rpc_result),
2294-
error: None,
2295-
};
2296-
let _ = client.send_response(&rpc_response).await;
2297+
let rpc_result = match response {
2298+
Some(UserInputResponse {
2299+
answer,
2300+
was_freeform,
2301+
}) => serde_json::json!({
2302+
"answer": answer,
2303+
"wasFreeform": was_freeform,
2304+
}),
2305+
None => serde_json::json!({ "noResponse": true }),
2306+
};
2307+
let rpc_response = JsonRpcResponse {
2308+
jsonrpc: "2.0".to_string(),
2309+
id: request_id,
2310+
result: Some(rpc_result),
2311+
error: None,
2312+
};
2313+
let _ = client.send_response(&rpc_response).await;
2314+
}
2315+
.instrument(span),
2316+
);
22972317
}
22982318

22992319
"exitPlanMode.request" => {

rust/tests/e2e/ask_user.rs

Lines changed: 145 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
use std::sync::Arc;
2+
use std::time::Duration;
23

34
use async_trait::async_trait;
45
use github_copilot_sdk::handler::{
5-
PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse,
6+
ApproveAllHandler, PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse,
67
};
7-
use github_copilot_sdk::{RequestId, SessionConfig, SessionId};
8-
use tokio::sync::mpsc;
8+
use github_copilot_sdk::tool::ToolHandler;
9+
use github_copilot_sdk::{
10+
Error, RequestId, SessionConfig, SessionId, Tool, ToolInvocation, ToolResult,
11+
};
12+
use serde_json::json;
13+
use tokio::sync::{Notify, mpsc};
914

1015
use super::support::{
1116
DEFAULT_TEST_TOKEN, assistant_message_content, recv_with_timeout, with_e2e_context,
@@ -147,6 +152,77 @@ async fn should_handle_freeform_user_input_response() {
147152
.await;
148153
}
149154

155+
/// Regression test for the per-session event-loop starvation bug where a pending
156+
/// `ask_user` (`userInput.request`) blocked the `tokio::select!` loop and starved
157+
/// a sibling tool call co-emitted in the same turn (github/copilot-experiences#12540).
158+
///
159+
/// The model emits both `set_marker` and `ask_user` in one assistant turn. The
160+
/// `set_marker` tool fires a `Notify`; the user-input handler waits on that
161+
/// `Notify` before answering. If `ask_user` were awaited inline, the loop could
162+
/// never dispatch the `set_marker` notification, so the handler would never
163+
/// observe the tool firing. With the handler spawned, both run concurrently and
164+
/// the handler observes the sibling tool while its own request is still pending.
165+
#[tokio::test]
166+
async fn ask_user_does_not_block_sibling_tool_call_in_same_turn() {
167+
with_e2e_context(
168+
"ask_user",
169+
"ask_user_does_not_block_sibling_tool_call_in_same_turn",
170+
|ctx| {
171+
Box::pin(async move {
172+
ctx.set_default_copilot_user();
173+
let client = ctx.start_client().await;
174+
175+
// Fired by `set_marker` when the sibling tool executes.
176+
let tool_fired = Arc::new(Notify::new());
177+
// Reports whether the user-input handler observed the sibling tool
178+
// firing while its own `ask_user` request was still pending.
179+
let (observed_tx, mut observed_rx) = mpsc::unbounded_channel();
180+
181+
let user_input_handler = Arc::new(SiblingAwareUserInputHandler {
182+
tool_fired: tool_fired.clone(),
183+
observed_tx,
184+
});
185+
let tools = vec![set_marker_tool(tool_fired.clone())];
186+
187+
let session = client
188+
.create_session(
189+
SessionConfig::default()
190+
.with_github_token(DEFAULT_TEST_TOKEN)
191+
.with_permission_handler(Arc::new(ApproveAllHandler))
192+
.with_user_input_handler(
193+
user_input_handler as Arc<dyn UserInputHandler>,
194+
)
195+
.with_tools(tools),
196+
)
197+
.await
198+
.expect("create session");
199+
200+
session
201+
.send_and_wait(
202+
"Call set_marker with value 'go' and, at the same time, use the ask_user \
203+
tool to ask me to choose between 'Option A' and 'Option B'. Wait for my \
204+
answer before continuing.",
205+
)
206+
.await
207+
.expect("send")
208+
.expect("assistant message");
209+
210+
let observed =
211+
recv_with_timeout(&mut observed_rx, "user input handler observation").await;
212+
assert!(
213+
observed,
214+
"ask_user handler must observe the sibling set_marker tool executing while \
215+
its own userInput.request is still pending (event loop must not be starved)"
216+
);
217+
218+
session.disconnect().await.expect("disconnect session");
219+
client.stop().await.expect("stop client");
220+
})
221+
},
222+
)
223+
.await;
224+
}
225+
150226
#[derive(Debug)]
151227
struct RecordedUserInputRequest {
152228
session_id: SessionId,
@@ -204,3 +280,69 @@ impl PermissionHandler for RecordingUserInputHandler {
204280
PermissionResult::approve_once()
205281
}
206282
}
283+
284+
/// A user-input handler that waits for a sibling tool to fire before answering,
285+
/// then reports whether it observed that tool while its own request was pending.
286+
struct SiblingAwareUserInputHandler {
287+
tool_fired: Arc<Notify>,
288+
observed_tx: mpsc::UnboundedSender<bool>,
289+
}
290+
291+
#[async_trait]
292+
impl UserInputHandler for SiblingAwareUserInputHandler {
293+
async fn handle(
294+
&self,
295+
_session_id: SessionId,
296+
_question: String,
297+
choices: Option<Vec<String>>,
298+
_allow_freeform: Option<bool>,
299+
) -> Option<UserInputResponse> {
300+
// Wait (bounded) for the sibling `set_marker` tool to execute. On the
301+
// buggy inline-await path the event loop is parked here, the tool
302+
// notification is never dispatched, and this times out.
303+
let observed = tokio::time::timeout(Duration::from_secs(30), self.tool_fired.notified())
304+
.await
305+
.is_ok();
306+
let _ = self.observed_tx.send(observed);
307+
308+
let answer = choices
309+
.as_ref()
310+
.and_then(|c| c.first())
311+
.cloned()
312+
.unwrap_or_else(|| "Option A".to_string());
313+
Some(UserInputResponse {
314+
answer,
315+
was_freeform: false,
316+
})
317+
}
318+
}
319+
320+
struct SetMarkerTool {
321+
tool_fired: Arc<Notify>,
322+
}
323+
324+
fn set_marker_tool(tool_fired: Arc<Notify>) -> Tool {
325+
Tool::new("set_marker")
326+
.with_description("Records a marker value")
327+
.with_parameters(json!({
328+
"type": "object",
329+
"properties": {
330+
"value": { "type": "string", "description": "Marker value" }
331+
},
332+
"required": ["value"]
333+
}))
334+
.with_handler(Arc::new(SetMarkerTool { tool_fired }))
335+
}
336+
337+
#[async_trait]
338+
impl ToolHandler for SetMarkerTool {
339+
async fn call(&self, invocation: ToolInvocation) -> Result<ToolResult, Error> {
340+
let value = invocation
341+
.arguments
342+
.get("value")
343+
.and_then(serde_json::Value::as_str)
344+
.unwrap_or_default();
345+
self.tool_fired.notify_one();
346+
Ok(ToolResult::Text(format!("MARKER_{}", value.to_uppercase())))
347+
}
348+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
models:
2+
- claude-sonnet-4.5
3+
conversations:
4+
- messages:
5+
- role: system
6+
content: ${system}
7+
- role: user
8+
content: Call set_marker with value 'go' and, at the same time, use the ask_user tool to ask me to choose between
9+
'Option A' and 'Option B'. Wait for my answer before continuing.
10+
- role: assistant
11+
tool_calls:
12+
- id: toolcall_0
13+
type: function
14+
function:
15+
name: set_marker
16+
arguments: '{"value":"go"}'
17+
- id: toolcall_1
18+
type: function
19+
function:
20+
name: ask_user
21+
arguments: '{"question":"Please choose between the following options:","choices":["Option A","Option B"]}'
22+
- role: tool
23+
tool_call_id: toolcall_0
24+
content: MARKER_GO
25+
- role: tool
26+
tool_call_id: toolcall_1
27+
content: "User selected: Option A"
28+
- role: assistant
29+
content: |-
30+
The marker is set (MARKER_GO) and you selected **Option A**.

0 commit comments

Comments
 (0)