fix(terminal): address CodeRabbit review findings

Author: edelauna

apps/vscode-e2e/src/suite/tools/fast-exit-shell-race.test.ts

@@ -90,7 +90,7 @@ suite("Fast-exit shell integration race", function () {
 						},
 						text: "FAST_EXIT_SHELL_RACE_E2E",
 					}),
-				timeout: 100_000, // TEMP: diagnostic probe, see if the marker ever arrives given patience
+				timeout: 60_000,
 			})
 
 			const elapsedMs = Date.now() - startedAt

src/core/tools/__tests__/executeCommandTool.spec.ts

@@ -187,12 +187,12 @@ describe("executeCommandTool", () => {
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 			})
 
-			// Verify - confirm the command was approved and result was pushed
-			// The custom path handling is tested in integration tests
+			// Verify - command approved, result pushed, and custom cwd passed to terminal
 			expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test")
 			expect(mockPushToolResult).toHaveBeenCalled()
-			const result = mockPushToolResult.mock.calls[0][0]
-			expect(result).toContain("Command")
+			const { TerminalRegistry } = await import("../../../integrations/terminal/TerminalRegistry")
+			const firstArg = (TerminalRegistry.getOrCreateTerminal as ReturnType<typeof vitest.fn>).mock.calls[0][0]
+			expect(firstArg).toBe("/custom/path")
 		})
 	})
 

src/integrations/terminal/TerminalProcess.ts

@@ -290,8 +290,12 @@ export class TerminalProcess extends BaseTerminalProcess {
 		// Only applies before any data arrives (chunkCount === 0).
 		const IDLE_TIMEOUT_MS = 3_000
 
+		// Hoist nextChunk outside the loop so that re-arming after an idle timeout
+		// reuses the same pending .next() promise instead of issuing a second call
+		// while the first is still unresolved (violates the async-iterator protocol
+		// and silently drops the first output chunk).
+		let nextChunk = iterator.next()
 		while (true) {
-			const nextChunk = iterator.next()
 			const racers: Promise<typeof DONE_SENTINEL | typeof IDLE_SENTINEL | IteratorResult<string>>[] = [
 				nextChunk,
 				shellExecutionComplete.then(() => DONE_SENTINEL as typeof DONE_SENTINEL),
@@ -325,6 +329,7 @@ export class TerminalProcess extends BaseTerminalProcess {
 					// silent command like `sleep 5` can legitimately produce zero output —
 					// elapsed time alone is not proof of completion. Re-arm the idle timer
 					// and keep waiting for a real chunk, the D marker, or the end event.
+					// Reuse the existing nextChunk promise — do NOT call iterator.next() again.
 					console.info(
 						`[Terminal Process] idle timeout fired but shell execution is running — re-arming (${chunkCount} chunks so far)`,
 					)
@@ -396,6 +401,11 @@ export class TerminalProcess extends BaseTerminalProcess {
 				)
 				break
 			}
+
+			// Advance to the next chunk only after the current one has been fully
+			// consumed. Calling iterator.next() here (not at the top of the loop)
+			// ensures there is never more than one pending .next() call at a time.
+			nextChunk = iterator.next()
 		}
 
 		if (!sawEndMarker && !streamEndedByEvent) {
@@ -546,7 +556,9 @@ export class TerminalProcess extends BaseTerminalProcess {
 			return `begin\n${command}\nend`
 		}
 
-		const scriptPath = path.join(os.tmpdir(), `roo-cmd-${Date.now()}-${Math.random().toString(36).slice(2)}.sh`)
+		const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), "roo-cmd-"))
+		this.scriptDir = scriptDir
+		const scriptPath = path.join(scriptDir, "cmd.sh")
 		fs.writeFileSync(scriptPath, command, { mode: 0o700 })
 		this.scriptPath = scriptPath
 
@@ -556,6 +568,7 @@ export class TerminalProcess extends BaseTerminalProcess {
 	}
 
 	private scriptPath: string | undefined
+	private scriptDir: string | undefined
 
 	private cleanupScriptFile() {
 		if (this.scriptPath) {
@@ -566,6 +579,14 @@ export class TerminalProcess extends BaseTerminalProcess {
 			}
 			this.scriptPath = undefined
 		}
+		if (this.scriptDir) {
+			try {
+				fs.rmdirSync(this.scriptDir)
+			} catch {
+				// Best-effort: ignore if already removed or non-empty.
+			}
+			this.scriptDir = undefined
+		}
 	}
 
 	public override continue() {

src/integrations/terminal/TerminalRegistry.ts

@@ -69,7 +69,11 @@ export class TerminalRegistry {
 						const process = terminal.process
 						const isOwnExecution =
 							!(process instanceof TerminalProcess) ||
-							process.ownExecution === undefined ||
+							// Allow undefined only when the process hasn't started yet (cold
+							// terminal: process is assigned but run() hasn't called executeCommand).
+							// Once isHot is true, ownExecution is always set — a stale start
+							// event on a reused terminal must match exactly.
+							(!process.isHot && process.ownExecution === undefined) ||
 							process.ownExecution === e.execution
 						if (!isOwnExecution) {
 							console.info(