Fix shell integration race condition (and other minor fixup/cleanup) #1660

KJ7LNW · 2025-03-15T02:56:00Z

Description

Made the terminal shell integration timeout configurable to resolve issues with long shell startup times. Previously, users would encounter "Shell Integration Unavailable" errors due to a hard-coded 4-second timeout. The timeout is now adjustable through Advanced Settings, allowing values from 1 to 60 seconds.

This change:

Makes shellIntegrationTimeout a static property in Terminal class
Adds UI controls in Advanced Settings
Persists the setting in VSCode globalState
Maintains the default 4000ms timeout while allowing customization

Test Procedure

Verified timeout persistence:
- Changed timeout in settings
- Restarted VSCode
- Confirmed new timeout was loaded and applied
Tested with slow shell startup:
- Added artificial delay to shell startup
- Confirmed shell integration works with longer timeout
- Verified error message suggests increasing timeout
Verified UI functionality:
- Slider updates immediately
- Shows correct units (seconds)
- Persists across sessions

Type of Change

✨ New feature (non-breaking change which adds functionality)

Pre-flight Checklist

Changes are limited to a single feature, bugfix or chore
Tests are passing (npm test) and code is formatted and linted
I have created a changeset using npm run changeset
I have reviewed contributor guidelines

Screenshots

Additional Notes

Thanks to:

filthy on discord for troubleshooting the shell integration timeout issues
@kiwina for suggesting making the timeout configurable

Important

Add configurable terminal shell integration timeout to VSCode extension, adjustable via Advanced Settings UI.

Behavior:
- Adds configurable terminalShellIntegrationTimeout to Terminal class, defaulting to 4000ms, adjustable via Advanced Settings.
- Updates ClineProvider.ts to initialize Terminal with persisted timeout value from global state.
- Handles timeout adjustment in AdvancedSettings.tsx and SettingsView.tsx.
UI:
- Adds slider in AdvancedSettings.tsx for terminalShellIntegrationTimeout, range 1000-60000ms.
- Persists setting changes in SettingsView.tsx.
State Management:
- Adds terminalShellIntegrationTimeout to ExtensionState and globalState.ts.
- Updates ExtensionStateContext.tsx to manage new setting.

^{This description was created by}^{for 185e396765560052ca7c1b35647393b374428748. It will automatically update as commits are pushed.}

changeset-bot · 2025-03-15T02:56:04Z

⚠️ No Changeset found

Latest commit: 1b2894c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

webview-ui/src/components/settings/AdvancedSettings.tsx

src/shared/globalState.ts

KJ7LNW · 2025-03-15T03:21:35Z

@ellipsis-dev try again

KJ7LNW · 2025-03-15T03:34:40Z

Tested, rebased, and ready to pull

* Update README.md * Create neat-apricots-search.md --------- Co-authored-by: Saoud Rizwan <[email protected]>

KJ7LNW · 2025-03-16T23:29:46Z

@cte , this pr contains a critical fix. there are the race conditions that may break some versions of vscode with terminal integration. I have fixed those in 94822a313d0f0fc90391187cfe1bfc18b9081d07

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 <[email protected]>

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 <[email protected]>

@FILTHY

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

Clear guidance for the AI system on: - Working directory constraints - Path handling requirements - Tool vs terminal directory behavior Signed-off-by: Eric Wheeler <[email protected]>

KJ7LNW · 2025-03-17T00:40:04Z

@hannesrudolph see minor changes to clarify directory issues.

Signed-off-by: Eric Wheeler <[email protected]>

cte · 2025-03-17T16:27:37Z

@KJ7LNW - Thanks for this. I'm not 100% sure about adding a new setting though. What do you think about just making the default timeout higher?

KJ7LNW · 2025-03-18T00:05:15Z

@KJ7LNW - Thanks for this. I'm not 100% sure about adding a new setting though. What do you think about just making the default timeout higher?

I have given this a lot of thought, there are two bounding problems:

Too high (ie, 30s), and people will think that there is a bigger problem than a shell integration problem, and the bug reports will be confusing without feedback: They will simply close the task or the entire program (or Roo task) before the long time out and open reports telling us that it completely froze while running a command.
Too low, and there are many circumstances where it will fail too early, as we have seen:
- People with slow Windows computers that take forever to load PowerShell
- New WSL shell prompts are also slow to load
- Edge cases like with one Roo user whose .bashrc was >400 lines (!) so adding integration to the bottom always timed out.
- Remote terminals could be slow to load, especially with possible reverse DNS ssh problems.

