Skip to content

Commit d2a0bb8

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`. Inbound JSON-RPC requests were dispatched by `handle_request(...).await` inline in the select loop, so a slow handler parked the reader and could not drain the next message. `ask_user` (`userInput.request`) awaits the user's answer (host backstop timeout: 5 min), so while it was pending the loop was frozen — starving a sibling tool call co-emitted in the same turn (e.g. `set_session_title` + `ask_user` — github/copilot-experiences#12540) and hanging the UI. Notifications didn't hit this because their slow interactive callbacks (permission/tool/elicitation) are already spawned. Move concurrency to the request-dispatch boundary, matching the other five SDKs: spawn each `handle_request` from the `requests.recv()` branch as its own task that awaits the handler and sends that request's response, instead of awaiting inline. This fixes starvation for every request handler — not just `userInput.request` but also `exitPlanMode`, `autoModeSwitch`, hooks, transforms, and canvas/session-FS providers. The Arc-backed dispatch context is cloned into the task so the future is `'static`; JSON-RPC permits concurrent requests and out-of-order responses, so nothing is serialized. Notifications remain inline (they only do fast dispatch work). The `userInput.request` arm itself is unchanged from `main`. 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 dispatch (~31s, starvation) and passes with the boundary spawn (~10s). The parallel dotnet/go/python/nodejs bindings and the bundled core already spawn per-request, but their `userInput.request` handlers are worth a follow-up audit for the same inline-await asymmetry (not changed here). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86146d70-7b81-497c-9d80-cd8dd8cf8a41
1 parent 3cbf4e7 commit d2a0bb8

3 files changed

Lines changed: 223 additions & 21 deletions

File tree

rust/src/session.rs

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,14 +1440,27 @@ fn spawn_event_loop(
14401440
loop {
14411441
// `mpsc::UnboundedReceiver::recv` and
14421442
// `CancellationToken::cancelled` are both cancel-safe per
1443-
// RFD 400. The selected branch's `await`'d handler is
1444-
// *not* mid-cancelled by the select — once a branch fires
1445-
// it runs to completion within the loop's iteration.
1446-
// Spawned child tasks inside `handle_notification`
1447-
// (permission/tool/elicitation callbacks) intentionally
1448-
// outlive the parent loop and own their own cleanup;
1449-
// this is RFD 400's "spawn background tasks to perform
1450-
// cancel-unsafe operations" pattern and is correct as-is.
1443+
// RFD 400.
1444+
//
1445+
// Inbound JSON-RPC *requests* are dispatched fire-and-forget:
1446+
// each `handle_request` runs in its own spawned task that
1447+
// awaits the handler and sends that request's response. This
1448+
// mirrors the other Copilot SDKs and moves concurrency to the
1449+
// request-dispatch boundary, so any slow handler — not just
1450+
// `userInput.request` (which can stay pending for the full
1451+
// input backstop of several minutes), but also `exitPlanMode`,
1452+
// `autoModeSwitch`, hooks, transforms, or canvas/session-FS
1453+
// providers — cannot park the reader loop and starve sibling
1454+
// requests or co-emitted notifications. JSON-RPC permits
1455+
// concurrent requests and out-of-order responses, so the SDK
1456+
// does not serialize them.
1457+
//
1458+
// `handle_notification` is awaited inline because it only
1459+
// performs fast dispatch work; its slow interactive callbacks
1460+
// (permission/tool/elicitation) are themselves spawned as child
1461+
// tasks. All of these spawned tasks intentionally outlive the
1462+
// parent loop and own their own cleanup — RFD 400's "spawn
1463+
// background tasks to perform cancel-unsafe operations" pattern.
14511464
tokio::select! {
14521465
_ = shutdown.cancelled() => break,
14531466
Some(notification) = notifications.recv() => {
@@ -1456,16 +1469,33 @@ fn spawn_event_loop(
14561469
).await;
14571470
}
14581471
Some(request) = requests.recv() => {
1459-
let ctx = RequestDispatchContext {
1460-
client: &client,
1461-
handlers: &handlers,
1462-
hooks: hooks.as_deref(),
1463-
transforms: transforms.as_deref(),
1464-
canvas_handler: canvas_handler.as_ref(),
1465-
session_fs_provider: session_fs_provider.as_ref(),
1466-
bearer_token_providers: &bearer_token_providers,
1467-
};
1468-
handle_request(&session_id, ctx, request).await;
1472+
// Clone the Arc-backed dispatch context into the task so
1473+
// the spawned `handle_request` future is `'static`. All
1474+
// clones are cheap (Arc refcount bumps / small maps).
1475+
let span = tracing::error_span!("session_request_handler", session_id = %session_id);
1476+
let session_id = session_id.clone();
1477+
let client = client.clone();
1478+
let handlers = handlers.clone();
1479+
let hooks = hooks.clone();
1480+
let transforms = transforms.clone();
1481+
let canvas_handler = canvas_handler.clone();
1482+
let session_fs_provider = session_fs_provider.clone();
1483+
let bearer_token_providers = bearer_token_providers.clone();
1484+
tokio::spawn(
1485+
async move {
1486+
let ctx = RequestDispatchContext {
1487+
client: &client,
1488+
handlers: &handlers,
1489+
hooks: hooks.as_deref(),
1490+
transforms: transforms.as_deref(),
1491+
canvas_handler: canvas_handler.as_ref(),
1492+
session_fs_provider: session_fs_provider.as_ref(),
1493+
bearer_token_providers: &bearer_token_providers,
1494+
};
1495+
handle_request(&session_id, ctx, request).await;
1496+
}
1497+
.instrument(span),
1498+
);
14691499
}
14701500
else => break,
14711501
}

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)