Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions crates/goose-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,15 @@ enum Command {
)]
fork: bool,

/// Open the session's conversation in $EDITOR before starting
#[arg(
long,
requires = "resume",
help = "Edit the session conversation in $EDITOR before starting",
long_help = "Open the session's conversation in your editor ($VISUAL / $EDITOR / vi) for modification before resuming. When combined with --fork, creates a new session from the edited result."
)]
edit: bool,

/// Show message history when resuming
#[arg(
long,
Expand Down Expand Up @@ -1465,6 +1474,7 @@ async fn handle_interactive_session(
identifier: Option<Identifier>,
resume: bool,
fork: bool,
edit: bool,
history: bool,
session_opts: SessionOptions,
extension_opts: ExtensionOptions,
Expand Down Expand Up @@ -1504,12 +1514,31 @@ async fn handle_interactive_session(
let goose_mode = Config::global().get_goose_mode().unwrap_or_default();
let mut session_id = get_or_create_session_id(identifier, resume, false, goose_mode).await?;

if fork {
if let Some(id) = session_id {
if edit || fork {
if let Some(ref id) = session_id {
let session_manager = SessionManager::instance();
let original = session_manager.get_session(&id, false).await?;
let copied = session_manager.copy_session(&id, original.name).await?;
session_id = Some(copied.id);
let original = session_manager.get_session(id, true).await?;

let target_id = if fork {
let copied = session_manager
.copy_session(id, original.name.clone())
.await?;
Comment on lines +1523 to +1525

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Defer creating the fork until edit succeeds

When --fork --edit is used and the editor exits non-zero or the YAML does not parse, this copy_session has already persisted a duplicate session with the unedited history; the command then returns before replace_conversation, leaving a stray most-recent fork even though the edit was rejected. Parse the edited conversation before creating the fork, or delete the copied session on any edit/replace failure.

Useful? React with 👍 / 👎.

let copied_id = copied.id.clone();
session_id = Some(copied.id);
copied_id
} else {
id.clone()
};

if edit {
let conversation = original
.conversation
.ok_or_else(|| anyhow::anyhow!("session has no messages to edit"))?;
let edited = crate::session::editor::edit_conversation(&conversation)?;
session_manager
.replace_conversation(&target_id, &edited)
.await?;
}
}
}

Expand Down Expand Up @@ -2081,6 +2110,7 @@ pub async fn cli() -> anyhow::Result<()> {
identifier,
resume,
fork,
edit,
history,
session_opts,
extension_opts,
Expand All @@ -2089,6 +2119,7 @@ pub async fn cli() -> anyhow::Result<()> {
identifier,
resume,
fork,
edit,
history,
session_opts,
extension_opts,
Expand Down
174 changes: 121 additions & 53 deletions crates/goose-cli/src/session/editor.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use anyhow::Result;
use anyhow::{Context, Result};
use goose::config::Config;
use goose::conversation::message::Message;
use goose::conversation::Conversation;
use std::fs;
use std::io::Read;
use std::io::Write;
use std::path::PathBuf;
use std::process::Command;
use tempfile::Builder;
Expand All @@ -21,9 +24,6 @@ pub fn resolve_editor_command() -> Option<String> {
)
}

/// 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>,
Expand All @@ -37,6 +37,57 @@ fn resolve_editor_from_sources(
None
}

/// Resolve the editor command, falling back to vi (or notepad on Windows).
pub fn resolve_editor_or_default() -> String {
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_or_default_from_sources(
config_editor.as_deref(),
visual.as_deref(),
editor_env.as_deref(),
)
}

fn resolve_editor_default() -> String {
if cfg!(windows) {
"notepad".to_string()
} else {
"vi".to_string()
}
}

fn resolve_editor_or_default_from_sources(
config_editor: Option<&str>,
visual: Option<&str>,
editor_env: Option<&str>,
) -> String {
resolve_editor_from_sources(config_editor, visual, editor_env)
.unwrap_or_else(resolve_editor_default)
}

/// Open a YAML temp file with the user's editor to edit a conversation.
/// Returns the edited conversation, or an error if the editor failed or YAML was invalid.
pub fn edit_conversation(conversation: &Conversation) -> Result<Conversation> {
let yaml = serde_yaml::to_string(conversation.messages())?;

let mut tmp = NamedTempFile::with_suffix(".yaml")?;
tmp.write_all(yaml.as_bytes())?;
tmp.flush()?;

let editor = resolve_editor_or_default();
let path = tmp.path().to_path_buf();

launch_editor(&editor, &path).with_context(|| format!("failed to launch editor '{editor}'"))?;

let edited = std::fs::read_to_string(&path)?;
let messages: Vec<Message> =
serde_yaml::from_str(&edited).context("invalid YAML — session unchanged")?;

Ok(Conversation::new_unvalidated(messages))
}

/// 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");
Expand Down Expand Up @@ -84,21 +135,36 @@ impl SymlinkCleanup {

impl Drop for SymlinkCleanup {
fn drop(&mut self) {
// Always try to clean up the symlink, ignoring any errors
let _ = std::fs::remove_file(&self.symlink_path);
}
}

/// Split an editor command into program and arguments.
///
/// Uses shell-word splitting only when the command contains quotes, so values like
/// `"/Applications/Sublime Text.app/.../subl" -w` work. Unquoted commands are split on
/// whitespace to avoid shlex stripping backslashes from Windows paths like
/// `C:\Windows\System32\notepad.exe`.
fn split_editor_command(editor_cmd: &str) -> Result<Vec<String>> {
if editor_cmd.contains(['"', '\'']) {
shlex::split(editor_cmd).ok_or_else(|| {
anyhow::anyhow!("Invalid editor command: unmatched quotes in '{editor_cmd}'")
})
} else {
Ok(editor_cmd.split_whitespace().map(String::from).collect())
}
}

/// Launch editor and wait for completion
fn launch_editor(editor_cmd: &str, file_path: &PathBuf) -> Result<()> {
use std::process::Stdio;

let parts: Vec<&str> = editor_cmd.split_whitespace().collect();
let parts = split_editor_command(editor_cmd)?;
if parts.is_empty() {
return Err(anyhow::anyhow!("Empty editor command"));
}

let mut cmd = Command::new(parts[0]);
let mut cmd = Command::new(&parts[0]);
if let Ok(cwd) = std::env::current_dir() {
cmd.current_dir(cwd);
}
Expand Down Expand Up @@ -414,64 +480,72 @@ 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"));
}
fn test_resolve_editor_resolution_priority() {
assert_eq!(
resolve_editor_from_sources(Some("config-val"), Some("visual-val"), Some("editor-val")),
Some("config-val".to_string())
);

#[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"));
}
assert_eq!(
resolve_editor_from_sources(Some(""), Some("visual-val"), Some("editor-val")),
Some("visual-val".to_string())
);

#[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"));
}
assert_eq!(
resolve_editor_from_sources(None, Some(""), Some("editor-val")),
Some("editor-val".to_string())
);

#[test]
fn test_resolve_editor_returns_none_when_nothing_set() {
let result = resolve_editor_from_sources(None, None, None);
assert_eq!(result, None);
}
assert_eq!(resolve_editor_from_sources(None, None, None), None);
assert_eq!(
resolve_editor_from_sources(Some(""), Some(""), Some("")),
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"));
let default_val = resolve_editor_default();
assert_eq!(
resolve_editor_or_default_from_sources(None, None, None),
default_val
);
assert_eq!(
resolve_editor_or_default_from_sources(Some(""), Some(""), Some("")),
default_val
);
}

#[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"));
}
fn test_split_editor_command() {
assert_eq!(
split_editor_command("code --wait").unwrap(),
vec!["code", "--wait"]
);

#[test]
fn test_resolve_editor_skips_all_empty() {
let result = resolve_editor_from_sources(Some(""), Some(""), Some(""));
assert_eq!(result, None);
}
assert_eq!(
split_editor_command(
r#""/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl" -w"#
)
.unwrap(),
vec![
"/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl",
"-w"
]
);

#[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"));
assert_eq!(
split_editor_command(r"C:\Windows\System32\notepad.exe").unwrap(),
vec![r"C:\Windows\System32\notepad.exe"]
);

assert!(split_editor_command(r#"code --wait "unclosed"#).is_err());
}

// --- 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"));
}

Expand All @@ -498,19 +572,15 @@ with multiple lines.
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",
Expand All @@ -522,7 +592,6 @@ with multiple lines.

#[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);
Expand All @@ -531,7 +600,6 @@ with multiple lines.

#[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);
Expand Down
2 changes: 1 addition & 1 deletion crates/goose-cli/src/session/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod builder;
mod completion;
mod editor;
pub mod editor;
mod elicitation;
mod export;
mod input;
Expand Down
7 changes: 7 additions & 0 deletions documentation/docs/guides/goose-cli-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ Start or resume interactive chat sessions.
- **`-n, --name <name>`**: Give the session a name
- **`--path <path>`**: Legacy parameter for specifying session by file path
- **`-r, --resume`**: Resume a previous session
- **`--edit`**: Open the session's conversation in your editor (`$VISUAL` / `$EDITOR` / `vi`) as YAML. Edit, trim, or rewrite messages, then save and close to continue the session with the edited conversation. Must be used with `--resume`. Can be combined with `--fork` to create a new session from the edited result.
- **`--fork`**: Create a new duplicate session with copied history. Must be used with `--resume`. Provide `--name` or `--session-id` to fork a specific session. Otherwise, forks the most recent session.
- **`--history`**: Show previous messages when resuming a session
- **`--container <container_id>`**: Run extensions inside a [Docker container](/docs/tutorials/goose-in-docker#running-extensions-in-docker-containers).
Expand Down Expand Up @@ -235,6 +236,12 @@ goose session --resume --fork --name my-project
# Fork the most recent session and show message history
goose session --resume --fork --history

# Edit a session's conversation in your editor
goose session --resume --session-id 20251108_2 --edit

# Edit and fork — create a new session from the edited conversation
goose session --resume --session-id 20251108_2 --fork --edit --history

# Start with extensions
goose session --with-extension "npx -y @modelcontextprotocol/server-memory"
goose session --with-builtin developer
Expand Down
Loading
Loading