From 0b877311eac9a6e02e9b3b247ca578a8c47e0586 Mon Sep 17 00:00:00 2001 From: Ivan Zatevakhin Date: Sun, 9 Aug 2026 17:10:35 +0100 Subject: [PATCH 1/2] refactor: introduce internal command seam --- src/acp_state.rs | 104 +++++++++----- src/app.rs | 47 ++++--- src/command.rs | 102 ++++++++++++++ src/handlers.rs | 28 ++-- src/input.rs | 6 +- src/lib.rs | 1 + src/mesh.rs | 34 ++--- src/protocol.rs | 285 +++++++++++++++++++++++++++++++++++--- src/runtime/event_loop.rs | 9 +- src/runtime/mod.rs | 5 +- 10 files changed, 508 insertions(+), 113 deletions(-) create mode 100644 src/command.rs diff --git a/src/acp_state.rs b/src/acp_state.rs index d1852d7..90db134 100644 --- a/src/acp_state.rs +++ b/src/acp_state.rs @@ -6,6 +6,7 @@ use crate::acp_client::{ DelegateModelOverrideInfo, DelegationUpdateNotification, DelegationUpdateState, }; use crate::app::{LogLevel, POPUP_SESSION_PAGE_TARGET, Popup, Screen}; +use crate::command::{Command, SessionListRequest}; use crate::domain::activity::{ ActivityState, DelegateChildState, DelegateEntry, DelegateStats, DelegateStatus, }; @@ -20,8 +21,8 @@ use crate::domain::session::{ }; use crate::domain::tool::ToolDetail; use crate::protocol::{ - ClientMsg, MeshInviteCreatedInfo, MeshNodesInfo, MeshStatusInfo, RemoteSessionAttachInfo, - RemoteSessionListInfo, SessionListRequest, + MeshInviteCreatedInfo, MeshNodesInfo, MeshStatusInfo, RemoteSessionAttachInfo, + RemoteSessionListInfo, }; use crate::tool_detail; @@ -182,7 +183,7 @@ pub(crate) enum AcpAppEvent { } impl crate::app::App { - pub(crate) fn handle_acp_event(&mut self, event: AcpAppEvent) -> Vec { + pub(crate) fn handle_acp_event(&mut self, event: AcpAppEvent) -> Vec { match event { AcpAppEvent::Initialized { agent_id, @@ -342,7 +343,7 @@ impl crate::app::App { is_replay, } => { self.apply_acp_session_update(&session_id, update, is_replay); - std::mem::take(&mut self.pending_commands) + self.drain_pending_commands() } AcpAppEvent::SessionReplay { session_id, @@ -357,7 +358,7 @@ impl crate::app::App { for update in updates { self.apply_acp_session_update(&session_id, update, true); } - std::mem::take(&mut self.pending_commands) + self.drain_pending_commands() } AcpAppEvent::UndoStack(undo_stack) => { self.undo_state = self.build_undo_state_from_server_stack(&undo_stack, None, None); @@ -385,7 +386,7 @@ impl crate::app::App { self.streaming_cache.invalidate(); self.set_status(LogLevel::Info, "session", "undone - reloading session"); if let Some(ref sid) = self.session_id { - return vec![ClientMsg::LoadSession { + return vec![Command::LoadSession { session_id: sid.clone(), cwd: self.current_session_cwd(), }]; @@ -420,7 +421,7 @@ impl crate::app::App { self.build_undo_state_from_server_stack(&stack, None, None); self.set_status(LogLevel::Info, "session", "redone - reloading session"); if let Some(ref sid) = self.session_id { - return vec![ClientMsg::LoadSession { + return vec![Command::LoadSession { session_id: sid.clone(), cwd: self.current_session_cwd(), }]; @@ -449,11 +450,11 @@ impl crate::app::App { self.popup = Popup::None; self.set_status(LogLevel::Info, "fork", "forked - loading session"); return vec![ - ClientMsg::LoadSession { + Command::LoadSession { session_id: forked_session_id.clone(), cwd: self.current_session_cwd(), }, - ClientMsg::SubscribeSession { + Command::SubscribeSession { session_id: forked_session_id, agent_id: self.agent_id.clone(), }, @@ -549,7 +550,7 @@ impl crate::app::App { self.auth_oauth_flow = None; self.auth_panel = crate::app::AuthPanel::List; } - vec![ClientMsg::ListAuthProviders] + vec![Command::ListAuthProviders] } AcpAppEvent::InfoLog { target, message } => { self.push_log(LogLevel::Info, target, message); @@ -584,7 +585,7 @@ impl crate::app::App { &mut self, profiles: Vec, backend_active_profile_id: Option, - ) -> Vec { + ) -> Vec { let previous_profile_id = self.active_profile_id.clone(); let is_available = |profile_id: &str| profiles.iter().any(|profile| profile.id == profile_id); @@ -610,7 +611,7 @@ impl crate::app::App { self.agents_profile_id = None; self.model_popup_agent_tab = 0; return desired_profile_id - .map(|profile_id| vec![ClientMsg::ListProfileAgents { profile_id }]) + .map(|profile_id| vec![Command::ListProfileAgents { profile_id }]) .unwrap_or_default(); } vec![] @@ -620,7 +621,7 @@ impl crate::app::App { &mut self, request: SessionListRequest, page: SessionListPage, - ) -> Vec { + ) -> Vec { let SessionListPage { mut groups, next_cursor, @@ -673,7 +674,7 @@ impl crate::app::App { if !hydrated && self.pending_session_group_loads.insert(Some(cwd.clone())) { - commands.push(ClientMsg::list_sessions_workspace(cwd)); + commands.push(Command::list_sessions_workspace(cwd)); } } None => { @@ -699,7 +700,7 @@ impl crate::app::App { && self.session_discovery_cursors.insert(cursor.clone()) { self.session_discovery_in_progress = true; - commands.push(ClientMsg::list_sessions_discovery(Some(cursor))); + commands.push(Command::list_sessions_discovery(Some(cursor))); } } SessionListRequest::WorkspaceFirstPage { cwd } => { @@ -780,14 +781,14 @@ impl crate::app::App { agent_id: String, session_id: String, profile_id: Option, - ) -> Vec { + ) -> Vec { self.session_id = Some(session_id.clone()); self.apply_session_profile_binding(&session_id, profile_id); self.agent_id = Some(agent_id); self.reset_active_session_view(); self.screen = Screen::Chat; self.set_status(LogLevel::Info, "session", "session created"); - let mut commands = vec![ClientMsg::SubscribeSession { + let mut commands = vec![Command::SubscribeSession { session_id: session_id.clone(), agent_id: self.agent_id.clone(), }]; @@ -795,7 +796,7 @@ impl crate::app::App { if self.agents_profile_id.as_deref() == Some(profile_id.as_str()) { commands.extend(self.delegate_model_commands_for_session(&session_id, &profile_id)); } else { - commands.push(ClientMsg::ListProfileAgents { profile_id }); + commands.push(Command::ListProfileAgents { profile_id }); } } commands @@ -806,7 +807,7 @@ impl crate::app::App { agent_id: String, session_id: String, profile_id: Option, - ) -> Vec { + ) -> Vec { self.activity = ActivityState::Idle; self.parent_session_id = self .pending_parent_session_id @@ -822,7 +823,7 @@ impl crate::app::App { Screen::Chat }; self.set_status(LogLevel::Debug, "activity", "ready"); - let mut commands = vec![ClientMsg::SetAgentMode { + let mut commands = vec![Command::SetAgentMode { mode: self.agent_mode.clone(), }]; if self.parent_session_id.is_none() @@ -831,7 +832,7 @@ impl crate::app::App { if self.agents_profile_id.as_deref() == Some(profile_id.as_str()) { commands.extend(self.delegate_model_commands_for_session(&session_id, &profile_id)); } else { - commands.push(ClientMsg::ListProfileAgents { profile_id }); + commands.push(Command::ListProfileAgents { profile_id }); } } commands @@ -2479,7 +2480,7 @@ mod tests { assert!(matches!( commands.as_slice(), - [ClientMsg::SetDelegateModel { + [Command::SetDelegateModel { session_id, agent_id, model_id: Some(model_id), @@ -2508,7 +2509,7 @@ mod tests { assert_eq!(app.selected_mesh_node_id(), Some("node-1")); assert!(matches!( replies.as_slice(), - [ClientMsg::ListRemoteSessions { node_id, offset: Some(0), limit: Some(50) }] + [Command::ListRemoteSessions { node_id, offset: 0, limit: 50 }] if node_id == "node-1" )); } @@ -2547,7 +2548,7 @@ mod tests { assert_eq!(app.session_remote_node_id("remote-1"), Some("node-1")); assert!(matches!( replies.as_slice(), - [ClientMsg::LoadSession { session_id, .. }] if session_id == "remote-1" + [Command::LoadSession { session_id, .. }] if session_id == "remote-1" )); } @@ -2572,7 +2573,7 @@ mod tests { assert_eq!(app.session_groups[0].next_cursor, None); assert!(replies.iter().any(|reply| matches!( reply, - ClientMsg::ListSessions { + Command::ListSessions { request: SessionListRequest::WorkspaceFirstPage { cwd }, cursor: None, .. @@ -2580,11 +2581,9 @@ mod tests { ))); assert!(replies.iter().any(|reply| matches!( reply, - ClientMsg::ListSessions { + Command::ListSessions { request: SessionListRequest::Discovery, cursor: Some(cursor), - cwd: None, - .. } if cursor == "opaque-root-2" ))); } @@ -2652,7 +2651,7 @@ mod tests { ); assert!(replies.iter().any(|reply| matches!( reply, - ClientMsg::ListSessions { + Command::ListSessions { request: SessionListRequest::WorkspaceFirstPage { cwd }, .. } if cwd == "/later" @@ -2737,8 +2736,8 @@ mod tests { assert!(matches!( replies.as_slice(), [ - ClientMsg::SubscribeSession { session_id, agent_id }, - ClientMsg::ListProfileAgents { profile_id }, + Command::SubscribeSession { session_id, agent_id }, + Command::ListProfileAgents { profile_id }, ] if session_id == "session-1" && agent_id.as_deref() == Some("agent-1") && profile_id == "code" @@ -2818,7 +2817,7 @@ mod tests { assert_eq!(app.delegate_entries.len(), 1); assert!(matches!( replies.as_slice(), - [ClientMsg::SetAgentMode { mode }] if mode == "plan" + [Command::SetAgentMode { mode }] if mode == "plan" )); } @@ -3590,7 +3589,7 @@ mod tests { ); assert!(matches!( replies.as_slice(), - [ClientMsg::LoadSession { session_id, cwd }] if session_id == "session-1" && cwd.is_none() + [Command::LoadSession { session_id, cwd }] if session_id == "session-1" && cwd.is_none() )); } @@ -3700,7 +3699,7 @@ mod tests { assert!(app.can_redo()); assert!(matches!( replies.as_slice(), - [ClientMsg::LoadSession { session_id, cwd }] if session_id == "session-1" && cwd.is_none() + [Command::LoadSession { session_id, cwd }] if session_id == "session-1" && cwd.is_none() )); } @@ -3745,8 +3744,8 @@ mod tests { assert!(matches!( replies.as_slice(), [ - ClientMsg::LoadSession { session_id: load_id, .. }, - ClientMsg::SubscribeSession { session_id: subscribe_id, agent_id } + Command::LoadSession { session_id: load_id, .. }, + Command::SubscribeSession { session_id: subscribe_id, agent_id } ] if load_id == "fork-1" && subscribe_id == "fork-1" && agent_id.as_deref() == Some("agent-1") )); } @@ -3797,6 +3796,39 @@ mod tests { assert!(app.can_redo()); } + #[test] + fn session_update_and_replay_drain_pending_commands() { + let mut app = app_with_active_session(); + app.pending_commands.push(Command::SubscribeSession { + session_id: "child-1".into(), + agent_id: Some("agent-1".into()), + }); + + let update_commands = app.handle_acp_event(AcpAppEvent::SessionUpdate { + session_id: TEST_SESSION_ID.into(), + is_replay: false, + update: AcpSessionUpdate::TurnStarted, + }); + + assert_eq!( + update_commands, + vec![Command::SubscribeSession { + session_id: "child-1".into(), + agent_id: Some("agent-1".into()), + }] + ); + assert!(app.pending_commands.is_empty()); + + app.pending_commands.push(Command::GetFileIndex); + let replay_commands = app.handle_acp_event(AcpAppEvent::SessionReplay { + session_id: TEST_SESSION_ID.into(), + updates: Vec::new(), + }); + + assert_eq!(replay_commands, vec![Command::GetFileIndex]); + assert!(app.pending_commands.is_empty()); + } + #[test] fn native_session_update_for_other_session_marks_activity_only() { let mut app = App::new(); diff --git a/src/app.rs b/src/app.rs index 0bf5320..d11f82b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -4,6 +4,7 @@ use std::time::{Duration, Instant}; use fuzzy_matcher::FuzzyMatcher; use fuzzy_matcher::skim::SkimMatcherV2; +use crate::command::Command; use crate::domain::activity::{ ActivityState, DelegateChildState, DelegateEntry, DelegateStats, PendingDelegateToolCall, SessionActivity, SessionOp, SessionStatsLite, @@ -21,7 +22,7 @@ use crate::highlight::Highlighter; use crate::markdown::CardBlock; use crate::mesh::{MeshFocus, MeshInviteFormField}; use crate::protocol::{ - ClientMsg, EventKind, MeshInviteCreatedInfo, MeshStatusInfo, RemoteNodeInfo, RemoteSessionInfo, + EventKind, MeshInviteCreatedInfo, MeshStatusInfo, RemoteNodeInfo, RemoteSessionInfo, }; use crate::ui::{CardCache, ElicitationUiState}; @@ -768,7 +769,7 @@ pub struct App { pub suppress_delegation_result: bool, /// Commands queued by event handlers (e.g. SubscribeSession for child sessions). /// Drained by native ACP after each event/replay batch. - pub pending_commands: Vec, + pub pending_commands: Vec, /// Child-session state observed before a delegation entry can be linked. pub pending_delegate_child_states: HashMap, pub pending_delegate_child_stats: HashMap, @@ -813,7 +814,11 @@ fn move_wrapping_cursor(cursor: usize, len: usize, delta: isize) -> usize { } impl App { - pub fn begin_session_discovery(&mut self) -> Option { + pub(crate) fn drain_pending_commands(&mut self) -> Vec { + std::mem::take(&mut self.pending_commands) + } + + pub fn begin_session_discovery(&mut self) -> Option { if self.session_discovery_in_progress || !self.pending_session_group_loads.is_empty() { return None; } @@ -822,30 +827,30 @@ impl App { self.pending_session_group_loads.clear(); self.hydrated_session_groups.clear(); self.session_discovery_in_progress = true; - Some(ClientMsg::list_sessions_browse()) + Some(Command::list_sessions_browse()) } - pub fn session_group_page_request(&mut self, group_idx: usize) -> Option { + pub fn session_group_page_request(&mut self, group_idx: usize) -> Option { let group = self.session_groups.get(group_idx)?; let cursor = group.next_cursor.clone()?; let cwd = group.cwd.clone()?; if !self.pending_session_group_loads.insert(Some(cwd.clone())) { return None; } - Some(ClientMsg::list_sessions_group(cwd, cursor)) + Some(Command::list_sessions_group(cwd, cursor)) } pub fn session_child_page_request( &mut self, group_idx: usize, parent_path: &[usize], - ) -> Option { + ) -> Option { let parent = self.session_by_path(group_idx, parent_path)?; let parent_session_id = parent.session_id.clone(); let cursor = parent.children_next_cursor.clone(); self.pending_session_child_loads .insert(parent_session_id.clone()); - Some(ClientMsg::list_session_children( + Some(Command::list_session_children( parent_session_id, cursor, SESSION_CHILD_PAGE_LIMIT, @@ -1072,13 +1077,13 @@ impl App { /// Cycle through `[auto, low, medium, high, max]` (wraps around). /// Updates `self.reasoning_effort` optimistically, saves the new value as /// the preference for the current `(mode, provider, model)` context, and - /// returns the [`ClientMsg`] to forward to the server. + /// returns the [`Command`] to forward to the server. /// /// Returns `None` if the current value is not a recognized level; in that /// case the state is left unchanged and no message is emitted (the caller /// should surface a warning to the user instead of silently coercing the /// unknown value to `low`). - pub fn cycle_reasoning_effort(&mut self) -> Option { + pub fn cycle_reasoning_effort(&mut self) -> Option { const LEVELS: &[Option<&str>] = &[None, Some("low"), Some("medium"), Some("high"), Some("max")]; let current = self.reasoning_effort.as_deref(); @@ -1096,9 +1101,9 @@ impl App { /// Set the reasoning effort to a specific level. /// `None` or `Some("auto")` both map to the "auto" (no override) state. - /// Updates `self.reasoning_effort` and returns the [`ClientMsg`] to forward to the server. + /// Updates `self.reasoning_effort` and returns the [`Command`] to forward to the server. /// Returns `None` if the level is invalid (state is unchanged). - pub fn set_reasoning_effort(&mut self, level: Option<&str>) -> Option { + pub fn set_reasoning_effort(&mut self, level: Option<&str>) -> Option { match validate_reasoning_effort(level) { Some(normalized) => { self.reasoning_effort = normalized; @@ -1107,7 +1112,7 @@ impl App { .as_deref() .unwrap_or("auto") .to_string(); - Some(ClientMsg::SetReasoningEffort { + Some(Command::SetReasoningEffort { reasoning_effort: effort_str, }) } @@ -2074,7 +2079,7 @@ impl App { &self, session_id: &str, profile_id: &str, - ) -> Vec { + ) -> Vec { let known_agents: HashSet<&str> = self.agents.iter().skip(1).map(|a| a.id.as_str()).collect(); self.delegate_model_preferences @@ -2082,7 +2087,7 @@ impl App { .into_iter() .flat_map(|preferences| preferences.iter()) .filter(|(agent_id, _)| known_agents.contains(agent_id.as_str())) - .map(|(agent_id, preference)| ClientMsg::SetDelegateModel { + .map(|(agent_id, preference)| Command::SetDelegateModel { session_id: session_id.to_string(), agent_id: agent_id.clone(), model_id: Some(preference.model_id.clone()), @@ -2239,12 +2244,12 @@ mod reasoning_effort_tests { } #[test] - fn cycle_returns_correct_client_msg() { + fn cycle_returns_correct_command() { let mut app = App::new(); // starts at auto let msg = app.cycle_reasoning_effort().expect("auto is a valid level"); // auto → low: should send "low" match msg { - ClientMsg::SetReasoningEffort { reasoning_effort } => { + Command::SetReasoningEffort { reasoning_effort } => { assert_eq!(reasoning_effort, "low"); } other => panic!("expected SetReasoningEffort, got {other:?}"), @@ -2258,7 +2263,7 @@ mod reasoning_effort_tests { let msg = app.cycle_reasoning_effort().expect("max is a valid level"); // max → auto: server expects "auto" string (not null) match msg { - ClientMsg::SetReasoningEffort { reasoning_effort } => { + Command::SetReasoningEffort { reasoning_effort } => { assert_eq!(reasoning_effort, "auto"); } other => panic!("expected SetReasoningEffort, got {other:?}"), @@ -2273,7 +2278,7 @@ mod reasoning_effort_tests { let msg = app.set_reasoning_effort(Some("high")); assert_eq!(app.reasoning_effort, Some("high".into())); match msg { - Some(ClientMsg::SetReasoningEffort { reasoning_effort }) => { + Some(Command::SetReasoningEffort { reasoning_effort }) => { assert_eq!(reasoning_effort, "high"); } other => panic!("expected SetReasoningEffort, got {other:?}"), @@ -2287,7 +2292,7 @@ mod reasoning_effort_tests { let msg = app.set_reasoning_effort(Some("auto")); assert_eq!(app.reasoning_effort, None); match msg { - Some(ClientMsg::SetReasoningEffort { reasoning_effort }) => { + Some(Command::SetReasoningEffort { reasoning_effort }) => { assert_eq!(reasoning_effort, "auto"); } other => panic!("expected SetReasoningEffort, got {other:?}"), @@ -2301,7 +2306,7 @@ mod reasoning_effort_tests { let msg = app.set_reasoning_effort(None); assert_eq!(app.reasoning_effort, None); match msg { - Some(ClientMsg::SetReasoningEffort { reasoning_effort }) => { + Some(Command::SetReasoningEffort { reasoning_effort }) => { assert_eq!(reasoning_effort, "auto"); } other => panic!("expected SetReasoningEffort, got {other:?}"), diff --git a/src/command.rs b/src/command.rs new file mode 100644 index 0000000..c912b44 --- /dev/null +++ b/src/command.rs @@ -0,0 +1,102 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SessionListRequest { + Discovery, + WorkspaceFirstPage { cwd: String }, + WorkspaceContinuation { cwd: String }, +} + +impl SessionListRequest { + pub fn cwd(&self) -> Option<&str> { + match self { + Self::Discovery => None, + Self::WorkspaceFirstPage { cwd } | Self::WorkspaceContinuation { cwd } => Some(cwd), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Command { + ListSessions { + request: SessionListRequest, + cursor: Option, + }, + ListRemoteSessions { + node_id: String, + offset: u32, + limit: u32, + }, + CreateMeshInvite { + mesh_name: Option, + ttl: Option, + max_uses: Option, + }, + ListSessionChildren { + parent_session_id: String, + cursor: Option, + limit: u32, + }, + SetReasoningEffort { + reasoning_effort: String, + }, + ListProfileAgents { + profile_id: String, + }, + SetDelegateModel { + session_id: String, + agent_id: String, + model_id: Option, + node_id: Option, + }, + LoadSession { + session_id: String, + cwd: Option, + }, + SubscribeSession { + session_id: String, + agent_id: Option, + }, + GetFileIndex, + SetAgentMode { + mode: String, + }, + ListAuthProviders, +} + +impl Command { + pub fn list_sessions_browse() -> Self { + Self::list_sessions_discovery(None) + } + + pub fn list_sessions_discovery(cursor: Option) -> Self { + Self::ListSessions { + request: SessionListRequest::Discovery, + cursor, + } + } + + pub fn list_sessions_workspace(cwd: String) -> Self { + Self::ListSessions { + request: SessionListRequest::WorkspaceFirstPage { cwd }, + cursor: None, + } + } + + pub fn list_sessions_group(cwd: String, cursor: String) -> Self { + Self::ListSessions { + request: SessionListRequest::WorkspaceContinuation { cwd }, + cursor: Some(cursor), + } + } + + pub fn list_session_children( + parent_session_id: String, + cursor: Option, + limit: u32, + ) -> Self { + Self::ListSessionChildren { + parent_session_id, + cursor, + limit, + } + } +} diff --git a/src/handlers.rs b/src/handlers.rs index ab1d580..f3bdef1 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -257,7 +257,7 @@ fn open_session_popup( app.session_cursor = 0; app.session_filter.clear(); if let Some(request) = app.begin_session_discovery() { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } Ok(()) } @@ -355,7 +355,7 @@ pub(crate) fn handle_mesh_popup_key( KeyCode::Up => match app.mesh_focus { crate::mesh::MeshFocus::Nodes => { if let Some(msg) = app.move_mesh_node_cursor(-1) { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; } } crate::mesh::MeshFocus::Sessions => app.move_remote_session_cursor(-1), @@ -363,7 +363,7 @@ pub(crate) fn handle_mesh_popup_key( KeyCode::Down => match app.mesh_focus { crate::mesh::MeshFocus::Nodes => { if let Some(msg) = app.move_mesh_node_cursor(1) { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; } } crate::mesh::MeshFocus::Sessions => app.move_remote_session_cursor(1), @@ -446,7 +446,7 @@ pub(crate) fn handle_mesh_invite_popup_key( }, KeyCode::Enter => { if let Some(msg) = app.mesh_invite_form_command() { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; app.set_status(app::LogLevel::Info, "mesh", "creating invite..."); } } @@ -578,7 +578,7 @@ pub(crate) fn handle_key( } match app.cycle_reasoning_effort() { Some(msg) => { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; app.set_status( app::LogLevel::Info, "model", @@ -884,7 +884,7 @@ pub(crate) fn handle_sessions_key( parent_path, } => { if let Some(request) = app.session_child_page_request(group_idx, &parent_path) { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } } SessionKeyAction::None => {} @@ -943,7 +943,7 @@ pub(crate) fn handle_sessions_key( app.session_child_page_request(group_idx, &parent_path) }; if let Some(request) = request { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } } SessionKeyAction::None => {} @@ -983,7 +983,7 @@ pub(crate) fn handle_session_popup_key( parent_path, } => { if let Some(request) = app.session_child_page_request(group_idx, &parent_path) { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } } SessionKeyAction::None => {} @@ -1039,7 +1039,7 @@ pub(crate) fn handle_session_popup_key( app.session_child_page_request(group_idx, &parent_path) }; if let Some(request) = request { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } } SessionKeyAction::NewSession | SessionKeyAction::None => {} @@ -1701,7 +1701,7 @@ pub(crate) fn handle_chat_key( // 3. Accept mention completion. if app.mention_state.is_some() && app.accept_selected_mention() { if let Some(msg) = app.request_file_index_if_needed() { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; } return Ok(AppAction::None); } @@ -1749,13 +1749,13 @@ pub(crate) fn handle_chat_key( && app.accept_selected_mention() && let Some(msg) = app.request_file_index_if_needed() { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; } } KeyCode::Char(c) if !key.modifiers.contains(KeyModifiers::CONTROL) && !input_blocked => { app.input_insert(c); if let Some(msg) = app.request_file_index_if_needed() { - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; } } KeyCode::Up => { @@ -1974,7 +1974,7 @@ fn try_execute_slash_command( return Ok(SlashResult::Handled); } let msg = app.set_reasoning_effort(Some(&level)).unwrap(); - cmd_tx.send(msg)?; + cmd_tx.send(msg.into())?; app.set_status( app::LogLevel::Info, "model", @@ -2031,7 +2031,7 @@ fn try_execute_slash_command( app.session_cursor = 0; app.session_filter.clear(); if let Some(request) = app.begin_session_discovery() { - cmd_tx.send(request)?; + cmd_tx.send(request.into())?; } } "delegates" => { diff --git a/src/input.rs b/src/input.rs index 5586f1d..d95e0d4 100644 --- a/src/input.rs +++ b/src/input.rs @@ -2,7 +2,7 @@ use fuzzy_matcher::FuzzyMatcher; use fuzzy_matcher::skim::SkimMatcherV2; use crate::app::{App, FileIndexEntryLite, MentionState, SlashCompletionState}; -use crate::protocol::ClientMsg; +use crate::command::Command; use crate::ui::build_input_visual_layout; impl App { @@ -206,11 +206,11 @@ impl App { }); } - pub fn request_file_index_if_needed(&mut self) -> Option { + pub fn request_file_index_if_needed(&mut self) -> Option { if self.mention_state.is_some() && self.file_index.is_empty() && !self.file_index_loading { self.file_index_loading = true; self.file_index_error = None; - return Some(ClientMsg::GetFileIndex); + return Some(Command::GetFileIndex); } None } diff --git a/src/lib.rs b/src/lib.rs index a927ab8..d2a0d52 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,7 @@ mod acp_client; mod acp_state; mod app; +mod command; mod config; mod domain; mod handlers; diff --git a/src/mesh.rs b/src/mesh.rs index 71b9468..fc008d2 100644 --- a/src/mesh.rs +++ b/src/mesh.rs @@ -1,9 +1,9 @@ use std::time::{Duration, Instant}; use crate::app::{App, LogLevel, Popup}; +use crate::command::Command; use crate::protocol::{ - ClientMsg, MeshInviteCreatedInfo, MeshNodesInfo, MeshStatusInfo, RemoteSessionInfo, - RemoteSessionListInfo, + MeshInviteCreatedInfo, MeshNodesInfo, MeshStatusInfo, RemoteSessionInfo, RemoteSessionListInfo, }; const INVITE_ERROR_TTL: Duration = Duration::from_secs(5); @@ -108,7 +108,7 @@ impl App { self.push_log(LogLevel::Info, "mesh", format!("mesh invite: {url}")); } - pub fn mesh_invite_form_command(&mut self) -> Option { + pub fn mesh_invite_form_command(&mut self) -> Option { let max_uses = match self.mesh_invite_max_uses.trim().parse::() { Ok(max_uses) if max_uses > 0 => max_uses, Ok(_) => { @@ -130,7 +130,7 @@ impl App { let mesh_name = (!self.mesh_invite_name.trim().is_empty()) .then(|| self.mesh_invite_name.trim().to_string()); self.clear_mesh_error(); - Some(ClientMsg::CreateMeshInvite { + Some(Command::CreateMeshInvite { mesh_name, ttl, max_uses: Some(max_uses), @@ -152,7 +152,7 @@ impl App { ); } - pub fn apply_mesh_nodes(&mut self, nodes: MeshNodesInfo) -> Vec { + pub fn apply_mesh_nodes(&mut self, nodes: MeshNodesInfo) -> Vec { self.mesh_node_count = Some(nodes.nodes.len() as u32); self.mesh_nodes = nodes.nodes; if self.mesh_node_cursor >= self.mesh_nodes.len() { @@ -164,10 +164,10 @@ impl App { format!("mesh nodes: {}", self.mesh_nodes.len()), ); self.selected_mesh_node_id() - .map(|node_id| ClientMsg::ListRemoteSessions { + .map(|node_id| Command::ListRemoteSessions { node_id: node_id.to_string(), - offset: Some(0), - limit: Some(50), + offset: 0, + limit: 50, }) .into_iter() .collect() @@ -192,21 +192,21 @@ impl App { session_id: &str, node_id: &str, attached: bool, - ) -> Vec { + ) -> Vec { self.remember_remote_session_node(session_id, node_id); if attached { self.popup = Popup::None; self.set_status(LogLevel::Info, "mesh", "remote session attached"); - vec![ClientMsg::LoadSession { + vec![Command::LoadSession { session_id: session_id.to_string(), cwd: self.current_session_cwd(), }] } else { self.set_status(LogLevel::Info, "mesh", "remote session created"); - vec![ClientMsg::ListRemoteSessions { + vec![Command::ListRemoteSessions { node_id: node_id.to_string(), - offset: Some(0), - limit: Some(50), + offset: 0, + limit: 50, }] } } @@ -235,7 +235,7 @@ impl App { .get(self.remote_session_cursor) } - pub fn move_mesh_node_cursor(&mut self, delta: isize) -> Option { + pub fn move_mesh_node_cursor(&mut self, delta: isize) -> Option { let len = self.mesh_nodes.len(); if len == 0 { self.mesh_node_cursor = 0; @@ -245,10 +245,10 @@ impl App { (self.mesh_node_cursor as isize + delta).rem_euclid(len as isize) as usize; self.remote_session_cursor = 0; self.selected_mesh_node_id() - .map(|node_id| ClientMsg::ListRemoteSessions { + .map(|node_id| Command::ListRemoteSessions { node_id: node_id.to_string(), - offset: Some(0), - limit: Some(50), + offset: 0, + limit: 50, }) } diff --git a/src/protocol.rs b/src/protocol.rs index 95b852b..7c7cf8b 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1,7 +1,10 @@ use serde::{Deserialize, Serialize}; +use crate::command::Command; use crate::domain::auth::{AuthMethod, AuthProviderEntry, OAuthFlowKind}; +pub(crate) use crate::command::SessionListRequest; + // --- Client → Server messages --- #[derive(Debug, Serialize)] @@ -11,22 +14,6 @@ pub enum SessionScope { Forks, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum SessionListRequest { - Discovery, - WorkspaceFirstPage { cwd: String }, - WorkspaceContinuation { cwd: String }, -} - -impl SessionListRequest { - pub fn cwd(&self) -> Option<&str> { - match self { - Self::Discovery => None, - Self::WorkspaceFirstPage { cwd } | Self::WorkspaceContinuation { cwd } => Some(cwd), - } - } -} - #[derive(Debug, Serialize)] #[serde(tag = "type", content = "data", rename_all = "snake_case")] pub enum ClientMsg { @@ -173,6 +160,86 @@ pub enum ClientMsg { }, } +impl From for ClientMsg { + fn from(command: Command) -> Self { + match command { + Command::ListSessions { request, cursor } => { + let (mode, limit, cwd, include_remote) = match &request { + SessionListRequest::Discovery => (None, None, None, Some(true)), + SessionListRequest::WorkspaceFirstPage { cwd } + | SessionListRequest::WorkspaceContinuation { cwd } => { + (Some("group".to_string()), Some(10), Some(cwd.clone()), None) + } + }; + Self::ListSessions { + request, + mode, + cursor, + limit, + cwd, + query: None, + include_remote, + session_scope: SessionScope::Root, + } + } + Command::ListRemoteSessions { + node_id, + offset, + limit, + } => Self::ListRemoteSessions { + node_id, + offset: Some(offset), + limit: Some(limit), + }, + Command::CreateMeshInvite { + mesh_name, + ttl, + max_uses, + } => Self::CreateMeshInvite { + mesh_name, + ttl, + max_uses, + }, + Command::ListSessionChildren { + parent_session_id, + cursor, + limit, + } => Self::ListSessionChildren { + parent_session_id, + cursor, + limit: Some(limit), + session_scope: SessionScope::Forks, + }, + Command::SetReasoningEffort { reasoning_effort } => { + Self::SetReasoningEffort { reasoning_effort } + } + Command::ListProfileAgents { profile_id } => Self::ListProfileAgents { profile_id }, + Command::SetDelegateModel { + session_id, + agent_id, + model_id, + node_id, + } => Self::SetDelegateModel { + session_id, + agent_id, + model_id, + node_id, + }, + Command::LoadSession { session_id, cwd } => Self::LoadSession { session_id, cwd }, + Command::SubscribeSession { + session_id, + agent_id, + } => Self::SubscribeSession { + session_id, + agent_id, + }, + Command::GetFileIndex => Self::GetFileIndex, + Command::SetAgentMode { mode } => Self::SetAgentMode { mode }, + Command::ListAuthProviders => Self::ListAuthProviders, + } + } +} + impl ClientMsg { pub fn list_sessions_browse() -> Self { Self::list_sessions_discovery(None) @@ -231,6 +298,192 @@ impl ClientMsg { } } +#[cfg(test)] +mod command_conversion_tests { + use super::{ClientMsg, SessionListRequest, SessionScope}; + use crate::command::Command; + + #[test] + fn list_session_commands_preserve_semantics_and_bound_wire_fields() { + let discovery = ClientMsg::from(Command::list_sessions_discovery(Some("root-2".into()))); + assert!(matches!( + discovery, + ClientMsg::ListSessions { + request: SessionListRequest::Discovery, + mode: None, + cursor: Some(cursor), + limit: None, + cwd: None, + query: None, + include_remote: Some(true), + session_scope: SessionScope::Root, + } if cursor == "root-2" + )); + + let first = ClientMsg::from(Command::list_sessions_workspace("/repo".into())); + assert!(matches!( + first, + ClientMsg::ListSessions { + request: SessionListRequest::WorkspaceFirstPage { cwd: request_cwd }, + mode: Some(mode), + cursor: None, + limit: Some(10), + cwd: Some(wire_cwd), + query: None, + include_remote: None, + session_scope: SessionScope::Root, + } if request_cwd == "/repo" && mode == "group" && wire_cwd == "/repo" + )); + + let continuation = ClientMsg::from(Command::list_sessions_group( + "/repo".into(), + "workspace-2".into(), + )); + assert!(matches!( + continuation, + ClientMsg::ListSessions { + request: SessionListRequest::WorkspaceContinuation { cwd: request_cwd }, + mode: Some(mode), + cursor: Some(cursor), + limit: Some(10), + cwd: Some(wire_cwd), + query: None, + include_remote: None, + session_scope: SessionScope::Root, + } if request_cwd == "/repo" + && mode == "group" + && cursor == "workspace-2" + && wire_cwd == "/repo" + )); + + let children = ClientMsg::from(Command::list_session_children( + "parent-1".into(), + Some("child-2".into()), + 25, + )); + assert!(matches!( + children, + ClientMsg::ListSessionChildren { + parent_session_id, + cursor: Some(cursor), + limit: Some(25), + session_scope: SessionScope::Forks, + } if parent_session_id == "parent-1" && cursor == "child-2" + )); + } + + #[test] + fn mesh_and_model_commands_convert_exactly() { + let remote = ClientMsg::from(Command::ListRemoteSessions { + node_id: "node-1".into(), + offset: 5, + limit: 50, + }); + assert!(matches!( + remote, + ClientMsg::ListRemoteSessions { + node_id, + offset: Some(5), + limit: Some(50), + } if node_id == "node-1" + )); + + let invite = ClientMsg::from(Command::CreateMeshInvite { + mesh_name: Some("team".into()), + ttl: Some("24h".into()), + max_uses: Some(3), + }); + assert!(matches!( + invite, + ClientMsg::CreateMeshInvite { + mesh_name: Some(mesh_name), + ttl: Some(ttl), + max_uses: Some(3), + } if mesh_name == "team" && ttl == "24h" + )); + + let effort = ClientMsg::from(Command::SetReasoningEffort { + reasoning_effort: "high".into(), + }); + assert!(matches!( + effort, + ClientMsg::SetReasoningEffort { reasoning_effort } if reasoning_effort == "high" + )); + + let agents = ClientMsg::from(Command::ListProfileAgents { + profile_id: "code".into(), + }); + assert!(matches!( + agents, + ClientMsg::ListProfileAgents { profile_id } if profile_id == "code" + )); + + let delegate = ClientMsg::from(Command::SetDelegateModel { + session_id: "session-1".into(), + agent_id: "reviewer".into(), + model_id: Some("provider/model".into()), + node_id: Some("node-2".into()), + }); + assert!(matches!( + delegate, + ClientMsg::SetDelegateModel { + session_id, + agent_id, + model_id: Some(model_id), + node_id: Some(node_id), + } if session_id == "session-1" + && agent_id == "reviewer" + && model_id == "provider/model" + && node_id == "node-2" + )); + } + + #[test] + fn session_and_input_commands_convert_exactly() { + let load = ClientMsg::from(Command::LoadSession { + session_id: "session-1".into(), + cwd: Some("/repo".into()), + }); + assert!(matches!( + load, + ClientMsg::LoadSession { + session_id, + cwd: Some(cwd), + } if session_id == "session-1" && cwd == "/repo" + )); + + let subscribe = ClientMsg::from(Command::SubscribeSession { + session_id: "session-1".into(), + agent_id: Some("agent-1".into()), + }); + assert!(matches!( + subscribe, + ClientMsg::SubscribeSession { + session_id, + agent_id: Some(agent_id), + } if session_id == "session-1" && agent_id == "agent-1" + )); + + assert!(matches!( + ClientMsg::from(Command::GetFileIndex), + ClientMsg::GetFileIndex + )); + + let mode = ClientMsg::from(Command::SetAgentMode { + mode: "plan".into(), + }); + assert!(matches!( + mode, + ClientMsg::SetAgentMode { mode } if mode == "plan" + )); + + assert!(matches!( + ClientMsg::from(Command::ListAuthProviders), + ClientMsg::ListAuthProviders + )); + } +} + #[cfg(test)] mod client_msg_tests { use super::ClientMsg; diff --git a/src/runtime/event_loop.rs b/src/runtime/event_loop.rs index 60c8ad0..b1b73e1 100644 --- a/src/runtime/event_loop.rs +++ b/src/runtime/event_loop.rs @@ -6,6 +6,7 @@ use tokio::sync::mpsc; use crate::{ app::{self, App}, + command::Command, handlers::{AppAction, handle_key, handle_mouse}, protocol::ClientMsg, server_manager::{self, ServerEvent, ServerState}, @@ -83,16 +84,16 @@ pub(super) async fn run_loop( } } Some(ServerChannelMsg::Acp(event)) = srv_rx.recv() => { - for reply in app.handle_acp_event(event) { - if let ClientMsg::LoadSession { ref session_id, .. } = reply { + for command in app.handle_acp_event(event) { + if let Command::LoadSession { ref session_id, .. } = command { let sid = session_id.clone(); - cmd_tx.send(reply)?; + cmd_tx.send(command.into())?; cmd_tx.send(ClientMsg::SubscribeSession { session_id: sid, agent_id: app.agent_id.clone(), })?; } else { - cmd_tx.send(reply)?; + cmd_tx.send(command.into())?; } } } diff --git a/src/runtime/mod.rs b/src/runtime/mod.rs index d410faf..d5769a8 100644 --- a/src/runtime/mod.rs +++ b/src/runtime/mod.rs @@ -3352,6 +3352,7 @@ mod runtime_tests { mod auth_tests { use super::*; use crate::app::AuthUiNotice; + use crate::command::Command; use crate::domain::auth::{ AuthProviderEntry, OAuthFlow, OAuthFlowKind, OAuthResult, OAuthResultStatus, OAuthStatus, }; @@ -3943,7 +3944,7 @@ mod auth_tests { })); assert_eq!(cmds.len(), 1); - assert!(matches!(cmds[0], ClientMsg::ListAuthProviders)); + assert!(matches!(cmds[0], Command::ListAuthProviders)); assert!(app.auth_oauth_flow.is_none()); assert_eq!(app.auth_panel, app::AuthPanel::List); assert_eq!( @@ -3976,7 +3977,7 @@ mod auth_tests { let cmds = app.handle_acp_event(AcpAppEvent::OAuthResult(result.clone())); assert_eq!(cmds.len(), 1); - assert!(matches!(cmds[0], ClientMsg::ListAuthProviders)); + assert!(matches!(cmds[0], Command::ListAuthProviders)); assert_eq!(app.auth_oauth_flow, Some(flow)); assert_eq!(app.auth_panel, app::AuthPanel::OAuthFlow); assert_eq!(app.auth_last_result, Some(result)); From 05436739ed71b0f4e04c3485144aaa17fae6ec62 Mon Sep 17 00:00:00 2001 From: Ivan Zatevakhin Date: Sun, 9 Aug 2026 17:20:51 +0100 Subject: [PATCH 2/2] fix: centralize session command mapping --- src/protocol.rs | 42 +++--------------------- src/runtime/event_loop.rs | 67 ++++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/src/protocol.rs b/src/protocol.rs index 7c7cf8b..81f6d68 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -242,46 +242,19 @@ impl From for ClientMsg { impl ClientMsg { pub fn list_sessions_browse() -> Self { - Self::list_sessions_discovery(None) + Command::list_sessions_browse().into() } pub fn list_sessions_discovery(cursor: Option) -> Self { - Self::ListSessions { - request: SessionListRequest::Discovery, - mode: None, - cursor, - limit: None, - cwd: None, - query: None, - include_remote: Some(true), - session_scope: SessionScope::Root, - } + Command::list_sessions_discovery(cursor).into() } pub fn list_sessions_workspace(cwd: String) -> Self { - Self::ListSessions { - request: SessionListRequest::WorkspaceFirstPage { cwd: cwd.clone() }, - mode: Some("group".to_string()), - cursor: None, - limit: Some(10), - cwd: Some(cwd), - query: None, - include_remote: None, - session_scope: SessionScope::Root, - } + Command::list_sessions_workspace(cwd).into() } pub fn list_sessions_group(cwd: String, cursor: String) -> Self { - Self::ListSessions { - request: SessionListRequest::WorkspaceContinuation { cwd: cwd.clone() }, - mode: Some("group".to_string()), - cursor: Some(cursor), - limit: Some(10), - cwd: Some(cwd), - query: None, - include_remote: None, - session_scope: SessionScope::Root, - } + Command::list_sessions_group(cwd, cursor).into() } pub fn list_session_children( @@ -289,12 +262,7 @@ impl ClientMsg { cursor: Option, limit: u32, ) -> Self { - Self::ListSessionChildren { - parent_session_id, - cursor, - limit: Some(limit), - session_scope: SessionScope::Forks, - } + Command::list_session_children(parent_session_id, cursor, limit).into() } } diff --git a/src/runtime/event_loop.rs b/src/runtime/event_loop.rs index b1b73e1..4599333 100644 --- a/src/runtime/event_loop.rs +++ b/src/runtime/event_loop.rs @@ -24,6 +24,26 @@ fn tick_from_elapsed(elapsed: Duration) -> u64 { (elapsed.as_millis() / 80) as u64 } +fn send_command( + cmd_tx: &mpsc::UnboundedSender, + command: Command, + agent_id: &Option, +) -> anyhow::Result<()> { + let load_session_id = match &command { + Command::LoadSession { session_id, .. } => Some(session_id.clone()), + _ => None, + }; + + cmd_tx.send(command.into())?; + if let Some(session_id) = load_session_id { + cmd_tx.send(ClientMsg::SubscribeSession { + session_id, + agent_id: agent_id.clone(), + })?; + } + Ok(()) +} + pub(super) async fn run_loop( terminal: &mut AppTerminal, app: &mut App, @@ -85,16 +105,7 @@ pub(super) async fn run_loop( } Some(ServerChannelMsg::Acp(event)) = srv_rx.recv() => { for command in app.handle_acp_event(event) { - if let Command::LoadSession { ref session_id, .. } = command { - let sid = session_id.clone(); - cmd_tx.send(command.into())?; - cmd_tx.send(ClientMsg::SubscribeSession { - session_id: sid, - agent_id: app.agent_id.clone(), - })?; - } else { - cmd_tx.send(command.into())?; - } + send_command(cmd_tx, command, &app.agent_id)?; } } Some(sup_event) = sup_rx.recv() => { @@ -186,7 +197,41 @@ pub(super) async fn run_loop( mod tests { use std::time::Duration; - use super::tick_from_elapsed; + use tokio::sync::mpsc; + + use super::{send_command, tick_from_elapsed}; + use crate::{command::Command, protocol::ClientMsg}; + + #[test] + fn load_session_command_sends_load_and_subscribe() { + let (tx, mut rx) = mpsc::unbounded_channel(); + + send_command( + &tx, + Command::LoadSession { + session_id: "session-1".into(), + cwd: Some("/repo".into()), + }, + &Some("agent-1".into()), + ) + .unwrap(); + + assert!(matches!( + rx.try_recv().unwrap(), + ClientMsg::LoadSession { + session_id, + cwd: Some(cwd), + } if session_id == "session-1" && cwd == "/repo" + )); + assert!(matches!( + rx.try_recv().unwrap(), + ClientMsg::SubscribeSession { + session_id, + agent_id: Some(agent_id), + } if session_id == "session-1" && agent_id == "agent-1" + )); + assert!(rx.try_recv().is_err()); + } #[test] fn tick_from_elapsed_zero_is_zero() {