CLI: Continue / Abort command execution #3949
Conversation
🦋 Changeset detectedLatest commit: 5bc6c66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR implements real-time command execution monitoring with Continue/Abort controls for the CLI. It introduces a synthetic ask message workaround to enable immediate user interaction with running commands before they produce output.
Key changes:
- Real-time command output streaming with synthetic
command_outputask messages created immediately on command start - Continue/Abort action menu for running commands with 0ms delay (vs 500ms for other approvals)
- Message deduplication logic to prevent conflicts between CLI-synthesized and backend-generated asks
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/src/ui/messages/utils/messageCompletion.ts | Removed command_output from non-rendering ask types to enable proper streaming display |
| cli/src/ui/messages/utils/tests/messageCompletion.test.ts | Updated test to verify command_output completion requires non-partial state |
| cli/src/ui/messages/extension/ask/index.ts | Added export for new AskCommandOutputMessage component |
| cli/src/ui/messages/extension/ask/AskCommandOutputMessage.tsx | New component displaying running commands with real-time output streaming (max 500 chars) |
| cli/src/ui/messages/extension/tests/ExtensionMessageRow.test.tsx | Removed test for say type command_output (now handled as ask type) |
| cli/src/ui/messages/extension/SayMessageRouter.tsx | Routes command_output to null (handled in AskMessageRouter instead) |
| cli/src/ui/messages/extension/AskMessageRouter.tsx | Routes command_output ask type to new AskCommandOutputMessage component |
| cli/src/ui/components/CommandInput.tsx | Changed placeholder text from "Awaiting approval..." to "Actions available:" |
| cli/src/ui/components/ApprovalMenu.tsx | Removed "[!] Action Required:" header and updated icon logic for approve actions |
| cli/src/state/hooks/useApprovalHandler.ts | Implemented sendTerminalOperation for Continue/Abort with 0ms delay for command_output asks |
| cli/src/state/atoms/ui.ts | Modified lastAskMessageAtom to allow partial command_output asks for real-time interaction |
| cli/src/state/atoms/extension.ts | Added message reconciliation with synthetic ask preservation and deduplication logic |
| cli/src/state/atoms/effects.ts | Implemented commandExecutionStatus handler creating synthetic asks and tracking output updates |
| cli/src/state/atoms/approval.ts | Added Continue/Abort options for command_output asks, preserving selection during streaming |
| cli/src/state/atoms/tests/effects-command-output.test.ts | New tests for synthetic ask creation on command start and completion handling |
| cli/src/state/atoms/tests/effects-command-output-duplicate.test.ts | New tests for duplicate ask filtering and synthetic ask lifecycle |
| cli/src/services/approvalDecision.ts | Added command_output case requiring manual approval (Continue/Abort decision) |
| // EXCEPT for command_output messages where we want to preserve selection | ||
| // as output streams in (to allow users to abort long-running commands) | ||
| const shouldResetSelection = isNewMessage && message?.ask !== "command_output" | ||
| if (shouldResetSelection) { |
There was a problem hiding this comment.
[nitpick] The variable isNewMessage is computed but only used in a condition that also checks message?.ask !== "command_output". The variable name and the new logic should be aligned. Consider renaming to shouldResetSelection and computing it as isNewMessage && message?.ask !== "command_output" directly, rather than computing it in two steps, to improve code clarity.
| output: activeUpdate.output || "", | ||
| }), | ||
| partial: !activeUpdate.completed, | ||
| isAnswered: activeUpdate.completed || false, |
There was a problem hiding this comment.
When activeUpdate.completed is true, the message's isAnswered is set to true (line 697). However, based on the comment in effects.ts (line 547: "Still needs user response"), a completed command should have partial: false but isAnswered: false until the user chooses to continue or abort. This logic may cause completed commands to not prompt for user action. Consider changing line 697 to isAnswered: false or isAnswered: msg.isAnswered to preserve the original answered state.
| isAnswered: activeUpdate.completed || false, | |
| isAnswered: msg.isAnswered, // kilocode_change |
pandemicsyn
left a comment
pandemicsyn
left a comment
There was a problem hiding this comment.
lgtm - one question inline but its not so much review feedback but more "help the newbie understand this"
|
|
||
| // Complete processing atomically | ||
| store.set(completeApprovalProcessingAtom) | ||
| } catch (error) { |
There was a problem hiding this comment.
Ok apologies in advanced - potentially dumb question, but sendTerminalOperation sets isAnswered=true and updates the chat message locally, and then calls sendMessage. If sendMessage throws, the catch only clears the processing atom, I think?
Does the chat entry stay isAnswered: true? If so, is that an issue?
There was a problem hiding this comment.
I should be test this but, in theory the extension continue pushing the state as isAnswered: false and clearing the completeApprovalProcessingAtom will be force to reupdate the ui showing the approval menu again.
There are multiples race conditions that this code handle on the CLI to deal with the extension message system.
Context
Resolve #3815
Implementation
Screenshots
2025-11-21_11-52-07.mp4
How to Test
Execute
To check if the stream of the command works