Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 48 additions & 18 deletions rust/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,14 +1440,27 @@ fn spawn_event_loop(
loop {
// `mpsc::UnboundedReceiver::recv` and
// `CancellationToken::cancelled` are both cancel-safe per
// RFD 400. The selected branch's `await`'d handler is
// *not* mid-cancelled by the select — once a branch fires
// it runs to completion within the loop's iteration.
// Spawned child tasks inside `handle_notification`
// (permission/tool/elicitation callbacks) intentionally
// outlive the parent loop and own their own cleanup;
// this is RFD 400's "spawn background tasks to perform
// cancel-unsafe operations" pattern and is correct as-is.
// RFD 400.
//
// Inbound JSON-RPC *requests* are dispatched fire-and-forget:
// each `handle_request` runs in its own spawned task that
// awaits the handler and sends that request's response. This
// mirrors the other Copilot SDKs and moves concurrency to the
// request-dispatch boundary, so any slow handler — not just
// `userInput.request` (which can stay pending for the full
// input backstop of several minutes), but also `exitPlanMode`,
// `autoModeSwitch`, hooks, transforms, or canvas/session-FS
// providers — cannot park the reader loop and starve sibling
// requests or co-emitted notifications. JSON-RPC permits
// concurrent requests and out-of-order responses, so the SDK
// does not serialize them.
//
// `handle_notification` is awaited inline because it only
// performs fast dispatch work; its slow interactive callbacks
// (permission/tool/elicitation) are themselves spawned as child
// tasks. All of these spawned tasks intentionally outlive the
// parent loop and own their own cleanup — RFD 400's "spawn
// background tasks to perform cancel-unsafe operations" pattern.
tokio::select! {
_ = shutdown.cancelled() => break,
Some(notification) = notifications.recv() => {
Expand All @@ -1456,16 +1469,33 @@ fn spawn_event_loop(
).await;
}
Some(request) = requests.recv() => {
let ctx = RequestDispatchContext {
client: &client,
handlers: &handlers,
hooks: hooks.as_deref(),
transforms: transforms.as_deref(),
canvas_handler: canvas_handler.as_ref(),
session_fs_provider: session_fs_provider.as_ref(),
bearer_token_providers: &bearer_token_providers,
};
handle_request(&session_id, ctx, request).await;
// Clone the Arc-backed dispatch context into the task so
// the spawned `handle_request` future is `'static`. All
// clones are cheap (Arc refcount bumps / small maps).
let span = tracing::error_span!("session_request_handler", session_id = %session_id);
let session_id = session_id.clone();
let client = client.clone();
let handlers = handlers.clone();
let hooks = hooks.clone();
let transforms = transforms.clone();
let canvas_handler = canvas_handler.clone();
let session_fs_provider = session_fs_provider.clone();
let bearer_token_providers = bearer_token_providers.clone();
tokio::spawn(
async move {
let ctx = RequestDispatchContext {
client: &client,
handlers: &handlers,
hooks: hooks.as_deref(),
transforms: transforms.as_deref(),
canvas_handler: canvas_handler.as_ref(),
session_fs_provider: session_fs_provider.as_ref(),
bearer_token_providers: &bearer_token_providers,
};
handle_request(&session_id, ctx, request).await;
}
.instrument(span),
);
}
else => break,
}
Expand Down
148 changes: 145 additions & 3 deletions rust/tests/e2e/ask_user.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use std::sync::Arc;
use std::time::Duration;

use async_trait::async_trait;
use github_copilot_sdk::handler::{
PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse,
ApproveAllHandler, PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse,
};
use github_copilot_sdk::{RequestId, SessionConfig, SessionId};
use tokio::sync::mpsc;
use github_copilot_sdk::tool::ToolHandler;
use github_copilot_sdk::{
Error, RequestId, SessionConfig, SessionId, Tool, ToolInvocation, ToolResult,
};
use serde_json::json;
use tokio::sync::{Notify, mpsc};

use super::support::{
DEFAULT_TEST_TOKEN, assistant_message_content, recv_with_timeout, with_e2e_context,
Expand Down Expand Up @@ -147,6 +152,77 @@ async fn should_handle_freeform_user_input_response() {
.await;
}

/// Regression test for the per-session event-loop starvation bug where a pending
/// `ask_user` (`userInput.request`) blocked the `tokio::select!` loop and starved
/// a sibling tool call co-emitted in the same turn (github/copilot-experiences#12540).
///
/// The model emits both `set_marker` and `ask_user` in one assistant turn. The
/// `set_marker` tool fires a `Notify`; the user-input handler waits on that
/// `Notify` before answering. If `ask_user` were awaited inline, the loop could
/// never dispatch the `set_marker` notification, so the handler would never
/// observe the tool firing. With the handler spawned, both run concurrently and
/// the handler observes the sibling tool while its own request is still pending.
#[tokio::test]
async fn ask_user_does_not_block_sibling_tool_call_in_same_turn() {
with_e2e_context(
"ask_user",
"ask_user_does_not_block_sibling_tool_call_in_same_turn",
|ctx| {
Box::pin(async move {
ctx.set_default_copilot_user();
let client = ctx.start_client().await;

// Fired by `set_marker` when the sibling tool executes.
let tool_fired = Arc::new(Notify::new());
// Reports whether the user-input handler observed the sibling tool
// firing while its own `ask_user` request was still pending.
let (observed_tx, mut observed_rx) = mpsc::unbounded_channel();

let user_input_handler = Arc::new(SiblingAwareUserInputHandler {
tool_fired: tool_fired.clone(),
observed_tx,
});
let tools = vec![set_marker_tool(tool_fired.clone())];

let session = client
.create_session(
SessionConfig::default()
.with_github_token(DEFAULT_TEST_TOKEN)
.with_permission_handler(Arc::new(ApproveAllHandler))
.with_user_input_handler(
user_input_handler as Arc<dyn UserInputHandler>,
)
.with_tools(tools),
)
.await
.expect("create session");

session
.send_and_wait(
"Call set_marker with value 'go' and, at the same time, use the ask_user \
tool to ask me to choose between 'Option A' and 'Option B'. Wait for my \
answer before continuing.",
)
.await
.expect("send")
.expect("assistant message");

let observed =
recv_with_timeout(&mut observed_rx, "user input handler observation").await;
assert!(
observed,
"ask_user handler must observe the sibling set_marker tool executing while \
its own userInput.request is still pending (event loop must not be starved)"
);

session.disconnect().await.expect("disconnect session");
client.stop().await.expect("stop client");
})
},
)
.await;
}

#[derive(Debug)]
struct RecordedUserInputRequest {
session_id: SessionId,
Expand Down Expand Up @@ -204,3 +280,69 @@ impl PermissionHandler for RecordingUserInputHandler {
PermissionResult::approve_once()
}
}

/// A user-input handler that waits for a sibling tool to fire before answering,
/// then reports whether it observed that tool while its own request was pending.
struct SiblingAwareUserInputHandler {
tool_fired: Arc<Notify>,
observed_tx: mpsc::UnboundedSender<bool>,
}

#[async_trait]
impl UserInputHandler for SiblingAwareUserInputHandler {
async fn handle(
&self,
_session_id: SessionId,
_question: String,
choices: Option<Vec<String>>,
_allow_freeform: Option<bool>,
) -> Option<UserInputResponse> {
// Wait (bounded) for the sibling `set_marker` tool to execute. On the
// buggy inline-await path the event loop is parked here, the tool
// notification is never dispatched, and this times out.
let observed = tokio::time::timeout(Duration::from_secs(30), self.tool_fired.notified())
.await
.is_ok();
let _ = self.observed_tx.send(observed);

let answer = choices
.as_ref()
.and_then(|c| c.first())
.cloned()
.unwrap_or_else(|| "Option A".to_string());
Some(UserInputResponse {
answer,
was_freeform: false,
})
}
}

struct SetMarkerTool {
tool_fired: Arc<Notify>,
}

fn set_marker_tool(tool_fired: Arc<Notify>) -> Tool {
Tool::new("set_marker")
.with_description("Records a marker value")
.with_parameters(json!({
"type": "object",
"properties": {
"value": { "type": "string", "description": "Marker value" }
},
"required": ["value"]
}))
.with_handler(Arc::new(SetMarkerTool { tool_fired }))
}

#[async_trait]
impl ToolHandler for SetMarkerTool {
async fn call(&self, invocation: ToolInvocation) -> Result<ToolResult, Error> {
let value = invocation
.arguments
.get("value")
.and_then(serde_json::Value::as_str)
.unwrap_or_default();
self.tool_fired.notify_one();
Ok(ToolResult::Text(format!("MARKER_{}", value.to_uppercase())))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
models:
- claude-sonnet-4.5
conversations:
- messages:
- role: system
content: ${system}
- role: user
content: Call set_marker with value 'go' and, at the same time, use the ask_user tool to ask me to choose between
'Option A' and 'Option B'. Wait for my answer before continuing.
- role: assistant
tool_calls:
- id: toolcall_0
type: function
function:
name: set_marker
arguments: '{"value":"go"}'
- id: toolcall_1
type: function
function:
name: ask_user
arguments: '{"question":"Please choose between the following options:","choices":["Option A","Option B"]}'
- role: tool
tool_call_id: toolcall_0
content: MARKER_GO
- role: tool
tool_call_id: toolcall_1
content: "User selected: Option A"
- role: assistant
content: |-
The marker is set (MARKER_GO) and you selected **Option A**.
Loading