-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add background parameter to execute_command tool #9023
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
base: main
Are you sure you want to change the base?
Conversation
- Add optional background boolean parameter to execute_command tool - When background=true, commands run without requiring user interaction - Commands return immediately without blocking when background is enabled - Output continues to stream to terminal UI in background - Add comprehensive test coverage for background execution Fixes #9022
|
Review completed. Found 2 issues that need attention:
Mention @roomote in a comment to trigger your PR Fixer agent and make changes to this pull request. |
| // If background=true, return immediately without waiting for process to complete | ||
| if (background) { | ||
| // Give a brief moment for initial output to be captured | ||
| await delay(100) | ||
|
|
||
| return [ | ||
| false, | ||
| [ | ||
| `Command is running in the background in terminal from '${terminal.getCurrentWorkingDirectory().toPosix()}'.`, | ||
| result && result.length > 0 ? `Initial output:\n${result}\n` : "", | ||
| "The command will continue running without blocking. You will be updated on the terminal status and new output in the future.", | ||
| ].join("\n"), | ||
| ] | ||
| } |
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.
The early return for background commands bypasses the cleanup code that sets task.terminalProcess = undefined (lines 312, 319). This means the process reference persists on the task even after this function completes. If another command is executed while a background command is still running, the new command will overwrite task.terminalProcess, orphaning the previous background process reference. Users won't be able to interact with (continue/abort) the background process via handleTerminalOperation since the reference will be lost.
Consider managing background process references differently, perhaps by storing them in a separate collection that tracks multiple concurrent processes, or documenting that only one command (background or foreground) should run at a time per task.
Fix it with Roo Code or mention @roomote and request a fix.
| return [ | ||
| false, | ||
| [ | ||
| `Command is running in the background in terminal from '${terminal.getCurrentWorkingDirectory().toPosix()}'.`, | ||
| result && result.length > 0 ? `Initial output:\n${result}\n` : "", |
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.
The background return logic checks result for initial output, but result is only set in the onCompleted callback (lines 221-229), which fires when the command finishes - not during execution. For long-running background commands, onCompleted won't be called within the 100ms delay window, so result will always be empty and "Initial output:" will never show anything useful. The actual output during this window is accumulated in accumulatedOutput (line 196) via the onLine callback, but that variable isn't used for the background return value. Consider using Terminal.compressTerminalOutput(accumulatedOutput, ...) instead of result to show actual captured output.
Fix it with Roo Code or mention @roomote and request a fix.
|
so use full <3 |
This PR attempts to address Issue #9022 by adding an optional
backgroundparameter to theexecute_commandtool.Summary
When
backgroundis set totrue:Changes
backgroundparameter to tool type definitions and TypeScript interfacesexecuteCommandTool.tsto handle the background parameterbackground=truereturn immediately after a brief delay to capture initial outputTesting
Example Usage
Fixes #9022
Feedback and guidance are welcome!
Important
Adds
backgroundparameter toexecute_commandtool for non-blocking command execution.backgroundparameter toexecute_commandtool, allowing commands to run without blocking.background=truereturn immediately, streaming output to terminal UI.executeCommandTool.tsto handlebackgroundparameter.executeCommandfunction to support immediate return for background commands.executeCommand.spec.ts, covering non-blocking behavior and parameter parsing.This description was created by
for 23011af. You can customize this summary. It will automatically update as commits are pushed.