Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jun 12, 2025

Related GitHub Issue

Closes: #4319

Description

This PR addresses a bug where a Roo-managed terminal would get stuck in a "busy" state if a user-initiated (manual) command was run in it. The root cause was that the onDidEndTerminalShellExecution event handler would return early for non-Roo-initiated processes without resetting the busy flag.

The fix is a one-line change in src/integrations/terminal/TerminalRegistry.ts to set terminal.busy = false before the early return, ensuring the terminal's state is accurately reset after any command finishes.

Test Procedure

The fix was verified manually by following the reproduction steps outlined in the issue:

  1. Run a simple command via Roo (e.g., echo test).
  2. In the same terminal, run a long-running manual command (e.g., sleep 30).
  3. While it's running, execute another command via Roo.
  4. Verification: A new terminal is correctly opened for the new command.
  5. After the manual sleep command finishes, execute another command via Roo.
  6. Verification: The original terminal is now correctly reused, as its busy flag was properly cleared.

Automated tests were not added because the existing test file src/integrations/terminal/__tests__/TerminalRegistry.test.ts has multiple pre-existing failures that are out of scope for this small bug fix.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Honestly, having trouble recreating this issue but this is a one line change if anyone else could verify.

Co-authored-by: Mnehmos [email protected] via #4449


Important

Fixes bug where terminals remain 'busy' after manual commands by resetting the busy flag in TerminalRegistry.

  • Behavior:
    • Fixes bug in TerminalRegistry where terminals remain 'busy' after manual commands by setting terminal.busy = false in onDidEndTerminalShellExecution.
  • Tests:
    • Adds unit tests in TerminalRegistry.test.ts to verify busy flag behavior for Roo and non-Roo terminals.
    • Tests include scenarios for shell execution start, end, and manual commands.

This description was created by Ellipsis for 91d4b56. You can customize this summary. It will automatically update as commits are pushed.

- Add terminal.busy = true when shell execution starts
- Add terminal.busy = false when shell execution ends for both Roo and non-Roo terminals
- Add comprehensive tests for busy flag management
- Fixes issue where terminals got stuck in busy state after manual commands
@hannesrudolph hannesrudolph requested review from cte, jr and mrubens as code owners June 12, 2025 04:03
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 12, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jun 12, 2025
@hannesrudolph hannesrudolph added lgtm This PR has been approved by a maintainer PR - Needs Review labels Jun 12, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

I managed to reproduce the issue, Roo shouldn't cancel running commands.

LGTM

@mrubens mrubens merged commit c17a07d into main Jun 12, 2025
25 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 12, 2025
@mrubens mrubens deleted the 4319 branch June 12, 2025 15:09
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Bug: Executing a command while another command is running cancels the running command

4 participants