Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Mar 7, 2025

Context

Earlier today I encountered another instance of the task UI hanging indefinitely after a command execution. I didn't have the logging in place to diagnose the issue, and I'm still trying to repro it in my dogfooding build, but in the meantime these are some theoretical edge cases that may be worth handling.

  • If for some reason an execution stream is undefined by we have a process object then emit a stream_unavailable event so that we can close the process out instead of waiting indefinitely for a stream that is never to going to be available. This is a theoretical fix; I don't know if it can happen in practice.
  • Add a 10s timeout to await streamAvailable using Promise.race. This ensures that we can unlock the UI if there's some delay on stream availability. Again, this is theoretical.
  • Adds additional logging.

Implementation

Screenshots

before after

How to Test

Get in Touch


Important

Prevent UI lock by handling unavailable streams and adding timeouts in terminal command executions.

  • Behavior:
    • Emit stream_unavailable event in TerminalManager if execution stream is undefined but process object exists.
    • Add 10s timeout to await streamAvailable in TerminalProcess using Promise.race to prevent indefinite UI lock.
  • Logging:
    • Add additional logging in TerminalManager and TerminalProcess for better diagnostics.
  • Events:
    • Add stream_unavailable event to TerminalProcessEvents in TerminalProcess.ts.

This description was created by Ellipsis for eee7bbe. It will automatically update as commits are pushed.

@cte cte requested a review from mrubens as a code owner March 7, 2025 05:42
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Mar 7, 2025
@cte cte changed the title Try to prevent additional cases in which terminal commands lock the task UI Theoretical handling of stream non-availability or delayed availability Mar 7, 2025
@hannesrudolph hannesrudolph moved this from New to PR [Greenlit] in Roo Code Roadmap Mar 7, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 7, 2025
@cte cte merged commit 9fd145a into main Mar 7, 2025
18 checks passed
@cte cte deleted the cte/terminal-reliability branch March 7, 2025 07:57
@github-project-automation github-project-automation bot moved this from PR [Greenlit] to Done in Roo Code Roadmap Mar 7, 2025
cte pushed a commit that referenced this pull request Mar 11, 2025
…ck the task UI"

This reverts commit eee7bbe which has
been superseded by PR #1365.

Fixes: #1435
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 size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants