Improve project action terminal readiness#3175
Conversation
…ed-project-actions # Conflicts: # apps/web/src/components/ChatView.browser.tsx # apps/web/src/components/ChatView.tsx
…ed-project-actions
…ed-project-actions
…ed-project-actions
…ed-project-actions
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 777cece. Configure here.
| }); | ||
| } | ||
| if (isInteractiveShellCommand(normalized)) { | ||
| return { hasRunningSubprocess: false, childCommand: null, processIds: [] }; |
There was a problem hiding this comment.
PS failure misreports busy shell
High Severity
When the full ps process-table pass fails after the direct PTY child is classified as an interactive shell, POSIX subprocess inspection returns idle (hasRunningSubprocess: false) without walking descendants. A command still running under that shell can be missed, so the UI may treat the action terminal as free and send a project script while work is still active.
Reviewed by Cursor Bugbot for commit 777cece. Configure here.
| ); | ||
| if (idleActionTerminal) { | ||
| return idleActionTerminal; | ||
| } |
There was a problem hiding this comment.
Action terminal prefix steals sessions
High Severity
resolveProjectActionTerminalId treats any action-{scriptId}-* id as a fallback for script {scriptId}. A script whose id is build-2 uses primary terminal action-build-2, but when build runs while action-build is busy the same id is reused. Different project actions can share one PTY and send commands to the wrong session.
Reviewed by Cursor Bugbot for commit 777cece. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces new terminal readiness detection and selection logic for project actions, representing significant new runtime behavior. Two high-severity unresolved review comments identify potential bugs in subprocess detection and terminal ID resolution that could cause commands to be sent to incorrect terminals. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
This changes project actions to use stable action-specific terminal sessions and wait until the target terminal is ready before writing the action command.
It also improves terminal subprocess detection so idle POSIX login shells are not treated as busy work, which lets action terminals be reused once they are actually sitting at a prompt.
What Changed
action-*terminal IDs for project scripts, including suffix allocation when existing action terminals are busy.ChatViewproject-script execution to prefer reusable action terminals, wait for readiness when needed, and only allocate regular terminals when explicitly requested.Action: lint.Why
When running a custom action, the terminal can open and receive the inserted action before the shell is ready. Once the terminal later becomes ready, it may no longer be able to receive that action. This change makes action execution wait for an input-ready prompt and improves terminal reuse so the action is sent to a usable shell session.
Validation
pnpm exec vp checkpnpm exec vp run typecheckpnpm exec vp test run apps/web/src/projectScriptTerminals.test.ts apps/server/src/terminal/Layers/Manager.test.ts packages/shared/src/terminalLabels.test.tsProof
Note
Medium Risk
Changes terminal busy-state semantics on POSIX and the project-script write path; behavior is covered by new tests but misclassification could still affect reuse or premature command injection.
Overview
Project scripts now target stable
action-*terminal sessions, wait for a shell prompt before sending the command, and reuse idle action terminals instead of always opening a new tab when the active one looks busy.Client: New helpers pick action terminal IDs (with suffixed fallbacks when busy), detect prompt readiness from terminal output (ANSI/OSC stripped), and block on the attach stream until ready or timeout.
ChatViewuses these for script runs and only allocates a regularterm-*whenpreferNewTerminalis set. A runtime command exposes the wait step over the existing RPC path.Server: POSIX subprocess polling now reads
pscommand names, walks the process tree, treats idle interactive shells as not busy, and surfaces the first non-shell descendant as the running command—so an action terminal sitting atbashcan be reused.Labels:
action-*terminal IDs render as readable Action: names in the UI.Reviewed by Cursor Bugbot for commit 777cece. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Improve project action terminal readiness by targeting stable terminal IDs and awaiting prompt detection
action-build,action-build-2) inprojectScriptTerminals.ts, with logic to pick an idle terminal or allocate the next available suffix.$,#,%,>); exposes both Stream-based and API-based wait utilities.ChatView.tsxto skip busy terminals when selecting a target, wait for a prompt in reusable shells before writing, and only set terminal dimensions for new server-side sessions.Manager.tsto traverse the full process tree, treating idle interactive shells as having no running subprocess and reporting the first non-shell descendant as the active process.Action: <suffix>human-readable labels foraction-*terminal IDs interminalLabels.ts.Macroscope summarized 777cece.