fix: improve terminal execution with shell integration status

When shell integration is not available, the system now provides clear feedback
about command execution status and maintains consistent event flow.

- Removed waitForShellIntegration property to simplify code flow
- Consolidated event emission to ensure consistent behavior
- Updated tests to verify correct event sequence
- Simplified shell integration detection with pWaitFor
This commit is contained in:
Eric Wheeler 2025-03-05 13:17:30 -08:00
parent 0e41241faa
commit 304fcf2fbc
3 changed files with 38 additions and 21 deletions

View file

@ -191,20 +191,24 @@ export class TerminalManager {
})
})
// if shell integration is already active, run the command immediately
if (terminalInfo.terminal.shellIntegration) {
process.waitForShellIntegration = false
process.run(terminalInfo.terminal, command)
} else {
// docs recommend waiting 3s for shell integration to activate
pWaitFor(() => terminalInfo.terminal.shellIntegration !== undefined, { timeout: 4000 }).finally(() => {
// Always use pWaitFor, which resolves immediately if shell integration is already available
pWaitFor(() => terminalInfo.terminal.shellIntegration !== undefined, { timeout: 4000 })
.then(() => {
const existingProcess = this.processes.get(terminalInfo.id)
if (existingProcess && existingProcess.waitForShellIntegration) {
existingProcess.waitForShellIntegration = false
if (existingProcess) {
existingProcess.run(terminalInfo.terminal, command)
} else {
console.error("[TerminalManager] existingProcess not found for terminal", terminalInfo.id)
}
})
.catch(() => {
// Shell integration did not become available within timeout
const existingProcess = this.processes.get(terminalInfo.id)
if (existingProcess) {
console.log("[TerminalManager] Shell integration not available. Command execution aborted.")
existingProcess.emit("no_shell_integration")
}
})
}
return mergePromise(process, promise)
}

View file

@ -32,7 +32,6 @@ const PROCESS_HOT_TIMEOUT_NORMAL = 2_000
const PROCESS_HOT_TIMEOUT_COMPILING = 15_000
export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
waitForShellIntegration: boolean = true
private isListening: boolean = true
private terminalInfo: Terminal | undefined
private lastEmitTime_ms: number = 0
@ -289,19 +288,23 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
this.isHot = false
this.emit("completed", this.removeEscapeSequences(this.fullOutput))
this.emit("continue")
} else {
terminal.sendText(command, true)
// For terminals without shell integration, we can't know when the command completes
// So we'll just emit the continue event after a delay
this.emit("completed")
this.emit("continue")
// Do not execute commands when shell integration is not available
console.warn(
"[TerminalProcess] Shell integration not available. Command sent without knowledge of response.",
)
this.emit("no_shell_integration")
// setTimeout(() => {
// console.log(`Emitting continue after delay for terminal`)
// // can't emit completed since we don't if the command actually completed, it could still be running server
// }, 500) // Adjust this delay as needed
// unknown, but trigger the event
this.emit(
"completed",
"<shell integration is not available, so terminal output and command execution status is unknown>",
)
}
this.emit("continue")
}
private emitRemainingBufferIfListening() {

View file

@ -107,13 +107,23 @@ describe("TerminalProcess", () => {
shellIntegration: undefined,
} as unknown as vscode.Terminal
// Set up event listeners to verify events are emitted
const noShellPromise = new Promise<void>((resolve) => {
terminalProcess.once("no_shell_integration", resolve)
})
const completedPromise = new Promise<void>((resolve) => {
terminalProcess.once("completed", (_output?: string) => resolve())
})
const continuePromise = new Promise<void>((resolve) => {
terminalProcess.once("continue", resolve)
})
await terminalProcess.run(noShellTerminal, "test command")
await noShellPromise
// Verify all expected events are emitted
await Promise.all([noShellPromise, completedPromise, continuePromise])
// Verify sendText is called with the command
expect(noShellTerminal.sendText).toHaveBeenCalledWith("test command", true)
})