Skip to content

Clear prompt input value when onDidEndShellExecution #286791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
anthonykim1 wants to merge 4 commits into from
Closed

Clear prompt input value when onDidEndShellExecution #286791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ecf6cc0
Clear prompt input value when onDidEndShellExecution
anthonykim1 Jan 9, 2026
f6a4ea1
logs to capture manually
anthonykim1 Jan 9, 2026
868cbac
more logs for testing
anthonykim1 Jan 9, 2026
10bce4e
Use terminal log instead
anthonykim1 Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/vs/platform/terminal/common/capabilities/commandDetection/promptInputModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ export interface IPromptInputModel extends IPromptInputModelState {
getCombinedString(emptyStringWhenEmpty?: boolean): string;

setShellType(shellType?: TerminalShellType): void;

/**
* Clears the prompt input value. This should be called when the command has finished
* and the value is no longer relevant as editable prompt input.
*/
clearValue(): void;
}

export interface IPromptInputModelState {
Expand Down Expand Up @@ -239,8 +245,10 @@ export class PromptInputModel extends Disposable implements IPromptInputModel {

private _handleCommandExecuted() {
if (this._state === PromptInputState.Execute) {
this._logService.trace('PromptInputModel#_handleCommandExecuted: already in Execute state');
return;
}
this._logService.trace('PromptInputModel#_handleCommandExecuted: Inside _handleCommandExecuted but not PromptInputState.Execute yet');

this._cursorIndex = -1;

Expand All @@ -261,6 +269,14 @@ export class PromptInputModel extends Disposable implements IPromptInputModel {
this._onDidChangeInput.fire(event);
}

/**
* Clears the prompt input value. This should be called when the command has finished
* and the value is no longer relevant as editable prompt input.
*/
clearValue(): void {
this._value = '';
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clearValue method only clears the _value field but leaves other state fields like _cursorIndex, _ghostTextIndex, _state, _commandStartMarker, and _commandStartX unchanged. This creates an inconsistent state where the value is empty but other tracking fields still reference the old command. Consider also resetting these fields or at minimum _cursorIndex and _ghostTextIndex to maintain state consistency. Looking at _handleCommandStart (line 219-220), it clears both _value and _cursorIndex together.

Suggested change
this._value = '';
this._value = '';
this._cursorIndex = 0;
this._ghostTextIndex = -1;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clearValue method doesn't trigger any events (onDidChangeInput) after clearing the value. This means subscribers won't be notified of the state change. Compare this to _handleCommandExecuted (line 267) which fires onDidChangeInput after modifying state, and _handleCommandStart (line 222) which also fires events. If extensions or other components are listening to these events to track the prompt state, they won't be notified when the value is cleared, potentially causing them to have stale state as well.

Suggested change
this._value = '';
this._value = '';
this._onDidChangeInput.fire(this._createStateObject());

Copilot uses AI. Check for mistakes.
}
Comment on lines +276 to +278
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new clearValue method lacks test coverage. There is a comprehensive test file at src/vs/platform/terminal/test/common/capabilities/commandDetection/promptInputModel.test.ts with tests for other prompt input model behaviors. Consider adding tests that verify clearValue properly clears the state and handles edge cases like calling clearValue multiple times or calling it in different states (Input, Execute, Unknown).

Copilot uses AI. Check for mistakes.

@throttle(0)
private _sync() {
try {
Expand Down
9 changes: 8 additions & 1 deletion src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { IWorkbenchEnvironmentService } from '../../services/environment/common/
import { extHostNamedCustomer, type IExtHostContext } from '../../services/extensions/common/extHostCustomers.js';
import { TerminalShellExecutionCommandLineConfidence } from '../common/extHostTypes.js';
import { IExtensionService } from '../../services/extensions/common/extensions.js';
import { ITerminalLogService } from '../../../platform/terminal/common/terminal.js';

@extHostNamedCustomer(MainContext.MainThreadTerminalShellIntegration)
export class MainThreadTerminalShellIntegration extends Disposable implements MainThreadTerminalShellIntegrationShape {
Expand All @@ -21,7 +22,8 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
extHostContext: IExtHostContext,
@ITerminalService private readonly _terminalService: ITerminalService,
@IWorkbenchEnvironmentService workbenchEnvironmentService: IWorkbenchEnvironmentService,
@IExtensionService private readonly _extensionService: IExtensionService
@IExtensionService private readonly _extensionService: IExtensionService,
@ITerminalLogService private readonly _logService: ITerminalLogService
) {
super();

Expand Down Expand Up @@ -98,10 +100,15 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
currentCommand = undefined;
const instanceId = e.instance.instanceId;
instanceDataListeners.get(instanceId)?.dispose();
// Clear the prompt input value before sending the event to the extension host
// so that subsequent executeCommand calls don't see stale input
// e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.clearValue();
this._logService.trace(`PromptInputModel#_onDidEndTerminalShellExecution# state of promptInputModel is : ${e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.state}`);
// Shell integration C (executed) and D (command finished) sequences should always be in
// their own events, so send this immediately. This means that the D sequence will not
// be included as it's currently being parsed when the command finished event fires.
this._proxy.$shellExecutionEnd(instanceId, e.data.command, convertToExtHostCommandLineConfidence(e.data), e.data.isTrusted, e.data.exitCode);
this._logService.trace(`PromptInputModel#_onDidEndTerminalShellExecution# state of promptInputModel after shellExecutionEnd is : ${e.instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.state}`);
}));

// Clean up after dispose
Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,10 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
// Determine whether to send ETX (ctrl+c) before running the command. Only do this when the
// command will be executed immediately or when command detection shows the prompt contains text.
if (shouldExecute && (!commandDetection || commandDetection.promptInputModel.value.length > 0)) {
this._logService.trace(`PromptInputModel#runCommand: Sending ETX (ctrl+c) before running command`);
this._logService.trace(`PromptInputModel#runCommand: state of promptinputmodel before sending ETX: ${commandDetection ? commandDetection.promptInputModel.state : 'no command detection'}`);
this._logService.trace(`PromptInputModel#runCommand: value of promptinputmodel before sending ETX: ${commandDetection ? commandDetection.promptInputModel.value : 'no command detection'}`);

await this.sendText('\x03', false);
// Wait a little before running the command to avoid the sequences being echoed while the ^C
// is being evaluated
Expand Down
Loading