Skip to content

Log runCommand for debugging #286625

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

Draft
anthonykim1 wants to merge 2 commits into
base: main
Choose a base branch
from
+18 −1
Draft

Log runCommand for debugging #286625

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77048c3
Fix Windows terminal announcement issue
anthonykim1 Jan 8, 2026
07501a0
log to figure out if we can avoid stalePromptInput after shellExecution
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
10 changes: 9 additions & 1 deletion 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 @@ -985,15 +985,19 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
store.add(this.capabilities.onDidAddCommandDetectionCapability(e => {
commandDetection = e;
if (commandDetection.promptInputModel.state === PromptInputState.Input) {
this._logService.trace('runCommand: Command detection became available with prompt input, proceeding to run command');
r();
} else {
store.add(commandDetection.promptInputModel.onDidStartInput(() => {
this._logService.trace('runCommand: Command detection prompt input started, proceeding to run command');
r();
}));
}
}));
}),
timeout(timeoutMs)
timeout(timeoutMs).then(() => {
this._logService.trace(`runCommand: Timed out waiting for command detection prompt input after ${timeoutMs}ms, proceeding to run command`);
Comment on lines +998 to +999
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The timeout promise's then() callback will execute after the Promise.race resolves, which means the log message will not appear when the timeout actually triggers the race resolution. The log statement inside the then() will execute regardless of which promise wins the race. To properly log when the timeout wins the race, the logging should happen in the race winner itself, not in a chained then().

Suggested change
timeout(timeoutMs).then(() => {
this._logService.trace(`runCommand: Timed out waiting for command detection prompt input after ${timeoutMs}ms, proceeding to run command`);
new Promise<void>(r => {
const timeoutDisposable = disposableTimeout(() => {
this._logService.trace(`runCommand: Timed out waiting for command detection prompt input after ${timeoutMs}ms, proceeding to run command`);
r();
}, timeoutMs);
store.add(timeoutDisposable);

Copilot uses AI. Check for mistakes.
})
]);
store.dispose();
}
Expand All @@ -1008,6 +1012,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(`runCommand: Sending ^C before running command: ${commandLine}`);
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Logging the command line may expose sensitive information such as passwords, API tokens, or other credentials that users might pass as command arguments. Consider either removing the commandLine from the log message or sanitizing/redacting it before logging.

Suggested change
this._logService.trace(`runCommand: Sending ^C before running command: ${commandLine}`);
this._logService.trace('runCommand: Sending ^C before running command');

Copilot uses AI. Check for mistakes.
this._logService.trace(`runCommand: The value of shouldExecute is: ${shouldExecute}`);
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

This log statement is redundant because this code is inside a conditional block that only executes when shouldExecute is true. Logging this value doesn't provide useful debugging information since it will always be true at this point.

Suggested change
this._logService.trace(`runCommand: The value of shouldExecute is: ${shouldExecute}`);

Copilot uses AI. Check for mistakes.
this._logService.trace(`runCommand: The value of command detection and prompt input model value is: ${commandDetection ? commandDetection.promptInputModel.value : 'N/A'}`);
Comment on lines +1018 to +1020
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

These three consecutive trace calls could be consolidated into a single, more readable log statement. This would reduce log noise and make it easier to parse debug output. Consider combining them into one trace call with a formatted multi-line string or a single line with all relevant information.

Suggested change
this._logService.trace(`runCommand: Sending ^C before running command: ${commandLine}`);
this._logService.trace(`runCommand: The value of shouldExecute is: ${shouldExecute}`);
this._logService.trace(`runCommand: The value of command detection and prompt input model value is: ${commandDetection ? commandDetection.promptInputModel.value : 'N/A'}`);
this._logService.trace(`runCommand: Sending ^C before running command`, {
commandLine,
shouldExecute,
promptInputValue: commandDetection ? commandDetection.promptInputModel.value : 'N/A'
});

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Logging the prompt input model value may expose sensitive information that users have typed at the terminal prompt, such as passwords or API tokens. Consider either removing this value from the log message or sanitizing/redacting it before logging.

Suggested change
this._logService.trace(`runCommand: The value of command detection and prompt input model value is: ${commandDetection ? commandDetection.promptInputModel.value : 'N/A'}`);
const promptInputInfo = commandDetection ? `length=${commandDetection.promptInputModel.value.length}` : 'N/A';
this._logService.trace(`runCommand: Command detection prompt input info: ${promptInputInfo}`);

Copilot uses AI. Check for mistakes.

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