diff --git a/README.md b/README.md index 1968c1bd..0d1e837b 100644 --- a/README.md +++ b/README.md @@ -294,6 +294,7 @@ This eliminates the need for polling—perfect for long-running processes like b | `PTY_MAX_BUFFER_LINES` | `50000` | Maximum lines to keep in output buffer per session | | `PTY_WEB_HOSTNAME` | `::1` | Hostname for the web server to bind to (IPv6 loopback by default) | | `PTY_WEB_PORT` | `0` (random) | Port for the web server (0 = random port) | +| `PTY_SANITIZE_OUTPUT` | enabled | Strip terminal control sequences from `pty_read` output and `` notifications; set to `0` or `false` to get the raw output | ### Permissions diff --git a/src/plugin/pty/buffer.ts b/src/plugin/pty/buffer.ts index 57308f04..fc1d353b 100644 --- a/src/plugin/pty/buffer.ts +++ b/src/plugin/pty/buffer.ts @@ -42,12 +42,18 @@ export class RingBuffer { return this.buffer } - search(pattern: RegExp): SearchMatch[] { + /** + * Returns the lines matching `pattern`. When `normalize` is given, each line + * is normalized first and the pattern is matched against (and returns) the + * normalized text. + */ + search(pattern: RegExp, normalize?: (line: string) => string): SearchMatch[] { const matches: SearchMatch[] = [] const lines: string[] = this.splitBufferLines() for (let i = 0; i < lines.length; i++) { - const line = lines[i] + const rawLine = lines[i] + const line = rawLine !== undefined && normalize ? normalize(rawLine) : rawLine if (line && pattern.test(line)) { matches.push({ lineNumber: i + 1, text: line }) } diff --git a/src/plugin/pty/manager.ts b/src/plugin/pty/manager.ts index e0550cf7..4268e6d7 100644 --- a/src/plugin/pty/manager.ts +++ b/src/plugin/pty/manager.ts @@ -127,11 +127,17 @@ class PTYManager { ) } - search(id: string, pattern: RegExp, offset: number = 0, limit?: number): SearchResult | null { + search( + id: string, + pattern: RegExp, + offset: number = 0, + limit?: number, + normalize?: (line: string) => string + ): SearchResult | null { return withSession( this.lifecycleManager, id, - (session) => this.outputManager.search(session, pattern, offset, limit), + (session) => this.outputManager.search(session, pattern, offset, limit, normalize), null ) } diff --git a/src/plugin/pty/notification-manager.ts b/src/plugin/pty/notification-manager.ts index 7e05609d..88f2ca15 100644 --- a/src/plugin/pty/notification-manager.ts +++ b/src/plugin/pty/notification-manager.ts @@ -2,6 +2,7 @@ import type { SessionNotifier } from '../../adapters/types.ts' import type { PTYSession } from './types.ts' import type { OpencodeClient } from '@opencode-ai/sdk' import { NOTIFICATION_LINE_TRUNCATE, NOTIFICATION_TITLE_TRUNCATE } from '../constants.ts' +import { isOutputSanitizationEnabled, sanitizeTerminalText } from './sanitize.ts' export class NotificationManager implements SessionNotifier { private client: OpencodeClient | null = null @@ -63,19 +64,22 @@ export class NotificationManager implements SessionNotifier { * context's `ctx.session.prompt`). */ export function buildExitNotification(session: PTYSession, exitCode: number): string { - const lineCount = session.buffer.length + const bufferLines = session.buffer.read() + const lineCount = bufferLines.length + const sanitize = isOutputSanitizationEnabled() let lastLine = '' - if (lineCount > 0) { - for (let i = lineCount - 1; i >= 0; i--) { - const bufferLines = session.buffer.read(i, 1) - const line = bufferLines[0] - if (line !== undefined && line.trim() !== '') { - lastLine = - line.length > NOTIFICATION_LINE_TRUNCATE - ? `${line.slice(0, NOTIFICATION_LINE_TRUNCATE)}...` - : line - break - } + for (let i = lineCount - 1; i >= 0; i--) { + const rawLine = bufferLines[i] + if (rawLine === undefined) continue + // Lines that hold only control sequences (e.g. the ConPTY preamble) are + // blank once sanitized and must not be reported as the last line. + const line = sanitize ? sanitizeTerminalText(rawLine) : rawLine + if (line.trim() !== '') { + lastLine = + line.length > NOTIFICATION_LINE_TRUNCATE + ? `${line.slice(0, NOTIFICATION_LINE_TRUNCATE)}...` + : line + break } } diff --git a/src/plugin/pty/output-manager.ts b/src/plugin/pty/output-manager.ts index 5db41630..e3803e96 100644 --- a/src/plugin/pty/output-manager.ts +++ b/src/plugin/pty/output-manager.ts @@ -17,8 +17,14 @@ export class OutputManager { return { lines, totalLines, offset, hasMore } } - search(session: PTYSession, pattern: RegExp, offset: number = 0, limit?: number): SearchResult { - const allMatches = session.buffer.search(pattern) + search( + session: PTYSession, + pattern: RegExp, + offset: number = 0, + limit?: number, + normalize?: (line: string) => string + ): SearchResult { + const allMatches = session.buffer.search(pattern, normalize) const totalMatches = allMatches.length const totalLines = session.buffer.length const paginatedMatches = diff --git a/src/plugin/pty/sanitize.ts b/src/plugin/pty/sanitize.ts new file mode 100644 index 00000000..29ea5baa --- /dev/null +++ b/src/plugin/pty/sanitize.ts @@ -0,0 +1,50 @@ +import { stripVTControlCharacters } from 'node:util' + +// String-type sequences: OSC (ESC ]), DCS (ESC P), SOS (ESC X), PM (ESC ^) and +// APC (ESC _), plus their C1 forms, up to BEL or ST (or the end of the line). +// `stripVTControlCharacters` only knows a restricted OSC payload alphabet and +// leaves e.g. Windows window titles (`ESC]0;C:\Program Files\...BEL`) behind. +const STRING_SEQUENCES = + // biome-ignore lint/suspicious/noControlCharactersInRegex: matching terminal control sequences is the point + /(?:\u001B[\]PX^_]|[\u0090\u0098\u009D-\u009F])[\s\S]*?(?:\u0007|\u001B\\|\u009C|$)/g + +// C0 controls except \t and \n, DEL, and C1 controls. Whatever survives the +// passes above (a lone ESC, \r, BEL, BS, ...) is still written to the terminal +// verbatim by hosts that render this text. +// biome-ignore lint/suspicious/noControlCharactersInRegex: matching control characters is the point +const CONTROL_CHARACTERS = /[\u0000-\u0008\u000B-\u001F\u007F-\u009F]/g + +// Non-global twin of CONTROL_CHARACTERS for the fast path; every sequence the +// passes below remove starts with one of these characters. +// biome-ignore lint/suspicious/noControlCharactersInRegex: matching control characters is the point +const HAS_CONTROL_CHARACTER = /[\u0000-\u0008\u000B-\u001F\u007F-\u009F]/ + +/** + * Whether text handed to the host (`pty_read` results and `` + * notifications) is stripped of terminal control sequences. + * + * Enabled by default; set `PTY_SANITIZE_OUTPUT=0` (or `false`) to receive the + * raw terminal output instead. + */ +export function isOutputSanitizationEnabled(): boolean { + const value = process.env.PTY_SANITIZE_OUTPUT?.trim().toLowerCase() + return value !== '0' && value !== 'false' +} + +/** + * Removes terminal control sequences (CSI, OSC, ...) and remaining control + * characters from a single line of PTY output, keeping tabs, and drops + * trailing whitespace (ConPTY pads erased lines with spaces up to the terminal + * width). + * + * Only applied where output leaves the plugin as text; the raw buffer is kept + * intact for the Web UI terminal. + */ +export function sanitizeTerminalText(text: string): string { + if (!HAS_CONTROL_CHARACTER.test(text)) { + return text.trimEnd() + } + return stripVTControlCharacters(text.replace(STRING_SEQUENCES, '')) + .replace(CONTROL_CHARACTERS, '') + .trimEnd() +} diff --git a/src/plugin/pty/tools/read.ts b/src/plugin/pty/tools/read.ts index cbc2309c..dbaf02d1 100644 --- a/src/plugin/pty/tools/read.ts +++ b/src/plugin/pty/tools/read.ts @@ -3,6 +3,7 @@ import { manager } from '../manager.ts' import { DEFAULT_READ_LIMIT, MAX_LINE_LENGTH } from '../../../shared/constants.ts' import { buildSessionNotFoundError } from '../utils.ts' import { formatLine } from '../formatters.ts' +import { isOutputSanitizationEnabled, sanitizeTerminalText } from '../sanitize.ts' import type { PTYSessionInfo } from '../types.ts' import DESCRIPTION from './read.txt' @@ -106,7 +107,9 @@ function handlePatternRead( ): string { const regex = validateAndCreateRegex(pattern, ignoreCase) - const result = manager.search(id, regex, offset, limit) + // Match against the text the agent will see, not the raw escape sequences. + const normalize = isOutputSanitizationEnabled() ? sanitizeTerminalText : undefined + const result = manager.search(id, regex, offset, limit, normalize) if (!result) { throw buildSessionNotFoundError(id) } @@ -170,8 +173,13 @@ function handlePlainRead( ) } + const sanitize = isOutputSanitizationEnabled() const formattedLines = result.lines.map((line, index) => - formatLine(line, result.offset + index + 1, MAX_LINE_LENGTH) + formatLine( + sanitize ? sanitizeTerminalText(line) : line, + result.offset + index + 1, + MAX_LINE_LENGTH + ) ) const paginationMessage = `(Buffer has more lines. Use offset=${result.offset + result.lines.length} to read beyond line ${result.offset + result.lines.length})` diff --git a/test/notification-manager.test.ts b/test/notification-manager.test.ts index b0cbb56f..089f3d94 100644 --- a/test/notification-manager.test.ts +++ b/test/notification-manager.test.ts @@ -1,7 +1,10 @@ -import { describe, expect, it, mock } from 'bun:test' +import { afterEach, describe, expect, it, mock } from 'bun:test' import type { OpencodeClient } from '@opencode-ai/sdk' import { RingBuffer } from '../src/plugin/pty/buffer.ts' -import { NotificationManager } from '../src/plugin/pty/notification-manager.ts' +import { + NotificationManager, + buildExitNotification, +} from '../src/plugin/pty/notification-manager.ts' import type { PTYSession } from '../src/plugin/pty/types.ts' type PromptPayload = { @@ -159,3 +162,63 @@ describe('NotificationManager', () => { expect(text).toContain('Process reached its PTY timeout and was stopped automatically.') }) }) + +describe('buildExitNotification', () => { + const original = process.env.PTY_SANITIZE_OUTPUT + + afterEach(() => { + if (original === undefined) { + delete process.env.PTY_SANITIZE_OUTPUT + } else { + process.env.PTY_SANITIZE_OUTPUT = original + } + }) + + function sessionWithOutput(output: string): PTYSession { + const buffer = new RingBuffer() + buffer.append(output) + return createSession({ buffer }) + } + + it('reports the last line without terminal control sequences', () => { + delete process.env.PTY_SANITIZE_OUTPUT + + const text = buildExitNotification( + sessionWithOutput('\x1b[?25l\x1b[2J\x1b[H\r\n\x1b[32mBuild completed.\x1b[0m\x1b[K\r\n'), + 0 + ) + + expect(text).toContain('Last Line: Build completed.\n') + expect(text).not.toContain('\x1b') + expect(text).not.toContain('\r') + }) + + it('skips trailing lines that only contain control sequences', () => { + delete process.env.PTY_SANITIZE_OUTPUT + + const text = buildExitNotification( + sessionWithOutput( + 'real output\r\n\x1b[?9001h\x1b[?1004h\x1b]0;C:\\Windows\\pwsh.exe\x07\x1b[K' + ), + 0 + ) + + expect(text).toContain('Last Line: real output\n') + }) + + it('reports an empty last line when the output is only control sequences', () => { + delete process.env.PTY_SANITIZE_OUTPUT + + const text = buildExitNotification(sessionWithOutput('\x1b[?9001h\x1b[?1004h\x1b[2J\x1b[H'), 0) + + expect(text).toContain('Output Lines: 1\nLast Line: \n') + }) + + it('keeps the raw last line when PTY_SANITIZE_OUTPUT=0', () => { + process.env.PTY_SANITIZE_OUTPUT = '0' + + const text = buildExitNotification(sessionWithOutput('\x1b[32mdone\x1b[0m\n'), 0) + + expect(text).toContain('Last Line: \x1b[32mdone\x1b[0m\n') + }) +}) diff --git a/test/pty-output-sanitize.test.ts b/test/pty-output-sanitize.test.ts new file mode 100644 index 00000000..3b2bd90a --- /dev/null +++ b/test/pty-output-sanitize.test.ts @@ -0,0 +1,115 @@ +import { afterAll, afterEach, describe, expect, it } from 'bun:test' +import { manager } from '../src/plugin/pty/manager.ts' +import { buildExitNotification } from '../src/plugin/pty/notification-manager.ts' +import { ptyRead } from '../src/plugin/pty/tools/read.ts' + +// End-to-end: a real PTY process writes colours, a window title, line erases +// and carriage returns. On Windows, ConPTY additionally prefixes its own mode, +// clear-screen and title sequences, so this also exercises the preamble. +const CHILD_OUTPUT = + '\x1b]0;child title\x07' + + '\x1b[31merror:\x1b[0m boom\r\n' + + '\x1b[2K\x1b[1Gprogress 100%\r\n' + + '\x1b[32mdone\x1b[0m\x1b[K\r\n' + +// Any C0/C1 control character other than tab and newline. +// biome-ignore lint/suspicious/noControlCharactersInRegex: asserting their absence +const CONTROL_CHARACTER = /[\u0000-\u0008\u000B-\u001F\u007F-\u009F]/ + +const ctx = { + sessionID: 'parent', + messageID: 'msg', + agent: 'agent', + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, + directory: process.cwd(), + worktree: process.cwd(), +} + +async function readTool(args: { id: string; pattern?: string }): Promise { + const result = await ptyRead.execute(args, ctx) + return typeof result === 'string' ? result : result.output +} + +/** Spawns the child through the manager and resolves with the notification text on exit. */ +async function runChild(): Promise<{ id: string; notification: string }> { + const exited = new Promise((resolve) => { + manager.setNotifier({ + sendExitNotification: (session, exitCode) => { + resolve(buildExitNotification(session, exitCode)) + }, + }) + }) + const info = manager.spawn({ + // The current Bun binary exists on every platform the tests run on, unlike `echo`. + command: process.execPath, + args: ['-e', 'process.stdout.write(process.env.PTY_TEST_OUTPUT)'], + env: { PTY_TEST_OUTPUT: CHILD_OUTPUT }, + description: 'sanitize e2e', + parentSessionId: 'parent', + notifyOnExit: true, + }) + const notification = await Promise.race([ + exited, + new Promise((_, reject) => + setTimeout(() => reject(new Error('PTY child did not exit')), 10000) + ), + ]) + return { id: info.id, notification } +} + +describe('PTY output sanitization (real PTY)', () => { + const original = process.env.PTY_SANITIZE_OUTPUT + + afterEach(() => { + if (original === undefined) { + delete process.env.PTY_SANITIZE_OUTPUT + } else { + process.env.PTY_SANITIZE_OUTPUT = original + } + }) + + afterAll(() => { + manager.setNotifier(null) + manager.clearAllSessions() + }) + + it('strips control sequences from pty_read, search and ', async () => { + delete process.env.PTY_SANITIZE_OUTPUT + const { id, notification } = await runChild() + + try { + // The raw buffer (used by the Web UI) keeps the terminal output. + expect(manager.getRawBuffer(id)?.raw).toContain('\x1b[') + + const read = await readTool({ id }) + expect(read).not.toMatch(CONTROL_CHARACTER) + expect(read).toContain('| error: boom\n') + expect(read).toContain('| progress 100%\n') + expect(read).toContain('| done\n') + + const search = await readTool({ id, pattern: '^error: boom$' }) + expect(search).not.toMatch(CONTROL_CHARACTER) + expect(search).toContain('| error: boom\n') + expect(search).toContain('(1 match from') + + expect(notification).not.toMatch(CONTROL_CHARACTER) + expect(notification).toContain('Exit Code: 0\n') + expect(notification).toContain('Last Line: done\n') + } finally { + manager.kill(id, true) + } + }) + + it('returns raw output when PTY_SANITIZE_OUTPUT=0', async () => { + process.env.PTY_SANITIZE_OUTPUT = '0' + const { id } = await runChild() + + try { + expect(await readTool({ id })).toContain('\x1b[') + } finally { + manager.kill(id, true) + } + }) +}) diff --git a/test/pty-tools.test.ts b/test/pty-tools.test.ts index e15083ac..01c062ba 100644 --- a/test/pty-tools.test.ts +++ b/test/pty-tools.test.ts @@ -1,9 +1,10 @@ -import { describe, it, expect, beforeEach, mock, spyOn, afterAll } from 'bun:test' +import { describe, it, expect, beforeEach, afterEach, mock, spyOn, afterAll } from 'bun:test' import { ptySpawn } from '../src/plugin/pty/tools/spawn.ts' import { ptyRead } from '../src/plugin/pty/tools/read.ts' import { ptyList } from '../src/plugin/pty/tools/list.ts' import { RingBuffer } from '../src/plugin/pty/buffer.ts' import { manager } from '../src/plugin/pty/manager.ts' +import { sanitizeTerminalText } from '../src/plugin/pty/sanitize.ts' describe('PTY Tools', () => { afterAll(() => { @@ -239,12 +240,76 @@ describe('PTY Tools', () => { const result = await ptyRead.execute(args, ctx) - expect(manager.search).toHaveBeenCalledWith('test-session-id', /line/, 0, 500) + expect(manager.search).toHaveBeenCalledWith( + 'test-session-id', + /line/, + 0, + 500, + sanitizeTerminalText + ) expect(result).toContain('') expect(result).toContain('00001| line 1') expect(result).toContain('(1 match from 2 total lines)') }) + describe('terminal control sequences', () => { + const original = process.env.PTY_SANITIZE_OUTPUT + const ctx = { + sessionID: 'parent', + messageID: 'msg', + agent: 'agent', + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, + directory: '/tmp', + worktree: '/tmp', + } + + beforeEach(() => { + spyOn(manager, 'read').mockReturnValue({ + lines: ['\x1b[?9001h\x1b[?1004h\x1b[2J\x1b[H', '\x1b[31merror:\x1b[0m boom\r'], + offset: 0, + hasMore: false, + totalLines: 2, + }) + }) + + afterEach(() => { + if (original === undefined) { + delete process.env.PTY_SANITIZE_OUTPUT + } else { + process.env.PTY_SANITIZE_OUTPUT = original + } + }) + + it('strips them from read output', async () => { + delete process.env.PTY_SANITIZE_OUTPUT + + const result = await ptyRead.execute({ id: 'test-session-id' }, ctx) + + expect(result).not.toContain('\x1b') + expect(result).not.toContain('\r') + expect(result).toContain('00001| \n') + expect(result).toContain('00002| error: boom\n') + }) + + it('keeps them when PTY_SANITIZE_OUTPUT=0', async () => { + process.env.PTY_SANITIZE_OUTPUT = '0' + + const result = await ptyRead.execute({ id: 'test-session-id' }, ctx) + + expect(result).toContain('00002| \x1b[31merror:\x1b[0m boom\r') + }) + + it('does not normalize search lines when PTY_SANITIZE_OUTPUT=0', async () => { + process.env.PTY_SANITIZE_OUTPUT = '0' + + await ptyRead.execute({ id: 'test-session-id', pattern: 'boom' }, ctx) + + expect(manager.search).toHaveBeenCalledWith('test-session-id', /boom/, 0, 500, undefined) + }) + }) + it('should throw for invalid session', async () => { spyOn(manager, 'get').mockReturnValue(null) @@ -378,6 +443,18 @@ describe('PTY Tools', () => { ]) }) + it('should match and return normalized lines when searching with a normalizer', () => { + const buffer = new RingBuffer(200) + buffer.append('\x1b[?25l\x1b[2J\n\x1b[31merror:\x1b[0m boom\nok 1\n') + + expect(buffer.search(/^error: boom$/, sanitizeTerminalText)).toEqual([ + { lineNumber: 2, text: 'error: boom' }, + ]) + // Digits inside escape sequences must not produce matches. + expect(buffer.search(/\d/, sanitizeTerminalText)).toEqual([{ lineNumber: 3, text: 'ok 1' }]) + expect(buffer.readRaw()).toContain('\x1b[31m') + }) + it('should clear buffer', () => { const buffer = new RingBuffer(100) buffer.append('line1\nline2') diff --git a/test/sanitize.test.ts b/test/sanitize.test.ts new file mode 100644 index 00000000..667d0a68 --- /dev/null +++ b/test/sanitize.test.ts @@ -0,0 +1,80 @@ +import { afterEach, describe, expect, it } from 'bun:test' +import { isOutputSanitizationEnabled, sanitizeTerminalText } from '../src/plugin/pty/sanitize.ts' + +// What ConPTY writes before the first byte of real output on Windows. +const CONPTY_PREAMBLE = + '\x1b[?9001h\x1b[?1004h\x1b[?25l\x1b[2J\x1b[m\x1b[H' + + '\x1b]0;C:\\Program Files\\PowerShell\\7\\pwsh.EXE\x07\x1b[?25h' + +describe('sanitizeTerminalText', () => { + it('removes SGR colours and erase sequences', () => { + expect(sanitizeTerminalText('\x1b[31merror:\x1b[0m boom\x1b[K')).toBe('error: boom') + }) + + it('removes terminal mode, cursor and screen sequences', () => { + expect(sanitizeTerminalText(CONPTY_PREAMBLE)).toBe('') + expect(sanitizeTerminalText('\x1b[12;1Hready\x1b[?7777h')).toBe('ready') + }) + + it('removes OSC sequences terminated by BEL or ST', () => { + expect(sanitizeTerminalText('\x1b]0;title\x07a')).toBe('a') + expect(sanitizeTerminalText('\x1b]2;~/my dir\x1b\\a')).toBe('a') + expect(sanitizeTerminalText('\x1b]8;;https://example.com\x1b\\link\x1b]8;;\x1b\\')).toBe('link') + }) + + it('removes DCS/APC strings and unterminated string sequences', () => { + expect(sanitizeTerminalText('a\x1bPq#0;1;2\x1b\\b')).toBe('ab') + expect(sanitizeTerminalText('a\x1b_payload\x1b\\b')).toBe('ab') + expect(sanitizeTerminalText('a\x1b]0;cut off')).toBe('a') + }) + + it('removes remaining control characters but keeps tabs', () => { + expect(sanitizeTerminalText('a\tb\r')).toBe('a\tb') + expect(sanitizeTerminalText('bell\x07 back\x08 nul\x00 del\x7f c1\x9b')).toBe( + 'bell back nul del c1' + ) + expect(sanitizeTerminalText('trailing escape\x1b')).toBe('trailing escape') + }) + + it('leaves plain and non-ASCII text untouched', () => { + expect(sanitizeTerminalText('héllo wörld ✓ 你好')).toBe('héllo wörld ✓ 你好') + expect(sanitizeTerminalText(' indented\tcode')).toBe(' indented\tcode') + }) + + it('drops trailing padding, with or without control sequences', () => { + // ConPTY renders an erased line as text padded with spaces to the width. + expect(sanitizeTerminalText(`done${' '.repeat(116)}`)).toBe('done') + expect(sanitizeTerminalText(`\x1b[32mdone${' '.repeat(116)}\x1b[0m\r`)).toBe('done') + }) +}) + +describe('isOutputSanitizationEnabled', () => { + const original = process.env.PTY_SANITIZE_OUTPUT + + afterEach(() => { + if (original === undefined) { + delete process.env.PTY_SANITIZE_OUTPUT + } else { + process.env.PTY_SANITIZE_OUTPUT = original + } + }) + + it('is enabled by default', () => { + delete process.env.PTY_SANITIZE_OUTPUT + expect(isOutputSanitizationEnabled()).toBe(true) + }) + + it('is disabled by 0 or false', () => { + for (const value of ['0', 'false', 'FALSE', ' false ']) { + process.env.PTY_SANITIZE_OUTPUT = value + expect(isOutputSanitizationEnabled()).toBe(false) + } + }) + + it('stays enabled for other values', () => { + for (const value of ['1', 'true', '']) { + process.env.PTY_SANITIZE_OUTPUT = value + expect(isOutputSanitizationEnabled()).toBe(true) + } + }) +})