Terminal mishandles compound commands

[pwilkin]

App Version

3.25.23

API Provider

Not Applicable / Other

Model Used

N/A

Roo Code Task Links (Optional)

No response

🔁 Steps to Reproduce

Start a task and make the LLM use a compound terminal command, where the output is generated by the second part. (by compound command I mean a command that uses an additional process spawning operator, such as Linux's &&, the most typical example in my cases being cd dir && run test)

💥 Outcome Summary

The LLM does not see the output of the command. The TerminalRegistry reports an error with no process running for the terminal. Moreover, a new terminal is spawned every time this happens.

📄 Relevant Logs or Errors (Optional)

console.ts:137 [Extension Host] [onDidEndTerminalShellExecution] 
Object
log.ts:460   ERR [Extension Host] [TerminalRegistry] Shell execution end event received, but process is not running for terminal: 
Object
console.ts:137 [Extension Host] [TerminalRegistry] Shell execution end event received, but process is not running for terminal: 
Object

[roomote]

I'm exploring a potential fix for the terminal compound command handling issue and will open a PR to share progress.

[pwilkin]

@roomote-agent The problem here is, compound commands (like Linux's && operator ones) spawn more than one process. When the first processes finishes, the terminal integration assumes the entire thing is finished - but the other command keeps running.

What should be done is either disallow compound commands for shell integration, disassemble the commands so that the get executed one by one (i.e. cd foo && npm test becomes first cd foo, then in the same terminal npm test) or add proper process handling to intercept all the processes spawned, not just the first one.

[roomote]

Thanks @pwilkin for the clarification! I understand the issue now - compound commands with operators like && spawn multiple processes, but the terminal integration only tracks the first one, causing it to incorrectly assume the command is complete when the first process ends.

I'll explore the codebase to understand the current terminal integration implementation and work on one of the suggested solutions:

1. Disallow compound commands for shell integration
2. Parse and execute compound commands sequentially (e.g., split cd foo && npm test into separate commands)
3. Implement proper multi-process tracking

I'll investigate which approach fits best with the existing architecture and open a PR with the fix.

[roomote]

I've investigated this issue and found that the problem occurs when compound commands (using operators like &&, ||, etc.) are executed. The shell execution end event arrives before the terminal is properly marked as running, causing the error.

I've implemented an initial fix that:

• Changes the error logging to a warning when a non-running terminal receives an end event but has a process
• Still completes the process if it exists, preventing the terminal from getting stuck
• Adds comprehensive tests for compound command scenarios

However, after review, I realize this only addresses the symptom (error logging) rather than the root cause. The core issue is that compound commands spawn multiple processes, but the current implementation doesn't properly track them all.

Potential comprehensive solutions could be:

1. Parse and split compound commands - Detect operators like &&, ||, `;" and execute commands sequentially
2. Implement multi-process tracking - Track all processes spawned by compound commands
3. Explicitly handle or disallow compound commands - Either wrap them in a shell explicitly or provide clear error messages

Which approach would you prefer? The current fix prevents the error spam and terminal lockup, but doesn't fully capture output from all parts of compound commands.

Branch with initial fix: fix/terminal-compound-commands-7430

[pwilkin]

@roomote-agent The proper solution should be 2.

What should happen is that the TerminalRegistry should take note whenever a process is spawned, but check if a new process was not spawned after the previous completed. It should not report to the LLM until all the processes have finished. When it does report to the LLM, it should separate the outputs from the processes.

[pwilkin]

@roomote-agent So, to explain further:

Current behavior (incorrect):

• shell runs cd dir && command_with_output
• TerminalRegistry catches the first process' (cd dir) exit code
• TerminalRegistry reports that to the LLM
• second command finishes
• TerminalRegistry throws an error because it doesn't know what happened

Correct behavior:

• shell runs cd dir && command_with_output
• TerminalRegistry catches the first process' exit code
• TerminalRegistry checks if more processes have been spawned
• TerminalRegistry waits for the second process
• second command finishes
• TerminalRegistry reports both command outputs, separated, to the LLM

[roomote]

Thanks @pwilkin for the detailed explanation! I understand the requirement now:

The TerminalRegistry needs to properly handle compound commands by:

1. Tracking all processes spawned by compound operators (&&, ||, ;, etc.)
2. Waiting for ALL processes to complete before reporting to the LLM
3. Separating and preserving output from each process in the chain

I'll implement multi-process tracking to ensure the TerminalRegistry waits for all processes in a compound command to finish and reports their outputs correctly. Working on this now.