Commit graph

28 commits

Author SHA1 Message Date
Eric Wheeler
0b0c438284 fix: standardize terminal integration timeout values
Replace hardcoded 3000ms timeout with configurable Terminal.shellIntegrationTimeout
in TerminalProcess.ts. This ensures consistent timeout behavior across all
terminal integration features and allows users to control both timeouts through
a single setting.

The error messages are also updated to display the dynamic timeout value,
providing clearer feedback when shell integration issues occur.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-04-10 21:18:37 -07:00
KJ7LNW
6301e90454
Fix shell integration race condition (and other minor fixup/cleanup) (#1660)
* fix: clarify PowerShell command completion workaround

The command completion detection approach in PowerShell requires an output
string to allow duplicate commands to execute in some versions of code.
Update the string to explicitly indicate it is a Roo PowerShell workaround,
making it clear in terminal output that this is intentional behavior rather
than a side effect.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* cleanup: improve terminal logging and error handling

No functional changes - purely improves error handling and logging clarity.

Terminal.ts:
- Handle undefined process state in setActiveStream without throwing
- Add terminal IDs to all log messages for better traceability
- Improve error message clarity in shell integration timeout

TerminalRegistry.ts:
- Reorganize shell execution event handlers for better flow
- Log shell execution events before processing for reliable debugging
- Add detailed context to terminal not found scenarios
- Include command and execution state in error messages

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* feat: make terminal shell integration timeout configurable

Users with long shell startup times were encountering "Shell Integration Unavailable" errors due to the hard-coded 4s timeout. The timeout is now configurable through Advanced Settings (1-60s).

Thanks @filthy for troubleshooting and @kiwina for suggesting making the timeout configurable.

Fixes #1654

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* critical fix: race condition that prevents command completion

Terminal running state is now managed in TerminalRegistry instead of Terminal to prevent race between stream close and shell completion.

While this race may not trigger on current VSCode versions, newer releases with additional terminal fixes may expose the issue. This proactively prevents "Shell execution end event received, but process is not running" errors.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* fix: improve command execution path reporting

Enhance clarity of command execution context and error reporting:

- Check to see if the directory changed because of the command
- Clarify execution path message
- Add explicit message when command exits with non-zero code

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* system instructions: clarify terminal directory operations

Clear guidance for the AI system on:
- Working directory constraints
- Path handling requirements
- Tool vs terminal directory behavior

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

* test: update snapshots for system prompt working directory instructions

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

---------

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Chris Estreich <cestreich@gmail.com>
2025-03-17 23:57:20 -07:00
Eric Wheeler
a25f1d949d fix: add PowerShell-specific command handling
PowerShell requires special handling for command output due to two issues:
- A sleep delay is required to prevent the ]633;D marker from losing the
  original output
- A counter is needed to work around a bug where identical commands are not executed

Changes:
- Add cmdCounter to Terminal class for unique command tracking
- Add PowerShell detection via platform and default shell profile
- Add sleep delay to ensure output is captured before command completion

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-11 20:54:15 -07:00
Eric Wheeler
4e0f868261 fix: improve error handling when start sequence not received but stream started
When VSCE start sequence (]633;C or ]133;C) is not received, but the
stream has started:

- Emit no_shell_integration event with clear error message
- Include preOutput in completed event for bug reporting
- Call continue() to ensure proper cleanup
- Return early to prevent further processing

This helps diagnose potential upstream VSCE bugs by providing more context
in the error messages and ensuring proper cleanup of terminal state.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
8de202ba09 fix: prevent UI freeze when terminal stream is unavailable
Add timeout and error handling to terminal stream initialization to prevent
UI from freezing when a stream is unavailable or never starts. This ensures
that if the VSCE shell integration stream does not start within 3 seconds:

- The streamAvailable promise is rejected with a clear error
- Event listeners are cleaned up to prevent memory leaks
- Terminal state is properly reset
- Execution continues rather than hanging indefinitely

This fixes a potential deadlock where the UI could freeze waiting for a
stream that never becomes available.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
a867595a6a doc: enhance shell integration error messages
Add descriptive messages to shell integration failures to help users
understand and resolve integration issues more effectively. This improves
the debugging experience by providing specific details about why shell
integration failed.