Having this configurable gives us an additional troubleshooting tool when someone has an issue: We can have them turn it up and see if it fixes the issue, and if it does, it very easily confirms that it is their configuration issue.

Note that the config slider will close #1654.

KJ7LNW requested review from cte and mrubens as code owners March 15, 2025 02:56

github-project-automation bot added this to Roo Code Roadmap Mar 15, 2025

github-project-automation bot moved this to New in Roo Code Roadmap Mar 15, 2025

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Mar 15, 2025

ellipsis-dev bot reviewed Mar 15, 2025

View reviewed changes

webview-ui/src/components/settings/AdvancedSettings.tsx Outdated Show resolved Hide resolved

src/shared/globalState.ts Outdated Show resolved Hide resolved

gxgl mentioned this pull request Mar 15, 2025

Roo-Code stuck after failing terminal #1654

Closed

ipattis pushed a commit to ipattis/roo-code that referenced this pull request Mar 15, 2025

Update README.md to include Getting Started (RooCodeInc#1660)

1935aab

* Update README.md * Create neat-apricots-search.md --------- Co-authored-by: Saoud Rizwan <[email protected]>

Eric Wheeler added 6 commits March 16, 2025 16:32

system instructions: clarify terminal directory operations

de5157a

Clear guidance for the AI system on: - Working directory constraints - Path handling requirements - Tool vs terminal directory behavior Signed-off-by: Eric Wheeler <[email protected]>

test: update snapshots for system prompt working directory instructions

7ff67c7

Signed-off-by: Eric Wheeler <[email protected]>

KJ7LNW changed the title ~~Feat/cleanup: Add configurable terminal shell integration timeout~~ Fix integration race conditions (and other minor fixup/cleanup) Mar 17, 2025

KJ7LNW changed the title ~~Fix integration race conditions (and other minor fixup/cleanup)~~ Fix shell integration race conditions (and other minor fixup/cleanup) Mar 17, 2025

KJ7LNW changed the title ~~Fix shell integration race conditions (and other minor fixup/cleanup)~~ Fix shell integration race condition (and other minor fixup/cleanup) Mar 17, 2025

hannesrudolph linked an issue Mar 17, 2025 that may be closed by this pull request

Roo-Code stuck after failing terminal #1654

Closed

hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap Mar 17, 2025

KJ7LNW mentioned this pull request Mar 18, 2025

Roo code can not read the result of executing commands with pyenv selected in vscode #1537

Closed

Merge branch 'main' into roo-misc-terminal-cleanup

1b2894c

cte approved these changes Mar 18, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 18, 2025

cte merged commit 6301e90 into RooCodeInc:main Mar 18, 2025
12 checks passed

github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap Mar 18, 2025

KJ7LNW mentioned this pull request Mar 18, 2025

Problem with calling terminal commands #1503

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix shell integration race condition (and other minor fixup/cleanup) #1660

Fix shell integration race condition (and other minor fixup/cleanup) #1660

Uh oh!

KJ7LNW commented Mar 15, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

changeset-bot bot commented Mar 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

KJ7LNW commented Mar 15, 2025

Uh oh!

KJ7LNW commented Mar 15, 2025

Uh oh!

KJ7LNW commented Mar 16, 2025

Uh oh!

KJ7LNW commented Mar 17, 2025

Uh oh!

cte commented Mar 17, 2025

Uh oh!

KJ7LNW commented Mar 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix shell integration race condition (and other minor fixup/cleanup) #1660

Fix shell integration race condition (and other minor fixup/cleanup) #1660

Uh oh!

Conversation

KJ7LNW commented Mar 15, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Procedure

Type of Change

Pre-flight Checklist

Screenshots

Additional Notes

Uh oh!

changeset-bot bot commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

Uh oh!

KJ7LNW commented Mar 15, 2025

Uh oh!

KJ7LNW commented Mar 15, 2025

Uh oh!

KJ7LNW commented Mar 16, 2025

Uh oh!

KJ7LNW commented Mar 17, 2025

Uh oh!

cte commented Mar 17, 2025

Uh oh!

KJ7LNW commented Mar 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KJ7LNW commented Mar 15, 2025 •

edited by ellipsis-dev bot

Loading

changeset-bot bot commented Mar 15, 2025 •

edited

Loading