-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: handle daemon processes without blocking UI (#8636) #8637
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 daemon process detection utility with common service patterns - Modify executeCommandTool to detect and handle daemons as background processes - Add configuration options for custom daemon patterns - Add comprehensive tests for daemon detection logic Fixes #8636
const daemonMessage = getDaemonMessage(command) | ||
await task.say("text", daemonMessage) | ||
|
||
return [ | ||
false, | ||
`Started ${serviceType} in the background from '${terminal.getCurrentWorkingDirectory().toPosix()}'.\n` + | ||
`The service is running and you can proceed with other tasks.\n` + | ||
`Current output:\n${result}\n` + | ||
`The terminal will continue to show output from this service.`, | ||
] |
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 result
variable will be empty for daemon processes because it's only set in the onCompleted
callback (line 238), which hasn't been called yet for long-running processes. This means the return message will show "Current output:\n\n" with no actual output, even though output has been captured in accumulatedOutput
.
The message should use the compressed version of accumulatedOutput
instead:
const daemonMessage = getDaemonMessage(command) | |
await task.say("text", daemonMessage) | |
return [ | |
false, | |
`Started ${serviceType} in the background from '${terminal.getCurrentWorkingDirectory().toPosix()}'.\n` + | |
`The service is running and you can proceed with other tasks.\n` + | |
`Current output:\n${result}\n` + | |
`The terminal will continue to show output from this service.`, | |
] | |
const daemonOutput = Terminal.compressTerminalOutput( | |
accumulatedOutput, | |
terminalOutputLineLimit, | |
terminalOutputCharacterLimit, | |
) | |
const daemonMessage = getDaemonMessage(command) | |
await task.say("text", daemonMessage) | |
return [ | |
false, | |
`Started ${serviceType} in the background from '${terminal.getCurrentWorkingDirectory().toPosix()}'.\n` + | |
`The service is running and you can proceed with other tasks.\n` + | |
`Current output:\n${daemonOutput}\n` + | |
`The terminal will continue to show output from this service.`, | |
] |
// Generic server patterns | ||
/\b(server|serve|watch|dev|start)\b.*$/i, |
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 generic pattern /\b(server|serve|watch|dev|start)\b.*$/i
is overly broad and could match non-daemon commands that happen to contain these words, such as cat server.log
, echo "starting process"
, or make start-build
. While the 3-second timeout provides a safeguard for short-running commands, this still introduces an unnecessary 3-second delay for these false positives.
Consider either:
- Making this pattern more specific (e.g., requiring these words to be the command itself or first argument, not just anywhere in the command)
- Removing this generic pattern and relying on the user-configurable
daemonCommands
setting for edge cases - Moving it to the user-defined patterns as an opt-in fallback rather than a built-in pattern
// Load user-defined daemon patterns from configuration | ||
if (!disableDaemonDetection) { | ||
clearUserDaemonPatterns() | ||
const userDaemonPatterns = vscode.workspace.getConfiguration(Package.name).get<string[]>("daemonCommands", []) | ||
if (userDaemonPatterns.length > 0) { | ||
addDaemonPatterns(userDaemonPatterns) | ||
} | ||
} |
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.
User-defined daemon patterns are cleared and reloaded from configuration on every command execution. For codebases with frequent command executions, this creates unnecessary overhead from repeatedly reading the VSCode configuration and recompiling regex patterns.
Consider loading these patterns once when the configuration changes (using vscode.workspace.onDidChangeConfiguration
) and caching them, rather than reloading on every command execution.
Description
This PR fixes issue #8636 where the terminal blocks indefinitely when running long-lived daemon processes like Spring Boot applications, Node.js servers, and other background services.
Problem
Previously, when users ran commands like
mvn spring-boot:run
ornpm start
, the UI would block waiting for the process to complete, preventing users from continuing with other tasks while services ran in the background.Solution
daemon-detector.ts
) that identifies common daemon/service commands using pattern matchingexecuteCommandTool.ts
to detect daemon processes and handle them differently:roo-cline.daemonCommands
: Custom patterns for additional daemon commandsroo-cline.disableDaemonDetection
: Option to disable the feature if neededSupported Daemon Types
mvn spring-boot:run
,gradle bootRun
,java -jar
npm start/dev/serve
,yarn start/dev
,nodemon
,pm2
python -m http.server
,flask run
,django runserver
rails server
dotnet run/watch
docker run
(without --rm),docker-compose up
(without -d)php -S
,php artisan serve
Testing
Review Score
Implementation reviewed with 95% confidence - ready for production
Fixes #8636
Important
Adds daemon process detection and handling to run background services without blocking the UI, with configuration options and comprehensive testing.
daemon-detector.ts
to identify daemon processes using pattern matching.executeCommandTool.ts
to handle daemon processes by allowing them to run in the background after a 3-second initial output check.roo-cline.daemonCommands
androo-cline.disableDaemonDetection
options inpackage.json
for custom daemon patterns and disabling detection.daemon-detector.spec.ts
with 32 test cases covering various daemon patterns.This description was created by
for e06469a. You can customize this summary. It will automatically update as commits are pushed.