- Add message parameter to no_shell_integration event
- Update UI to display specific error messages
- Update troubleshooting documentation link

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
7dadf4cd1b refactor: remove redundant terminal ID from event handling
As pointed out by @cte, passing and checking terminal IDs in events is
unnecessary since a TerminalProcess instance can never be associated with a
different Terminal instance. The event handling is already properly scoped
to the specific TerminalProcess instance.

- Remove terminal ID parameter from shell_execution_complete event
- Remove terminal ID parameter from stream_available event
- Update all event handlers to remove ID checks
- Update all test cases to match new event signatures

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
5c9f7722b6 docs: document TerminalProcess stability guidelines
The TerminalProcess class is a critical component for VSCode shell integration.
This documentation explains why changes must be minimal and carefully considered:

- Performance optimizations using index-based operations and zero-copy implementation
- Accuracy requirements for handling terminal output and escape sequences
- Complex integration with VSCode shell features and command execution
- Careful handling of stream data and escape sequence processing
- Backwards compatibility considerations for VSCode releases

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
25e46a244d fix: terminal output not showing after command completion
Fix an issue where background running terminals that complete their
execution do not report the final output of their command. Previously,
output was reported while the command was active, but after termination
the remaining output was not provided within the 'inactive terminals'
section of environment details.

- Implement terminal process queue system to track completed processes
- Store command and output retrieval state per process
- Add helper methods to manage the process queue efficiently
- Update getEnvironmentDetails to properly display output from completed processes

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
24d8ef91ce fix: emit line event when command output starts
This fixes an issue where commands that wait for input (like 'cat'
without arguments) would hang indefinitely because no 'line' event was
emitted. The terminal would receive the VSCode shell integration marker
indicating command output has started, but since there was no actual
output yet, the UI would not proceed.

By emitting an empty line event when command output starts, we ensure
the UI can proceed even when a command is waiting for input, preventing
the task from hanging.

Thank you @cte for pointing this out in the PR development process.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
9f5a67ecf8 refactor: remove TerminalManager after migration
- Delete TerminalManager.ts as functionality has been migrated
- Remove TerminalManager import and usage from tests
- Remove outdated TerminalManager references from comments
- Fix TypeScript types in TerminalRegistry event handlers

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
3157bf29a8 refactor: move interpretExitCode from TerminalManager to TerminalProcess
- Make interpretExitCode a static method in TerminalProcess
- Update all references to use the static method
- Add comprehensive unit tests for exit code interpretation
- Test with real shell commands for different exit conditions

This change improves code organization by moving the exit code interpretation
logic to the appropriate class, making it more maintainable and reusable.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
59745a7058 refactor: establish natural terminal hierarchy
Move terminal state access from TerminalManager to TerminalRegistry to establish
a clear hierarchical relationship between components. This change centralizes
terminal management in TerminalRegistry and eliminates duplicate state tracking
in TerminalManager.

The hierarchy flows from TerminalRegistry (managing all terminals) to Terminal
(encapsulating a terminal instance) to TerminalProcess (running within a terminal).

Key changes:
- Remove `processes` map from TerminalManager
- Add static getUnretrievedOutput and isProcessHot methods to TerminalRegistry, which manages all terminals globally

Test updates:
- Modify test setup to create Terminal instances
- Remove processes map usage from tests
- Update process creation and command execution flow in tests

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
304fcf2fbc fix: improve terminal execution with shell integration status
When shell integration is not available, the system now provides clear feedback
about command execution status and maintains consistent event flow.

- Removed waitForShellIntegration property to simplify code flow
- Consolidated event emission to ensure consistent behavior
- Updated tests to verify correct event sequence
- Simplified shell integration detection with pWaitFor
2025-03-10 20:55:05 -07:00
Eric Wheeler
daf36f38ea refactor: move interpretExitCode from TerminalManager to TerminalProcess
This commit moves the interpretExitCode method from TerminalManager to
TerminalProcess class, as part of the terminal refactoring effort. The
method is responsible for translating exit codes into detailed results,
including signal information.

