fix: cancel interrupted endpoint actions without trapping dialogs

This commit is contained in:
Ogulcan Celik
2026-09-06 02:13:08 +03:00
parent 50c24c1d7d
commit 0876886bdc
9 changed files with 424 additions and 57 deletions
+1 -1
View File
@@ -50,7 +50,7 @@ pub(super) fn apply_profiles(
state.retire_endpoint_graphics(&endpoint_id);
if let Some(shell) = state.shell.as_mut() {
for request_id in cancelled {
shell.discard_endpoint_result(&request_id);
shell.cancel_endpoint_request(&request_id);
}
shell.retire_endpoint(&endpoint_id);
}
+34 -25
View File
@@ -91,35 +91,44 @@ impl EndpointCommands {
&mut self,
endpoint_id: &ClientEndpointId,
endpoints: &mut EndpointRegistry,
) -> io::Result<()> {
) -> Vec<String> {
let lane = self.lanes.entry(endpoint_id.clone()).or_default();
let mut cancelled = Vec::new();
if lane.in_flight.is_some() {
return Ok(());
return cancelled;
}
let Some(queued) = lane.queued.pop_front() else {
return Ok(());
};
if !endpoints.accepts(endpoint_id, queued.generation) {
return Ok(());
while let Some(queued) = lane.queued.pop_front() {
let request_id = queued.request.id.clone();
if !endpoints.accepts(endpoint_id, queued.generation) {
cancelled.push(request_id);
continue;
}
let request = match serde_json::to_string(&queued.request) {
Ok(request) => request,
Err(error) => {
tracing::warn!(%error, %request_id, "could not encode endpoint request");
cancelled.push(request_id);
continue;
}
};
let message = ClientMessage::ClientShellEndpointRequest {
boot_id: queued.boot_id.clone(),
request,
};
if endpoints.send_to(endpoint_id, &message) != EndpointSendOutcome::Sent {
cancelled.push(request_id);
continue;
}
lane.in_flight = Some(InFlightCommand {
generation: queued.generation,
boot_id: queued.boot_id,
request_id,
response: Vec::new(),
sent_at: Instant::now(),
});
break;
}
let request_id = queued.request.id.clone();
let request = serde_json::to_string(&queued.request)
.map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?;
let message = ClientMessage::ClientShellEndpointRequest {
boot_id: queued.boot_id.clone(),
request,
};
if endpoints.send_to(endpoint_id, &message) != EndpointSendOutcome::Sent {
return Ok(());
}
lane.in_flight = Some(InFlightCommand {
generation: queued.generation,
boot_id: queued.boot_id,
request_id,
response: Vec::new(),
sent_at: Instant::now(),
});
Ok(())
cancelled
}
pub(super) fn accepts_response(
+7 -3
View File
@@ -1663,8 +1663,10 @@ async fn run_client_loop(
completed.result,
)
} else {
shell.discard_endpoint_result(&completed.request_id);
(false, Vec::new())
(
shell.cancel_endpoint_request(&completed.request_id),
Vec::new(),
)
}
},
);
@@ -1675,13 +1677,15 @@ async fn run_client_loop(
&mut state,
&mut prefix_input_source,
);
let replay_mouse = dispatch_client_shell_actions(
let (replay_mouse, dispatch_repaint) = dispatch_client_shell_actions(
actions,
&mut endpoint_commands,
&mut write_stream,
state.shell.as_mut(),
&mut state.detached_process_children,
&event_tx,
)?;
let repaint = repaint || dispatch_repaint;
if replay_mouse.is_empty() {
if repaint {
if let Some(frame) = state.shell.as_mut().and_then(|shell| {
+29 -3
View File
@@ -507,8 +507,25 @@ impl ClientShellState {
outcome.actions
}
pub(crate) fn discard_endpoint_result(&mut self, request_id: &str) {
self.pending_requests.remove(request_id);
pub(crate) fn cancel_endpoint_request(&mut self, request_id: &str) -> bool {
let Some(pending) = self.pending_requests.get(request_id) else {
return false;
};
let boot_id = pending.boot_id.clone();
let (repaint, actions) = self.handle_endpoint_result(
&boot_id,
request_id,
Err(ClientShellEndpointError {
code: Some("endpoint_cancelled".into()),
message: "This server action was interrupted. Check its state before retrying."
.into(),
}),
);
debug_assert!(
actions.is_empty(),
"cancellation must not start another action"
);
repaint
}
pub(crate) fn handle_endpoint_result(
@@ -549,6 +566,12 @@ impl ClientShellState {
"Server timed out",
format!("This server did not respond to {}.", pending.method_name),
),
"endpoint_cancelled" => (
ClientEndpointNoticeKind::Unavailable,
"cancelled".to_owned(),
"Action interrupted",
error.message.clone(),
),
"server_unavailable" => (
ClientEndpointNoticeKind::Unavailable,
"server".to_owned(),
@@ -656,6 +679,9 @@ impl ClientShellState {
Some("endpoint returned an unexpected selection result".to_owned());
(true, fallback())
}
Err(error) if error.code.as_deref() == Some("endpoint_cancelled") => {
(true, Vec::new())
}
Err(_) => (true, fallback()),
};
}
@@ -767,7 +793,7 @@ impl ClientShellState {
Err(error)
if matches!(
error.code.as_deref(),
Some("stale_content" | "stale_target")
Some("stale_content" | "stale_target" | "endpoint_cancelled")
) =>
{
self.url_click_consumes_until_up = completed_before_release;
+4 -1
View File
@@ -122,7 +122,10 @@ impl ClientShellState {
pub(crate) fn mark_endpoint_disconnected(&mut self, endpoint_id: &ClientEndpointId) {
self.set_endpoint_status(endpoint_id, ClientEndpointStatus::Reconnecting);
if endpoint_id == &self.active_endpoint_id {
self.pending_requests.clear();
let pending = self.pending_requests.keys().cloned().collect::<Vec<_>>();
for request_id in pending {
self.cancel_endpoint_request(&request_id);
}
self.pending_integration_installs = 0;
self.pane_scroll_in_flight.clear();
self.pane_scroll_queued.clear();
+5 -1
View File
@@ -313,6 +313,9 @@ impl ClientShellState {
(true, Vec::new())
}
PendingEndpointKind::IntegrationInstall => {
let cancelled = result
.as_ref()
.is_err_and(|error| error.code.as_deref() == Some("endpoint_cancelled"));
self.pending_integration_installs =
self.pending_integration_installs.saturating_sub(1);
if let Some(ClientShellOverlay::Settings(settings)) = self.overlay.as_mut() {
@@ -328,7 +331,8 @@ impl ClientShellState {
}
settings.installing_integrations = self.pending_integration_installs > 0;
}
let actions = if self.pending_integration_installs == 0
let actions = if !cancelled
&& self.pending_integration_installs == 0
&& matches!(self.overlay, Some(ClientShellOverlay::Settings(_)))
{
let mut deferred = ClientShellInput::default();
+298
View File
@@ -0,0 +1,298 @@
use super::*;
use crate::client::endpoint::{ClientEndpointId, ClientEndpointStatus};
fn pending_popup() -> (ClientShellState, Vec<ClientShellAction>) {
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
let binding = crate::config::CustomCommandKeybind {
bindings: crate::config::ActionKeybinds::prefix("t"),
label: "prefix+t".into(),
command: "popup-command".into(),
action: crate::config::CustomCommandAction::Popup,
description: None,
width: None,
height: None,
};
let mut projection = snapshot();
projection
.commands
.push(crate::protocol::ClientShellCommand {
command_id: "cmd_popup".into(),
binding_label: binding.label.clone(),
binding_labels: binding.bindings.labels(),
action: crate::protocol::ClientShellCommandAction::Popup,
description: None,
});
state.set_snapshot(Box::new(projection));
state.set_pane_surface(surface());
let mut outcome = ClientShellInput::default();
state.record_binding(crate::input::KeybindMatch::Command(binding), &mut outcome);
assert!(state.popup_pending);
(state, outcome.actions)
}
fn pending_worktree() -> (ClientShellState, Vec<ClientShellAction>) {
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
state.set_snapshot(Box::new(snapshot()));
state.set_pane_surface(surface());
let mut outcome = ClientShellInput::default();
state.record_binding(
crate::input::KeybindMatch::Action(crate::input::KeybindAction::NewWorktree),
&mut outcome,
);
let [ClientShellAction::Endpoint { request, .. }] = &outcome.actions[..] else {
panic!("expected worktree preparation");
};
state.handle_endpoint_result("boot-1", &request.id, Ok(worktree_list_result(None)));
state.handle_input_bytes(b"feature/reconnect");
let outcome = state.handle_input_bytes(b"\r");
assert!(matches!(
&state.overlay,
Some(ClientShellOverlay::WorktreeCreate(create)) if create.creating
));
(state, outcome.actions)
}
fn request_id(actions: &[ClientShellAction]) -> &str {
let [ClientShellAction::Endpoint { request, .. }] = actions else {
panic!("expected one endpoint request");
};
&request.id
}
#[test]
fn cancelling_popup_request_unblocks_input_and_ignores_late_success() {
let (mut state, actions) = pending_popup();
let id = request_id(&actions);
assert!(state.cancel_endpoint_request(id));
assert!(!state.popup_pending);
assert!(state.pending_requests.is_empty());
assert!(state
.handle_endpoint_result("boot-1", id, Ok(crate::api::schema::ResponseResult::Ok {}))
.1
.is_empty());
assert!(!state.popup_pending);
assert!(!state.handle_input_bytes(b"x").requests.is_empty());
}
#[test]
fn disconnect_cancels_worktree_dialog_before_same_server_reconnect() {
let (mut state, _) = pending_worktree();
state.mark_endpoint_disconnected(&ClientEndpointId::Local);
assert!(state.pending_requests.is_empty());
assert!(matches!(
&state.overlay,
Some(ClientShellOverlay::WorktreeCreate(create)) if !create.creating
));
state.set_endpoint_status(&ClientEndpointId::Local, ClientEndpointStatus::Online);
state.set_endpoint_snapshot(&ClientEndpointId::Local, Box::new(snapshot()));
assert!(state.activate_endpoint_projection(&ClientEndpointId::Local));
state.set_pane_surface(surface());
state.handle_raw_events(vec![RawInputEvent::Key(crate::input::TerminalKey::new(
KeyCode::Esc,
KeyModifiers::NONE,
))]);
assert!(state.overlay.is_none());
assert!(!state.handle_input_bytes(b"x").requests.is_empty());
}
struct TestTransport {
fail: bool,
}
impl crate::client::endpoint::EndpointTransport for TestTransport {
fn send(&mut self, _: &ClientMessage) -> std::io::Result<()> {
if self.fail {
Err(std::io::ErrorKind::BrokenPipe.into())
} else {
Ok(())
}
}
}
#[test]
fn dispatcher_cancels_worktree_requests_on_frozen_surface_or_failed_send() {
use crate::client::endpoint::{EndpointNegotiation, EndpointRegistry};
use crate::client::endpoint_commands::EndpointCommands;
for fail_send in [false, true] {
let (mut state, actions) = pending_worktree();
let mut endpoints = EndpointRegistry::new(
TestTransport { fail: fail_send },
1,
EndpointNegotiation::default(),
);
endpoints.set_surface_active(&ClientEndpointId::Local, fail_send);
let mut commands = EndpointCommands::default();
let (tx, _rx) = tokio::sync::mpsc::channel(16);
let (replay, repaint) = crate::client::shell_runtime::dispatch_client_shell_actions(
actions,
&mut commands,
&mut endpoints,
Some(&mut state),
&mut Vec::new(),
&tx,
)
.unwrap();
assert!(repaint);
assert!(replay.is_empty());
assert!(state.pending_requests.is_empty());
assert!(matches!(
&state.overlay,
Some(ClientShellOverlay::WorktreeCreate(create)) if !create.creating
));
assert!(state
.visible_endpoint_notice
.as_ref()
.is_some_and(|notice| { notice.title == "Action interrupted" }));
assert!(commands.disconnect(&ClientEndpointId::Local).is_empty());
}
}
#[test]
fn stale_queued_request_is_cancelled_without_blocking_the_current_generation() {
use crate::client::endpoint::{EndpointNegotiation, EndpointRegistry};
use crate::client::endpoint_commands::EndpointCommands;
let (mut state, actions) = pending_popup();
let stale_id = request_id(&actions).to_owned();
let current = state.focus_endpoint_target(ClientEndpointFocusTarget::Workspace("ws_1".into()));
let current_id = request_id(&current).to_owned();
let mut commands = EndpointCommands::default();
for (generation, actions) in [(1, actions), (2, current)] {
for action in actions {
let ClientShellAction::Endpoint {
endpoint_id,
boot_id,
request,
} = action
else {
panic!("expected endpoint request");
};
commands.enqueue(endpoint_id, generation, boot_id, request);
}
}
let mut endpoints = EndpointRegistry::new(
TestTransport { fail: false },
2,
EndpointNegotiation::default(),
);
let cancelled = commands.send_next(&ClientEndpointId::Local, &mut endpoints);
assert_eq!(cancelled, vec![stale_id.clone()]);
state.cancel_endpoint_request(&stale_id);
assert!(!state.popup_pending);
assert!(!commands.accepts_response(&ClientEndpointId::Local, 1, "boot-1", &stale_id));
assert!(commands.accepts_response(&ClientEndpointId::Local, 2, "boot-1", &current_id));
assert!(state.pending_requests.contains_key(&current_id));
}
#[test]
fn cancelled_integration_install_does_not_queue_a_refresh() {
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
state.set_snapshot(Box::new(snapshot()));
state.open_settings_overlay();
let Some(ClientShellOverlay::Settings(settings)) = state.overlay.as_mut() else {
panic!("settings overlay");
};
settings.installing_integrations = true;
state.pending_integration_installs = 1;
let mut outcome = ClientShellInput::default();
assert!(state.push_endpoint_method_with_kind(
crate::api::schema::Method::IntegrationInstall(
crate::api::schema::IntegrationInstallParams {
target: crate::api::schema::IntegrationTarget::Pi,
}
),
PendingEndpointKind::IntegrationInstall,
&mut outcome,
));
assert!(state.cancel_endpoint_request(request_id(&outcome.actions)));
assert!(state.pending_requests.is_empty());
assert_eq!(state.pending_integration_installs, 0);
assert!(matches!(
&state.overlay,
Some(ClientShellOverlay::Settings(settings))
if !settings.installing_integrations && !settings.loading_integrations
));
}
#[test]
fn cancelled_selection_does_not_replay_its_fallback_key() {
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
state.set_snapshot(Box::new(snapshot()));
state.set_pane_surface(surface());
state.selection = Some(crate::selection::Selection::absolute_range(
"pane_1".into(),
(0, 0),
(0, 2),
));
let mut outcome = ClientShellInput::default();
state.request_selection_copy_with_fallback(
&mut outcome,
Some(crate::input::TerminalKey::new(
KeyCode::Char('c'),
KeyModifiers::CONTROL,
)),
);
let (_, actions) = state.handle_endpoint_result(
"boot-1",
request_id(&outcome.actions),
Err(ClientShellEndpointError {
code: Some("endpoint_cancelled".into()),
message: "cancelled".into(),
}),
);
assert!(actions.is_empty());
assert!(state.pending_requests.is_empty());
}
#[test]
fn cancelled_link_activation_does_not_replay_mouse_input() {
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
state.set_snapshot(Box::new(snapshot()));
state.set_pane_surface(surface());
state.compose(100, 28).unwrap();
let pane_id = state.hits.panes[0].pane_id.clone();
let inner_rect = state.hits.panes[0].inner_rect;
let mut outcome = ClientShellInput::default();
state.push_endpoint_method_with_kind(
crate::api::schema::Method::PaneLinkActivate(crate::api::schema::PaneLinkActivateParams {
pane_id: pane_id.clone(),
viewport_row: 0,
col: 0,
content_revision: None,
offset_from_bottom: None,
}),
PendingEndpointKind::PaneLinkActivate {
pane_id,
inner_rect,
fallback_events: vec![MouseEvent {
kind: crossterm::event::MouseEventKind::Down(crossterm::event::MouseButton::Left),
column: inner_rect.x,
row: inner_rect.y,
modifiers: KeyModifiers::NONE,
}],
},
&mut outcome,
);
let (_, actions) = state.handle_endpoint_result(
"boot-1",
request_id(&outcome.actions),
Err(ClientShellEndpointError {
code: Some("endpoint_cancelled".into()),
message: "cancelled".into(),
}),
);
assert!(actions.is_empty());
assert!(state.url_click_consumes_until_up);
}
#[test]
fn another_machine_disconnect_does_not_cancel_active_popup() {
let (mut state, _) = pending_popup();
let remote = ClientEndpointId::Ssh(
crate::client::endpoint::ProfileId::parse("0123456789abcdef0123456789abcdef").unwrap(),
);
state.mark_endpoint_disconnected(&remote);
assert!(state.popup_pending);
assert_eq!(state.pending_requests.len(), 1);
}
+1
View File
@@ -211,6 +211,7 @@ fn surface_with_popup() -> PaneSurfaceFrame {
mod agents_worktrees_notifications;
mod chrome_context;
mod copy;
mod endpoint_requests;
mod endpoints;
#[path = "input.rs"]
mod input_domain;
+45 -23
View File
@@ -4,10 +4,12 @@ pub(super) fn dispatch_client_shell_actions(
actions: Vec<shell::ClientShellAction>,
endpoint_commands: &mut endpoint_commands::EndpointCommands,
endpoints: &mut endpoint::EndpointRegistry,
mut shell: Option<&mut shell::ClientShellState>,
detached_process_children: &mut Vec<std::process::Child>,
event_tx: &tokio::sync::mpsc::Sender<ClientLoopEvent>,
) -> Result<Vec<crossterm::event::MouseEvent>, ClientError> {
) -> Result<(Vec<crossterm::event::MouseEvent>, bool), ClientError> {
let mut replay_mouse = Vec::new();
let mut repaint = false;
for action in actions {
match action {
shell::ClientShellAction::Endpoint {
@@ -15,15 +17,12 @@ pub(super) fn dispatch_client_shell_actions(
boot_id,
request,
} => {
if endpoints.active_id() == &endpoint_id && endpoints.active_surface_available() {
if let Some(connection) = endpoints.connection(&endpoint_id) {
endpoint_commands.enqueue(
endpoint_id,
connection.generation,
boot_id,
request,
);
}
if let Some(connection) = endpoints.connection(&endpoint_id).filter(|_| {
endpoints.active_id() == &endpoint_id && endpoints.active_surface_available()
}) {
endpoint_commands.enqueue(endpoint_id, connection.generation, boot_id, request);
} else if let Some(shell) = shell.as_deref_mut() {
repaint |= shell.cancel_endpoint_request(&request.id);
}
}
shell::ClientShellAction::ClipboardWrite(bytes) => {
@@ -67,11 +66,14 @@ pub(super) fn dispatch_client_shell_actions(
// must reject it; completion below resumes the committed owner's lane.
if endpoints.active_surface_available() {
let active_endpoint = endpoints.active_id().clone();
endpoint_commands
.send_next(&active_endpoint, endpoints)
.map_err(ClientError::ConnectionLost)?;
let cancelled = endpoint_commands.send_next(&active_endpoint, endpoints);
if let Some(shell) = shell {
for request_id in cancelled {
repaint |= shell.cancel_endpoint_request(&request_id);
}
}
}
Ok(replay_mouse)
Ok((replay_mouse, repaint))
}
pub(super) fn client_shell_resize_message(
@@ -164,7 +166,7 @@ fn install_pending_activation(
.unwrap_or_default();
if let Some(shell) = state.shell.as_mut() {
for request_id in retired {
shell.discard_endpoint_result(&request_id);
shell.cancel_endpoint_request(&request_id);
}
}
*next_surface_serial = next_surface_serial.saturating_add(1);
@@ -209,13 +211,19 @@ pub(super) fn begin_endpoint_activation(
if already_active {
if let (Some(shell), Some(target)) = (state.shell.as_mut(), target) {
let actions = shell.focus_endpoint_target(target);
dispatch_client_shell_actions(
let (_, repaint) = dispatch_client_shell_actions(
actions,
endpoint_commands,
endpoints,
Some(shell),
&mut state.detached_process_children,
event_tx,
)?;
if repaint {
if let Some(frame) = shell.compose(state.reported_size.0, state.reported_size.1) {
state.present_frame(frame);
}
}
}
return Ok(());
}
@@ -365,6 +373,15 @@ pub(super) fn complete_endpoint_activation(
| endpoint::ActivationCompletion::AwaitingPresentationEffects => unreachable!(),
};
state.unfreeze_presentation();
if successor.is_none() {
let active_endpoint = endpoints.active_id().clone();
let cancelled = endpoint_commands.send_next(&active_endpoint, endpoints);
if let Some(shell) = state.shell.as_mut() {
for request_id in cancelled {
shell.cancel_endpoint_request(&request_id);
}
}
}
let (cleanup, frame) = {
let shell = state.shell.as_mut().expect("checked client shell");
(
@@ -383,10 +400,6 @@ pub(super) fn complete_endpoint_activation(
force: true,
}));
}
let active_endpoint = endpoints.active_id().clone();
endpoint_commands
.send_next(&active_endpoint, endpoints)
.map_err(ClientError::ConnectionLost)?;
Ok(None)
}
@@ -462,7 +475,7 @@ pub(super) fn handle_endpoint_disconnect(
let cancelled = endpoint_commands.disconnect(endpoint_id);
let unavailable = state.shell.as_mut().and_then(|shell| {
for request_id in cancelled {
shell.discard_endpoint_result(&request_id);
shell.cancel_endpoint_request(&request_id);
}
shell.mark_endpoint_disconnected(endpoint_id);
endpoint_was_active.then(|| format!("{} {notice}", shell.endpoint_label(endpoint_id)))
@@ -520,7 +533,7 @@ pub(super) fn handle_endpoint_attention(
let cancelled = endpoint_commands.disconnect(endpoint_id);
let unavailable = state.shell.as_mut().and_then(|shell| {
for request_id in cancelled {
shell.discard_endpoint_result(&request_id);
shell.cancel_endpoint_request(&request_id);
}
shell.set_endpoint_status(endpoint_id, endpoint::ClientEndpointStatus::Attention);
endpoint_was_active.then(|| format!("{}: {message}", shell.endpoint_label(endpoint_id)))
@@ -644,13 +657,22 @@ pub(super) fn finish_client_shell_input(
query_host_terminal_theme();
}
sync_client_shell_keyboard_report_all(state)?;
let replay = dispatch_client_shell_actions(
let (replay, dispatch_repaint) = dispatch_client_shell_actions(
outcome.actions,
endpoint_commands,
endpoints,
state.shell.as_mut(),
&mut state.detached_process_children,
event_tx,
)?;
let frame = if dispatch_repaint {
state
.shell
.as_mut()
.and_then(|shell| shell.compose(state.reported_size.0, state.reported_size.1))
} else {
frame
};
debug_assert!(
replay.is_empty(),
"mouse replay only follows endpoint results"