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

[KJ7LNW]

Current Behavior

When executing a command through Roo while another command is already running in the terminal, the running command gets canceled.

Expected behavior

Open a new terminal instance for the new command

Steps to reproduce

1. Run any simple command to open a new terminal echo test
2. Manually execute a long-running command in the terminal (e.g., sleep 60)
3. While that command is still running, try to execute another command through Roo
4. Observe that the first command gets canceled (indicated by ^C) and the new command executes

Terminal transcript showing the issue

]$ echo test
test
]$ sleep 60
^C #<< interrupted
echo test2
]$ echo test2
test

Scoping

Set Terminal.busy when a command is running in the terminal.

Roo is capable of recognizing when a command has been run in a terminal and the terminal should be marked busy: [onDidStartTerminalShellExecution] {command: 'sleep 60', terminalId: 2}

When the command terminates it should be no longer busy: [TerminalRegistry] Shell execution end event received, but process is not running for terminal

Version

• App version: 3.16.5
• API Provider: Anthropic

[dosubot]

Based on the documentation, Roo Code manages terminal command execution using a TerminalRegistry, which is responsible for getting or creating terminals, setting up working directory contexts, and monitoring command execution status. When a command is executed, Roo Code should ideally recognize if a terminal is "busy" and either queue the command or open a new terminal instance for the next command. However, your report shows that executing a new command through Roo while another is running cancels the running command, rather than opening a new terminal as expected.

I did not find any existing tickets or discussions in the documentation that specifically address this issue of command cancellation when a new command is executed. The release notes for version 3.15 mention improvements in terminal command handling and reliability, but do not describe this specific scenario or a fix for it. The documentation also does not provide details on how Roo Code decides when to open a new terminal instance versus reusing an existing one, nor does it describe configuration options to change this behavior.

If you are looking to avoid this interruption, you may want to check if there are any settings or workspace configurations that control terminal instance management, but based on the available documentation, this behavior appears to be a limitation or bug in the current implementation. For more details on how Roo Code manages terminal commands, you can refer to the terminal command execution documentation and the release notes for v3.15.

If you need implementation-level guidance or a workaround, you may need to look into how TerminalRegistry marks terminals as busy and whether you can manually intervene to force new terminal creation for each command. If you have access to the codebase, reviewing the logic around terminal allocation and busy state management would be the next step.

To reply, just mention my name @dosu.

---

^{How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other}   

[Mnehmos]

1. Introduction

This document provides a detailed analysis of GitHub Issue #4319, which describes a bug where executing a command in a Roo-managed terminal that is already running a manual command (e.g., sleep 60) cancels the running command. The expected behavior is for Roo to open a new terminal instance for the new command, leaving the existing process untouched.

2. Problem Analysis

The root cause of this issue lies in the terminal reuse logic within the TerminalRegistry. When a user manually executes a command in a terminal previously opened by Roo, the TerminalRegistry is not aware of this new process. The onDidStartTerminalShellExecution event fires, but because the command was not initiated via the executeCommandTool, the corresponding RooTerminal object is not marked as busy.

Consequently, when the user subsequently asks Roo to execute a new command, the getOrCreateTerminal function finds the existing terminal, sees that its busy flag is false, and reuses it. This reuse sends the new command to the terminal, which interrupts and cancels the manually-executed process.

3. Codebase Analysis

The investigation focused on the following key files:

• src/integrations/terminal/TerminalRegistry.ts: This class is responsible for creating, tracking, and reusing terminal instances. The primary function of interest is getOrCreateTerminal, which contains the logic for selecting an available terminal. The event handlers for onDidStartTerminalShellExecution and onDidEndTerminalShellExecution are also critical as they are intended to track the busy state of terminals.

• src/core/tools/executeCommandTool.ts: This tool is the entry point for all command executions initiated by Roo. It calls TerminalRegistry.getOrCreateTerminal to acquire a terminal instance before running the command.

• src/integrations/terminal/types.ts (and its implementations like Terminal.ts): These define the RooTerminal interface and its implementations, which encapsulate the state of a terminal, including its busy status and the currently running process.

The core flaw is the disconnect between VS Code's terminal state and Roo's internal tracking. Roo only considers a terminal "busy" if it initiated the command itself. It does not account for user-initiated commands within the same terminal window.

4. Proposed Solution

To address this, the TerminalRegistry must be updated to more accurately reflect the true state of the underlying VS Code terminal.

1. Enhance onDidStartTerminalShellExecution Logic:
   In TerminalRegistry.ts, the onDidStartTerminalShellExecution handler should be modified. When it detects a shell execution starting in a registered Roo terminal, it should immediately mark that terminal as busy, regardless of whether Roo initiated the command. A new property, perhaps isExternallyBusy, could be added to the RooTerminal interface to differentiate between Roo-initiated commands and manual ones.

2. Update getOrCreateTerminal:
   The getOrCreateTerminal function must be updated to respect this new state. It should not reuse any terminal that is marked as busy or isExternallyBusy. This will force the creation of a new terminal when the user asks Roo to execute a command while another is running manually, which is the desired behavior.

