fix(vscode): reclaim unobserved fallback terminals (#12352)

Classify unobservable terminal outcomes so cleanup and reporting share one source of truth. Reclaim managed sendText fallbacks at the disclosed next-acquisition boundary while preserving markerless, continued, detached, and uncertain-error terminals. Make cleanup, CWD reservations, process listeners, and detached logs failure-safe.
This commit is contained in:
Dominic Cooney
2026-07-23 08:22:11 +09:00
committed by GitHub
parent a5211a3a5c
commit 8b4d1973b5
8 changed files with 1026 additions and 131 deletions
@@ -2,6 +2,7 @@ import assert from "node:assert/strict"
import { afterEach, beforeEach, describe, it } from "mocha"
import * as sinon from "sinon"
import * as vscode from "vscode"
import { setVscodeHostProviderMock } from "@/test/host-provider-test-utils"
import { VscodeTerminalManager } from "./VscodeTerminalManager"
import { TerminalInfo, TerminalRegistry } from "./VscodeTerminalRegistry"
@@ -13,6 +14,24 @@ function createNeverEndingStream(): AsyncIterable<string> {
}
}
function createMarkerlessStream(): AsyncIterable<string> {
return {
async *[Symbol.asyncIterator]() {
yield "remote output\n"
yield "user@remote:~$ "
await new Promise(() => {})
},
}
}
function createFailingStream(error: Error): AsyncIterable<string> {
return {
async *[Symbol.asyncIterator]() {
throw error
},
}
}
describe("VscodeTerminalManager", () => {
let sandbox: sinon.SinonSandbox
let manager: VscodeTerminalManager
@@ -24,10 +43,12 @@ describe("VscodeTerminalManager", () => {
afterEach(() => {
manager.disposeAll()
TerminalRegistry.disposeTerminalsPendingCleanup()
sandbox.restore()
})
it("returns after timing out a reused terminal cwd command", async () => {
it("creates a fresh terminal after timing out an unconfirmed reused-terminal cwd change", async () => {
setVscodeHostProviderMock()
const targetCwd = "/tmp/cline-target"
const executeCommandStub = sandbox.stub().returns({
read: () => createNeverEndingStream(),
@@ -57,13 +78,423 @@ describe("VscodeTerminalManager", () => {
assert.equal(didResolve, false)
await sandbox.clock.tickAsync(1)
const terminal = (await terminalPromise) as unknown as TerminalInfo
try {
assert.notEqual(terminal, terminalInfo)
assert.equal(terminalInfo.busy, false)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
assert.equal(terminal.busy, true)
assert.equal(getAllTerminalsStub.called, true)
assert.equal(executeCommandStub.calledOnceWith(`cd "${targetCwd}"`), true)
} finally {
terminal.terminal.dispose()
TerminalRegistry.removeTerminal(terminal.id)
}
})
it("reuses a terminal after its cwd change is confirmed", async () => {
const targetCwd = "/tmp/cline-target"
let currentCwd = vscode.Uri.file("/tmp/cline-original")
const terminalInfo: TerminalInfo = {
id: 1,
busy: false,
lastCommand: "",
lastActive: Date.now(),
terminal: {
shellIntegration: {
get cwd() {
return currentCwd
},
executeCommand: () => ({
read: () => ({
async *[Symbol.asyncIterator]() {
currentCwd = vscode.Uri.file(targetCwd)
},
}),
}),
},
show: sandbox.stub(),
} as unknown as vscode.Terminal,
}
sandbox.stub(TerminalRegistry, "getAllTerminals").returns([terminalInfo])
const terminalPromise = manager.getOrCreateTerminal(targetCwd)
await sandbox.clock.tickAsync(100)
const terminal = await terminalPromise
assert.equal(terminal, terminalInfo)
assert.equal(terminalInfo.busy, true)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
})
it("releases a reused terminal reservation when showing it fails", async () => {
const terminalInfo: TerminalInfo = {
id: 1,
busy: false,
lastCommand: "",
lastActive: Date.now(),
terminal: {
shellIntegration: { cwd: vscode.Uri.file("/tmp/cline-original") },
show: sandbox.stub().throws(new Error("terminal closed")),
} as unknown as vscode.Terminal,
}
sandbox.stub(TerminalRegistry, "getAllTerminals").returns([terminalInfo])
await assert.rejects(manager.getOrCreateTerminal("/tmp/cline-target"), /terminal closed/)
assert.equal(terminalInfo.busy, false)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
assert.equal(getAllTerminalsStub.called, true)
assert.equal(executeCommandStub.calledOnceWith(`cd "${targetCwd}"`), true)
})
it("creates a fresh terminal when the reused-terminal cwd command cannot start", async () => {
setVscodeHostProviderMock()
const terminalInfo: TerminalInfo = {
id: 1,
busy: false,
lastCommand: "",
lastActive: Date.now(),
terminal: {
shellIntegration: {
cwd: vscode.Uri.file("/tmp/cline-original"),
executeCommand: () => {
throw new Error("cwd command failed")
},
},
show: sandbox.stub(),
} as unknown as vscode.Terminal,
}
sandbox.stub(TerminalRegistry, "getAllTerminals").returns([terminalInfo])
const terminal = (await manager.getOrCreateTerminal("/tmp/cline-target")) as unknown as TerminalInfo
try {
assert.notEqual(terminal, terminalInfo)
assert.equal(terminal.busy, true)
assert.equal(terminalInfo.busy, false)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined)
} finally {
terminal.terminal.dispose()
TerminalRegistry.removeTerminal(terminal.id)
}
})
it("creates a fresh terminal when the reused-terminal cwd command stream fails", async () => {
setVscodeHostProviderMock()
const terminalInfo: TerminalInfo = {
id: 1,
busy: false,
lastCommand: "",
lastActive: Date.now(),
terminal: {
shellIntegration: {
cwd: vscode.Uri.file("/tmp/cline-original"),
executeCommand: () => ({ read: () => createFailingStream(new Error("cwd stream failed")) }),
},
show: sandbox.stub(),
} as unknown as vscode.Terminal,
}
sandbox.stub(TerminalRegistry, "getAllTerminals").returns([terminalInfo])
const terminal = (await manager.getOrCreateTerminal("/tmp/cline-target")) as unknown as TerminalInfo
try {
assert.notEqual(terminal, terminalInfo)
assert.equal(terminal.busy, true)
assert.equal(terminalInfo.busy, false)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined)
} finally {
terminal.terminal.dispose()
TerminalRegistry.removeTerminal(terminal.id)
}
})
it("releases a reused terminal reservation when it closes during cwd setup", async () => {
let exitStatus: vscode.TerminalExitStatus | undefined
const terminalInfo: TerminalInfo = {
id: 1,
busy: false,
lastCommand: "",
lastActive: Date.now(),
terminal: {
get exitStatus() {
return exitStatus
},
shellIntegration: {
cwd: vscode.Uri.file("/tmp/cline-original"),
executeCommand: () => ({ read: createNeverEndingStream }),
},
show: sandbox.stub(),
} as unknown as vscode.Terminal,
}
sandbox.stub(TerminalRegistry, "getAllTerminals").returns([terminalInfo])
const rejectedAcquisition = assert.rejects(manager.getOrCreateTerminal("/tmp/cline-target"), /exited while preparing/)
exitStatus = { code: undefined, reason: vscode.TerminalExitReason.Unknown }
await sandbox.clock.tickAsync(5000)
await rejectedAcquisition
assert.equal(terminalInfo.busy, false)
assert.equal(terminalInfo.pendingCwdChange, undefined)
assert.equal(terminalInfo.cwdResolved, undefined)
})
it("reserves different terminals for parallel acquisitions", async () => {
setVscodeHostProviderMock()
const first = (await manager.getOrCreateTerminal("/tmp/cline-parallel")) as unknown as TerminalInfo
const second = (await manager.getOrCreateTerminal("/tmp/cline-parallel")) as unknown as TerminalInfo
try {
assert.notEqual(first.id, second.id)
assert.equal(first.busy, true)
assert.equal(second.busy, true)
} finally {
first.terminal.dispose()
second.terminal.dispose()
TerminalRegistry.removeTerminal(first.id)
TerminalRegistry.removeTerminal(second.id)
}
})
it("rejects the command and releases the terminal when process startup fails", async () => {
const terminalInfo: TerminalInfo = {
id: 1,
busy: true,
lastCommand: "",
lastActive: Date.now(),
terminal: {
shellIntegration: {
executeCommand: () => {
throw new Error("command startup failed")
},
},
show: sandbox.stub(),
} as unknown as vscode.Terminal,
}
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"failing-command",
)
await assert.rejects(process, /command startup failed/)
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined)
})
it("does not reuse a terminal after its command stream fails", async () => {
setVscodeHostProviderMock()
const terminalInfo = TerminalRegistry.createTerminal("/tmp/cline-stream-error")
sandbox.stub(terminalInfo.terminal, "shellIntegration").get(() => ({
cwd: vscode.Uri.file("/tmp/cline-stream-error"),
executeCommand: () => ({ read: () => createFailingStream(new Error("command stream failed")) }),
}))
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"long-running-command",
)
await assert.rejects(process, /command stream failed/)
const nextTerminal = (await manager.getOrCreateTerminal("/tmp/cline-stream-error")) as unknown as TerminalInfo
try {
assert.notEqual(nextTerminal.id, terminalInfo.id)
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined)
} finally {
terminalInfo.terminal.dispose()
nextTerminal.terminal.dispose()
TerminalRegistry.removeTerminal(nextTerminal.id)
}
})
it("continues terminal acquisition when pending cleanup fails", async () => {
setVscodeHostProviderMock()
const failedCleanup = TerminalRegistry.createTerminal()
const successfulCleanup = TerminalRegistry.createTerminal()
const failedDispose = sandbox.stub(failedCleanup.terminal, "dispose").throws(new Error("dispose failed"))
const successfulDispose = sandbox.spy(successfulCleanup.terminal, "dispose")
TerminalRegistry.queueTerminalForCleanup(failedCleanup)
TerminalRegistry.queueTerminalForCleanup(successfulCleanup)
let acquiredTerminal: TerminalInfo | undefined
let didRestoreFailedDispose = false
try {
acquiredTerminal = (await manager.getOrCreateTerminal("/tmp/cline-after-cleanup-error")) as unknown as TerminalInfo
assert.equal(successfulDispose.calledOnce, true)
assert.notEqual(acquiredTerminal.id, failedCleanup.id)
failedDispose.restore()
didRestoreFailedDispose = true
const retryDispose = sandbox.spy(failedCleanup.terminal, "dispose")
TerminalRegistry.disposeTerminalsPendingCleanup()
assert.equal(retryDispose.calledOnce, true)
} finally {
if (!didRestoreFailedDispose) {
failedDispose.restore()
}
acquiredTerminal?.terminal.dispose()
if (acquiredTerminal) {
TerminalRegistry.removeTerminal(acquiredTerminal.id)
}
TerminalRegistry.disposeTerminalsPendingCleanup()
}
})
it("disposes an already-exited fallback terminal exactly once", () => {
const terminalInfo = TerminalRegistry.createTerminal()
const disposeSpy = sandbox.spy(terminalInfo.terminal, "dispose")
sandbox.stub(terminalInfo.terminal, "exitStatus").get(() => ({
code: 0,
reason: vscode.TerminalExitReason.Process,
}))
TerminalRegistry.queueTerminalForCleanup(terminalInfo)
TerminalRegistry.disposeTerminalsPendingCleanup()
TerminalRegistry.disposeTerminalsPendingCleanup()
assert.equal(disposeSpy.calledOnce, true)
})
it("defers fallback terminal disposal until the next terminal acquisition", async () => {
setVscodeHostProviderMock()
const terminalInfo = TerminalRegistry.createTerminal()
sandbox.stub(terminalInfo.terminal, "shellIntegration").get(() => undefined)
sandbox.stub(terminalInfo.terminal, "sendText")
const disposeSpy = sandbox.spy(terminalInfo.terminal, "dispose")
let nextTerminal: TerminalInfo | undefined
let nextManager: VscodeTerminalManager | undefined
try {
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"sleep 999",
)
await sandbox.clock.tickAsync(4000)
await sandbox.clock.tickAsync(3000)
await process
assert.deepEqual(process.getCompletionDetails?.().unobservedCommand, {
source: "sendText",
ownership: "managed",
})
assert.equal(disposeSpy.called, false, "the command must not be killed when the fallback result resolves")
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined, "the terminal must be evicted from reuse")
nextManager = new VscodeTerminalManager()
nextTerminal = (await nextManager.getOrCreateTerminal("/tmp/cline-next-command")) as unknown as TerminalInfo
assert.equal(disposeSpy.calledOnce, true, "the next acquisition must reclaim the fallback terminal")
} finally {
nextManager?.disposeAll()
nextTerminal?.terminal.dispose()
if (nextTerminal) {
TerminalRegistry.removeTerminal(nextTerminal.id)
}
if (!disposeSpy.called) {
terminalInfo.terminal.dispose()
TerminalRegistry.removeTerminal(terminalInfo.id)
}
}
})
it("preserves a detached fallback terminal across the next terminal acquisition", async () => {
setVscodeHostProviderMock()
const terminalInfo = TerminalRegistry.createTerminal()
sandbox.stub(terminalInfo.terminal, "shellIntegration").get(() => undefined)
sandbox.stub(terminalInfo.terminal, "sendText")
const disposeSpy = sandbox.spy(terminalInfo.terminal, "dispose")
let nextTerminal: TerminalInfo | undefined
try {
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"sleep 999",
)
const unobservedCommand = new Promise<void>((resolve) => process.once("unobserved_command", () => resolve()))
process.detach()
await sandbox.clock.tickAsync(4000)
await sandbox.clock.tickAsync(3000)
await unobservedCommand
nextTerminal = (await manager.getOrCreateTerminal("/tmp/cline-next-command")) as unknown as TerminalInfo
assert.equal(disposeSpy.called, false, "Proceed While Running transfers terminal ownership to the user")
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined, "detached terminals must not be reused")
} finally {
nextTerminal?.terminal.dispose()
if (nextTerminal) {
TerminalRegistry.removeTerminal(nextTerminal.id)
}
terminalInfo.terminal.dispose()
TerminalRegistry.removeTerminal(terminalInfo.id)
}
})
it("preserves a continued fallback terminal across the next terminal acquisition", async () => {
setVscodeHostProviderMock()
const terminalInfo = TerminalRegistry.createTerminal()
sandbox.stub(terminalInfo.terminal, "shellIntegration").get(() => undefined)
sandbox.stub(terminalInfo.terminal, "sendText")
const disposeSpy = sandbox.spy(terminalInfo.terminal, "dispose")
let nextTerminal: TerminalInfo | undefined
try {
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"sleep 999",
)
const unobservedCommand = new Promise<void>((resolve) => process.once("unobserved_command", () => resolve()))
process.continue()
await sandbox.clock.tickAsync(4000)
await sandbox.clock.tickAsync(3000)
await unobservedCommand
nextTerminal = (await manager.getOrCreateTerminal("/tmp/cline-next-command")) as unknown as TerminalInfo
assert.equal(disposeSpy.called, false, "stopping the wait relinquishes cleanup ownership")
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined, "continued terminals must not be reused")
} finally {
nextTerminal?.terminal.dispose()
if (nextTerminal) {
TerminalRegistry.removeTerminal(nextTerminal.id)
}
terminalInfo.terminal.dispose()
TerminalRegistry.removeTerminal(terminalInfo.id)
}
})
it("preserves a markerless shell-integration terminal across the next terminal acquisition", async () => {
setVscodeHostProviderMock()
const terminalInfo = TerminalRegistry.createTerminal()
sandbox.stub(terminalInfo.terminal, "shellIntegration").get(() => ({
executeCommand: () => ({ read: createMarkerlessStream }),
}))
const disposeSpy = sandbox.spy(terminalInfo.terminal, "dispose")
let nextTerminal: TerminalInfo | undefined
try {
const process = manager.runCommand(
terminalInfo as unknown as Parameters<VscodeTerminalManager["runCommand"]>[0],
"remote-command",
)
await sandbox.clock.tickAsync(15_000)
await process
assert.deepEqual(process.getCompletionDetails?.().unobservedCommand, {
source: "markerlessShellIntegration",
ownership: "managed",
})
nextTerminal = (await manager.getOrCreateTerminal("/tmp/cline-next-command")) as unknown as TerminalInfo
assert.equal(disposeSpy.called, false, "an SSH or nested-shell session remains user-owned")
assert.equal(TerminalRegistry.getTerminal(terminalInfo.id), undefined, "markerless terminals must not be reused")
} finally {
nextTerminal?.terminal.dispose()
if (nextTerminal) {
TerminalRegistry.removeTerminal(nextTerminal.id)
}
terminalInfo.terminal.dispose()
TerminalRegistry.removeTerminal(terminalInfo.id)
}
})
})
@@ -3,8 +3,9 @@ import { getShell, getShellForProfile } from "@utils/shell"
import pWaitFor from "p-wait-for"
import * as vscode from "vscode"
import {
TerminalInfo as ITerminalInfo,
TerminalProcessResultPromise as ITerminalProcessResultPromise,
getUnobservedTerminalCommandDisposition,
type TerminalInfo as ITerminalInfo,
type TerminalProcessResultPromise as ITerminalProcessResultPromise,
} from "@/integrations/terminal/types"
import { Logger } from "@/shared/services/Logger"
import { mergePromise, VscodeTerminalProcess } from "./VscodeTerminalProcess"
@@ -13,6 +14,8 @@ import { TerminalInfo, TerminalRegistry } from "./VscodeTerminalRegistry"
const CWD_COMMAND_TIMEOUT_MS = 5000
const CWD_STATE_TIMEOUT_MS = 1000
type CwdChangeResult = "observed" | "unobserved"
/*
TerminalManager:
- Creates/reuses terminals
@@ -106,7 +109,8 @@ export class VscodeTerminalManager {
// Check if CWD has been updated to match the expected path
if (this.isCwdMatchingExpected(terminalInfo)) {
const resolver = terminalInfo.cwdResolved.resolve
terminalInfo.pendingCwdChange = undefined
// Keep the target until the acquisition's finally block so the
// caller can confirm the resolved state before handing off.
terminalInfo.cwdResolved = undefined
resolver()
}
@@ -149,7 +153,7 @@ export class VscodeTerminalManager {
// VS Code shell integration sometimes finishes the internal `cd` command without
// reporting completion through the execution stream. Timeout this setup step so
// the user's actual command is still sent instead of leaving the chat stuck.
private async runCwdChangeCommand(terminalInfo: TerminalInfo, cwd: string): Promise<boolean> {
private async runCwdChangeCommand(terminalInfo: TerminalInfo, cwd: string): Promise<CwdChangeResult> {
const command = `cd "${cwd}"`
const shellIntegration = terminalInfo.terminal.shellIntegration
@@ -159,7 +163,7 @@ export class VscodeTerminalManager {
`[TerminalManager] Shell integration executeCommand is unavailable while changing terminal ${terminalInfo.id} cwd. Proceeding after ${CWD_COMMAND_TIMEOUT_MS}ms.`,
)
await new Promise((resolve) => setTimeout(resolve, CWD_COMMAND_TIMEOUT_MS))
return true
return "unobserved"
}
let timeout: NodeJS.Timeout | undefined
@@ -181,14 +185,21 @@ export class VscodeTerminalManager {
])
} catch (error) {
Logger.warn(`[TerminalManager] Failed to observe terminal ${terminalInfo.id} cwd command completion`, error)
return true
throw error
} finally {
if (timeout) {
clearTimeout(timeout)
}
}
return didTimeOut
return didTimeOut ? "unobserved" : "observed"
}
private runTerminalProcess(process: VscodeTerminalProcess, terminal: vscode.Terminal, command: string): void {
void process.run(terminal, command).catch((error) => {
process.releaseActiveExecutionResources()
process.emit("error", error instanceof Error ? error : new Error(String(error)))
})
}
runCommand(terminalInfo: ITerminalInfo, command: string): ITerminalProcessResultPromise {
@@ -198,6 +209,12 @@ export class VscodeTerminalManager {
Logger.log(`[TerminalManager] Running command on terminal ${vscodeTerminalInfo.id}: "${command}"`)
Logger.log(`[TerminalManager] Terminal ${vscodeTerminalInfo.id} busy state before: ${vscodeTerminalInfo.busy}`)
try {
vscodeTerminalInfo.terminal.show()
} catch (error) {
vscodeTerminalInfo.busy = false
throw error
}
vscodeTerminalInfo.busy = true
vscodeTerminalInfo.lastCommand = command
const process = new VscodeTerminalProcess()
@@ -207,14 +224,23 @@ export class VscodeTerminalManager {
Logger.log(`[TerminalManager] Terminal ${vscodeTerminalInfo.id} completed, setting busy to false`)
vscodeTerminalInfo.busy = false
})
process.once("error", () => {
// A stream/API failure does not prove the launched command stopped.
// Evict the terminal from Cline reuse without disposing potentially
// active user work.
this.evictTerminal(vscodeTerminalInfo)
})
// if shell integration is not available, remove terminal so it does not get reused as it may be running a long-running process
process.once("no_shell_integration", () => {
Logger.log(`no_shell_integration received for terminal ${vscodeTerminalInfo.id}`)
// Remove the terminal so we can't reuse it (in case it's running a long-running process)
TerminalRegistry.removeTerminal(vscodeTerminalInfo.id)
this.terminalIds.delete(vscodeTerminalInfo.id)
this.processes.delete(vscodeTerminalInfo.id)
process.once("unobserved_command", (outcome) => {
Logger.log(`unobserved_command (${outcome.source}) received for terminal ${vscodeTerminalInfo.id}`)
this.evictTerminal(vscodeTerminalInfo)
// Markerless streams (for example, an SSH session) and commands Cline no
// longer owns remain open. Ordinary managed sendText fallbacks are
// reclaimed at the next acquisition, after this tool result can report
// that their completion is indeterminate.
if (getUnobservedTerminalCommandDisposition(outcome) === "disposeBeforeNextTerminalAcquisition") {
TerminalRegistry.queueTerminalForCleanup(vscodeTerminalInfo)
}
})
const promise = new Promise<void>((resolve, reject) => {
@@ -230,7 +256,7 @@ export class VscodeTerminalManager {
// if shell integration is already active, run the command immediately
if (vscodeTerminalInfo.terminal.shellIntegration) {
process.waitForShellIntegration = false
process.run(vscodeTerminalInfo.terminal, command)
this.runTerminalProcess(process, vscodeTerminalInfo.terminal, command)
} else {
// docs recommend waiting 3s for shell integration to activate
Logger.log(
@@ -254,7 +280,7 @@ export class VscodeTerminalManager {
const existingProcess = this.processes.get(vscodeTerminalInfo.id)
if (existingProcess && existingProcess.waitForShellIntegration) {
existingProcess.waitForShellIntegration = false
existingProcess.run(vscodeTerminalInfo.terminal, command)
this.runTerminalProcess(existingProcess, vscodeTerminalInfo.terminal, command)
}
})
}
@@ -262,13 +288,28 @@ export class VscodeTerminalManager {
return mergePromise(process, promise)
}
/**
* A pre-start cancellation takes effect immediately for the tool result. The
* in-flight acquisition still owns its exact reservation until it settles;
* release that reservation here without starting or disposing the terminal.
*/
releaseTerminalReservation(terminalInfo: ITerminalInfo): void {
const vscodeTerminalInfo = terminalInfo as unknown as TerminalInfo
vscodeTerminalInfo.busy = false
}
/**
* @param profileId Terminal profile to create/match the terminal with.
* Defaults to the current setting; callers that captured the profile
* earlier (e.g. when the model request was built) pass it here so a
* settings change does not switch shells under an in-flight tool call.
* The returned terminal is reserved until runCommand() takes ownership.
*/
async getOrCreateTerminal(cwd: string, profileId: string = this.defaultTerminalProfile): Promise<ITerminalInfo> {
// A fallback terminal becomes cleanup-eligible when its unobserved-command
// outcome is emitted. Dispose the snapshot of eligible terminals before
// selecting a terminal for this acquisition.
TerminalRegistry.disposeTerminalsPendingCleanup()
const terminals = TerminalRegistry.getAllTerminals()
const expectedShellPath = profileId !== "default" ? getShellForProfile(profileId) : undefined
// Resolve effective shell for comparison (so "default" and "zsh" match on macOS)
@@ -298,6 +339,9 @@ export class VscodeTerminalManager {
})
if (matchingTerminal) {
Logger.log(`[TerminalManager] Found matching terminal ${matchingTerminal.id} in correct cwd`)
// Reserve synchronously before returning so parallel acquisitions cannot
// select this terminal before runCommand() marks it busy.
matchingTerminal.busy = true
this.terminalIds.add(matchingTerminal.id)
// Cast to ITerminalInfo for interface compatibility
return matchingTerminal as unknown as ITerminalInfo
@@ -310,45 +354,78 @@ export class VscodeTerminalManager {
)
if (availableTerminal) {
availableTerminal.busy = true
// Set up promise and tracking for CWD change
const cwdPromise = new Promise<void>((resolve, reject) => {
availableTerminal.pendingCwdChange = cwd
availableTerminal.cwdResolved = { resolve, reject }
})
// Showing the reused terminal gives VS Code a chance to initialize shell integration.
// runCommand() below waits up to shellIntegrationTimeout for executeCommand before falling back.
availableTerminal.terminal.show()
let didHandOffReservation = false
try {
const didCwdCommandTimeOut = await this.runCwdChangeCommand(availableTerminal, cwd)
// Set up promise and tracking for CWD change after reserving the
// terminal so parallel acquisitions cannot select it.
const cwdPromise = new Promise<void>((resolve, reject) => {
availableTerminal.pendingCwdChange = cwd
availableTerminal.cwdResolved = { resolve, reject }
})
// Showing the reused terminal gives VS Code a chance to initialize shell integration.
// runCommand() below waits up to shellIntegrationTimeout for executeCommand before falling back.
availableTerminal.terminal.show()
let cwdChangeResult: CwdChangeResult | undefined
try {
cwdChangeResult = await this.runCwdChangeCommand(availableTerminal, cwd)
} catch (error) {
// The user's command has not started. The failed setup command may
// still change this terminal later, so evict it and continue with a
// fresh terminal rooted at the requested cwd.
Logger.warn(
`[TerminalManager] Failed to prepare terminal ${availableTerminal.id} for "${cwd}"; creating a new terminal`,
error,
)
this.evictTerminal(availableTerminal)
}
// Add a small delay to ensure terminal is ready after cd
if (!didCwdCommandTimeOut) {
if (cwdChangeResult === "observed") {
await new Promise((resolve) => setTimeout(resolve, 100))
}
// Either resolve immediately if CWD already updated or wait for event/timeout
if (this.isCwdMatchingExpected(availableTerminal)) {
const isCwdConfirmed = this.isCwdMatchingExpected(availableTerminal)
if (isCwdConfirmed) {
if (availableTerminal.cwdResolved) {
availableTerminal.cwdResolved.resolve()
}
} else if (!didCwdCommandTimeOut) {
} else if (cwdChangeResult === "observed") {
await Promise.race([cwdPromise, new Promise((resolve) => setTimeout(resolve, CWD_STATE_TIMEOUT_MS))])
}
if (cwdChangeResult !== undefined && availableTerminal.terminal.exitStatus !== undefined) {
TerminalRegistry.removeTerminal(availableTerminal.id)
throw new Error("The terminal's shell process exited while preparing to run the command.")
}
if (cwdChangeResult !== undefined && this.isCwdMatchingExpected(availableTerminal)) {
this.terminalIds.add(availableTerminal.id)
didHandOffReservation = true
return availableTerminal as unknown as ITerminalInfo
}
// Never run a command in a terminal whose working directory could
// not be confirmed. The setup command may still take effect later,
// so evict this terminal and create a fresh one at the requested cwd.
Logger.warn(
`[TerminalManager] Could not confirm terminal ${availableTerminal.id} changed to "${cwd}"; creating a new terminal`,
)
this.evictTerminal(availableTerminal)
} finally {
availableTerminal.pendingCwdChange = undefined
availableTerminal.cwdResolved = undefined
availableTerminal.busy = false
if (!didHandOffReservation) {
availableTerminal.busy = false
}
}
this.terminalIds.add(availableTerminal.id)
// Cast to ITerminalInfo for interface compatibility
return availableTerminal as unknown as ITerminalInfo
}
}
// If all terminals are busy or don't match shell profile, create a new one with the configured shell
const newTerminalInfo = TerminalRegistry.createTerminal(cwd, expectedShellPath)
newTerminalInfo.busy = true
this.terminalIds.add(newTerminalInfo.id)
// Cast to ITerminalInfo for interface compatibility
return newTerminalInfo as unknown as ITerminalInfo
@@ -400,4 +477,10 @@ export class VscodeTerminalManager {
// skipped during reuse matching.
this.defaultTerminalProfile = profileId
}
private evictTerminal(terminalInfo: TerminalInfo): void {
this.terminalIds.delete(terminalInfo.id)
this.processes.delete(terminalInfo.id)
TerminalRegistry.removeTerminal(terminalInfo.id)
}
}
@@ -245,8 +245,10 @@ describe("TerminalProcess (Integration Tests)", () => {
;(emitSpy as sinon.SinonSpy).calledWith("completed").should.be.true()
;(emitSpy as sinon.SinonSpy).calledWith("continue").should.be.true()
// This event should be emitted for terminals without shell integration
;(emitSpy as sinon.SinonSpy).calledWith("no_shell_integration").should.be.true()
;(emitSpy as sinon.SinonSpy)
.calledWith("unobserved_command", { source: "sendText", ownership: "managed" })
.should.be.true()
process.getCompletionDetails().unobservedCommand?.should.eql({ source: "sendText", ownership: "managed" })
})
// The following tests require shell integration and controlled terminal output
@@ -530,7 +532,9 @@ describe("TerminalProcess (Integration Tests)", () => {
// The buffered pre-C output is emitted as fallback output
;(emitSpy as sinon.SinonSpy).calledWith("line", "remote output").should.be.true()
// The terminal must be evicted from the reuse pool
;(emitSpy as sinon.SinonSpy).calledWith("no_shell_integration").should.be.true()
;(emitSpy as sinon.SinonSpy)
.calledWith("unobserved_command", { source: "markerlessShellIntegration", ownership: "managed" })
.should.be.true()
})
it("should complete after the max quiet time even without a prompt", async () => {
@@ -549,7 +553,9 @@ describe("TerminalProcess (Integration Tests)", () => {
await runPromise
;(emitSpy as sinon.SinonSpy).calledWith("completed").should.be.true()
;(emitSpy as sinon.SinonSpy).calledWith("continue").should.be.true()
;(emitSpy as sinon.SinonSpy).calledWith("no_shell_integration").should.be.true()
;(emitSpy as sinon.SinonSpy)
.calledWith("unobserved_command", { source: "markerlessShellIntegration", ownership: "managed" })
.should.be.true()
})
it("should complete when no data ever arrives", async () => {
@@ -604,7 +610,7 @@ describe("TerminalProcess (Integration Tests)", () => {
;(emitSpy as sinon.SinonSpy).calledWith("line", "build finished").should.be.true()
;(emitSpy as sinon.SinonSpy).calledWith("completed").should.be.true()
// Shell integration worked — the terminal stays reusable
;(emitSpy as sinon.SinonSpy).calledWith("no_shell_integration").should.be.false()
;(emitSpy as sinon.SinonSpy).calledWith("unobserved_command").should.be.false()
})
it("should complete when the terminal closes mid-command", async () => {
@@ -15,7 +15,12 @@ import {
PROCESS_HOT_TIMEOUT_NORMAL,
TRUNCATE_KEEP_LINES,
} from "@/integrations/terminal/constants"
import type { ITerminalProcess, TerminalCompletionDetails, TerminalProcessEvents } from "@/integrations/terminal/types"
import type {
ITerminalProcess,
TerminalCompletionDetails,
TerminalProcessEvents,
UnobservedTerminalCommand,
} from "@/integrations/terminal/types"
import type { MarkerlessCompletionCause } from "@/services/telemetry/TelemetryService"
import { Logger } from "@/shared/services/Logger"
import { Osc633EventType, Osc633Parser } from "./osc633Parser"
@@ -37,7 +42,7 @@ type StreamReadOutcome = { kind: "data"; data: string } | { kind: "streamEnd" }
* - 'completed': Emitted when the process completes
* - 'continue': Emitted when continue() is called
* - 'error': Emitted on process errors
* - 'no_shell_integration': Emitted when shell integration is not available
* - 'unobserved_command': Emitted when command completion cannot be observed
*/
export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> implements ITerminalProcess {
waitForShellIntegration = true
@@ -50,10 +55,16 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
private exitCode: number | null | undefined = undefined
private signal: NodeJS.Signals | null = null
private terminalClosedMidCommand = false
private unobservedCommand: UnobservedTerminalCommand | undefined
private ownership: "managed" | "continued" | "detached" = "managed"
private activeCloseDisposable: vscode.Disposable | undefined
private activeEndEventDisposable: vscode.Disposable | undefined
private activeIterator: AsyncIterator<string> | undefined
async run(terminal: vscode.Terminal, command: string) {
this.exitCode = undefined
this.signal = null
this.unobservedCommand = undefined
// The pty may already be dead (exitStatus is set when the shell process
// terminates). executeCommand()/sendText() on a dead terminal never
@@ -120,6 +131,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
resolveExitCode.resolve(e.exitCode)
}
})
this.activeEndEventDisposable = endEventDisposable
// Track terminal closure so a dying pty can't leave the read loop
// blocked forever — the read() stream does not necessarily end when
@@ -135,6 +147,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
resolveTerminalClosed()
}
})
this.activeCloseDisposable = closeDisposable
// The stream is pulled manually (rather than with for-await) so each
// read can be raced against the markerless-completion timers and
@@ -142,6 +155,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
// is kept and reused on the next iteration — iterator.next() must
// not be called again while a previous read is still pending.
const iterator = stream[Symbol.asyncIterator]()
this.activeIterator = iterator
let pendingRead: Promise<IteratorResult<string>> | undefined
const readNext = async (idleTimeoutMs: number | undefined): Promise<StreamReadOutcome> => {
pendingRead ??= iterator.next()
@@ -322,6 +336,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
}
closeDisposable.dispose()
this.activeCloseDisposable = undefined
// Release the stream iterator. On the markerless/terminal-closed
// paths a read is still pending; return() lets a well-behaved
// iterator clean up instead of holding the stream open. Not
@@ -332,6 +347,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
} catch {
// The iterator does not support early termination.
}
this.activeIterator = undefined
this.emitRemainingBufferIfListening()
// Await the exit code from onDidEndTerminalShellExecution.
@@ -352,6 +368,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
}),
])
endEventDisposable.dispose()
this.activeEndEventDisposable = undefined
if (exitCodeEventTimedOut) {
// A lost exit code silently reports the command as successful to the
@@ -451,15 +468,15 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
)
}
this.emit("completed", this.getCompletionDetails())
this.emit("continue")
// A terminal whose shell isn't emitting completion markers (e.g. it
// is inside an ssh session) must not be reused for later commands:
// this event makes the manager evict it from the reuse pool, exactly
// like a terminal that never had shell integration.
// tell the manager to evict it without conflating this path with the
// sendText fallback, which has different cleanup semantics.
if (completedWithoutMarkers) {
this.emit("no_shell_integration")
this.markCommandUnobserved("markerlessShellIntegration")
}
this.emit("completed", this.getCompletionDetails())
this.emit("continue")
} else {
// no shell integration detected, we'll fallback to running the command and capturing the terminal's output after some time
telemetryService.captureTerminalOutputFailure(TerminalOutputFailureReason.NO_SHELL_INTEGRATION, "vscode")
@@ -483,9 +500,9 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
}
// 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.markCommandUnobserved("sendText")
this.emit("completed", this.getCompletionDetails())
this.emit("continue")
this.emit("no_shell_integration")
}
}
@@ -516,6 +533,9 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
}
continue() {
if (this.ownership === "managed") {
this.ownership = "continued"
}
this.emitRemainingBufferIfListening()
this.isListening = false
this.removeAllListeners("line")
@@ -531,6 +551,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
* terminal stays busy and is not eligible for reuse until then.
*/
detach() {
this.ownership = "detached"
// Flush any partial line (no trailing newline yet) so it reaches
// listeners before the awaited promise resolves; otherwise it would be
// dropped from both the partial output and the log capture if the
@@ -539,6 +560,25 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
this.emit("continue")
}
releaseActiveExecutionResources(): void {
this.activeCloseDisposable?.dispose()
this.activeCloseDisposable = undefined
this.activeEndEventDisposable?.dispose()
this.activeEndEventDisposable = undefined
try {
this.activeIterator?.return?.()?.catch?.(() => {})
} catch {
// The iterator does not support early termination.
}
this.activeIterator = undefined
}
private markCommandUnobserved(source: UnobservedTerminalCommand["source"]): void {
const command = { source, ownership: this.ownership }
this.unobservedCommand = command
this.emit("unobserved_command", command)
}
/**
* Get output that hasn't been retrieved yet.
* Truncates if output is too large to prevent context window overflow.
@@ -565,6 +605,7 @@ export class VscodeTerminalProcess extends EventEmitter<TerminalProcessEvents> i
exitCode: this.exitCode,
signal: this.signal,
terminalClosed: this.terminalClosedMidCommand,
unobservedCommand: this.unobservedCommand,
}
}
@@ -1,4 +1,5 @@
import * as vscode from "vscode"
import { Logger } from "@/shared/services/Logger"
export interface TerminalInfo {
terminal: vscode.Terminal
@@ -18,6 +19,7 @@ export interface TerminalInfo {
// Since we have promises keeping track of terminal processes, we get the added benefit of keep track of busy terminals even after a task is closed.
export class TerminalRegistry {
private static terminals: TerminalInfo[] = []
private static terminalsPendingCleanup = new Map<number, TerminalInfo>()
private static nextTerminalId = 1
static createTerminal(cwd?: string | vscode.Uri | undefined, shellPath?: string): TerminalInfo {
@@ -73,6 +75,36 @@ export class TerminalRegistry {
TerminalRegistry.terminals = TerminalRegistry.terminals.filter((t) => t.id !== id)
}
/**
* Evict a terminal now and remember it for disposal at the next terminal
* acquisition boundary. Keeping this queue in the global registry preserves
* cleanup ownership across task-scoped terminal-manager replacement. If no
* later command needs a terminal, leave the unobservable command alone: it
* may still be running, and without another acquisition it cannot contribute
* to the terminal pile-up this queue prevents.
*/
static queueTerminalForCleanup(terminalInfo: TerminalInfo): void {
TerminalRegistry.removeTerminal(terminalInfo.id)
TerminalRegistry.terminalsPendingCleanup.set(terminalInfo.id, terminalInfo)
}
/** Dispose every terminal that was cleanup-eligible when this call began. */
static disposeTerminalsPendingCleanup(): void {
const pending = Array.from(TerminalRegistry.terminalsPendingCleanup.entries())
for (const [id, terminalInfo] of pending) {
// Remove ownership before dispose(), which may synchronously trigger
// terminal-close listeners that acquire another terminal. Restore it if
// disposal fails so the resource is never silently lost.
TerminalRegistry.terminalsPendingCleanup.delete(id)
try {
terminalInfo.terminal.dispose()
} catch (error) {
TerminalRegistry.terminalsPendingCleanup.set(id, terminalInfo)
Logger.warn(`[TerminalRegistry] Failed to dispose fallback terminal ${id}; cleanup will be retried`, error)
}
}
}
static getAllTerminals(): TerminalInfo[] {
TerminalRegistry.terminals = TerminalRegistry.terminals.filter((t) => !TerminalRegistry.isTerminalClosed(t.terminal))
return TerminalRegistry.terminals
+26 -4
View File
@@ -16,6 +16,22 @@ import type { EventEmitter } from "events"
/**
* Event types for terminal process
*/
export interface UnobservedTerminalCommand {
/** The execution path that could not provide authoritative completion. */
source: "sendText" | "markerlessShellIntegration"
/** Who owns the still-running command when observation ends. */
ownership: "managed" | "continued" | "detached"
}
export type UnobservedTerminalCommandDisposition = "disposeBeforeNextTerminalAcquisition" | "preserve"
/** Derive cleanup and reporting policy from the same unobserved-command snapshot. */
export function getUnobservedTerminalCommandDisposition(
command: UnobservedTerminalCommand,
): UnobservedTerminalCommandDisposition {
return command.source === "sendText" && command.ownership === "managed" ? "disposeBeforeNextTerminalAcquisition" : "preserve"
}
export interface TerminalCompletionDetails {
/** Process exit code when available */
exitCode?: number | null
@@ -28,6 +44,12 @@ export interface TerminalCompletionDetails {
* closing mid-`npm test` must not be read as tests passing.
*/
terminalClosed?: boolean
/**
* The snapshotted outcome when VS Code could not observe completion. The
* command may have completed or may still be running; cleanup and reporting
* must both derive from this value.
*/
unobservedCommand?: UnobservedTerminalCommand
}
export interface TerminalProcessEvents {
@@ -35,7 +57,7 @@ export interface TerminalProcessEvents {
continue: []
completed: [details?: TerminalCompletionDetails]
error: [error: Error]
no_shell_integration: []
unobserved_command: [outcome: UnobservedTerminalCommand]
}
/**
@@ -47,7 +69,7 @@ export interface TerminalProcessEvents {
* - 'completed': Emitted when the process completes
* - 'continue': Emitted when continue() is called
* - 'error': Emitted on process errors
* - 'no_shell_integration': Emitted when shell integration is not available
* - 'unobserved_command': Emitted when command completion cannot be observed
*/
export interface ITerminalProcess extends EventEmitter<TerminalProcessEvents> {
/**
@@ -152,8 +174,8 @@ export type TerminalProcessResultPromise = Promise<void> &
on(event: "continue", listener: () => void): TerminalProcessResultPromise
/** Listen for error events */
on(event: "error", listener: (error: Error) => void): TerminalProcessResultPromise
/** Listen for no shell integration event */
on(event: "no_shell_integration", listener: () => void): TerminalProcessResultPromise
/** Listen for an unobserved command outcome */
on(event: "unobserved_command", listener: (outcome: UnobservedTerminalCommand) => void): TerminalProcessResultPromise
/** Listen once for any event */
once(event: string, listener: (...args: any[]) => void): TerminalProcessResultPromise
}
@@ -143,6 +143,18 @@ function createFakeTerminalProcess(options: { lines?: string[]; completionDetail
return fakeProcess as unknown as ReturnType<VscodeTerminalManager["runCommand"]>
}
function createRejectedTerminalProcess(error: Error) {
const emitter = new EventEmitter()
const promise = new Promise<void>((_resolve, reject) => setTimeout(() => reject(error), 0))
const fakeProcess = Object.assign(emitter, {
then: promise.then.bind(promise),
catch: promise.catch.bind(promise),
finally: promise.finally.bind(promise),
getCompletionDetails: () => ({}),
})
return fakeProcess as unknown as ReturnType<VscodeTerminalManager["runCommand"]>
}
function createFakeTerminalManager(process: ReturnType<VscodeTerminalManager["runCommand"]>): VscodeTerminalManager {
return {
getOrCreateTerminal: async () => ({ terminal: { show: () => {} } }) as never,
@@ -175,12 +187,20 @@ function createControllableTerminalProcess() {
return {
process: fakeProcess as unknown as ReturnType<VscodeTerminalManager["runCommand"]>,
emitLine: (line: string) => emitter.emit("line", line),
fail: (error: Error) => emitter.emit("error", error),
complete: (details?: TerminalCompletionDetails) => {
emitter.emit("completed", details)
emitter.emit("continue")
resolvePromise()
},
fail: (error: Error) => emitter.emit("error", error),
}
}
function createControllableUnobservedTerminalProcess() {
const controlled = createControllableTerminalProcess()
return {
...controlled,
completeUnobserved: () => controlled.complete({ unobservedCommand: { source: "sendText", ownership: "detached" } }),
}
}
@@ -339,6 +359,45 @@ describe("executeForeground", () => {
}
})
it("throws an indeterminate CommandExitError when command completion cannot be observed", async () => {
const process = createFakeTerminalProcess({
lines: ["partial output"],
completionDetails: { unobservedCommand: { source: "sendText", ownership: "managed" } },
})
const terminalManager = createFakeTerminalManager(process)
try {
await executeForeground("long-running-cmd", "/workspace", terminalManager, 1000)
expect.unreachable("expected executeForeground to reject indeterminate completion")
} catch (error) {
expect(error).toBeInstanceOf(CommandExitError)
expect((error as InstanceType<typeof CommandExitError>).output).toContain("must not be assumed to have succeeded")
expect((error as InstanceType<typeof CommandExitError>).output).toContain(
"The terminal remains open for now, but starting another foreground command will attempt to close it",
)
expect((error as InstanceType<typeof CommandExitError>).output).toContain("partial output")
}
})
it("says markerless terminals will be preserved when completion cannot be observed", async () => {
const process = createFakeTerminalProcess({
completionDetails: {
unobservedCommand: { source: "markerlessShellIntegration", ownership: "managed" },
},
})
try {
await executeForeground("remote-command", "/workspace", createFakeTerminalManager(process), 1000)
expect.unreachable("expected executeForeground to reject indeterminate completion")
} catch (error) {
expect(error).toBeInstanceOf(CommandExitError)
expect((error as InstanceType<typeof CommandExitError>).output).toContain(
"left open and will not be closed automatically",
)
expect((error as InstanceType<typeof CommandExitError>).output).not.toContain("next foreground command")
}
})
it("unregisters its foreground handle when the command completes normally", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const terminalManager = createFakeTerminalManager(createFakeTerminalProcess({ lines: ["hello"] }))
@@ -348,6 +407,29 @@ describe("executeForeground", () => {
expect(result).toBe("hello")
expect(coordinator.isRunning).toBe(false)
})
it("removes per-call listeners when the command completes", async () => {
const process = createFakeTerminalProcess({ lines: ["hello"] })
await executeForeground("echo hello", "/workspace", createFakeTerminalManager(process), 1000)
expect(process.listenerCount("line")).toBe(0)
})
it("removes per-call and abort listeners when the command rejects", async () => {
const process = createRejectedTerminalProcess(new Error("stream failed"))
const abortController = new AbortController()
const removeAbortListener = vi.spyOn(abortController.signal, "removeEventListener")
await expect(
executeForeground("failing-command", "/workspace", createFakeTerminalManager(process), 1000, abortController.signal),
).rejects.toThrow("stream failed")
expect(process.listenerCount("line")).toBe(0)
expect(process.listenerCount("completed")).toBe(0)
expect(process.listenerCount("continue")).toBe(0)
expect(removeAbortListener).toHaveBeenCalledWith("abort", expect.any(Function))
})
})
describe("executeForeground — Proceed While Running", () => {
@@ -390,6 +472,130 @@ describe("executeForeground — Proceed While Running", () => {
fs.rmSync(logFilePath!, { force: true })
})
it("does not label a detached unobserved command as completed in its log", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const { process, completeUnobserved } = createControllableUnobservedTerminalProcess()
const resultPromise = executeForeground(
"devserver",
"/workspace",
createFakeTerminalManager(process),
100_000,
undefined,
coordinator,
)
await waitFor(() => coordinator.isRunning)
expect(coordinator.proceedWhileRunning()).toBe(1)
const result = await resultPromise
const logFilePath = /redirected to this file[^:]*: (.+)$/m.exec(result)?.[1]?.trim()
expect(logFilePath).toBeTruthy()
completeUnobserved()
await waitFor(() => {
try {
return fs.readFileSync(logFilePath!, "utf8").includes("completion could not be observed")
} catch {
return false
}
})
const log = fs.readFileSync(logFilePath!, "utf8")
expect(log).toContain("the command may still be running")
expect(log).not.toContain("[Command completed]")
fs.rmSync(logFilePath!, { force: true })
})
it("does not label a detached terminal closure as completed in its log", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const { process, complete } = createControllableTerminalProcess()
const resultPromise = executeForeground(
"devserver",
"/workspace",
createFakeTerminalManager(process),
100_000,
undefined,
coordinator,
)
await waitFor(() => coordinator.isRunning)
expect(coordinator.proceedWhileRunning()).toBe(1)
const result = await resultPromise
const logFilePath = /redirected to this file[^:]*: (.+)$/m.exec(result)?.[1]?.trim()
expect(logFilePath).toBeTruthy()
complete({ terminalClosed: true })
await waitFor(() => {
try {
return fs.readFileSync(logFilePath!, "utf8").includes("Terminal closed while the command was running")
} catch {
return false
}
})
const log = fs.readFileSync(logFilePath!, "utf8")
expect(log).toContain("output may be incomplete")
expect(log).not.toContain("[Command completed]")
fs.rmSync(logFilePath!, { force: true })
})
it("records a command failure that occurs after detaching and closes the log", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const { process, fail } = createControllableTerminalProcess()
const resultPromise = executeForeground(
"devserver",
"/workspace",
createFakeTerminalManager(process),
100_000,
undefined,
coordinator,
)
await waitFor(() => coordinator.isRunning)
expect(coordinator.proceedWhileRunning()).toBe(1)
const result = await resultPromise
const logFilePath = /redirected to this file[^:]*: (.+)$/m.exec(result)?.[1]?.trim()
expect(logFilePath).toBeTruthy()
fail(new Error("stream failed"))
await waitFor(() => {
try {
return fs.readFileSync(logFilePath!, "utf8").includes("[Command failed after detaching: stream failed]")
} catch {
return false
}
})
expect(fs.readFileSync(logFilePath!, "utf8")).not.toContain("[Command completed]")
fs.rmSync(logFilePath!, { force: true })
})
it("records a detached failure even after command output reaches the log cap", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const { process, emitLine, fail } = createControllableTerminalProcess()
const resultPromise = executeForeground(
"devserver",
"/workspace",
createFakeTerminalManager(process),
100_000,
undefined,
coordinator,
)
await waitFor(() => coordinator.isRunning)
expect(coordinator.proceedWhileRunning()).toBe(1)
const result = await resultPromise
const logFilePath = /redirected to this file[^:]*: (.+)$/m.exec(result)?.[1]?.trim()
expect(logFilePath).toBeTruthy()
emitLine("x".repeat(PROCEED_LOG_MAX_BYTES))
fail(new Error("stream failed after cap"))
await waitFor(() => {
try {
return fs.readFileSync(logFilePath!, "utf8").includes("[Command failed after detaching: stream failed after cap]")
} catch {
return false
}
})
fs.rmSync(logFilePath!, { force: true })
})
it("detaches each parallel command into its own log file", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const first = createControllableTerminalProcess()
@@ -500,12 +706,17 @@ describe("executeForeground — Proceed While Running", () => {
releaseTerminal = resolve
})
const runCommand = vi.fn()
const terminalInfo = { terminal: { show: () => {} }, busy: true }
const releaseTerminalReservation = vi.fn(() => {
terminalInfo.busy = false
})
const terminalManager = {
getOrCreateTerminal: async () => {
await terminalGate
return { terminal: { show: () => {} } } as never
return terminalInfo as never
},
runCommand,
releaseTerminalReservation,
} as unknown as VscodeTerminalManager
const resultPromise = executeForeground(
@@ -525,11 +736,41 @@ describe("executeForeground — Proceed While Running", () => {
releaseTerminal()
await new Promise((resolve) => setTimeout(resolve, 0))
expect(runCommand).not.toHaveBeenCalled()
expect(releaseTerminalReservation).toHaveBeenCalledWith(terminalInfo)
expect(terminalInfo.busy).toBe(false)
})
it("releases the reservation when acquisition and abort settle in the same promise turn", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const abortController = new AbortController()
const terminalInfo = { terminal: { show: () => {} }, busy: true }
const runCommand = vi.fn()
const releaseTerminalReservation = vi.fn(() => {
terminalInfo.busy = false
})
const terminalManager = {
getOrCreateTerminal: () =>
Promise.resolve(terminalInfo as never).then((terminal) => {
abortController.abort()
return terminal
}),
runCommand,
releaseTerminalReservation,
} as unknown as VscodeTerminalManager
await expect(
executeForeground("cancelled-as-acquired", "/workspace", terminalManager, 1000, abortController.signal, coordinator),
).rejects.toThrow("Command execution aborted")
expect(runCommand).not.toHaveBeenCalled()
expect(releaseTerminalReservation).toHaveBeenCalledWith(terminalInfo)
expect(terminalInfo.busy).toBe(false)
expect(coordinator.isRunning).toBe(false)
})
it("detach requested during terminal acquisition applies once the command starts", async () => {
const coordinator = new SdkForegroundCommandCoordinator()
const { process, emitLine, complete } = createControllableTerminalProcess()
const { process, emitLine, completeUnobserved } = createControllableUnobservedTerminalProcess()
let releaseTerminal!: () => void
const terminalGate = new Promise<void>((resolve) => {
releaseTerminal = resolve
@@ -560,15 +801,18 @@ describe("executeForeground — Proceed While Running", () => {
releaseTerminal()
await waitFor(() => process.listenerCount("line") > 0)
emitLine("started late")
complete({ exitCode: 0 })
completeUnobserved()
await waitFor(() => {
try {
return fs.readFileSync(logFilePath!, "utf8").includes("[Command completed with exit code 0]")
return fs.readFileSync(logFilePath!, "utf8").includes("completion could not be observed")
} catch {
return false
}
})
expect(fs.readFileSync(logFilePath!, "utf8")).toContain("started late")
const log = fs.readFileSync(logFilePath!, "utf8")
expect(log).toContain("started late")
expect(log).toContain("the command may still be running")
expect(log).not.toContain("[Command completed]")
fs.rmSync(logFilePath!, { force: true })
})
+107 -71
View File
@@ -27,7 +27,11 @@ import * as fs from "fs"
import { StateManager } from "@/core/storage/StateManager"
import type { VscodeTerminalManager } from "@/hosts/vscode/terminal/VscodeTerminalManager"
import { MAX_UNRETRIEVED_LINES } from "@/integrations/terminal/constants"
import type { ITerminalProcess } from "@/integrations/terminal/types"
import {
getUnobservedTerminalCommandDisposition,
type ITerminalProcess,
type TerminalCompletionDetails,
} from "@/integrations/terminal/types"
import { Logger } from "@/shared/services/Logger"
import { getShellForProfile } from "@/utils/shell"
import type { SdkForegroundCommandCoordinator } from "./sdk-foreground-command-coordinator"
@@ -49,6 +53,7 @@ export const VSCODE_FOREGROUND_RUN_COMMANDS_TIMEOUT_MS = 60 * 60 * 1000
* for the files themselves.
*/
export const PROCEED_LOG_MAX_BYTES = 10 * 1024 * 1024
const PROCEED_LOG_FINAL_MESSAGE_MAX_CHARS = 4096
/** Options for creating the VSCode run_commands tool. */
export interface VscodeRunCommandsToolOptions {
@@ -144,7 +149,9 @@ function createDetachedCommandLog(terminalCommand: string, existingLines: string
}
settled = true
removeAttachedListeners()
tryWriteLine(message)
// Output obeys the strict cap, but reserve a small bounded allowance for
// the terminal status so a full log never hides completion or failure.
stream.write(`${message.slice(0, PROCEED_LOG_FINAL_MESSAGE_MAX_CHARS)}\n`)
stream.end()
}
@@ -159,15 +166,19 @@ function createDetachedCommandLog(terminalCommand: string, existingLines: string
process.removeListener("line", onLine)
}
}
const onCompleted = (details?: { exitCode?: number | null }): void => {
const onCompleted = (details?: TerminalCompletionDetails): void => {
const exitCode = details?.exitCode
end(
exitCode !== undefined && exitCode !== null
? `[Command completed with exit code ${exitCode}]`
: "[Command completed]",
details?.terminalClosed
? "[Terminal closed while the command was running; output may be incomplete]"
: details?.unobservedCommand
? "[Command completion could not be observed; the command may still be running]"
: exitCode !== undefined && exitCode !== null
? `[Command completed with exit code ${exitCode}]`
: "[Command completed]",
)
}
const onError = (error: Error): void => end(`[Command failed before log capture completed: ${error.message}]`)
const onError = (error: Error): void => end(`[Command failed after detaching: ${error.message}]`)
removeAttachedListeners = () => {
process.removeListener("line", onLine)
process.removeListener("completed", onCompleted)
@@ -257,7 +268,6 @@ export async function executeForeground(
const terminalPromise = terminalManager.getOrCreateTerminal(cwd, terminalProfileId)
const startDetached = (terminalInfo: Awaited<typeof terminalPromise>, log: DetachedCommandLog): void => {
try {
terminalInfo.terminal.show()
const process = terminalManager.runCommand(terminalInfo, terminalCommand)
log.attach(process)
void process.catch((error) => log.fail(error))
@@ -277,6 +287,11 @@ export async function executeForeground(
log.fail(outcome.error)
}
}
const finishAbortedAcquisition = (outcome: Awaited<typeof acquisition>): void => {
if (outcome.type === "terminal") {
terminalManager.releaseTerminalReservation(outcome.terminalInfo)
}
}
const firstOutcome = await Promise.race([
acquisition,
preStartControl.then((control) => ({ type: "control" as const, control })),
@@ -284,6 +299,10 @@ export async function executeForeground(
if (firstOutcome.type === "control") {
if (firstOutcome.control === "abort") {
// Acquisition is already in flight and may return a synchronously
// reserved terminal after this tool result settles. Consume it so a
// pre-start cancellation cannot leave that terminal permanently busy.
void acquisition.then(finishAbortedAcquisition)
throw new Error("Command execution aborted")
}
@@ -300,6 +319,7 @@ export async function executeForeground(
// terminal outcome cannot overwrite a detach or abort that happened while
// the promise continuation was pending.
if (state.phase === "aborted") {
finishAbortedAcquisition(firstOutcome)
throw new Error("Command execution aborted")
}
if (state.phase === "detached") {
@@ -316,7 +336,6 @@ export async function executeForeground(
state.phase = "started"
const { terminalInfo } = firstOutcome
terminalInfo.terminal.show()
const process = terminalManager.runCommand(terminalInfo, terminalCommand)
const outputLines: string[] = []
@@ -343,72 +362,89 @@ export async function executeForeground(
}
process.on("line", bufferLine)
applyAbort = () => process.continue()
try {
applyAbort = () => process.continue()
applyDetach = () => {
if (detachedLog !== undefined) {
return
applyDetach = () => {
if (detachedLog !== undefined) {
return
}
detachedLog = createDetachedCommandLog(terminalCommand, outputLines)
detachedLog.attach(process)
telemetryService.captureTerminalUserIntervention(TerminalUserInterventionAction.PROCESS_WHILE_RUNNING, "vscode")
// detach() flushes any partial line (reaching both bufferLine and
// the log) before resolving the awaited promise. After that the
// partial output is final: stop buffering so the remaining
// (log-only) output doesn't mutate outputLines while it's read.
process.detach()
process.removeListener("line", bufferLine)
}
detachedLog = createDetachedCommandLog(terminalCommand, outputLines)
detachedLog.attach(process)
telemetryService.captureTerminalUserIntervention(TerminalUserInterventionAction.PROCESS_WHILE_RUNNING, "vscode")
// detach() flushes any partial line (reaching both bufferLine and
// the log) before resolving the awaited promise. After that the
// partial output is final: stop buffering so the remaining
// (log-only) output doesn't mutate outputLines while it's read.
process.detach()
// Wait for completion (or detach, which also resolves the promise)
await process
if (abortSignal?.aborted) {
throw new Error("Command execution aborted")
}
const bufferedOutput =
droppedLines > 0
? [...outputLines, `\n... (${droppedLines} earlier lines dropped) ...\n`].join("\n")
: outputLines.join("\n")
const output = truncateCommandOutput(bufferedOutput.trim(), {
maxChars: maxOutputChars,
})
if (detachedLog !== undefined) {
return formatDetachedResult(detachedLog.path, output)
}
const completionDetails = process.getCompletionDetails?.()
// A terminal closed mid-command has no exit code and no reliable output —
// whatever the command was doing (e.g. running a test suite) was interrupted,
// so this must never look like success to the agent.
if (completionDetails?.terminalClosed) {
const result =
output.length > 0
? `[Terminal closed while the command was running; output may be incomplete]\n${output}`
: "[Terminal closed while the command was running; no output was captured]"
throw new CommandExitError(1, result)
}
if (completionDetails?.unobservedCommand) {
const disposition = getUnobservedTerminalCommandDisposition(completionDetails.unobservedCommand)
const lifecycle =
disposition === "disposeBeforeNextTerminalAcquisition"
? "The terminal remains open for now, but starting another foreground command will attempt to close it, stopping the command if it is still running."
: "The terminal has been left open and will not be closed automatically."
const result =
output.length > 0
? `[Command completion could not be observed; the command may still be running and must not be assumed to have succeeded. ${lifecycle}]\n${output}`
: `[Command completion could not be observed; the command may still be running and must not be assumed to have succeeded. ${lifecycle}]`
throw new CommandExitError(1, result)
}
// Plumb the exit code from onDidEndTerminalShellExecution through to the tool
// result. When shell integration reports a non-zero exit code, throw
// CommandExitError so the SDK's shell tool wrapper marks the result as
// `success: false` and includes the exit code in the error message —
// matching the background (child_process) executor's behavior.
// If no exit code was captured after an observed completion, return the
// output as-is. Unobserved completion is handled explicitly above.
const exitCode = completionDetails?.exitCode
if (exitCode !== undefined && exitCode !== null && exitCode !== 0) {
const result =
output.length > 0
? `[Command exited with code ${exitCode}]\n${output}`
: `[Command exited with code ${exitCode}]`
throw new CommandExitError(exitCode, result)
}
return output
} finally {
process.removeListener("line", bufferLine)
}
// Wait for completion (or detach, which also resolves the promise)
await process
if (abortSignal?.aborted) {
throw new Error("Command execution aborted")
}
const bufferedOutput =
droppedLines > 0
? [...outputLines, `\n... (${droppedLines} earlier lines dropped) ...\n`].join("\n")
: outputLines.join("\n")
const output = truncateCommandOutput(bufferedOutput.trim(), {
maxChars: maxOutputChars,
})
if (detachedLog !== undefined) {
return formatDetachedResult(detachedLog.path, output)
}
const completionDetails = process.getCompletionDetails?.()
// A terminal closed mid-command has no exit code and no reliable output —
// whatever the command was doing (e.g. running a test suite) was interrupted,
// so this must never look like success to the agent.
if (completionDetails?.terminalClosed) {
const result =
output.length > 0
? `[Terminal closed while the command was running; output may be incomplete]\n${output}`
: "[Terminal closed while the command was running; no output was captured]"
throw new CommandExitError(1, result)
}
// Plumb the exit code from onDidEndTerminalShellExecution through to the tool
// result. When shell integration reports a non-zero exit code, throw
// CommandExitError so the SDK's shell tool wrapper marks the result as
// `success: false` and includes the exit code in the error message —
// matching the background (child_process) executor's behavior.
// If no exit code was captured (shell integration present but not reporting
// completion for this execution — e.g. a command run inside an ssh session),
// we can't determine success/failure, so we return the output as-is
// (success: true).
const exitCode = completionDetails?.exitCode
if (exitCode !== undefined && exitCode !== null && exitCode !== 0) {
const result =
output.length > 0 ? `[Command exited with code ${exitCode}]\n${output}` : `[Command exited with code ${exitCode}]`
throw new CommandExitError(exitCode, result)
}
return output
} finally {
abortSignal?.removeEventListener("abort", onAbort)
unregister?.()