Skip to content

Commit fdd8ada

Browse files
authored
feat: add /edit command to cli for on-demand prompt editing (#8566)
Signed-off-by: Adam Miller <admiller@redhat.com>
1 parent 0766d34 commit fdd8ada

4 files changed

Lines changed: 373 additions & 101 deletions

File tree

crates/goose-cli/src/session/editor.rs

Lines changed: 220 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,53 @@
11
use anyhow::Result;
2+
use goose::config::Config;
23
use std::fs;
34
use std::io::Read;
45
use std::path::PathBuf;
56
use std::process::Command;
67
use tempfile::Builder;
78
use tempfile::NamedTempFile;
89

9-
/// Create temporary markdown file with conversation history
10-
fn create_temp_file(messages: &[&str]) -> Result<NamedTempFile> {
11-
let temp_file = Builder::new()
12-
.prefix("goose_prompt_")
13-
.suffix(".md")
14-
.tempfile()?;
10+
/// Resolve the editor command from config and environment variables.
11+
/// Checks GOOSE_PROMPT_EDITOR, then $VISUAL, then $EDITOR.
12+
pub fn resolve_editor_command() -> Option<String> {
13+
let config = Config::global();
14+
let config_editor = config.get_goose_prompt_editor().ok().flatten();
15+
let visual = std::env::var("VISUAL").ok();
16+
let editor_env = std::env::var("EDITOR").ok();
17+
resolve_editor_from_sources(
18+
config_editor.as_deref(),
19+
visual.as_deref(),
20+
editor_env.as_deref(),
21+
)
22+
}
23+
24+
/// Inner resolution logic, separated for testability.
25+
/// Checks sources in priority order: config, VISUAL, EDITOR.
26+
/// Skips empty strings at each level.
27+
fn resolve_editor_from_sources(
28+
config_editor: Option<&str>,
29+
visual: Option<&str>,
30+
editor_env: Option<&str>,
31+
) -> Option<String> {
32+
for cmd in [config_editor, visual, editor_env].into_iter().flatten() {
33+
if !cmd.is_empty() {
34+
return Some(cmd.to_string());
35+
}
36+
}
37+
None
38+
}
39+
40+
/// Build the markdown template content for the editor prompt.
41+
fn build_template(messages: &[&str], prefill: Option<&str>) -> String {
1542
let mut content = String::from("# Goose Prompt Editor\n\n");
1643

1744
content.push_str("# Your prompt:\n\n");
45+
if let Some(text) = prefill {
46+
if !text.is_empty() {
47+
content.push_str(text);
48+
content.push('\n');
49+
}
50+
}
1851

1952
if !messages.is_empty() {
2053
content.push_str("# Recent conversation for context (newest first):\n\n");
@@ -24,7 +57,17 @@ fn create_temp_file(messages: &[&str]) -> Result<NamedTempFile> {
2457
content.push('\n');
2558
}
2659

27-
fs::write(temp_file.path(), content)?;
60+
content
61+
}
62+
63+
/// Create temporary markdown file with conversation history and optional prefill text
64+
fn create_temp_file(messages: &[&str], prefill: Option<&str>) -> Result<NamedTempFile> {
65+
let temp_file = Builder::new()
66+
.prefix("goose_prompt_")
67+
.suffix(".md")
68+
.tempfile()?;
69+
70+
fs::write(temp_file.path(), build_template(messages, prefill))?;
2871
Ok(temp_file)
2972
}
3073

@@ -80,8 +123,12 @@ fn launch_editor(editor_cmd: &str, file_path: &PathBuf) -> Result<()> {
80123
}
81124

82125
/// Main function to get input from editor
83-
pub fn get_editor_input(editor_cmd: &str, messages: &[&str]) -> Result<(String, bool)> {
84-
let temp_file = create_temp_file(messages)?;
126+
pub fn get_editor_input(
127+
editor_cmd: &str,
128+
messages: &[&str],
129+
prefill: Option<&str>,
130+
) -> Result<(String, bool)> {
131+
let temp_file = create_temp_file(messages, prefill)?;
85132
let temp_path = temp_file.path().to_path_buf();
86133

87134
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,
98145

99146
let _cleanup_guard = SymlinkCleanup::new(symlink_path.clone());
100147

101-
let _original_template = {
102-
let mut template_content = String::from("# Goose Prompt Editor\n\n");
103-
template_content.push_str("# Your prompt:\n\n");
104-
if !messages.is_empty() {
105-
template_content.push_str("# Recent conversation for context (newest first):\n\n");
106-
for message in messages.iter().rev() {
107-
template_content.push_str(&format!("{}\n", message));
108-
}
109-
template_content.push('\n');
110-
}
111-
template_content
112-
};
148+
let _original_template = build_template(messages, prefill);
113149

114150
launch_editor(editor_cmd, &symlink_path)?;
115151

@@ -209,7 +245,7 @@ This is the user's input
209245
fn test_create_temp_file_with_messages() {
210246
let messages = vec!["## User: Hello", "## Assistant: Hi there!"];
211247

212-
let temp_file = create_temp_file(&messages).unwrap();
248+
let temp_file = create_temp_file(&messages, None).unwrap();
213249
let path = temp_file.path();
214250

215251
assert!(path.exists());
@@ -224,6 +260,33 @@ This is the user's input
224260
assert!(content.contains("# Recent conversation for context (newest first):"));
225261
}
226262

263+
#[test]
264+
fn test_create_temp_file_with_prefill() {
265+
let messages = vec!["## User: Hello"];
266+
let temp_file = create_temp_file(&messages, Some("fix the login bug")).unwrap();
267+
let content = fs::read_to_string(temp_file.path()).unwrap();
268+
269+
assert!(content.contains("# Your prompt:"));
270+
assert!(content.contains("fix the login bug"));
271+
// Prefill text should appear before conversation context
272+
let prefill_pos = content.find("fix the login bug").unwrap();
273+
let context_pos = content.find("# Recent conversation for context").unwrap();
274+
assert!(
275+
prefill_pos < context_pos,
276+
"Prefill text should appear before conversation context"
277+
);
278+
}
279+
280+
#[test]
281+
fn test_create_temp_file_without_prefill() {
282+
let messages = vec!["## User: Hello"];
283+
let temp_file = create_temp_file(&messages, None).unwrap();
284+
let content = fs::read_to_string(temp_file.path()).unwrap();
285+
286+
assert!(content.contains("# Your prompt:"));
287+
assert!(!content.contains("fix the login bug"));
288+
}
289+
227290
#[test]
228291
fn test_create_temp_file_with_prefix_suffix() {
229292
let temp_file = Builder::new()
@@ -280,7 +343,7 @@ with multiple lines.
280343
"## User: Third message (newest)",
281344
];
282345

283-
let temp_file = create_temp_file(&messages).unwrap();
346+
let temp_file = create_temp_file(&messages, None).unwrap();
284347
let content = fs::read_to_string(temp_file.path()).unwrap();
285348

286349
let newest_first = [
@@ -314,7 +377,7 @@ with multiple lines.
314377
use std::panic;
315378

316379
let messages = vec!["## User: Test message for panic cleanup"];
317-
let temp_file = create_temp_file(&messages).unwrap();
380+
let temp_file = create_temp_file(&messages, None).unwrap();
318381
let temp_path = temp_file.path().to_path_buf();
319382

320383
let symlink_path = PathBuf::from(format!("test_panic_cleanup_{}.md", std::process::id()));
@@ -351,13 +414,145 @@ with multiple lines.
351414
);
352415
}
353416

417+
// --- resolve_editor_from_sources tests ---
418+
419+
#[test]
420+
fn test_resolve_editor_returns_config_when_set() {
421+
let result = resolve_editor_from_sources(Some("code"), Some("vim"), Some("nano"));
422+
assert_eq!(result.as_deref(), Some("code"));
423+
}
424+
425+
#[test]
426+
fn test_resolve_editor_falls_back_to_visual() {
427+
let result = resolve_editor_from_sources(None, Some("vim"), Some("nano"));
428+
assert_eq!(result.as_deref(), Some("vim"));
429+
}
430+
431+
#[test]
432+
fn test_resolve_editor_falls_back_to_editor_env() {
433+
let result = resolve_editor_from_sources(None, None, Some("nano"));
434+
assert_eq!(result.as_deref(), Some("nano"));
435+
}
436+
437+
#[test]
438+
fn test_resolve_editor_returns_none_when_nothing_set() {
439+
let result = resolve_editor_from_sources(None, None, None);
440+
assert_eq!(result, None);
441+
}
442+
443+
#[test]
444+
fn test_resolve_editor_skips_empty_config() {
445+
let result = resolve_editor_from_sources(Some(""), Some("vim"), None);
446+
assert_eq!(result.as_deref(), Some("vim"));
447+
}
448+
449+
#[test]
450+
fn test_resolve_editor_skips_empty_visual() {
451+
let result = resolve_editor_from_sources(None, Some(""), Some("nano"));
452+
assert_eq!(result.as_deref(), Some("nano"));
453+
}
454+
455+
#[test]
456+
fn test_resolve_editor_skips_all_empty() {
457+
let result = resolve_editor_from_sources(Some(""), Some(""), Some(""));
458+
assert_eq!(result, None);
459+
}
460+
461+
#[test]
462+
fn test_resolve_editor_skips_empty_config_and_visual() {
463+
let result = resolve_editor_from_sources(Some(""), Some(""), Some("emacs"));
464+
assert_eq!(result.as_deref(), Some("emacs"));
465+
}
466+
467+
// --- build_template edge case tests ---
468+
469+
#[test]
470+
fn test_build_template_empty_prefill_string() {
471+
let content = build_template(&["## User: Hello"], Some(""));
472+
// Empty prefill should not appear in content
473+
assert!(content.contains("# Your prompt:\n\n#"));
474+
// Should go directly to conversation context
475+
assert!(content.contains("# Recent conversation for context"));
476+
}
477+
478+
#[test]
479+
fn test_build_template_prefill_with_no_messages() {
480+
let content = build_template(&[], Some("fix the bug"));
481+
assert!(content.contains("# Your prompt:\n\nfix the bug\n"));
482+
assert!(!content.contains("# Recent conversation for context"));
483+
}
484+
485+
#[test]
486+
fn test_build_template_no_prefill_no_messages() {
487+
let content = build_template(&[], None);
488+
assert_eq!(content, "# Goose Prompt Editor\n\n# Your prompt:\n\n");
489+
}
490+
491+
#[test]
492+
fn test_build_template_prefill_with_messages() {
493+
let content = build_template(&["## User: Hi", "## Assistant: Hello"], Some("do stuff"));
494+
assert!(content.contains("do stuff"));
495+
assert!(content.contains("## User: Hi"));
496+
let prefill_pos = content.find("do stuff").unwrap();
497+
let context_pos = content.find("# Recent conversation").unwrap();
498+
assert!(prefill_pos < context_pos);
499+
}
500+
501+
// --- extract_user_input with prefilled content tests ---
502+
503+
#[test]
504+
fn test_extract_user_input_with_prefill_kept() {
505+
// Simulates a user who opened the editor with prefill and kept it unchanged
506+
let content = build_template(&["## User: Hello"], Some("fix the login bug"));
507+
let result = extract_user_input(&content);
508+
assert_eq!(result, "fix the login bug");
509+
}
510+
511+
#[test]
512+
fn test_extract_user_input_with_prefill_edited() {
513+
// Simulates a user who edited the prefill text
514+
let mut content = build_template(&["## User: Hello"], Some("fix the login bug"));
515+
content = content.replace(
516+
"fix the login bug",
517+
"fix the login bug and also the signup flow",
518+
);
519+
let result = extract_user_input(&content);
520+
assert_eq!(result, "fix the login bug and also the signup flow");
521+
}
522+
523+
#[test]
524+
fn test_extract_user_input_prefill_replaced() {
525+
// Simulates a user who deleted the prefill and wrote something new
526+
let mut content = build_template(&["## User: Hello"], Some("fix the login bug"));
527+
content = content.replace("fix the login bug\n", "completely different prompt\n");
528+
let result = extract_user_input(&content);
529+
assert_eq!(result, "completely different prompt");
530+
}
531+
532+
#[test]
533+
fn test_extract_user_input_prefill_cleared() {
534+
// Simulates a user who deleted the prefill and left nothing
535+
let mut content = build_template(&["## User: Hello"], Some("fix the login bug"));
536+
content = content.replace("fix the login bug\n", "");
537+
let result = extract_user_input(&content);
538+
assert_eq!(result, "");
539+
}
540+
541+
#[test]
542+
fn test_extract_user_input_multiline_with_prefill() {
543+
let mut content = build_template(&["## User: Hello"], Some("line one"));
544+
content = content.replace("line one\n", "line one\nline two\nline three\n");
545+
let result = extract_user_input(&content);
546+
assert_eq!(result, "line one\nline two\nline three");
547+
}
548+
354549
#[test]
355550
#[cfg(unix)]
356551
fn test_symlink_creation_and_cleanup() {
357552
use std::os::unix::fs;
358553

359554
let messages = vec!["## User: Test message"];
360-
let temp_file = create_temp_file(&messages).unwrap();
555+
let temp_file = create_temp_file(&messages, None).unwrap();
361556
let temp_path = temp_file.path().to_path_buf();
362557

363558
let symlink_path = PathBuf::from(format!("test_symlink_cleanup_{}.md", std::process::id()));

0 commit comments

Comments
 (0)