Changes include:
- Moved interpretExitCode method to TerminalProcess class
- Updated imports in TerminalManager and Cline to reference
  ExitCodeDetails from TerminalProcess
- Added findTerminalIdByVscodeTerminal helper method in TerminalManager
- Added comprehensive unit tests for interpretExitCode in
  TerminalProcess
- Tests cover undefined exit codes, normal exit codes (0-127), and
  signal exit codes (128+)

This change improves code organization by placing the exit code
interpretation logic closer to where it's primarily used, in the
TerminalProcess class.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
3a950bbc15 refactor: Rename TerminalInfo to Terminal and relocate to Terminal.ts
Transformed the TerminalInfo interface into a proper Terminal class and
moved it to its own file. This improves code organization and
encapsulation by centralizing terminal-related functionality.

The change establishes a clearer object model for terminal management,
setting the foundation for a more maintainable terminal architecture.
All references throughout the codebase have been updated to use the new
Terminal class while preserving existing functionality.

Tests have been updated and verified to ensure compatibility with the
new structure.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
2025-03-10 20:55:05 -07:00
Eric Wheeler
070a36baa2 Revert "Smart truncation for terminal output"
This reverts commit 7eee3e0878.

Middle-out truncation is a really great feature and it should still be
implemented, however it unnecessarily interferes with #1365 because it
hooked into the low-level chunk management that comes directly from VSCE
shell integration.

The best place to hook OutputBuilder is as follows depending on the
state of terminal interaction:

1. Foreground terminals:

Cline.ts:
	 executeCommandTool(...) {
	     process.on("line", (line) => {
		    lines.push(line)
		    ...
	    }
	 }

2. For background terminals: hook in at the point that getUnretrievedOutput is consumed for active or
   inactive terminals in Cline.ts:getEnvironmentDetails()

Please note:

The Terminal classes are very sensitive to change, partially because of
the complicated way that shell integration works with VSCE, and
partially because of the way that Cline interacts with the Terminal*
class abstractions that make VSCE shell integration easier to work with.

At the point that PR#1365 is merged, it is unlikely that any Terminal*
classes will need to be modified substantially.  Generally speaking, we
should think of this is a stable interface and minimize changes.

Reverts: #1390
2025-03-10 20:54:53 -07:00
Eric Wheeler
384b469bf1 Revert "Handle outputless commands"
This reverts commit 710284cc3d which has
been superseded by PR #1365.

Fixes: #1416
2025-03-10 20:50:10 -07:00
Eric Wheeler
7e0a4dd426 Revert "Try to prevent additional cases in which terminal commands lock the task UI"
This reverts commit eee7bbe104 which has
been superseded by PR #1365.

Fixes: #1435
2025-03-10 20:50:10 -07:00
cte
eee7bbe104 Try to prevent additional cases in which terminal commands lock the task UI 2025-03-06 21:34:03 -08:00
cte
710284cc3d Handle outputless commands 2025-03-05 21:28:42 -08:00
cte
7eee3e0878 Smart truncation for terminal output 2025-03-05 19:36:52 -08:00
Chris Estreich
4b66ce8255 Improve terminal command execution output parsing 2025-02-28 09:38:48 -08:00
Matt Rubens
dc83617b4d
Revert "Stop removing commas from terminal output" 2025-02-27 09:51:05 -05:00
Matt Rubens
5c5bf85020 Stop removing commas from terminal output 2025-02-26 21:36:19 -05:00
Mark Percival
93e70c62f1
Chore: Prettier for consistant formatting (#794)
* Chore: Pretier for consistant formatting

- TODO: This PR needs to be updated by Saoud after he runs `npm install` & `npm format:fix` and commits the results of the prettier changes.

* Revert prettier config

* Run npm install

* Fix prettier config and ignore package lock

* Run format

---------

Co-authored-by: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com>
2024-11-21 13:13:54 -08:00
Saoud Rizwan
c04dfc76cb Misc refactor cline 2024-10-09 01:49:59 -04:00
Saoud Rizwan
c2aff91973 Refactor TerminalManager 2024-09-24 12:54:05 -04:00