feat: enhancing new terminal feature #5852
Conversation
Term command is a new way to use goose without the builtin REPL - instead the session is tied right into your terminal
- Fix bash/zsh: properly quote goose binary path to handle spaces - Extract session creation logic to ensure_terminal_session helper - Reduces code duplication between handle_term_log and handle_term_run
|
some small enhancements: #5852 will take a look at other feedback |
There was a problem hiding this comment.
Pull request overview
This PR improves the terminal integration feature by making terminal sessions less visible (using SessionType::Hidden), simplifying command syntax to not require quotes, and adding an optional command-not-found handler for bash/zsh shells.
- Changed terminal sessions from
SessionType::UsertoSessionType::Hiddenso they don't appear in session lists - Updated
goose term runto accept multiple arguments without quotes (e.g.,gt list filesinstead ofgt "list files") - Added
--with-command-not-foundflag togoose term initfor bash/zsh that automatically sends unknown commands to goose
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/goose-cli/src/commands/term.rs | Changed session type to Hidden, added command_not_found_handler for bash/zsh, updated prompt parameter to accept Vec and join with spaces, added zsh prompt indicator |
| crates/goose-cli/src/cli.rs | Removed non_empty_string validator, updated Run command to accept multiple arguments, added with_command_not_found CLI flag, updated help text and examples |
|
blackgirlbytes
left a comment
blackgirlbytes
left a comment
There was a problem hiding this comment.
good on the docs side. approving so i dont block you
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
I like this better. not entirely sure why we need the json file though, or is that just as an extra way to override the session ids? something something projects
{
"terminal_sessions": {
"/Users/micn/Development/goose": "20251124_2",
"/Users/micn/Documents/code/goose": "20251124_3"
}
}
``` yeah it just glues it to session id. |
michaelneale
changed the title
| SessionManager::update_session(&session.id) | ||
| .user_provided_name(session_name) | ||
| .apply() | ||
| .await?; | ||
|
|
||
| Ok(session.id) |
There was a problem hiding this comment.
Lines 24-26 call create_session with session_name, then lines 28-31 immediately call update_session to set the same session_name via user_provided_name. This appears redundant - verify if create_session already sets the user_provided_name field from the session_name parameter, or if this update is actually necessary.
| SessionManager::update_session(&session.id) | |
| .user_provided_name(session_name) | |
| .apply() | |
| .await?; | |
| Ok(session.id) | |
| Ok(session.id) |
| let goose_bin = std::env::current_exe() | ||
| .map(|p| p.to_string_lossy().into_owned()) | ||
| .unwrap_or_else(|_| "goose".to_string()); |
There was a problem hiding this comment.
The goose_bin path is interpolated into shell scripts without proper escaping. If the executable path contains spaces or special shell metacharacters, the generated shell initialization scripts will be malformed or could be exploited. Consider using shell-safe escaping for the path before interpolation, or wrap it in quotes with proper escaping.
|
|
||
| The `goose term` commands let you talk to goose directly from your shell prompt. Instead of switching to a separate REPL session, you stay in your terminal and call goose when you need it. |
There was a problem hiding this comment.
The documentation doesn't mention the gt alias that is created by the shell initialization scripts. Consider adding this to the documentation since the PR description specifically mentions this feature ("you can use gt type anything you want"). The missing documentation could confuse users who see this alias but don't know what it does.
| fn read_and_clear_shell_history(session_id: &str) -> Result<Vec<String>> { | ||
| let path = shell_history_path(session_id)?; | ||
|
|
||
| if !path.exists() { | ||
| return Ok(Vec::new()); | ||
| } | ||
|
|
||
| let content = fs::read_to_string(&path)?; | ||
| let commands: Vec<String> = content | ||
| .lines() | ||
| .filter(|line| !line.trim().is_empty()) | ||
| .map(|s| s.to_string()) | ||
| .collect(); | ||
|
|
||
| fs::write(&path, "")?; |
There was a problem hiding this comment.
Potential race condition: read_and_clear_shell_history performs a non-atomic read-then-write operation. If multiple @goose commands run concurrently, commands could be lost between the read and clear operations. Consider using file locking or atomic operations to ensure commands aren't dropped.
|
work has shifted over here: #5887 |
Building on: #5847
Some small tweaks, naming session, aliases, and more. Doens't use session DB to catch up on missed terminal entries keeping things cleanerl
Also - you can use
gt type anything you want- no quotes necessary.also just type in
any command you wantand it will send it to gooseThis also doesn't use changes to schema or session manager - an alternative approach.