-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[draft] [feat] add term command #5847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Term command is a new way to use goose without the builtin REPL - instead the session is tied right into your terminal
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new goose term command that enables terminal-integrated sessions, allowing users to interact with goose directly from their shell prompt without entering a separate REPL. Each terminal maintains its own persistent session with shell command history awareness, so goose automatically sees what commands you've run when you ask questions.
Key changes:
- New terminal integration command with shell-specific initialization scripts for bash, zsh, fish, and PowerShell
- Database schema migration to v7 adding a
shell_commandstable to track command history - New CLI handlers for
term init,term log, andterm runsubcommands
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| documentation/docs/guides/terminal-integration.md | New documentation explaining terminal integration setup and usage |
| crates/goose/src/session/session_manager.rs | Database schema bump to v7, new shell_commands table with migration logic and query methods |
| crates/goose-cli/src/commands/term.rs | New command handlers for term init/log/run with shell script generation |
| crates/goose-cli/src/commands/mod.rs | Module declaration for new term command |
| crates/goose-cli/src/cli.rs | CLI integration with new Term command enum and shell type enum |
| "fish" => { | ||
| format!( | ||
| r#"set -gx GOOSE_TERMINAL_ID "{terminal_id}" | ||
| alias gt='{goose_bin} term run' |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fish shell aliases don't support arguments like bash/zsh. Use alias gt '{goose_bin} term run' (function-style without =) or create a proper function instead.
| alias gt='{goose_bin} term run' | |
| function gt; {goose_bin} term run $argv; end |
| function goose_preexec --on-event fish_preexec | ||
| string match -q -r '^goose term' -- $argv[1]; and return | ||
| string match -q -r '^gt ' -- $argv[1]; and return | ||
| {goose_bin} term log $argv[1] 2>/dev/null & |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing quotes around $argv[1] in Fish shell. Commands with spaces or special characters will fail. Use {goose_bin} term log \"$argv[1]\" 2>/dev/null &
| {goose_bin} term log $argv[1] 2>/dev/null & | |
| {goose_bin} term log "$argv[1]" 2>/dev/null & |
| "powershell" | "pwsh" => { | ||
| format!( | ||
| r#"$env:GOOSE_TERMINAL_ID = "{terminal_id}" | ||
| Set-Alias -Name gt -Value {{ {goose_bin} term run $args }} |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PowerShell Set-Alias -Value doesn't accept scriptblocks with parameters. Use a function instead: function gt {{ & {goose_bin} term run @args }}
| Set-Alias -Name gt -Value {{ {goose_bin} term run $args }} | |
| function gt { {goose_bin} term run $args } |
| let session = SessionManager::create_session_with_id( | ||
| session_name.clone(), | ||
| working_dir.clone(), | ||
| session_name.clone(), | ||
| SessionType::User, | ||
| ) | ||
| .await?; | ||
|
|
||
| SessionManager::update_session(&session.id) | ||
| .user_provided_name(session_name.clone()) | ||
| .apply() | ||
| .await?; |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated session creation logic appears in both handle_term_log (lines 110-121) and handle_term_run (lines 151-163). Extract to a helper function like ensure_terminal_session to reduce duplication.
| [[ "$1" =~ ^goose\ term ]] && return | ||
| [[ "$1" =~ ^gt\ ]] && return |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex pattern ^gt\ (line 28) requires gt followed by space but won't match standalone gt command. Should be ^gt(\s|$) or ^gt to handle both cases.
| [[ "$1" =~ ^goose\ term ]] && return | ||
| [[ "$1" =~ ^gt\ ]] && return |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as bash: regex pattern ^gt\ won't match standalone gt command. Should be ^gt(\s|$) or ^gt.
| # Log commands to goose | ||
| function goose_preexec --on-event fish_preexec | ||
| string match -q -r '^goose term' -- $argv[1]; and return | ||
| string match -q -r '^gt ' -- $argv[1]; and return |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pattern ^gt requires space after gt but won't match standalone gt. Should be '^gt(\s|$)' or '^gt'.
| string match -q -r '^gt ' -- $argv[1]; and return | |
| string match -q -r '^gt(\\s|$)' -- $argv[1]; and return |
| Set-PSReadLineKeyHandler -Chord Enter -ScriptBlock {{ | ||
| $line = $null | ||
| [Microsoft.PowerShell.PSConsoleReadLine]::GetBufferState([ref]$line, [ref]$null) | ||
| if ($line -notmatch '^goose term' -and $line -notmatch '^gt ') {{ |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pattern ^gt won't match standalone gt command. Should be ^gt(\s|$) or ^gt\b.
| gt "what does this error mean?" | ||
| ``` | ||
|
|
||
| Goose responds, you read the answer, and you're back at your prompt. The conversation lives alongside your work, not in a separate window you have to manage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Goose responds, you read the answer, and you're back at your prompt. The conversation lives alongside your work, not in a separate window you have to manage. | |
| goose responds, you read the answer, and you're back at your prompt. The conversation lives alongside your work, not in a separate window you have to manage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| $line = $null | ||
| [Microsoft.PowerShell.PSConsoleReadLine]::GetBufferState([ref]$line, [ref]$null) | ||
| if ($line -notmatch '^goose term' -and $line -notmatch '^gt ') {{ | ||
| Start-Job -ScriptBlock {{ {goose_bin} term log $using:line }} | Out-Null |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goose binary path contains spaces, this command will fail. Quote the path in PowerShell: & '{goose_bin}' term log $using:line
| Start-Job -ScriptBlock {{ {goose_bin} term log $using:line }} | Out-Null | |
| Start-Job -ScriptBlock {{ '{goose_bin}' term log $using:line }} | Out-Null |
| goose_preexec() {{ | ||
| [[ "$1" =~ ^goose\ term ]] && return | ||
| [[ "$1" =~ ^gt\ ]] && return | ||
| ("{goose_bin}" term log "$1" &) 2>/dev/null |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goose binary path contains spaces, this command will fail. Quote the path: ("'${goose_bin}'" term log "$1" &) or use escaped quotes within the path variable itself.
| goose_preexec() {{ | ||
| [[ "$1" =~ ^goose\ term ]] && return | ||
| [[ "$1" =~ ^gt\ ]] && return | ||
| ("{goose_bin}" term log "$1" &) 2>/dev/null |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goose binary path contains spaces, this command will fail. Quote the path: ("'${goose_bin}'" term log "$1" &)
| @@ -0,0 +1,75 @@ | |||
| # Terminal Integration | |||
|
|
|||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is such a cool feature, but since it's not released yet, can you make the document unlisted? We can remove it when it gets released.
The way you do that..is in the frontmatter write "unlisted: true"
| @@ -0,0 +1,75 @@ | |||
| # Terminal Integration | |||
|
|
|||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is such a cool feature, but since it's not released yet, can you make the document unlisted? We can remove it when it gets released.
The way you do that..is in the frontmatter write "unlisted: true"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And noting to myself that we should have this mentioned in the CLI commands section
- 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
|
nice - I did note when I ran something which didn't involve a tool call, it didn't always return results... (ie ask "how are you") - if it uses a tool call seems all ok., but intermittent... digging in. Also ... command_not_found_handler() {
local cmd="$*"
gt "$cmd"
}I added that to it and works well (even with the eval, automatically, if not too invasive!). Very cool and innovative! (I also wonder if we could add a nice little oh my zsh like decoration to know goose is on) I do think, somehow, dont' want each invocation to show up as a "term..." shell in the session history if possible? |
| use utoipa::ToSchema; | ||
|
|
||
| const CURRENT_SCHEMA_VERSION: i32 = 6; | ||
| const CURRENT_SCHEMA_VERSION: i32 = 7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if we are changing the schema - we can make the term sessions not show up the usual way? (wonder if there is a less invasive way).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like having these term: specific sessions not show up in the history for things like the goose desktop? could do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - I have a PR that does that, Session::Hidden is fine.
| .await | ||
| } | ||
|
|
||
| async fn add_shell_command( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused as to why we need to track these separately? as a special case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is used by the preexec hook so that goose actually gets a record of the commands that you ran by hand in your terminal between interactions with goose. could make this optional but i find it very useful to not have to tell it what i was doing
DOsinga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems pretty awesome!
I would prefer it if we can do this without making changes to the session manager. I don't think you need the create_with_session_id in this flow and I wonder if we can get the commands out of the sessions.
there's also a bunch of LLM-y comments in there that I could do without.
| .await | ||
| .is_err() | ||
| { | ||
| let session = SessionManager::create_session_with_id( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to create a session with a specific id, why not just use the name of the session here instead? or alternatively, we could create the session the first time and then write that session id to the config file; that way you wouldn't have to set the environment variable either
|
|
||
| /// Handle `goose term init <shell>` - print shell initialization script | ||
| pub fn handle_term_init(shell: &str) -> Result<()> { | ||
| let terminal_id = Uuid::new_v4().to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually you could just create the session here and then use the session id of that session as the terminal_id
| let session = SessionManager::get_session(&session_name, false).await.ok(); | ||
| let total_tokens = session.as_ref().and_then(|s| s.total_tokens).unwrap_or(0) as usize; | ||
|
|
||
| let model_name = Config::global() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we store the model and provider now in the session - should probably get it from there
| } | ||
| }; | ||
|
|
||
| let commands = SessionManager::get_shell_commands_since_last_message(&session_id).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure I follow what we are fetching here. doesn't each session add at least one message to the conversation so isn't this then automatically empty?
in general though, adding shell commands to the session manager seems rather specific for this application. couldn't we just list the messages from session and exclude the <shell_history></shell_history> from that to get the same result? or if that doesn't work, maybe add a message content type?
|
my enhancements so far: #5852 |
|
all of this has been pulled over here: #5887 |
Term command is a new way to use goose without the builtin REPL
Type of Change