Skip to content

Commit bdf3e17

Browse files
edelaunaedelauna
committed
fix(TerminalProcess): warning even when shell opens
1 parent 68aab69 commit bdf3e17

3 files changed

Lines changed: 53 additions & 84 deletions

File tree

‎src/integrations/terminal/TerminalProcess.ts‎

Lines changed: 19 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as vscode from "vscode"
2-
import { inspect } from "util"
32

43
import type { ExitCodeDetails } from "./types"
54
import { BaseTerminalProcess } from "./BaseTerminalProcess"
@@ -118,11 +117,9 @@ export class TerminalProcess extends BaseTerminalProcess {
118117
isPowerShell: Terminal.isActiveShellPowerShell(),
119118
isFish: Terminal.isActiveShellFish(),
120119
}
121-
const isPowerShell = shellKind.isPowerShell
122-
123-
if (isPowerShell) {
124-
let commandToExecute = command
120+
let commandToExecute = command
125121

122+
if (shellKind.isPowerShell) {
126123
// Only add the PowerShell counter workaround if enabled
127124
if (Terminal.getPowershellCounter()) {
128125
commandToExecute += ` ; "(Roo/PS Workaround: ${this.terminal.cmdCounter++})" > $null`
@@ -132,14 +129,17 @@ export class TerminalProcess extends BaseTerminalProcess {
132129
if (Terminal.getCommandDelay() > 0) {
133130
commandToExecute += ` ; start-sleep -milliseconds ${Terminal.getCommandDelay()}`
134131
}
135-
136-
terminal.shellIntegration.executeCommand(
137-
this.prepareCommandForShellIntegration(commandToExecute, shellKind),
138-
)
139-
} else {
140-
terminal.shellIntegration.executeCommand(this.prepareCommandForShellIntegration(command, shellKind))
141132
}
142133

134+
const execution = terminal.shellIntegration.executeCommand(
135+
this.prepareCommandForShellIntegration(commandToExecute, shellKind),
136+
)
137+
138+
// VS Code only captures data written after read() is first called, so read
139+
// the execution stream immediately instead of waiting for the global start
140+
// event to deliver the same execution later.
141+
this.terminal.setActiveStream(execution.read())
142+
143143
this.isHot = true
144144

145145
// Wait for stream to be available
@@ -164,9 +164,6 @@ export class TerminalProcess extends BaseTerminalProcess {
164164
return
165165
}
166166

167-
let preOutput = ""
168-
let commandOutputStarted = false
169-
170167
/*
171168
* Extract clean output from raw accumulated output. FYI:
172169
* ]633 is a custom sequence number used by VSCode shell integration:
@@ -179,22 +176,14 @@ export class TerminalProcess extends BaseTerminalProcess {
179176

180177
// Process stream data
181178
for await (let data of stream) {
182-
// Check for command output start marker
183-
if (!commandOutputStarted) {
184-
preOutput += data
185-
const match = this.matchAfterVsceStartMarkers(data)
186-
187-
if (match !== undefined) {
188-
commandOutputStarted = true
189-
data = match
190-
this.fullOutput = "" // Reset fullOutput when command actually starts
191-
this.emit("line", "") // Trigger UI to proceed
192-
} else {
193-
continue
194-
}
179+
const match = this.fullOutput === "" ? this.matchAfterVsceStartMarkers(data) : undefined
180+
181+
if (match !== undefined) {
182+
data = match
183+
this.emit("line", "") // Trigger UI to proceed
195184
}
196185

197-
// Command output started, accumulate data without filtering.
186+
// Accumulate data without filtering.
198187
// notice to future programmers: do not add escape sequence
199188
// filtering here: fullOutput cannot change in length (see getUnretrievedOutput),
200189
// and chunks may not be complete so you cannot rely on detecting or removing escape sequences mid-stream.
@@ -221,32 +210,8 @@ export class TerminalProcess extends BaseTerminalProcess {
221210

222211
this.isHot = false
223212

224-
if (commandOutputStarted) {
225-
// Emit any remaining output before completing
226-
this.emitRemainingBufferIfListening()
227-
} else {
228-
const inspectPreOutput = inspect(preOutput, { colors: false, breakLength: Infinity })
229-
230-
// executeCommand() has already been called, so an empty stream cannot prove
231-
// the command was never submitted. Treat the status as submitted/unknown to
232-
// avoid replaying a potentially side-effecting command through Execa.
233-
const errorMsg =
234-
"VSCE output start escape sequence (]633;C or ]133;C) not received after command submission. Command execution status is unknown."
235-
236-
console.error(`[Terminal Process] ${errorMsg} preOutput: ${inspectPreOutput}`)
237-
238-
this.emit("no_shell_integration", { message: errorMsg, commandSubmitted: true })
239-
240-
this.emit(
241-
"completed",
242-
"<VSCE shell integration markers not found: terminal output and command execution status is unknown>\n" +
243-
`<preOutput>${inspectPreOutput}</preOutput>\n` +
244-
"AI MODEL: You MUST notify the user with the information above so they can open a bug report.",
245-
)
246-
247-
this.continue()
248-
return
249-
}
213+
// Emit any remaining output before completing.
214+
this.emitRemainingBufferIfListening()
250215

251216
// fullOutput begins after C marker so we only need to trim off D marker
252217
// (if D exists, see VSCode bug# 237208):

‎src/integrations/terminal/TerminalRegistry.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ export class TerminalRegistry {
4848
try {
4949
const startDisposable = vscode.window.onDidStartTerminalShellExecution?.(
5050
async (e: vscode.TerminalShellExecutionStartEvent) => {
51-
// Get a handle to the stream as early as possible:
52-
const stream = e.execution.read()
5351
const terminal = this.getTerminalByVSCETerminal(e.terminal)
5452

5553
console.info("[onDidStartTerminalShellExecution]", {
@@ -58,6 +56,12 @@ export class TerminalRegistry {
5856
})
5957

6058
if (terminal) {
59+
if (terminal.running) {
60+
return
61+
}
62+
63+
// Get a handle to the stream as early as possible.
64+
const stream = e.execution.read()
6165
terminal.setActiveStream(stream)
6266
terminal.busy = true // Mark terminal as busy when shell execution starts
6367
} else {

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

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ describe("TerminalProcess", () => {
5050

5151
// Create a process for testing
5252
terminalProcess = new TestTerminalProcess(mockTerminalInfo)
53+
mockTerminalInfo.process = terminalProcess
5354

5455
TerminalRegistry["terminals"].push(mockTerminalInfo)
5556

@@ -142,10 +143,11 @@ describe("TerminalProcess", () => {
142143
})
143144

144145
it.each([
145-
["PowerShell", ". {\necho one\necho two\n}"],
146-
["fish", "begin\necho one\necho two\nend"],
147-
])("uses the %s multiline wrapper", async (profile, expectedCommand) => {
148-
Terminal.setTerminalProfile(profile)
146+
["PowerShell", true, false, ". {\necho one\necho two\n}"],
147+
["fish", false, true, "begin\necho one\necho two\nend"],
148+
])("uses the %s multiline wrapper", async (_profile, isPowerShell, isFish, expectedCommand) => {
149+
const psSpy = vi.spyOn(Terminal, "isActiveShellPowerShell").mockReturnValue(isPowerShell)
150+
const fishSpy = vi.spyOn(Terminal, "isActiveShellFish").mockReturnValue(isFish)
149151

150152
try {
151153
mockStream = (async function* () {
@@ -165,7 +167,8 @@ describe("TerminalProcess", () => {
165167

166168
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(expectedCommand)
167169
} finally {
168-
Terminal.setTerminalProfile(undefined)
170+
psSpy.mockRestore()
171+
fishSpy.mockRestore()
169172
}
170173
})
171174

@@ -218,24 +221,20 @@ describe("TerminalProcess", () => {
218221
consoleWarnSpy.mockRestore()
219222
})
220223

221-
it("emits no_shell_integration with commandSubmitted=true when stream is empty after submission", async () => {
222-
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
223-
224-
let details: { message: string; commandSubmitted: boolean } | undefined
224+
it("completes without warning when the execution stream is empty after submission", async () => {
225+
const noShellIntegrationSpy = vi.fn()
226+
let completedOutput: string | undefined
225227

226228
const eventPromises = Promise.all([
227229
new Promise<void>((resolve) =>
228-
terminalProcess.once("no_shell_integration", (d) => {
229-
details = d
230+
terminalProcess.once("completed", (output?: string) => {
231+
completedOutput = output
230232
resolve()
231233
}),
232234
),
233-
new Promise<void>((resolve) => terminalProcess.once("completed", (_output?: string) => resolve())),
234235
new Promise<void>((resolve) => terminalProcess.once("continue", resolve)),
235236
])
236237

237-
// Empty stream: simulates VS Code firing onDidStartTerminalShellExecution
238-
// before the shell has fully initialised on a freshly-created terminal.
239238
async function* emptyStream(): AsyncGenerator<string> {
240239
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
241240
return
@@ -246,32 +245,31 @@ describe("TerminalProcess", () => {
246245
mockExecution = { read: vi.fn().mockReturnValue(mockStream) }
247246
mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
248247

248+
terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
249+
249250
const runPromise = terminalProcess.run("test command")
250-
terminalProcess.emit("stream_available", mockStream)
251251
await runPromise
252252
await eventPromises
253253

254-
expect(details?.commandSubmitted).toBe(true)
255-
consoleErrorSpy.mockRestore()
254+
expect(mockExecution.read).toHaveBeenCalledTimes(1)
255+
expect(completedOutput).toBe("")
256+
expect(noShellIntegrationSpy).not.toHaveBeenCalled()
256257
})
257258

258-
it("emits no_shell_integration with commandSubmitted=true when stream has data but no ]633;C", async () => {
259-
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
260-
261-
let details: { message: string; commandSubmitted: boolean } | undefined
259+
it("captures execution output even when VS Code does not include start markers", async () => {
260+
const noShellIntegrationSpy = vi.fn()
261+
let completedOutput: string | undefined
262262

263263
const eventPromises = Promise.all([
264264
new Promise<void>((resolve) =>
265-
terminalProcess.once("no_shell_integration", (d) => {
266-
details = d
265+
terminalProcess.once("completed", (output?: string) => {
266+
completedOutput = output
267267
resolve()
268268
}),
269269
),
270-
new Promise<void>((resolve) => terminalProcess.once("completed", (_output?: string) => resolve())),
271270
new Promise<void>((resolve) => terminalProcess.once("continue", resolve)),
272271
])
273272

274-
// Stream has output but never emits ]633;C — genuine shell integration failure.
275273
mockStream = (async function* () {
276274
yield "some output without marker\n"
277275
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
@@ -280,13 +278,15 @@ describe("TerminalProcess", () => {
280278
mockExecution = { read: vi.fn().mockReturnValue(mockStream) }
281279
mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
282280

281+
terminalProcess.once("no_shell_integration", noShellIntegrationSpy)
282+
283283
const runPromise = terminalProcess.run("test command")
284-
terminalProcess.emit("stream_available", mockStream)
285284
await runPromise
286285
await eventPromises
287286

288-
expect(details?.commandSubmitted).toBe(true)
289-
consoleErrorSpy.mockRestore()
287+
expect(mockExecution.read).toHaveBeenCalledTimes(1)
288+
expect(completedOutput).toBe("some output without marker\n")
289+
expect(noShellIntegrationSpy).not.toHaveBeenCalled()
290290
})
291291

292292
it("sets hot state for compiling commands", async () => {

0 commit comments

Comments
 (0)