Nice errors for unsupported ACP slash commands (#37393)
If we get back slash-commands that aren't supported, tell the user that this is the problem. Release Notes: - Improve error messages for unsupported ACP slash-commands --------- Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
This commit is contained in:
co-authored by
Conrad Irwin
parent
88a79750cc
commit
5e01fb8f1c
@@ -11,7 +11,7 @@ use collections::HashMap;
|
||||
use editor::{Editor, EditorMode, MinimapVisibility};
|
||||
use gpui::{
|
||||
AnyEntity, App, AppContext as _, Entity, EntityId, EventEmitter, FocusHandle, Focusable,
|
||||
ScrollHandle, TextStyleRefinement, WeakEntity, Window,
|
||||
ScrollHandle, SharedString, TextStyleRefinement, WeakEntity, Window,
|
||||
};
|
||||
use language::language_settings::SoftWrap;
|
||||
use project::Project;
|
||||
@@ -32,6 +32,7 @@ pub struct EntryViewState {
|
||||
entries: Vec<Entry>,
|
||||
prompt_capabilities: Rc<Cell<acp::PromptCapabilities>>,
|
||||
available_commands: Rc<RefCell<Vec<acp::AvailableCommand>>>,
|
||||
agent_name: SharedString,
|
||||
}
|
||||
|
||||
impl EntryViewState {
|
||||
@@ -42,6 +43,7 @@ impl EntryViewState {
|
||||
prompt_store: Option<Entity<PromptStore>>,
|
||||
prompt_capabilities: Rc<Cell<acp::PromptCapabilities>>,
|
||||
available_commands: Rc<RefCell<Vec<acp::AvailableCommand>>>,
|
||||
agent_name: SharedString,
|
||||
) -> Self {
|
||||
Self {
|
||||
workspace,
|
||||
@@ -51,6 +53,7 @@ impl EntryViewState {
|
||||
entries: Vec::new(),
|
||||
prompt_capabilities,
|
||||
available_commands,
|
||||
agent_name,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,6 +93,7 @@ impl EntryViewState {
|
||||
self.prompt_store.clone(),
|
||||
self.prompt_capabilities.clone(),
|
||||
self.available_commands.clone(),
|
||||
self.agent_name.clone(),
|
||||
"Edit message - @ to include context",
|
||||
editor::EditorMode::AutoHeight {
|
||||
min_lines: 1,
|
||||
@@ -476,6 +480,7 @@ mod tests {
|
||||
None,
|
||||
Default::default(),
|
||||
Default::default(),
|
||||
"Test Agent".into(),
|
||||
)
|
||||
});
|
||||
|
||||
|
||||
@@ -22,8 +22,8 @@ use futures::{
|
||||
};
|
||||
use gpui::{
|
||||
Animation, AnimationExt as _, AppContext, ClipboardEntry, Context, Entity, EntityId,
|
||||
EventEmitter, FocusHandle, Focusable, Image, ImageFormat, Img, KeyContext, Subscription, Task,
|
||||
TextStyle, WeakEntity, pulsating_between,
|
||||
EventEmitter, FocusHandle, Focusable, Image, ImageFormat, Img, KeyContext, SharedString,
|
||||
Subscription, Task, TextStyle, WeakEntity, pulsating_between,
|
||||
};
|
||||
use language::{Buffer, Language, language_settings::InlayHintKind};
|
||||
use language_model::LanguageModelImage;
|
||||
@@ -49,8 +49,8 @@ use theme::ThemeSettings;
|
||||
use ui::{
|
||||
ActiveTheme, AnyElement, App, ButtonCommon, ButtonLike, ButtonStyle, Color, Element as _,
|
||||
FluentBuilder as _, Icon, IconName, IconSize, InteractiveElement, IntoElement, Label,
|
||||
LabelCommon, LabelSize, ParentElement, Render, SelectableButton, SharedString, Styled,
|
||||
TextSize, TintColor, Toggleable, Window, div, h_flex,
|
||||
LabelCommon, LabelSize, ParentElement, Render, SelectableButton, Styled, TextSize, TintColor,
|
||||
Toggleable, Window, div, h_flex,
|
||||
};
|
||||
use util::{ResultExt, debug_panic};
|
||||
use workspace::{Workspace, notifications::NotifyResultExt as _};
|
||||
@@ -65,6 +65,7 @@ pub struct MessageEditor {
|
||||
prompt_store: Option<Entity<PromptStore>>,
|
||||
prompt_capabilities: Rc<Cell<acp::PromptCapabilities>>,
|
||||
available_commands: Rc<RefCell<Vec<acp::AvailableCommand>>>,
|
||||
agent_name: SharedString,
|
||||
_subscriptions: Vec<Subscription>,
|
||||
_parse_slash_command_task: Task<()>,
|
||||
}
|
||||
@@ -89,6 +90,7 @@ impl MessageEditor {
|
||||
prompt_store: Option<Entity<PromptStore>>,
|
||||
prompt_capabilities: Rc<Cell<acp::PromptCapabilities>>,
|
||||
available_commands: Rc<RefCell<Vec<acp::AvailableCommand>>>,
|
||||
agent_name: SharedString,
|
||||
placeholder: impl Into<Arc<str>>,
|
||||
mode: EditorMode,
|
||||
window: &mut Window,
|
||||
@@ -179,6 +181,7 @@ impl MessageEditor {
|
||||
prompt_store,
|
||||
prompt_capabilities,
|
||||
available_commands,
|
||||
agent_name,
|
||||
_subscriptions: subscriptions,
|
||||
_parse_slash_command_task: Task::ready(()),
|
||||
}
|
||||
@@ -731,10 +734,52 @@ impl MessageEditor {
|
||||
})
|
||||
}
|
||||
|
||||
fn validate_slash_commands(
|
||||
text: &str,
|
||||
available_commands: &[acp::AvailableCommand],
|
||||
agent_name: &str,
|
||||
) -> Result<()> {
|
||||
if let Some(parsed_command) = SlashCommandCompletion::try_parse(text, 0) {
|
||||
if let Some(command_name) = parsed_command.command {
|
||||
// Check if this command is in the list of available commands from the server
|
||||
let is_supported = available_commands
|
||||
.iter()
|
||||
.any(|cmd| cmd.name == command_name);
|
||||
|
||||
if !is_supported {
|
||||
return Err(anyhow!(
|
||||
"The /{} command is not supported by {}.\n\nAvailable commands: {}",
|
||||
command_name,
|
||||
agent_name,
|
||||
if available_commands.is_empty() {
|
||||
"none".to_string()
|
||||
} else {
|
||||
available_commands
|
||||
.iter()
|
||||
.map(|cmd| format!("/{}", cmd.name))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
}
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn contents(
|
||||
&self,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Task<Result<(Vec<acp::ContentBlock>, Vec<Entity<Buffer>>)>> {
|
||||
// Check for unsupported slash commands before spawning async task
|
||||
let text = self.editor.read(cx).text(cx);
|
||||
let available_commands = self.available_commands.borrow().clone();
|
||||
if let Err(err) =
|
||||
Self::validate_slash_commands(&text, &available_commands, &self.agent_name)
|
||||
{
|
||||
return Task::ready(Err(err));
|
||||
}
|
||||
|
||||
let contents = self
|
||||
.mention_set
|
||||
.contents(&self.prompt_capabilities.get(), cx);
|
||||
@@ -744,7 +789,7 @@ impl MessageEditor {
|
||||
let contents = contents.await?;
|
||||
let mut all_tracked_buffers = Vec::new();
|
||||
|
||||
editor.update(cx, |editor, cx| {
|
||||
let result = editor.update(cx, |editor, cx| {
|
||||
let mut ix = 0;
|
||||
let mut chunks: Vec<acp::ContentBlock> = Vec::new();
|
||||
let text = editor.text(cx);
|
||||
@@ -837,9 +882,9 @@ impl MessageEditor {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
(chunks, all_tracked_buffers)
|
||||
})
|
||||
Ok((chunks, all_tracked_buffers))
|
||||
})?;
|
||||
result
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1573,6 +1618,7 @@ mod tests {
|
||||
None,
|
||||
Default::default(),
|
||||
Default::default(),
|
||||
"Test Agent".into(),
|
||||
"Test",
|
||||
EditorMode::AutoHeight {
|
||||
min_lines: 1,
|
||||
@@ -1650,6 +1696,140 @@ mod tests {
|
||||
pretty_assertions::assert_matches!(content.as_slice(), [acp::ContentBlock::Text { .. }]);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_slash_command_validation(cx: &mut gpui::TestAppContext) {
|
||||
init_test(cx);
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
"/test",
|
||||
json!({
|
||||
".zed": {
|
||||
"tasks.json": r#"[{"label": "test", "command": "echo"}]"#
|
||||
},
|
||||
"src": {
|
||||
"main.rs": "fn main() {}",
|
||||
},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await;
|
||||
let context_store = cx.new(|cx| ContextStore::fake(project.clone(), cx));
|
||||
let history_store = cx.new(|cx| HistoryStore::new(context_store, cx));
|
||||
let prompt_capabilities = Rc::new(Cell::new(acp::PromptCapabilities::default()));
|
||||
// Start with no available commands - simulating Claude which doesn't support slash commands
|
||||
let available_commands = Rc::new(RefCell::new(vec![]));
|
||||
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
let workspace_handle = workspace.downgrade();
|
||||
let message_editor = workspace.update_in(cx, |_, window, cx| {
|
||||
cx.new(|cx| {
|
||||
MessageEditor::new(
|
||||
workspace_handle.clone(),
|
||||
project.clone(),
|
||||
history_store.clone(),
|
||||
None,
|
||||
prompt_capabilities.clone(),
|
||||
available_commands.clone(),
|
||||
"Claude Code".into(),
|
||||
"Test",
|
||||
EditorMode::AutoHeight {
|
||||
min_lines: 1,
|
||||
max_lines: None,
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
});
|
||||
let editor = message_editor.update(cx, |message_editor, _| message_editor.editor.clone());
|
||||
|
||||
// Test that slash commands fail when no available_commands are set (empty list means no commands supported)
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.set_text("/file test.txt", window, cx);
|
||||
});
|
||||
|
||||
let contents_result = message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx))
|
||||
.await;
|
||||
|
||||
// Should fail because available_commands is empty (no commands supported)
|
||||
assert!(contents_result.is_err());
|
||||
let error_message = contents_result.unwrap_err().to_string();
|
||||
assert!(error_message.contains("not supported by Claude Code"));
|
||||
assert!(error_message.contains("Available commands: none"));
|
||||
|
||||
// Now simulate Claude providing its list of available commands (which doesn't include file)
|
||||
available_commands.replace(vec![acp::AvailableCommand {
|
||||
name: "help".to_string(),
|
||||
description: "Get help".to_string(),
|
||||
input: None,
|
||||
}]);
|
||||
|
||||
// Test that unsupported slash commands trigger an error when we have a list of available commands
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.set_text("/file test.txt", window, cx);
|
||||
});
|
||||
|
||||
let contents_result = message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx))
|
||||
.await;
|
||||
|
||||
assert!(contents_result.is_err());
|
||||
let error_message = contents_result.unwrap_err().to_string();
|
||||
assert!(error_message.contains("not supported by Claude Code"));
|
||||
assert!(error_message.contains("/file"));
|
||||
assert!(error_message.contains("Available commands: /help"));
|
||||
|
||||
// Test that supported commands work fine
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.set_text("/help", window, cx);
|
||||
});
|
||||
|
||||
let contents_result = message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx))
|
||||
.await;
|
||||
|
||||
// Should succeed because /help is in available_commands
|
||||
assert!(contents_result.is_ok());
|
||||
|
||||
// Test that regular text works fine
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.set_text("Hello Claude!", window, cx);
|
||||
});
|
||||
|
||||
let (content, _) = message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(content.len(), 1);
|
||||
if let acp::ContentBlock::Text(text) = &content[0] {
|
||||
assert_eq!(text.text, "Hello Claude!");
|
||||
} else {
|
||||
panic!("Expected ContentBlock::Text");
|
||||
}
|
||||
|
||||
// Test that @ mentions still work
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.set_text("Check this @", window, cx);
|
||||
});
|
||||
|
||||
// The @ mention functionality should not be affected
|
||||
let (content, _) = message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(content.len(), 1);
|
||||
if let acp::ContentBlock::Text(text) = &content[0] {
|
||||
assert_eq!(text.text, "Check this @");
|
||||
} else {
|
||||
panic!("Expected ContentBlock::Text");
|
||||
}
|
||||
}
|
||||
|
||||
struct MessageEditorItem(Entity<MessageEditor>);
|
||||
|
||||
impl Item for MessageEditorItem {
|
||||
@@ -1725,6 +1905,7 @@ mod tests {
|
||||
None,
|
||||
prompt_capabilities.clone(),
|
||||
available_commands.clone(),
|
||||
"Test Agent".into(),
|
||||
"Test",
|
||||
EditorMode::AutoHeight {
|
||||
max_lines: None,
|
||||
@@ -1957,6 +2138,7 @@ mod tests {
|
||||
None,
|
||||
prompt_capabilities.clone(),
|
||||
Default::default(),
|
||||
"Test Agent".into(),
|
||||
"Test",
|
||||
EditorMode::AutoHeight {
|
||||
max_lines: None,
|
||||
|
||||
@@ -342,6 +342,7 @@ impl AcpThreadView {
|
||||
prompt_store.clone(),
|
||||
prompt_capabilities.clone(),
|
||||
available_commands.clone(),
|
||||
agent.name(),
|
||||
placeholder,
|
||||
editor::EditorMode::AutoHeight {
|
||||
min_lines: MIN_EDITOR_LINES,
|
||||
@@ -366,6 +367,7 @@ impl AcpThreadView {
|
||||
prompt_store.clone(),
|
||||
prompt_capabilities.clone(),
|
||||
available_commands.clone(),
|
||||
agent.name(),
|
||||
)
|
||||
});
|
||||
|
||||
@@ -495,8 +497,25 @@ impl AcpThreadView {
|
||||
Ok(thread) => {
|
||||
let action_log = thread.read(cx).action_log().clone();
|
||||
|
||||
this.available_commands
|
||||
.replace(thread.read(cx).available_commands());
|
||||
let mut available_commands = thread.read(cx).available_commands();
|
||||
|
||||
if connection
|
||||
.auth_methods()
|
||||
.iter()
|
||||
.any(|method| method.id.0.as_ref() == "claude-login")
|
||||
{
|
||||
available_commands.push(acp::AvailableCommand {
|
||||
name: "login".to_owned(),
|
||||
description: "Authenticate".to_owned(),
|
||||
input: None,
|
||||
});
|
||||
available_commands.push(acp::AvailableCommand {
|
||||
name: "logout".to_owned(),
|
||||
description: "Authenticate".to_owned(),
|
||||
input: None,
|
||||
});
|
||||
}
|
||||
this.available_commands.replace(available_commands);
|
||||
|
||||
this.prompt_capabilities
|
||||
.set(thread.read(cx).prompt_capabilities());
|
||||
@@ -914,6 +933,40 @@ impl AcpThreadView {
|
||||
return;
|
||||
}
|
||||
|
||||
let text = self.message_editor.read(cx).text(cx);
|
||||
let text = text.trim();
|
||||
if text == "/login" || text == "/logout" {
|
||||
let ThreadState::Ready { thread, .. } = &self.thread_state else {
|
||||
return;
|
||||
};
|
||||
|
||||
let connection = thread.read(cx).connection().clone();
|
||||
if !connection
|
||||
.auth_methods()
|
||||
.iter()
|
||||
.any(|method| method.id.0.as_ref() == "claude-login")
|
||||
{
|
||||
return;
|
||||
};
|
||||
let this = cx.weak_entity();
|
||||
let agent = self.agent.clone();
|
||||
window.defer(cx, |window, cx| {
|
||||
Self::handle_auth_required(
|
||||
this,
|
||||
AuthRequired {
|
||||
description: None,
|
||||
provider_id: Some(language_model::ANTHROPIC_PROVIDER_ID),
|
||||
},
|
||||
agent,
|
||||
connection,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
cx.notify();
|
||||
return;
|
||||
}
|
||||
|
||||
let contents = self
|
||||
.message_editor
|
||||
.update(cx, |message_editor, cx| message_editor.contents(cx));
|
||||
|
||||
Reference in New Issue
Block a user