From 8d24c614997735bcdbffafc30fedff046607479c Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 15 Apr 2026 19:18:40 +0300 Subject: [PATCH 1/3] fix: use OutputChannel for build logs instead of pseudoterminal Replace TerminalSession with TerminalOutputChannel backed by a VS Code OutputChannel. CliContext now takes a plain write callback instead of an EventEmitter. --- src/api/workspace.ts | 10 ++--- src/remote/terminalOutputChannel.ts | 19 +++++++++ src/remote/terminalSession.ts | 39 ------------------- src/remote/workspaceStateMachine.ts | 20 +++++----- test/mocks/testHelpers.ts | 27 ++++++------- .../unit/remote/terminalOutputChannel.test.ts | 39 +++++++++++++++++++ .../unit/remote/workspaceStateMachine.test.ts | 8 ++-- 7 files changed, 88 insertions(+), 74 deletions(-) create mode 100644 src/remote/terminalOutputChannel.ts delete mode 100644 src/remote/terminalSession.ts create mode 100644 test/unit/remote/terminalOutputChannel.test.ts diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 7d392836..3c23e28f 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -50,7 +50,7 @@ interface CliContext { auth: CliAuth; binPath: string; workspace: Workspace; - writeEmitter: vscode.EventEmitter; + write: (data: string) => void; featureSet: FeatureSet; } @@ -70,13 +70,13 @@ function runCliCommand(ctx: CliContext, args: string[]): Promise { const proc = spawn(cmd, { shell: true }); proc.stdout.on("data", (data: Buffer) => { - ctx.writeEmitter.fire(data.toString().replace(/\r?\n/g, "\r\n")); + ctx.write(data.toString().replace(/\r?\n/g, "\r\n")); }); let capturedStderr = ""; proc.stderr.on("data", (data: Buffer) => { const text = data.toString(); - ctx.writeEmitter.fire(text.replace(/\r?\n/g, "\r\n")); + ctx.write(text.replace(/\r?\n/g, "\r\n")); capturedStderr += text; }); @@ -126,7 +126,7 @@ export async function updateWorkspace(ctx: CliContext): Promise { // REST API fallback for older CLIs. if (ctx.workspace.latest_build.status === "running") { - ctx.writeEmitter.fire("Stopping workspace for update...\r\n"); + ctx.write("Stopping workspace for update...\r\n"); const stopBuild = await ctx.restClient.stopWorkspace(ctx.workspace.id); const stoppedJob = await ctx.restClient.waitForBuild(stopBuild); if (stoppedJob?.status === "canceled") { @@ -134,7 +134,7 @@ export async function updateWorkspace(ctx: CliContext): Promise { } } - ctx.writeEmitter.fire("Starting workspace with updated template...\r\n"); + ctx.write("Starting workspace with updated template...\r\n"); await ctx.restClient.updateWorkspaceVersion(ctx.workspace); return ctx.restClient.getWorkspace(ctx.workspace.id); } diff --git a/src/remote/terminalOutputChannel.ts b/src/remote/terminalOutputChannel.ts new file mode 100644 index 00000000..17c39762 --- /dev/null +++ b/src/remote/terminalOutputChannel.ts @@ -0,0 +1,19 @@ +import * as vscode from "vscode"; + +/** Adapts terminal-style output (\r\n) for a VS Code OutputChannel (\n). */ +export class TerminalOutputChannel implements vscode.Disposable { + private readonly channel: vscode.OutputChannel; + + constructor(name: string) { + this.channel = vscode.window.createOutputChannel(name); + this.channel.show(true); + } + + write(data: string): void { + this.channel.append(data.replace(/\r/g, "")); + } + + dispose(): void { + this.channel.dispose(); + } +} diff --git a/src/remote/terminalSession.ts b/src/remote/terminalSession.ts deleted file mode 100644 index 358134a1..00000000 --- a/src/remote/terminalSession.ts +++ /dev/null @@ -1,39 +0,0 @@ -import * as vscode from "vscode"; - -/** - * Manages a terminal and its associated write emitter as a single unit. - * Ensures both are created together and disposed together properly. - */ -export class TerminalSession implements vscode.Disposable { - public readonly writeEmitter: vscode.EventEmitter; - public readonly terminal: vscode.Terminal; - - constructor(name: string) { - this.writeEmitter = new vscode.EventEmitter(); - this.terminal = vscode.window.createTerminal({ - name, - location: vscode.TerminalLocation.Panel, - // Spin makes this gear icon spin! - iconPath: new vscode.ThemeIcon("gear~spin"), - pty: { - onDidWrite: this.writeEmitter.event, - close: () => undefined, - open: () => undefined, - }, - }); - this.terminal.show(true); - } - - dispose(): void { - try { - this.writeEmitter.dispose(); - } catch { - // Ignore disposal errors - } - try { - this.terminal.dispose(); - } catch { - // Ignore disposal errors - } - } -} diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index 20ef8f66..b533ffb8 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -9,7 +9,7 @@ import { import { maybeAskAgent } from "../promptUtils"; import { vscodeProposed } from "../vscodeProposed"; -import { TerminalSession } from "./terminalSession"; +import { TerminalOutputChannel } from "./terminalOutputChannel"; import type { ProvisionerJobLog, @@ -30,7 +30,7 @@ import type { AuthorityParts } from "../util"; * Streams build and agent logs, and handles socket lifecycle. */ export class WorkspaceStateMachine implements vscode.Disposable { - private readonly terminal: TerminalSession; + private readonly terminal: TerminalOutputChannel; private readonly buildLogStream = new LazyStream(); private readonly agentLogStream = new LazyStream(); @@ -45,7 +45,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly logger: Logger, private readonly cliAuth: CliAuth, ) { - this.terminal = new TerminalSession("Workspace Build"); + this.terminal = new TerminalOutputChannel("Coder: Workspace Build"); } /** @@ -102,12 +102,10 @@ export class WorkspaceStateMachine implements vscode.Disposable { }); this.logger.info(`Waiting for ${workspaceName}`); - const write = (line: string) => - this.terminal.writeEmitter.fire(line + "\r\n"); await this.buildLogStream.open(() => streamBuildLogs( this.workspaceClient, - write, + (line) => this.terminal.write(line + "\r\n"), workspace.latest_build.id, ), ); @@ -183,10 +181,12 @@ export class WorkspaceStateMachine implements vscode.Disposable { }); this.logger.debug(`Running agent ${agent.name} startup scripts`); - const writeAgent = (line: string) => - this.terminal.writeEmitter.fire(line + "\r\n"); await this.agentLogStream.open(() => - streamAgentLogs(this.workspaceClient, writeAgent, agent.id), + streamAgentLogs( + this.workspaceClient, + (line) => this.terminal.write(line + "\r\n"), + agent.id, + ), ); return false; } @@ -229,7 +229,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { auth: this.cliAuth, binPath: this.binaryPath, workspace, - writeEmitter: this.terminal.writeEmitter, + write: (data: string) => this.terminal.write(data), featureSet: this.featureSet, }; } diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index 052de4b4..a94b7257 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -947,35 +947,30 @@ export class MockContextManager { } /** - * Mock TerminalSession that captures all content written to the terminal. + * Mock TerminalOutputChannel that captures all written content. * Use `lastInstance` to get the most recently created instance (set in the constructor), - * which is useful when the real TerminalSession is created inside the class under test. + * which is useful when the real class is created inside the class under test. */ -export class MockTerminalSession { - static lastInstance: MockTerminalSession | undefined; +export class MockTerminalOutputChannel { + static lastInstance: MockTerminalOutputChannel | undefined; private readonly _lines: string[] = []; - readonly writeEmitter = { - fire: vi.fn((data: string) => { - this._lines.push(data); - }), - event: vi.fn(), - dispose: vi.fn(), - }; - readonly terminal = { show: vi.fn(), dispose: vi.fn() }; + readonly write = vi.fn((data: string) => { + this._lines.push(data); + }); readonly dispose = vi.fn(); constructor(_name?: string) { - MockTerminalSession.lastInstance = this; + MockTerminalOutputChannel.lastInstance = this; } - /** All lines written via writeEmitter.fire(). */ + /** All lines written via write(). */ get lines(): readonly string[] { return this._lines; } - /** Concatenated terminal content. */ + /** Concatenated content. */ get content(): string { return this._lines.join(""); } @@ -983,6 +978,6 @@ export class MockTerminalSession { /** Reset captured content and mock call history. */ clear(): void { this._lines.length = 0; - this.writeEmitter.fire.mockClear(); + this.write.mockClear(); } } diff --git a/test/unit/remote/terminalOutputChannel.test.ts b/test/unit/remote/terminalOutputChannel.test.ts new file mode 100644 index 00000000..3c47e3e9 --- /dev/null +++ b/test/unit/remote/terminalOutputChannel.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import { TerminalOutputChannel } from "@/remote/terminalOutputChannel"; + +const mockAppend = vi.fn(); + +vi.mock("vscode", () => ({ + window: { + createOutputChannel: vi.fn(() => ({ + append: mockAppend, + show: vi.fn(), + dispose: vi.fn(), + })), + }, +})); + +describe("TerminalOutputChannel", () => { + beforeEach(() => { + mockAppend.mockClear(); + }); + + it("strips \\r from \\r\\n line endings", () => { + const channel = new TerminalOutputChannel("test"); + channel.write("hello\r\nworld\r\n"); + expect(mockAppend).toHaveBeenCalledWith("hello\nworld\n"); + }); + + it("strips bare \\r characters", () => { + const channel = new TerminalOutputChannel("test"); + channel.write("progress\r50%\r100%\n"); + expect(mockAppend).toHaveBeenCalledWith("progress50%100%\n"); + }); + + it("passes plain text through unchanged", () => { + const channel = new TerminalOutputChannel("test"); + channel.write("no special chars"); + expect(mockAppend).toHaveBeenCalledWith("no special chars"); + }); +}); diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index e45c21c9..24840124 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -12,7 +12,7 @@ import { WorkspaceStateMachine } from "@/remote/workspaceStateMachine"; import { createMockLogger, MockProgress, - MockTerminalSession, + MockTerminalOutputChannel, MockUserInteraction, } from "../../mocks/testHelpers"; import { @@ -46,9 +46,9 @@ vi.mock("@/promptUtils", () => ({ maybeAskAgent: vi.fn(), })); -vi.mock("@/remote/terminalSession", async () => { +vi.mock("@/remote/terminalOutputChannel", async () => { const helpers = await import("../../mocks/testHelpers"); - return { TerminalSession: helpers.MockTerminalSession }; + return { TerminalOutputChannel: helpers.MockTerminalOutputChannel }; }); const DEFAULT_PARTS: Readonly = { @@ -92,7 +92,7 @@ function setup(startupMode: StartupMode = "start") { describe("WorkspaceStateMachine", () => { beforeEach(() => { vi.clearAllMocks(); - MockTerminalSession.lastInstance = undefined; + MockTerminalOutputChannel.lastInstance = undefined; vi.mocked(maybeAskAgent).mockImplementation((agents) => Promise.resolve(agents.length > 0 ? agents[0] : undefined), ); From 764af7ed8d3aa77769b0369feb9db0bf84f503c9 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 15 Apr 2026 19:19:49 +0300 Subject: [PATCH 2/3] fix: strip ANSI escape sequences from build log output OutputChannels render ANSI codes as raw text. Use strip-ansi (already a project dependency) to clean the output. --- src/remote/terminalOutputChannel.ts | 5 +++-- test/unit/remote/terminalOutputChannel.test.ts | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/remote/terminalOutputChannel.ts b/src/remote/terminalOutputChannel.ts index 17c39762..40c6c774 100644 --- a/src/remote/terminalOutputChannel.ts +++ b/src/remote/terminalOutputChannel.ts @@ -1,6 +1,7 @@ +import stripAnsi from "strip-ansi"; import * as vscode from "vscode"; -/** Adapts terminal-style output (\r\n) for a VS Code OutputChannel (\n). */ +/** Adapts terminal-style output for a VS Code OutputChannel. Strips ANSI escape sequences and carriage returns. */ export class TerminalOutputChannel implements vscode.Disposable { private readonly channel: vscode.OutputChannel; @@ -10,7 +11,7 @@ export class TerminalOutputChannel implements vscode.Disposable { } write(data: string): void { - this.channel.append(data.replace(/\r/g, "")); + this.channel.append(stripAnsi(data).replace(/\r/g, "")); } dispose(): void { diff --git a/test/unit/remote/terminalOutputChannel.test.ts b/test/unit/remote/terminalOutputChannel.test.ts index 3c47e3e9..06ac6afb 100644 --- a/test/unit/remote/terminalOutputChannel.test.ts +++ b/test/unit/remote/terminalOutputChannel.test.ts @@ -31,6 +31,12 @@ describe("TerminalOutputChannel", () => { expect(mockAppend).toHaveBeenCalledWith("progress50%100%\n"); }); + it("strips ANSI escape sequences", () => { + const channel = new TerminalOutputChannel("test"); + channel.write("\x1b[0;1mBold text\x1b[0m normal\r\n"); + expect(mockAppend).toHaveBeenCalledWith("Bold text normal\n"); + }); + it("passes plain text through unchanged", () => { const channel = new TerminalOutputChannel("test"); channel.write("no special chars"); From 7db73e980bc89d6e09f4fb1d268a51564e88a0f7 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 16 Apr 2026 00:18:19 +0300 Subject: [PATCH 3/3] refactor: add MockOutputChannel and improve test setup Add a fully-implemented MockOutputChannel to testHelpers, add LogLevel to the vscode mock runtime, and use a setup function in the TerminalOutputChannel tests for black-box input/output assertions. --- test/mocks/testHelpers.ts | 36 +++++++++++ test/mocks/vscode.runtime.ts | 8 +++ .../unit/remote/terminalOutputChannel.test.ts | 59 +++++++------------ 3 files changed, 66 insertions(+), 37 deletions(-) diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index a94b7257..fe119a7d 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -946,6 +946,42 @@ export class MockContextManager { readonly dispose = vi.fn(); } +/** Mock VS Code OutputChannel that captures all appended content. */ +export class MockOutputChannel implements vscode.LogOutputChannel { + readonly name: string; + readonly logLevel = vscode.LogLevel.Info; + readonly onDidChangeLogLevel: vscode.Event = vi.fn(); + + private _content: string[] = []; + + constructor(name = "mock") { + this.name = name; + } + + get content(): string[] { + return this._content; + } + + append = vi.fn((value: string) => this._content.push(value)); + appendLine = vi.fn((value: string) => this._content.push(value + "\n")); + replace = vi.fn((value: string) => { + this._content = [value]; + }); + clear = vi.fn(() => { + this._content = []; + }); + dispose = vi.fn(() => { + this._content = []; + }); + show = vi.fn(); + hide = vi.fn(); + trace = vi.fn(); + debug = vi.fn(); + info = vi.fn(); + warn = vi.fn(); + error = vi.fn(); +} + /** * Mock TerminalOutputChannel that captures all written content. * Use `lastInstance` to get the most recently created instance (set in the constructor), diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index f8e3b490..ffdf5daf 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -33,6 +33,14 @@ export const ColorThemeKind = E({ HighContrastLight: 4, }); export const ExtensionMode = E({ Production: 1, Development: 2, Test: 3 }); +export const LogLevel = E({ + Off: 0, + Trace: 1, + Debug: 2, + Info: 3, + Warning: 4, + Error: 5, +}); export const UIKind = E({ Desktop: 1, Web: 2 }); export const InputBoxValidationSeverity = E({ Info: 1, diff --git a/test/unit/remote/terminalOutputChannel.test.ts b/test/unit/remote/terminalOutputChannel.test.ts index 06ac6afb..e8aaf854 100644 --- a/test/unit/remote/terminalOutputChannel.test.ts +++ b/test/unit/remote/terminalOutputChannel.test.ts @@ -1,45 +1,30 @@ -import { describe, expect, it, vi, beforeEach } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; import { TerminalOutputChannel } from "@/remote/terminalOutputChannel"; -const mockAppend = vi.fn(); +import { MockOutputChannel } from "../../mocks/testHelpers"; -vi.mock("vscode", () => ({ - window: { - createOutputChannel: vi.fn(() => ({ - append: mockAppend, - show: vi.fn(), - dispose: vi.fn(), - })), - }, -})); +vi.mocked(vscode.window.createOutputChannel).mockImplementation( + (name: string) => new MockOutputChannel(name), +); -describe("TerminalOutputChannel", () => { - beforeEach(() => { - mockAppend.mockClear(); - }); +function setup(input: string): MockOutputChannel { + const channel = new TerminalOutputChannel("test"); + channel.write(input); + return vi.mocked(vscode.window.createOutputChannel).mock.results.at(-1)! + .value as MockOutputChannel; +} - it("strips \\r from \\r\\n line endings", () => { - const channel = new TerminalOutputChannel("test"); - channel.write("hello\r\nworld\r\n"); - expect(mockAppend).toHaveBeenCalledWith("hello\nworld\n"); - }); - - it("strips bare \\r characters", () => { - const channel = new TerminalOutputChannel("test"); - channel.write("progress\r50%\r100%\n"); - expect(mockAppend).toHaveBeenCalledWith("progress50%100%\n"); - }); - - it("strips ANSI escape sequences", () => { - const channel = new TerminalOutputChannel("test"); - channel.write("\x1b[0;1mBold text\x1b[0m normal\r\n"); - expect(mockAppend).toHaveBeenCalledWith("Bold text normal\n"); - }); - - it("passes plain text through unchanged", () => { - const channel = new TerminalOutputChannel("test"); - channel.write("no special chars"); - expect(mockAppend).toHaveBeenCalledWith("no special chars"); +describe("TerminalOutputChannel", () => { + it.each([ + ["converts \\r\\n to \\n", "hello\r\nworld\r\n", "hello\nworld\n"], + ["strips bare \\r", "progress\r50%\r100%\n", "progress50%100%\n"], + ["strips ANSI escape sequences", "\x1b[0;1mBold\x1b[0m text", "Bold text"], + ["strips ANSI color codes", "\x1b[32m✔ Success\x1b[0m\r\n", "✔ Success\n"], + ["passes plain text unchanged", "hello world", "hello world"], + ["handles empty string", "", ""], + ])("%s", (_label, input, expected) => { + expect(setup(input).content.join("")).toBe(expected); }); });