From fdd8ada03243bc7ee52a1e3d2e250856391df88f Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Thu, 16 Apr 2026 13:51:40 -0500 Subject: [PATCH] feat: add /edit command to cli for on-demand prompt editing (#8566) Signed-off-by: Adam Miller --- crates/goose-cli/src/session/editor.rs | 245 ++++++++++++++++++++++--- crates/goose-cli/src/session/input.rs | 194 ++++++++++++-------- crates/goose-cli/src/session/mod.rs | 32 +++- crates/goose/src/config/base.rs | 1 + 4 files changed, 372 insertions(+), 100 deletions(-) diff --git a/crates/goose-cli/src/session/editor.rs b/crates/goose-cli/src/session/editor.rs index 2709f1a1..1e42d606 100644 --- a/crates/goose-cli/src/session/editor.rs +++ b/crates/goose-cli/src/session/editor.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use goose::config::Config; use std::fs; use std::io::Read; use std::path::PathBuf; @@ -6,15 +7,47 @@ use std::process::Command; use tempfile::Builder; use tempfile::NamedTempFile; -/// Create temporary markdown file with conversation history -fn create_temp_file(messages: &[&str]) -> Result { - let temp_file = Builder::new() - .prefix("goose_prompt_") - .suffix(".md") - .tempfile()?; +/// Resolve the editor command from config and environment variables. +/// Checks GOOSE_PROMPT_EDITOR, then $VISUAL, then $EDITOR. +pub fn resolve_editor_command() -> Option { + let config = Config::global(); + let config_editor = config.get_goose_prompt_editor().ok().flatten(); + let visual = std::env::var("VISUAL").ok(); + let editor_env = std::env::var("EDITOR").ok(); + resolve_editor_from_sources( + config_editor.as_deref(), + visual.as_deref(), + editor_env.as_deref(), + ) +} + +/// Inner resolution logic, separated for testability. +/// Checks sources in priority order: config, VISUAL, EDITOR. +/// Skips empty strings at each level. +fn resolve_editor_from_sources( + config_editor: Option<&str>, + visual: Option<&str>, + editor_env: Option<&str>, +) -> Option { + for cmd in [config_editor, visual, editor_env].into_iter().flatten() { + if !cmd.is_empty() { + return Some(cmd.to_string()); + } + } + None +} + +/// Build the markdown template content for the editor prompt. +fn build_template(messages: &[&str], prefill: Option<&str>) -> String { let mut content = String::from("# Goose Prompt Editor\n\n"); content.push_str("# Your prompt:\n\n"); + if let Some(text) = prefill { + if !text.is_empty() { + content.push_str(text); + content.push('\n'); + } + } if !messages.is_empty() { content.push_str("# Recent conversation for context (newest first):\n\n"); @@ -24,7 +57,17 @@ fn create_temp_file(messages: &[&str]) -> Result { content.push('\n'); } - fs::write(temp_file.path(), content)?; + content +} + +/// Create temporary markdown file with conversation history and optional prefill text +fn create_temp_file(messages: &[&str], prefill: Option<&str>) -> Result { + let temp_file = Builder::new() + .prefix("goose_prompt_") + .suffix(".md") + .tempfile()?; + + fs::write(temp_file.path(), build_template(messages, prefill))?; Ok(temp_file) } @@ -80,8 +123,12 @@ fn launch_editor(editor_cmd: &str, file_path: &PathBuf) -> Result<()> { } /// Main function to get input from editor -pub fn get_editor_input(editor_cmd: &str, messages: &[&str]) -> Result<(String, bool)> { - let temp_file = create_temp_file(messages)?; +pub fn get_editor_input( + editor_cmd: &str, + messages: &[&str], + prefill: Option<&str>, +) -> Result<(String, bool)> { + let temp_file = create_temp_file(messages, prefill)?; let temp_path = temp_file.path().to_path_buf(); let symlink_path = PathBuf::from(".goose_prompt_temp.md"); @@ -98,18 +145,7 @@ pub fn get_editor_input(editor_cmd: &str, messages: &[&str]) -> Result<(String, let _cleanup_guard = SymlinkCleanup::new(symlink_path.clone()); - let _original_template = { - let mut template_content = String::from("# Goose Prompt Editor\n\n"); - template_content.push_str("# Your prompt:\n\n"); - if !messages.is_empty() { - template_content.push_str("# Recent conversation for context (newest first):\n\n"); - for message in messages.iter().rev() { - template_content.push_str(&format!("{}\n", message)); - } - template_content.push('\n'); - } - template_content - }; + let _original_template = build_template(messages, prefill); launch_editor(editor_cmd, &symlink_path)?; @@ -209,7 +245,7 @@ This is the user's input fn test_create_temp_file_with_messages() { let messages = vec!["## User: Hello", "## Assistant: Hi there!"]; - let temp_file = create_temp_file(&messages).unwrap(); + let temp_file = create_temp_file(&messages, None).unwrap(); let path = temp_file.path(); assert!(path.exists()); @@ -224,6 +260,33 @@ This is the user's input assert!(content.contains("# Recent conversation for context (newest first):")); } + #[test] + fn test_create_temp_file_with_prefill() { + let messages = vec!["## User: Hello"]; + let temp_file = create_temp_file(&messages, Some("fix the login bug")).unwrap(); + let content = fs::read_to_string(temp_file.path()).unwrap(); + + assert!(content.contains("# Your prompt:")); + assert!(content.contains("fix the login bug")); + // Prefill text should appear before conversation context + let prefill_pos = content.find("fix the login bug").unwrap(); + let context_pos = content.find("# Recent conversation for context").unwrap(); + assert!( + prefill_pos < context_pos, + "Prefill text should appear before conversation context" + ); + } + + #[test] + fn test_create_temp_file_without_prefill() { + let messages = vec!["## User: Hello"]; + let temp_file = create_temp_file(&messages, None).unwrap(); + let content = fs::read_to_string(temp_file.path()).unwrap(); + + assert!(content.contains("# Your prompt:")); + assert!(!content.contains("fix the login bug")); + } + #[test] fn test_create_temp_file_with_prefix_suffix() { let temp_file = Builder::new() @@ -280,7 +343,7 @@ with multiple lines. "## User: Third message (newest)", ]; - let temp_file = create_temp_file(&messages).unwrap(); + let temp_file = create_temp_file(&messages, None).unwrap(); let content = fs::read_to_string(temp_file.path()).unwrap(); let newest_first = [ @@ -314,7 +377,7 @@ with multiple lines. use std::panic; let messages = vec!["## User: Test message for panic cleanup"]; - let temp_file = create_temp_file(&messages).unwrap(); + let temp_file = create_temp_file(&messages, None).unwrap(); let temp_path = temp_file.path().to_path_buf(); let symlink_path = PathBuf::from(format!("test_panic_cleanup_{}.md", std::process::id())); @@ -351,13 +414,145 @@ with multiple lines. ); } + // --- resolve_editor_from_sources tests --- + + #[test] + fn test_resolve_editor_returns_config_when_set() { + let result = resolve_editor_from_sources(Some("code"), Some("vim"), Some("nano")); + assert_eq!(result.as_deref(), Some("code")); + } + + #[test] + fn test_resolve_editor_falls_back_to_visual() { + let result = resolve_editor_from_sources(None, Some("vim"), Some("nano")); + assert_eq!(result.as_deref(), Some("vim")); + } + + #[test] + fn test_resolve_editor_falls_back_to_editor_env() { + let result = resolve_editor_from_sources(None, None, Some("nano")); + assert_eq!(result.as_deref(), Some("nano")); + } + + #[test] + fn test_resolve_editor_returns_none_when_nothing_set() { + let result = resolve_editor_from_sources(None, None, None); + assert_eq!(result, None); + } + + #[test] + fn test_resolve_editor_skips_empty_config() { + let result = resolve_editor_from_sources(Some(""), Some("vim"), None); + assert_eq!(result.as_deref(), Some("vim")); + } + + #[test] + fn test_resolve_editor_skips_empty_visual() { + let result = resolve_editor_from_sources(None, Some(""), Some("nano")); + assert_eq!(result.as_deref(), Some("nano")); + } + + #[test] + fn test_resolve_editor_skips_all_empty() { + let result = resolve_editor_from_sources(Some(""), Some(""), Some("")); + assert_eq!(result, None); + } + + #[test] + fn test_resolve_editor_skips_empty_config_and_visual() { + let result = resolve_editor_from_sources(Some(""), Some(""), Some("emacs")); + assert_eq!(result.as_deref(), Some("emacs")); + } + + // --- build_template edge case tests --- + + #[test] + fn test_build_template_empty_prefill_string() { + let content = build_template(&["## User: Hello"], Some("")); + // Empty prefill should not appear in content + assert!(content.contains("# Your prompt:\n\n#")); + // Should go directly to conversation context + assert!(content.contains("# Recent conversation for context")); + } + + #[test] + fn test_build_template_prefill_with_no_messages() { + let content = build_template(&[], Some("fix the bug")); + assert!(content.contains("# Your prompt:\n\nfix the bug\n")); + assert!(!content.contains("# Recent conversation for context")); + } + + #[test] + fn test_build_template_no_prefill_no_messages() { + let content = build_template(&[], None); + assert_eq!(content, "# Goose Prompt Editor\n\n# Your prompt:\n\n"); + } + + #[test] + fn test_build_template_prefill_with_messages() { + let content = build_template(&["## User: Hi", "## Assistant: Hello"], Some("do stuff")); + assert!(content.contains("do stuff")); + assert!(content.contains("## User: Hi")); + let prefill_pos = content.find("do stuff").unwrap(); + let context_pos = content.find("# Recent conversation").unwrap(); + assert!(prefill_pos < context_pos); + } + + // --- extract_user_input with prefilled content tests --- + + #[test] + fn test_extract_user_input_with_prefill_kept() { + // Simulates a user who opened the editor with prefill and kept it unchanged + let content = build_template(&["## User: Hello"], Some("fix the login bug")); + let result = extract_user_input(&content); + assert_eq!(result, "fix the login bug"); + } + + #[test] + fn test_extract_user_input_with_prefill_edited() { + // Simulates a user who edited the prefill text + let mut content = build_template(&["## User: Hello"], Some("fix the login bug")); + content = content.replace( + "fix the login bug", + "fix the login bug and also the signup flow", + ); + let result = extract_user_input(&content); + assert_eq!(result, "fix the login bug and also the signup flow"); + } + + #[test] + fn test_extract_user_input_prefill_replaced() { + // Simulates a user who deleted the prefill and wrote something new + let mut content = build_template(&["## User: Hello"], Some("fix the login bug")); + content = content.replace("fix the login bug\n", "completely different prompt\n"); + let result = extract_user_input(&content); + assert_eq!(result, "completely different prompt"); + } + + #[test] + fn test_extract_user_input_prefill_cleared() { + // Simulates a user who deleted the prefill and left nothing + let mut content = build_template(&["## User: Hello"], Some("fix the login bug")); + content = content.replace("fix the login bug\n", ""); + let result = extract_user_input(&content); + assert_eq!(result, ""); + } + + #[test] + fn test_extract_user_input_multiline_with_prefill() { + let mut content = build_template(&["## User: Hello"], Some("line one")); + content = content.replace("line one\n", "line one\nline two\nline three\n"); + let result = extract_user_input(&content); + assert_eq!(result, "line one\nline two\nline three"); + } + #[test] #[cfg(unix)] fn test_symlink_creation_and_cleanup() { use std::os::unix::fs; let messages = vec!["## User: Test message"]; - let temp_file = create_temp_file(&messages).unwrap(); + let temp_file = create_temp_file(&messages, None).unwrap(); let temp_path = temp_file.path().to_path_buf(); let symlink_path = PathBuf::from(format!("test_symlink_cleanup_{}.md", std::process::id())); diff --git a/crates/goose-cli/src/session/input.rs b/crates/goose-cli/src/session/input.rs index af9db363..8bc067dd 100644 --- a/crates/goose-cli/src/session/input.rs +++ b/crates/goose-cli/src/session/input.rs @@ -26,6 +26,7 @@ pub enum InputResult { Recipe(Option), Compact, ToggleFullToolOutput, + Edit(Option), } #[derive(Debug)] @@ -88,22 +89,43 @@ pub fn get_newline_key() -> char { .unwrap_or('j') } +/// Determine whether the editor should be used for every prompt. +/// +/// When `goose_prompt_editor` is configured, defaults to `true` (backward compat). +/// Users can override by explicitly setting `goose_prompt_editor_always` to `false`. +/// When no editor is configured, defaults to `false`. +fn should_use_editor_always( + prompt_editor: Option<&str>, + editor_always_override: Option, +) -> bool { + let has_editor = prompt_editor.map(|s| !s.is_empty()).unwrap_or(false); + editor_always_override.unwrap_or(has_editor) +} + pub fn get_input( editor: &mut Editor, conversation_messages: Option<&Vec>, ) -> Result { let config = Config::global(); - if let Ok(Some(editor_cmd)) = config.get_goose_prompt_editor() { - let messages = extract_recent_messages(conversation_messages); - let message_refs: Vec<&str> = messages.iter().map(|s| s.as_str()).collect(); - let (message, has_meaningful_content) = - crate::session::editor::get_editor_input(&editor_cmd, &message_refs)?; + let prompt_editor = config.get_goose_prompt_editor().ok().flatten(); + let editor_always_override = config.get_goose_prompt_editor_always().ok().flatten(); + let editor_always = should_use_editor_always(prompt_editor.as_deref(), editor_always_override); - if !has_meaningful_content { - return get_regular_input(editor); + if editor_always { + if let Ok(Some(editor_cmd)) = config.get_goose_prompt_editor() { + if !editor_cmd.is_empty() { + let messages = extract_recent_messages(conversation_messages); + let message_refs: Vec<&str> = messages.iter().map(|s| s.as_str()).collect(); + let (message, has_meaningful_content) = + crate::session::editor::get_editor_input(&editor_cmd, &message_refs, None)?; + + if has_meaningful_content { + editor.add_history_entry(message.as_str())?; + return Ok(InputResult::Message(message)); + } + // Empty editor content — fall through to inline prompt + } } - editor.add_history_entry(message.as_str())?; - return Ok(InputResult::Message(message)); } let completion_cache = editor @@ -164,67 +186,6 @@ pub fn get_input( } } -fn get_regular_input( - editor: &mut Editor, -) -> Result { - let completion_cache = editor - .helper() - .map(|h| h.completion_cache.clone()) - .ok_or_else(|| anyhow::anyhow!("Editor helper not set"))?; - - let newline_key = get_newline_key(); - editor.bind_sequence( - rustyline::KeyEvent( - rustyline::KeyCode::Char(newline_key), - rustyline::Modifiers::CTRL, - ), - rustyline::EventHandler::Simple(rustyline::Cmd::Newline), - ); - - editor.bind_sequence( - rustyline::KeyEvent(rustyline::KeyCode::Char('c'), rustyline::Modifiers::CTRL), - rustyline::EventHandler::Conditional(Box::new(CtrlCHandler::new(completion_cache))), - ); - - let prompt = get_input_prompt_string(); - - let input = match editor.readline(&prompt) { - Ok(text) => text, - Err(e) => match e { - rustyline::error::ReadlineError::Interrupted => return Ok(InputResult::Exit), - rustyline::error::ReadlineError::Eof => return Ok(InputResult::Exit), - _ => return Err(e.into()), - }, - }; - - // Add valid input to history (history saving to file is handled in the Session::interactive method) - if !input.trim().is_empty() { - editor.add_history_entry(input.as_str())?; - } - - // Handle non-slash commands first - if !input.starts_with('/') { - let trimmed = input.trim(); - if trimmed.is_empty() - || trimmed.eq_ignore_ascii_case("exit") - || trimmed.eq_ignore_ascii_case("quit") - { - return Ok(if trimmed.is_empty() { - InputResult::Retry - } else { - InputResult::Exit - }); - } - return Ok(InputResult::Message(trimmed.to_string())); - } - - // Handle slash commands - match handle_slash_command(&input) { - Some(result) => Ok(result), - None => Ok(InputResult::Message(input.trim().to_string())), - } -} - fn handle_slash_command(input: &str) -> Option { let input = input.trim(); @@ -241,6 +202,8 @@ fn handle_slash_command(input: &str) -> Option { const CMD_RECIPE: &str = "/recipe"; const CMD_COMPACT: &str = "/compact"; const CMD_SUMMARIZE_DEPRECATED: &str = "/summarize"; + const CMD_EDIT: &str = "/edit"; + const CMD_EDIT_WITH_SPACE: &str = "/edit "; match input { "/exit" | "/quit" => Some(InputResult::Exit), @@ -309,6 +272,18 @@ fn handle_slash_command(input: &str) -> Option { Some(InputResult::Compact) } "/r" => Some(InputResult::ToggleFullToolOutput), + s if s == CMD_EDIT => Some(InputResult::Edit(None)), + s if s.starts_with(CMD_EDIT_WITH_SPACE) => { + let prefill = s + .strip_prefix(CMD_EDIT_WITH_SPACE) + .unwrap_or_default() + .trim(); + if prefill.is_empty() { + Some(InputResult::Edit(None)) + } else { + Some(InputResult::Edit(Some(prefill.to_string()))) + } + } _ => None, } } @@ -440,6 +415,8 @@ fn print_help() { /recipe [filepath] - Generate a recipe from the current conversation and save it to the specified filepath (must end with .yaml). If no filepath is provided, it will be saved to ./recipe.yaml. /compact - Compact the current conversation to reduce context length while preserving key information. +/edit [text] - Open your prompt editor to compose a message. Optionally pre-fill with text. + Uses $GOOSE_PROMPT_EDITOR, $VISUAL, or $EDITOR (in that order). /? or /help - Display this help message /clear - Clears the current chat history @@ -451,7 +428,7 @@ Up/Down arrows - Navigate through command history" } /// Extract recent messages for editor context -fn extract_recent_messages(conversation_messages: Option<&Vec>) -> Vec { +pub(super) fn extract_recent_messages(conversation_messages: Option<&Vec>) -> Vec { match conversation_messages { Some(messages) => { // Return the messages in reverse chronological order (newest first) @@ -465,9 +442,13 @@ fn extract_recent_messages(conversation_messages: Option<&Vec>) -> Vec, + conversation_messages: &[String], ) -> Result<()> { match input { InputResult::Message(content) => { @@ -635,6 +636,35 @@ impl CliSession { history.save(editor); self.handle_compact().await?; } + InputResult::Edit(prefill) => { + history.save(editor); + match crate::session::editor::resolve_editor_command() { + Some(editor_cmd) => { + let messages: Vec<&str> = + conversation_messages.iter().map(|s| s.as_str()).collect(); + match crate::session::editor::get_editor_input( + &editor_cmd, + &messages, + prefill.as_deref(), + ) { + Ok((message, true)) => { + self.handle_message_input(&message, history, editor).await?; + } + Ok((_, false)) => {} + Err(e) => { + output::render_error(&format!("Failed to open editor: {}", e)); + } + } + } + None => { + output::render_error( + "No editor found. Set one with:\n \ + goose configure set goose_prompt_editor \"vim\"\n \ + or set $VISUAL or $EDITOR in your shell.", + ); + } + } + } } Ok(()) } diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index b6698b5a..93b562ca 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -1058,6 +1058,7 @@ config_value!(GOOSE_MODE, GooseMode); config_value!(GOOSE_PROVIDER, String); config_value!(GOOSE_MODEL, String); config_value!(GOOSE_PROMPT_EDITOR, Option); +config_value!(GOOSE_PROMPT_EDITOR_ALWAYS, Option); config_value!(GOOSE_MAX_ACTIVE_AGENTS, usize); config_value!(GOOSE_DISABLE_SESSION_NAMING, bool); config_value!(GEMINI3_THINKING_LEVEL, String);