From 9ab9bdb451e554c0e9c048d400ce937bcc0753e2 Mon Sep 17 00:00:00 2001 From: Josh Carver Date: Tue, 30 Jun 2026 06:45:05 -0700 Subject: [PATCH] refactor(aether-cli): Consolidate and cleanup slash command expansion logic --- crates/aether-cli/src/acp/agent_runtime.rs | 36 ++++----------- crates/aether-cli/src/acp/session_actor.rs | 3 +- crates/aether-cli/src/acp/slash_commands.rs | 32 ++----------- crates/aether-cli/src/headless/run.rs | 47 +------------------ crates/aether-cli/src/init/build_settings.rs | 6 +-- crates/aether-cli/src/init/mod.rs | 5 +- crates/aether-cli/src/slash_commands.rs | 48 ++++++++++++++++++-- crates/internal-evals/tests/plan_agent.rs | 26 ++++++----- 8 files changed, 81 insertions(+), 122 deletions(-) diff --git a/crates/aether-cli/src/acp/agent_runtime.rs b/crates/aether-cli/src/acp/agent_runtime.rs index 713132ef7..d8e12b100 100644 --- a/crates/aether-cli/src/acp/agent_runtime.rs +++ b/crates/aether-cli/src/acp/agent_runtime.rs @@ -1,6 +1,7 @@ use super::agent_key::AgentKey; use super::error::SessionError; use crate::runtime::{Runtime, RuntimeBuilder}; +use crate::slash_commands::{SlashCommandError, list_prompts}; use aether_auth::OAuthCredentialStorage; use aether_auth::OAuthHandler; use aether_core::agent_spec::AgentSpec; @@ -13,11 +14,11 @@ use mcp_utils::client::{ ElicitingOAuthHandler, McpClientEvent, McpConnectionDetails, McpError, McpServer, McpServerStatusEntry, OAuthHandlerFactory, }; -use rmcp::model::{GetPromptResult, Prompt as McpPrompt}; +use rmcp::model::Prompt as McpPrompt; use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Arc; -use tokio::sync::{mpsc, oneshot, watch}; +use tokio::sync::{mpsc, watch}; use tokio::task::JoinHandle; /// Capacity of the channel that fans runtime events from every spawned agent @@ -92,32 +93,15 @@ impl AgentRuntime { .map_err(|e| SessionError::CommandChannel(format!("failed to sync active conversation: {e}"))) } - pub(crate) async fn list_prompts(&self) -> Result, SessionError> { - let (tx, rx) = oneshot::channel(); - self.mcp_tx - .send(McpCommand::ListPrompts { tx }) - .await - .map_err(|e| SessionError::CommandChannel(format!("failed to send ListPrompts command: {e}")))?; - - rx.await - .map_err(|e| SessionError::CommandChannel(format!("failed to receive prompts: {e}")))? - .map_err(SessionError::McpOperation) + pub(crate) fn mcp_tx(&self) -> &mpsc::Sender { + &self.mcp_tx } - pub(crate) async fn get_prompt( - &self, - name: String, - arguments: Option>, - ) -> Result { - let (tx, rx) = oneshot::channel(); - self.mcp_tx - .send(McpCommand::GetPrompt { name, arguments, tx }) - .await - .map_err(|e| SessionError::CommandChannel(format!("failed to send GetPrompt command: {e}")))?; - - rx.await - .map_err(|e| SessionError::CommandChannel(format!("failed to receive prompt: {e}")))? - .map_err(SessionError::McpOperation) + pub(crate) async fn list_prompts(&self) -> Result, SessionError> { + list_prompts(&self.mcp_tx).await.map_err(|error| match error { + SlashCommandError::CommandChannel(message) => SessionError::CommandChannel(message), + other => SessionError::McpOperation(other.to_string()), + }) } pub(crate) async fn authenticate_mcp_server(&self, name: &str) -> Result<(), SessionError> { diff --git a/crates/aether-cli/src/acp/session_actor.rs b/crates/aether-cli/src/acp/session_actor.rs index 90b3b7d6f..7e1064dc7 100644 --- a/crates/aether-cli/src/acp/session_actor.rs +++ b/crates/aether-cli/src/acp/session_actor.rs @@ -30,7 +30,8 @@ use super::protocol::events::{ }; use super::session_config_state::{SessionConfigState, Switch}; use super::session_store::{SessionStore, should_persist_session_event}; -use super::slash_commands::{dedupe_commands_by_name, expand_slash_command_in_content, send_available_commands}; +use super::slash_commands::{expand_slash_command_in_content, send_available_commands}; +use crate::slash_commands::dedupe_commands_by_name; /// Capacity of the per-session command channel feeding the actor loop. const SESSION_COMMAND_CHANNEL_CAPACITY: usize = 50; diff --git a/crates/aether-cli/src/acp/slash_commands.rs b/crates/aether-cli/src/acp/slash_commands.rs index feb3fc984..4632eeae7 100644 --- a/crates/aether-cli/src/acp/slash_commands.rs +++ b/crates/aether-cli/src/acp/slash_commands.rs @@ -5,19 +5,13 @@ use llm::ContentBlock; use tracing::{error, info}; use super::agent_runtime::AgentRuntime; -use crate::acp::error::SessionError; -pub(crate) use crate::slash_commands::dedupe_commands_by_name; -use crate::slash_commands::{ - SlashCommandError, find_prompt_name, parse_slash_command, parse_slash_command_arguments, prompt_result_text, -}; +use crate::slash_commands::{expand_slash_command, parse_slash_command}; pub(crate) async fn expand_slash_command_in_content( runtime: &AgentRuntime, mut content: Vec, ) -> Vec { - if let Some(ContentBlock::Text { text }) = content.first() - && text.starts_with('/') - { + if let Some(ContentBlock::Text { text }) = content.first() { let expanded = expand_slash_command_text(runtime, text.clone()).await; content[0] = ContentBlock::text(expanded); } @@ -29,7 +23,7 @@ async fn expand_slash_command_text(runtime: &AgentRuntime, text: String) -> Stri return text; }; - match expand_slash_command(runtime, slash_command.command_name, slash_command.args_text).await { + match expand_slash_command(runtime.mcp_tx(), slash_command.command_name, slash_command.args_text).await { Ok(expanded) => { info!("Expanded slash command -> {} chars", expanded.len()); expanded @@ -41,26 +35,6 @@ async fn expand_slash_command_text(runtime: &AgentRuntime, text: String) -> Stri } } -async fn expand_slash_command( - runtime: &AgentRuntime, - command_name: &str, - args_text: &str, -) -> Result { - let arguments = parse_slash_command_arguments(args_text); - let prompts = runtime.list_prompts().await.map_err(slash_command_error)?; - let prompt_name = find_prompt_name(&prompts, command_name)?; - let prompt_result = runtime.get_prompt(prompt_name, arguments).await.map_err(slash_command_error)?; - - prompt_result_text(&prompt_result) -} - -fn slash_command_error(error: super::error::SessionError) -> SlashCommandError { - match error { - SessionError::CommandChannel(message) => SlashCommandError::CommandChannel(message), - other => SlashCommandError::McpOperation(other.to_string()), - } -} - pub(crate) fn send_available_commands( connection: &ConnectionTo, acp_session_id: SessionId, diff --git a/crates/aether-cli/src/headless/run.rs b/crates/aether-cli/src/headless/run.rs index df33531f3..1ebf0883d 100644 --- a/crates/aether-cli/src/headless/run.rs +++ b/crates/aether-cli/src/headless/run.rs @@ -1,19 +1,16 @@ use aether_core::core::Prompt; use aether_core::events::{AgentMessage, Command}; use aether_core::mcp::run_mcp_task::McpCommand; -use rmcp::model::{GetPromptResult, Prompt as McpPrompt}; use std::io; use std::process::ExitCode; -use tokio::sync::{mpsc, oneshot}; +use tokio::sync::mpsc; use tracing::error; use super::error::CliError; use super::{CliEventKind, RunConfig}; use crate::output::OutputFormat; use crate::runtime::RuntimeBuilder; -use crate::slash_commands::{ - SlashCommandError, find_prompt_name, parse_slash_command, parse_slash_command_arguments, prompt_result_text, -}; +use crate::slash_commands::{expand_slash_command, parse_slash_command}; pub async fn run(config: RunConfig) -> Result { setup_tracing(config.verbose); @@ -54,46 +51,6 @@ async fn expand_prompt(mcp_tx: &mpsc::Sender, prompt: String) -> Str } } -async fn expand_slash_command( - mcp_tx: &mpsc::Sender, - command_name: &str, - args_text: &str, -) -> Result { - let arguments = parse_slash_command_arguments(args_text); - let prompts = list_prompts(mcp_tx).await?; - let prompt_name = find_prompt_name(&prompts, command_name)?; - let prompt_result = get_prompt(mcp_tx, prompt_name, arguments).await?; - prompt_result_text(&prompt_result) -} - -async fn list_prompts(mcp_tx: &mpsc::Sender) -> Result, SlashCommandError> { - let (tx, rx) = oneshot::channel(); - mcp_tx - .send(McpCommand::ListPrompts { tx }) - .await - .map_err(|e| SlashCommandError::CommandChannel(format!("failed to send ListPrompts command: {e}")))?; - - rx.await - .map_err(|e| SlashCommandError::CommandChannel(format!("failed to receive prompts: {e}")))? - .map_err(SlashCommandError::McpOperation) -} - -async fn get_prompt( - mcp_tx: &mpsc::Sender, - name: String, - arguments: Option>, -) -> Result { - let (tx, rx) = oneshot::channel(); - mcp_tx - .send(McpCommand::GetPrompt { name, arguments, tx }) - .await - .map_err(|e| SlashCommandError::CommandChannel(format!("failed to send GetPrompt command: {e}")))?; - - rx.await - .map_err(|e| SlashCommandError::CommandChannel(format!("failed to receive prompt: {e}")))? - .map_err(SlashCommandError::McpOperation) -} - async fn stream_output( mut rx: mpsc::Receiver, format: OutputFormat, diff --git a/crates/aether-cli/src/init/build_settings.rs b/crates/aether-cli/src/init/build_settings.rs index ec4b3f702..28bfea367 100644 --- a/crates/aether-cli/src/init/build_settings.rs +++ b/crates/aether-cli/src/init/build_settings.rs @@ -56,7 +56,7 @@ pub fn build_batteries_included_settings( } } -pub fn build_batteries_included_plan_agent( +fn build_batteries_included_plan_agent( display: &str, model: String, reasoning_effort: Option, @@ -83,7 +83,7 @@ pub fn build_batteries_included_plan_agent( } } -pub fn build_batteries_included_build_agent( +fn build_batteries_included_build_agent( display: &str, model: String, reasoning_effort: Option, @@ -108,7 +108,7 @@ pub fn build_batteries_included_build_agent( } } -pub fn build_batteries_included_explore_agent( +fn build_batteries_included_explore_agent( model: String, reasoning_effort: Option, scope: InitScope, diff --git a/crates/aether-cli/src/init/mod.rs b/crates/aether-cli/src/init/mod.rs index fdc1e0375..af8ad44ce 100644 --- a/crates/aether-cli/src/init/mod.rs +++ b/crates/aether-cli/src/init/mod.rs @@ -3,10 +3,7 @@ mod harness; mod recommendations; mod tui_runner; use aether_project::user_settings_path; -pub use build_settings::{ - Preset, build_batteries_included_build_agent, build_batteries_included_explore_agent, - build_batteries_included_plan_agent, build_batteries_included_settings, -}; +pub use build_settings::{Preset, build_batteries_included_settings}; pub use harness::HarnessIntegration; use llm::catalog::Provider; use recommendations::recommended_for_provider; diff --git a/crates/aether-cli/src/slash_commands.rs b/crates/aether-cli/src/slash_commands.rs index e33fa8e88..42d42f95e 100644 --- a/crates/aether-cli/src/slash_commands.rs +++ b/crates/aether-cli/src/slash_commands.rs @@ -1,7 +1,9 @@ +use aether_core::mcp::run_mcp_task::McpCommand; use agent_client_protocol::schema::AvailableCommand; use rmcp::model::{GetPromptResult, Prompt as McpPrompt, PromptMessageContent}; use std::collections::HashSet; use thiserror::Error; +use tokio::sync::{mpsc, oneshot}; pub(crate) struct SlashCommand<'a> { pub(crate) command_name: &'a str, @@ -38,7 +40,47 @@ pub(crate) fn dedupe_commands_by_name(commands: Vec) -> Vec Result { +pub(crate) async fn expand_slash_command( + mcp_tx: &mpsc::Sender, + command_name: &str, + args_text: &str, +) -> Result { + let arguments = parse_slash_command_arguments(args_text); + let prompts = list_prompts(mcp_tx).await?; + let prompt_name = find_prompt_name(&prompts, command_name)?; + let prompt_result = get_prompt(mcp_tx, prompt_name, arguments).await?; + prompt_result_text(&prompt_result) +} + +pub(crate) async fn list_prompts(mcp_tx: &mpsc::Sender) -> Result, SlashCommandError> { + let (tx, rx) = oneshot::channel(); + mcp_tx + .send(McpCommand::ListPrompts { tx }) + .await + .map_err(|e| SlashCommandError::CommandChannel(format!("failed to send ListPrompts command: {e}")))?; + + rx.await + .map_err(|e| SlashCommandError::CommandChannel(format!("failed to receive prompts: {e}")))? + .map_err(SlashCommandError::McpOperation) +} + +async fn get_prompt( + mcp_tx: &mpsc::Sender, + name: String, + arguments: Option>, +) -> Result { + let (tx, rx) = oneshot::channel(); + mcp_tx + .send(McpCommand::GetPrompt { name, arguments, tx }) + .await + .map_err(|e| SlashCommandError::CommandChannel(format!("failed to send GetPrompt command: {e}")))?; + + rx.await + .map_err(|e| SlashCommandError::CommandChannel(format!("failed to receive prompt: {e}")))? + .map_err(SlashCommandError::McpOperation) +} + +fn find_prompt_name(prompts: &[McpPrompt], command_name: &str) -> Result { prompts .iter() .find(|p| p.name.split("__").last().unwrap_or("") == command_name) @@ -46,7 +88,7 @@ pub(crate) fn find_prompt_name(prompts: &[McpPrompt], command_name: &str) -> Res .ok_or_else(|| SlashCommandError::NotFound(command_name.to_string())) } -pub(crate) fn prompt_result_text(prompt_result: &GetPromptResult) -> Result { +fn prompt_result_text(prompt_result: &GetPromptResult) -> Result { prompt_result .messages .first() @@ -57,7 +99,7 @@ pub(crate) fn prompt_result_text(prompt_result: &GetPromptResult) -> Result Option> { +fn parse_slash_command_arguments(args_text: &str) -> Option> { if args_text.is_empty() { None } else { diff --git a/crates/internal-evals/tests/plan_agent.rs b/crates/internal-evals/tests/plan_agent.rs index 8e7b13d42..c9a8a237b 100644 --- a/crates/internal-evals/tests/plan_agent.rs +++ b/crates/internal-evals/tests/plan_agent.rs @@ -4,20 +4,23 @@ use aether_project::{AetherSettings, McpSourceSpec}; use internal_evals::{EvalAgent, EvalHarnessError, batteries_included_settings}; use std::fs::read_to_string; +const NOTES_TXT_CONTENT: &str = "old value\n"; +const EDIT_NOTES_PROMPT: &str = + "/plan Edit notes.txt, replace 'old value' with 'new value'. Don't make a plan, just make change."; + #[tokio::test] async fn plan_agent_reports_missing_non_plan_edit_tools_eval() -> Result<(), EvalHarnessError> { - let workspace = Workspace::from_files([("notes.txt", "old value\n")])?; - let prompt = - "/plan Edit notes.txt, replace 'old value' with 'new value'. Don't make a plan, just make change.".to_string(); + let workspace = Workspace::from_files([("notes.txt", NOTES_TXT_CONTENT)])?; - let (_container, stream) = EvalAgent::new().agent("Plan").run(&workspace, Task::new(prompt)).await?; + let (_container, stream) = + EvalAgent::new().agent("Plan").run(&workspace, Task::new(EDIT_NOTES_PROMPT.to_string())).await?; let trace = Transcript::from_stream(stream).await?; let final_message = final_text_message(&trace); assert_eq!(trace.tool_call_count("coding__edit_file"), 0); assert_eq!(trace.tool_call_count("coding__write_file"), 0); assert_eq!(trace.tool_call_count("plan__write_plan"), 0); - assert_eq!(read_to_string(workspace.join("notes.txt"))?, "old value\n"); + assert_eq!(read_to_string(workspace.join("notes.txt"))?, NOTES_TXT_CONTENT); assert!( final_message.contains("I don't have tools to modify non-plan files, you must switch to another agent"), "unexpected final message: {final_message}" @@ -28,19 +31,20 @@ async fn plan_agent_reports_missing_non_plan_edit_tools_eval() -> Result<(), Eva #[tokio::test] async fn plan_prompt_with_edit_tools_plans_before_modifying_non_plan_files_eval() -> Result<(), EvalHarnessError> { - let workspace = Workspace::from_files([("notes.txt", "old value\n")])?; - let prompt = - "/plan Edit notes.txt, replace 'old value' with 'new value'. Don't make a plan, just make change.".to_string(); + let workspace = Workspace::from_files([("notes.txt", NOTES_TXT_CONTENT)])?; let settings = build_agent_settings_with_plan_mcp()?; - let (_container, stream) = - EvalAgent::new().settings(settings).agent("Build").run(&workspace, Task::new(prompt)).await?; + let (_container, stream) = EvalAgent::new() + .settings(settings) + .agent("Build") + .run(&workspace, Task::new(EDIT_NOTES_PROMPT.to_string())) + .await?; let trace = Transcript::from_stream(stream).await?; let final_message = final_text_message(&trace).to_lowercase(); assert_eq!(trace.tool_call_count("coding__edit_file"), 0); assert_eq!(trace.tool_call_count("coding__write_file"), 0); - assert_eq!(read_to_string(workspace.join("notes.txt"))?, "old value\n"); + assert_eq!(read_to_string(workspace.join("notes.txt"))?, NOTES_TXT_CONTENT); assert!( final_message.contains("would you like to exit plan mode?"), "expected explicit approval request before editing, got: {final_message}"