3. Reliable State-Clearing:
   The onDidEndTerminalShellExecution event should clear the busy/isExternallyBusy flag. Given the known reliability issues with this event (as seen in issue When Roo runs command in the terminal gets stuck even if the command is finished #4384), a fallback mechanism may be necessary. A simple approach could be to add a "Force New Terminal" option to the UI, or to implement a timeout after which an externally-busy terminal is considered free again, though this is less ideal. The primary fix, however, is to prevent reuse of a busy terminal.

5. Implementation Plan

1. Modify RooTerminal Interface: Add a new boolean property isExternallyBusy to the RooTerminal interface in src/integrations/terminal/types.ts.

2. Update TerminalRegistry.ts:

   • In the onDidStartTerminalShellExecution handler, if a terminal is found, set terminal.isExternallyBusy = true;.
   • In the onDidEndTerminalShellExecution handler, set terminal.isExternallyBusy = false;.
   • In getOrCreateTerminal, update the find logic to exclude terminals where t.busy or t.isExternallyBusy is true.

3. Testing:

   • Create a new test case in src/integrations/terminal/__tests__/TerminalRegistry.test.ts that simulates the scenario described in the issue:
     1. Create a terminal.
     2. Simulate a manual command start by firing onDidStartTerminalShellExecution for that terminal.
     3. Call getOrCreateTerminal.
     4. Assert that a new terminal instance is created, rather than reusing the existing one.

6. Conclusion

This approach directly addresses the identified problem by making Roo's internal state management more aware of external terminal activity. By preventing the reuse of any busy terminal, it ensures that user-initiated commands are not interrupted, aligning the tool's behavior with user expectations.

[KJ7LNW]

A new property, perhaps isExternallyBusy, could be added to the RooTerminal interface to differentiate between Roo-initiated commands and manual ones.

No, please keep it simple: use the existing busy flag otherwise we do not have a single source of truth for terminal busy-ness.

[Mnehmos]

Proposed Solution

The solution will focus on ensuring the existing RooTerminal.busy flag accurately reflects the state of the underlying VS Code terminal, thereby maintaining a single source of truth for terminal busy-ness.

Modifications to src/integrations/terminal/TerminalRegistry.ts

1. Enhance onDidStartTerminalShellExecution Logic:
   • Modify the onDidStartTerminalShellExecution handler. When a shell execution starts in a RooTerminal (identified by its terminalId), set the existing terminal.busy flag to true. This will ensure that any command, whether initiated by Roo or manually by the user, marks the terminal as busy.
2. Update getOrCreateTerminal Logic:
   • The getOrCreateTerminal function's logic for finding an available terminal should already be checking t.busy. With the onDidStartTerminalShellExecution handler correctly setting t.busy for manual commands, this function will naturally be forced to create a new terminal when an existing one is busy, achieving the desired behavior.
3. Ensure onDidEndTerminalShellExecution Clears Flag:
   • Verify that the onDidEndTerminalShellExecution handler correctly resets terminal.busy to false when a command finishes. While issue When Roo runs command in the terminal gets stuck even if the command is finished #4384 points to potential reliability issues with this event, ensuring the flag is set correctly on start is the primary fix for this issue. Any lingering issues with onDidEndTerminalShellExecution reliability would be a separate, follow-up task.

No Changes to src/integrations/terminal/types.ts

As per KJ7LNW's feedback, no new properties like isExternallyBusy will be added to the RooTerminal interface. The existing busy flag is sufficient.

Implementation Plan

1. Locate and Analyze TerminalRegistry.ts:
   • Read the contents of src/integrations/terminal/TerminalRegistry.ts to understand its current implementation, specifically the onDidStartTerminalShellExecution, onDidEndTerminalShellExecution, and getOrCreateTerminal methods.
2. Modify onDidStartTerminalShellExecution:
   • Implement the logic to set terminal.busy = true; within the onDidStartTerminalShellExecution handler for the relevant RooTerminal instance.
3. Verify getOrCreateTerminal Behavior:
   • Confirm that the existing logic in getOrCreateTerminal correctly checks the busy flag and will create a new terminal if busy is true. No direct code changes are expected here, but verification is crucial.
4. Verify onDidEndTerminalShellExecution Behavior:
   • Confirm that the onDidEndTerminalShellExecution handler correctly sets terminal.busy = false;.
5. Develop New Test Case:
   • Create a new test case in src/integrations/terminal/__tests__/TerminalRegistry.test.ts. This test should:
     • Simulate the creation of a RooTerminal.
     • Manually trigger the onDidStartTerminalShellExecution event for that terminal (simulating a user-initiated command).
     • Call TerminalRegistry.getOrCreateTerminal.
     • Assert that a new RooTerminal instance is returned, confirming that the original terminal was correctly identified as busy and not reused.

source_insights:

• artifact_id: 4319
  summary: >
  The issue describes a bug where executing a command via Roo while another command is running in the terminal cancels the running command. The expected behavior is to open a new terminal instance. Key comments from Mnehmos and KJ7LNW provide conflicting initial proposals, with KJ7LNW's comment ("No, please keep it simple: use the existing busy flag otherwise we do not have a single source of truth for terminal busy-ness.") being the definitive directive.

[KJ7LNW]

that it's pretty good, it should basically be a two line change