fix(TerminalProcess): warning even when shell opens

Author: edelauna

src/integrations/terminal/Terminal.ts

@@ -15,6 +15,8 @@ export class Terminal extends BaseTerminal {
 
 	public cmdCounter: number = 0
 
+	public activeShellExecution?: vscode.TerminalShellExecution
+
 	constructor(id: number, terminal: vscode.Terminal | undefined, cwd: string) {
 		super("vscode", id, cwd, Terminal.getReuseKey())
 

src/integrations/terminal/TerminalProcess.ts

@@ -1,5 +1,4 @@
 import * as vscode from "vscode"
-import { inspect } from "util"
 
 import type { ExitCodeDetails } from "./types"
 import { BaseTerminalProcess } from "./BaseTerminalProcess"
@@ -118,11 +117,9 @@ export class TerminalProcess extends BaseTerminalProcess {
 			isPowerShell: Terminal.isActiveShellPowerShell(),
 			isFish: Terminal.isActiveShellFish(),
 		}
-		const isPowerShell = shellKind.isPowerShell
-
-		if (isPowerShell) {
-			let commandToExecute = command
+		let commandToExecute = command
 
+		if (shellKind.isPowerShell) {
 			// Only add the PowerShell counter workaround if enabled
 			if (Terminal.getPowershellCounter()) {
 				commandToExecute += ` ; "(Roo/PS Workaround: ${this.terminal.cmdCounter++})" > $null`
@@ -132,12 +129,22 @@ export class TerminalProcess extends BaseTerminalProcess {
 			if (Terminal.getCommandDelay() > 0) {
 				commandToExecute += ` ; start-sleep -milliseconds ${Terminal.getCommandDelay()}`
 			}
+		}
 
-			terminal.shellIntegration.executeCommand(
+		try {
+			const execution = terminal.shellIntegration.executeCommand(
 				this.prepareCommandForShellIntegration(commandToExecute, shellKind),
 			)
-		} else {
-			terminal.shellIntegration.executeCommand(this.prepareCommandForShellIntegration(command, shellKind))
+
+			this.terminal.activeShellExecution = execution
+
+			// VS Code only captures data written after read() is first called, so read
+			// the execution stream immediately instead of waiting for the global start
+			// event to deliver the same execution later.
+			this.terminal.setActiveStream(execution.read())
+		} catch (error) {
+			this.terminal.activeShellExecution = undefined
+			throw error
 		}
 
 		this.isHot = true
@@ -164,9 +171,6 @@ export class TerminalProcess extends BaseTerminalProcess {
 			return
 		}
 
-		let preOutput = ""
-		let commandOutputStarted = false
-
 		/*
 		 * Extract clean output from raw accumulated output. FYI:
 		 * ]633 is a custom sequence number used by VSCode shell integration:
@@ -179,22 +183,14 @@ export class TerminalProcess extends BaseTerminalProcess {
 
 		// Process stream data
 		for await (let data of stream) {
-			// Check for command output start marker
-			if (!commandOutputStarted) {
-				preOutput += data
-				const match = this.matchAfterVsceStartMarkers(data)
-
-				if (match !== undefined) {
-					commandOutputStarted = true
-					data = match
-					this.fullOutput = "" // Reset fullOutput when command actually starts
-					this.emit("line", "") // Trigger UI to proceed
-				} else {
-					continue
-				}
+			const match = this.fullOutput === "" ? this.matchAfterVsceStartMarkers(data) : undefined
+
+			if (match !== undefined) {
+				data = match
+				this.emit("line", "") // Trigger UI to proceed
 			}
 
-			// Command output started, accumulate data without filtering.
+			// Accumulate data without filtering.
 			// notice to future programmers: do not add escape sequence
 			// filtering here: fullOutput cannot change in length (see getUnretrievedOutput),
 			// and chunks may not be complete so you cannot rely on detecting or removing escape sequences mid-stream.
@@ -218,35 +214,12 @@ export class TerminalProcess extends BaseTerminalProcess {
 
 		// Wait for shell execution to complete.
 		await shellExecutionComplete
+		this.terminal.activeShellExecution = undefined
 
 		this.isHot = false
 
-		if (commandOutputStarted) {
-			// Emit any remaining output before completing
-			this.emitRemainingBufferIfListening()
-		} else {
-			const inspectPreOutput = inspect(preOutput, { colors: false, breakLength: Infinity })
-
-			// executeCommand() has already been called, so an empty stream cannot prove
-			// the command was never submitted. Treat the status as submitted/unknown to
-			// avoid replaying a potentially side-effecting command through Execa.
-			const errorMsg =
-				"VSCE output start escape sequence (]633;C or ]133;C) not received after command submission. Command execution status is unknown."
-
-			console.error(`[Terminal Process] ${errorMsg} preOutput: ${inspectPreOutput}`)
-
-			this.emit("no_shell_integration", { message: errorMsg, commandSubmitted: true })
-
-			this.emit(
-				"completed",
-				"<VSCE shell integration markers not found: terminal output and command execution status is unknown>\n" +
-					`<preOutput>${inspectPreOutput}</preOutput>\n` +
-					"AI MODEL: You MUST notify the user with the information above so they can open a bug report.",
-			)
-
-			this.continue()
-			return
-		}
+		// Emit any remaining output before completing.
+		this.emitRemainingBufferIfListening()
 
 		// fullOutput begins after C marker so we only need to trim off D marker
 		// (if D exists, see VSCode bug# 237208):

src/integrations/terminal/TerminalRegistry.ts

@@ -48,16 +48,20 @@ export class TerminalRegistry {
 		try {
 			const startDisposable = vscode.window.onDidStartTerminalShellExecution?.(
 				async (e: vscode.TerminalShellExecutionStartEvent) => {
-					// Get a handle to the stream as early as possible:
-					const stream = e.execution.read()
 					const terminal = this.getTerminalByVSCETerminal(e.terminal)
 
 					console.info("[onDidStartTerminalShellExecution]", {
 						command: e.execution?.commandLine?.value,
 						terminalId: terminal?.id,
 					})
 
-					if (terminal) {
+					if (terminal instanceof Terminal) {
+						if (terminal.activeShellExecution === e.execution) {
+							return
+						}
+
+						// Get a handle to the stream as early as possible.
+						const stream = e.execution.read()
 						terminal.setActiveStream(stream)
 						terminal.busy = true // Mark terminal as busy when shell execution starts
 					} else {
@@ -94,6 +98,10 @@ export class TerminalRegistry {
 						return
 					}
 
+					if (terminal instanceof Terminal && terminal.activeShellExecution === e.execution) {
+						terminal.activeShellExecution = undefined
+					}
+
 					if (!terminal.running) {
 						console.error(
 							"[TerminalRegistry] Shell execution end event received, but process is not running for terminal:",

src/integrations/terminal/__tests__/TerminalProcess.spec.ts

@@ -50,6 +50,7 @@ describe("TerminalProcess", () => {
 
 		// Create a process for testing
 		terminalProcess = new TestTerminalProcess(mockTerminalInfo)
+		mockTerminalInfo.process = terminalProcess
 
 		TerminalRegistry["terminals"].push(mockTerminalInfo)
 
@@ -142,10 +143,11 @@ describe("TerminalProcess", () => {
 		})
 
 		it.each([
-			["PowerShell", ". {\necho one\necho two\n}"],
-			["fish", "begin\necho one\necho two\nend"],
-		])("uses the %s multiline wrapper", async (profile, expectedCommand) => {
-			Terminal.setTerminalProfile(profile)
+			["PowerShell", true, false, ". {\necho one\necho two\n}"],
+			["fish", false, true, "begin\necho one\necho two\nend"],
+		])("uses the %s multiline wrapper", async (_profile, isPowerShell, isFish, expectedCommand) => {
+			const psSpy = vi.spyOn(Terminal, "isActiveShellPowerShell").mockReturnValue(isPowerShell)
+			const fishSpy = vi.spyOn(Terminal, "isActiveShellFish").mockReturnValue(isFish)
 
 			try {
 				mockStream = (async function* () {
@@ -165,7 +167,8 @@ describe("TerminalProcess", () => {
 
 				expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(expectedCommand)
 			} finally {
-				Terminal.setTerminalProfile(undefined)
+				psSpy.mockRestore()
+				fishSpy.mockRestore()
 			}
 		})
 
@@ -218,24 +221,20 @@ describe("TerminalProcess", () => {
 			consoleWarnSpy.mockRestore()
 		})
 
-		it("emits no_shell_integration with commandSubmitted=true when stream is empty after submission", async () => {
-			const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
-
-			let details: { message: string; commandSubmitted: boolean } | undefined
+		it("completes without warning when the execution stream is empty after submission", async () => {
+			const noShellIntegrationSpy = vi.fn()
+			let completedOutput: string | undefined
 
 			const eventPromises = Promise.all([
 				new Promise<void>((resolve) =>
-					terminalProcess.once("no_shell_integration", (d) => {
-						details = d
+					terminalProcess.once("completed", (output?: string) => {
+						completedOutput = output
 						resolve()
 					}),
 				),
-				new Promise<void>((resolve) => terminalProcess.once("completed", (_output?: string) => resolve())),
 				new Promise<void>((resolve) => terminalProcess.once("continue", resolve)),
 			])
 
-			// Empty stream: simulates VS Code firing onDidStartTerminalShellExecution
-			// before the shell has fully initialised on a freshly-created terminal.
 			async function* emptyStream(): AsyncGenerator<string> {
 				terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
 				return
@@ -246,32 +245,31 @@ describe("TerminalProcess", () => {
 			mockExecution = { read: vi.fn().mockReturnValue(mockStream) }
 			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
 
+			terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
+
 			const runPromise = terminalProcess.run("test command")
-			terminalProcess.emit("stream_available", mockStream)
 			await runPromise
 			await eventPromises
 
-			expect(details?.commandSubmitted).toBe(true)
-			consoleErrorSpy.mockRestore()
+			expect(mockExecution.read).toHaveBeenCalledTimes(1)
+			expect(completedOutput).toBe("")
+			expect(noShellIntegrationSpy).not.toHaveBeenCalled()
 		})
 
-		it("emits no_shell_integration with commandSubmitted=true when stream has data but no ]633;C", async () => {
-			const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
-
-			let details: { message: string; commandSubmitted: boolean } | undefined
+		it("captures execution output even when VS Code does not include start markers", async () => {
+			const noShellIntegrationSpy = vi.fn()
+			let completedOutput: string | undefined
 
 			const eventPromises = Promise.all([
 				new Promise<void>((resolve) =>
-					terminalProcess.once("no_shell_integration", (d) => {
-						details = d
+					terminalProcess.once("completed", (output?: string) => {
+						completedOutput = output
 						resolve()
 					}),
 				),
-				new Promise<void>((resolve) => terminalProcess.once("completed", (_output?: string) => resolve())),
 				new Promise<void>((resolve) => terminalProcess.once("continue", resolve)),
 			])
 
-			// Stream has output but never emits ]633;C — genuine shell integration failure.
 			mockStream = (async function* () {
 				yield "some output without marker\n"
 				terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
@@ -280,13 +278,15 @@ describe("TerminalProcess", () => {
 			mockExecution = { read: vi.fn().mockReturnValue(mockStream) }
 			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
 
+			terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
+
 			const runPromise = terminalProcess.run("test command")
-			terminalProcess.emit("stream_available", mockStream)
 			await runPromise
 			await eventPromises
 
-			expect(details?.commandSubmitted).toBe(true)
-			consoleErrorSpy.mockRestore()
+			expect(mockExecution.read).toHaveBeenCalledTimes(1)
+			expect(completedOutput).toBe("some output without marker\n")
+			expect(noShellIntegrationSpy).not.toHaveBeenCalled()
 		})
 
 		it("sets hot state for compiling commands", async () => {