From 54757c072ed1f9e04fcf6fd8543f3e4cf634c706 Mon Sep 17 00:00:00 2001 From: dhaern Date: Mon, 5 Oct 2026 14:18:19 +0000 Subject: [PATCH] fix(pty): retain output by line and enforce buffer limits Store retained output as lines so counts are constant-time and reads and searches avoid splitting the full buffer. Trim discarded lines in one batch and find the exit-notification tail with a single read. Normalize negative read and search offsets, including the tool's next-page hint. Intentional contract changes: honor PTY_MAX_BUFFER_LINES alongside PTY_MAX_BUFFER_SIZE, retaining whole lines until either limit is reached; only a single oversized line is sliced to its tail. Report byteLength as UTF-8 bytes instead of UTF-16 code units. Remove the exported no-op RingBuffer.flush(). Invalid or non-positive limits fall back to the existing defaults. An oversized final line that ends with a newline stays visible to pty_read and exit notifications. Reads use native slice pagination, and the line-density explanation lives in the README rather than the tool prompt. --- README.md | 3 ++ src/plugin/pty/buffer.ts | 67 +++++++++++++------------- src/plugin/pty/notification-manager.ts | 19 ++------ src/plugin/pty/output-manager.ts | 2 + src/plugin/pty/session-lifecycle.ts | 9 +--- src/plugin/pty/tools/read.ts | 2 +- src/plugin/pty/tools/read.txt | 2 +- test/output-manager.test.ts | 16 ++++++ test/pty-tools.test.ts | 33 ++++++++++--- 9 files changed, 90 insertions(+), 63 deletions(-) create mode 100644 test/output-manager.test.ts diff --git a/README.md b/README.md index 1968c1bd..aa0929f2 100644 --- a/README.md +++ b/README.md @@ -292,9 +292,12 @@ This eliminates the need for polling—perfect for long-running processes like b | Variable | Default | Description | | ---------------------- | ---------- | -------------------------------------------------- | | `PTY_MAX_BUFFER_LINES` | `50000` | Maximum lines to keep in output buffer per session | +| `PTY_MAX_BUFFER_SIZE` | `1000000` | Maximum characters 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) | +The buffer discards older whole lines when either limit is exceeded, whichever comes first. A single oversized line keeps only its tail. With the default character limit, 50,000 lines fit only when they average at most 20 characters including newlines. Invalid or non-positive limits use the defaults. + ### Permissions This plugin respects OpenCode's [permission settings](https://opencode.ai/docs/permissions/) for the `bash` tool. Commands spawned via `pty_spawn` are checked against your `permission.bash` configuration. diff --git a/src/plugin/pty/buffer.ts b/src/plugin/pty/buffer.ts index 57308f04..3c7da294 100644 --- a/src/plugin/pty/buffer.ts +++ b/src/plugin/pty/buffer.ts @@ -1,5 +1,6 @@ -// Default buffer size in characters (approximately 1MB) -const DEFAULT_MAX_BUFFER_SIZE = parseInt(process.env.PTY_MAX_BUFFER_SIZE || '1000000', 10) +// A missing, invalid or non-positive limit falls back to the default. +const envLimit = (name: string, fallback: number) => + Math.max(0, Number(process.env[name])) || fallback export interface SearchMatch { lineNumber: number @@ -7,47 +8,52 @@ export interface SearchMatch { } export class RingBuffer { - private buffer: string = '' - private maxSize: number + private lines: string[] = [''] + private size = 0 - constructor(maxSize: number = DEFAULT_MAX_BUFFER_SIZE) { - this.maxSize = maxSize - } + constructor( + private readonly maxSize = envLimit('PTY_MAX_BUFFER_SIZE', 1_000_000), + private readonly maxLines = envLimit('PTY_MAX_BUFFER_LINES', 50_000) + ) {} append(data: string): void { - this.buffer += data - if (this.buffer.length > this.maxSize) { - this.buffer = this.buffer.slice(-this.maxSize) + const lines = data.split('\n') + this.lines[this.lines.length - 1] += lines[0] ?? '' + for (let i = 1; i < lines.length; i++) { + this.lines.push(lines[i] ?? '') } - } + this.size += data.length - private splitBufferLines(): string[] { - const lines: string[] = this.buffer.split('\n') - // Remove empty string at end if buffer doesn't end with newline - if (lines.length && lines[lines.length - 1] === '') { - lines.pop() + let start = 0 + while ( + this.length - start > 1 && + (this.length - start > this.maxLines || this.size > this.maxSize) + ) { + this.size -= (this.lines[start]?.length ?? 0) + 1 + start++ + } + if (start > 0) this.lines.splice(0, start) + if (this.size > this.maxSize) { + this.lines[0] = this.lines[0]?.slice(this.size - this.maxSize) ?? '' + this.size = this.maxSize } - return lines } read(offset: number = 0, limit?: number): string[] { - if (this.buffer === '') return [] - const lines: string[] = this.splitBufferLines() + const lines = this.lines.slice(0, this.length) const start = Math.max(0, offset) const end = limit !== undefined ? start + limit : lines.length return lines.slice(start, end) } readRaw(): string { - return this.buffer + return this.lines.join('\n') } search(pattern: RegExp): SearchMatch[] { const matches: SearchMatch[] = [] - const lines: string[] = this.splitBufferLines() - - for (let i = 0; i < lines.length; i++) { - const line = lines[i] + for (let i = 0; i < this.length; i++) { + const line = this.lines[i] if (line && pattern.test(line)) { matches.push({ lineNumber: i + 1, text: line }) } @@ -56,20 +62,15 @@ export class RingBuffer { } get length(): number { - if (this.buffer === '') return 0 - const lines = this.splitBufferLines() - return lines.length + return this.lines.length - (this.lines[this.lines.length - 1] === '' ? 1 : 0) } get byteLength(): number { - return this.buffer.length - } - - flush(): void { - // No-op in new implementation + return Buffer.byteLength(this.readRaw()) } clear(): void { - this.buffer = '' + this.lines = [''] + this.size = 0 } } diff --git a/src/plugin/pty/notification-manager.ts b/src/plugin/pty/notification-manager.ts index 7e05609d..54d2c90e 100644 --- a/src/plugin/pty/notification-manager.ts +++ b/src/plugin/pty/notification-manager.ts @@ -64,20 +64,11 @@ export class NotificationManager implements SessionNotifier { */ export function buildExitNotification(session: PTYSession, exitCode: number): string { const lineCount = session.buffer.length - 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 - } - } - } + const line = session.buffer.read().findLast((line) => line.trim() !== '') ?? '' + const lastLine = + line.length > NOTIFICATION_LINE_TRUNCATE + ? `${line.slice(0, NOTIFICATION_LINE_TRUNCATE)}...` + : line const displayTitle = session.description ?? session.title const truncatedTitle = diff --git a/src/plugin/pty/output-manager.ts b/src/plugin/pty/output-manager.ts index 5db41630..5bb4a05e 100644 --- a/src/plugin/pty/output-manager.ts +++ b/src/plugin/pty/output-manager.ts @@ -11,6 +11,7 @@ export class OutputManager { } read(session: PTYSession, offset: number = 0, limit?: number): ReadResult { + offset = Math.max(0, offset) const lines = session.buffer.read(offset, limit) const totalLines = session.buffer.length const hasMore = offset + lines.length < totalLines @@ -18,6 +19,7 @@ export class OutputManager { } search(session: PTYSession, pattern: RegExp, offset: number = 0, limit?: number): SearchResult { + offset = Math.max(0, offset) const allMatches = session.buffer.search(pattern) const totalMatches = allMatches.length const totalLines = session.buffer.length diff --git a/src/plugin/pty/session-lifecycle.ts b/src/plugin/pty/session-lifecycle.ts index 6927998b..7b157c26 100644 --- a/src/plugin/pty/session-lifecycle.ts +++ b/src/plugin/pty/session-lifecycle.ts @@ -123,14 +123,7 @@ export class SessionLifecycleManager { session.process?.onExit(({ exitCode, signal }) => { this.clearSessionTimeout(session.id) - // Flush any remaining incomplete line in the buffer - session.buffer.flush() - - if (session.status === 'killing') { - session.status = 'killed' - } else { - session.status = 'exited' - } + session.status = session.status === 'killing' ? 'killed' : 'exited' session.exitCode = exitCode session.exitSignal = signal // When the process stopped, so a reader can report how long it ran: the diff --git a/src/plugin/pty/tools/read.ts b/src/plugin/pty/tools/read.ts index cbc2309c..971a8cc3 100644 --- a/src/plugin/pty/tools/read.ts +++ b/src/plugin/pty/tools/read.ts @@ -127,7 +127,7 @@ function handlePatternRead( formatLine(match.text, match.lineNumber, MAX_LINE_LENGTH) ) - const paginationMessage = `(${result.matches.length} of ${result.totalMatches} matches shown. Use offset=${offset + result.matches.length} to see more.)` + const paginationMessage = `(${result.matches.length} of ${result.totalMatches} matches shown. Use offset=${result.offset + result.matches.length} to see more.)` const endMessage = `(${result.totalMatches} match${result.totalMatches === 1 ? '' : 'es'} from ${result.totalLines} total lines)` return appendSessionReminders( diff --git a/src/plugin/pty/tools/read.txt b/src/plugin/pty/tools/read.txt index 5ea8b2b5..7ed83c6b 100644 --- a/src/plugin/pty/tools/read.txt +++ b/src/plugin/pty/tools/read.txt @@ -14,7 +14,7 @@ Returns: - Total line count in the buffer - Indicator if more lines are available -The buffer stores up to PTY_MAX_BUFFER_LINES (default: 50000) lines. Older lines are discarded when the limit is reached. +The buffer stores up to PTY_MAX_BUFFER_LINES (default: 50000) lines or PTY_MAX_BUFFER_SIZE (default: 1000000) characters, whichever limit is reached first. Older lines are discarded whole; a single oversized line keeps only its tail. Pattern Filtering: - When `pattern` is set, lines are FILTERED FIRST using the regex, then offset/limit apply to the MATCHES diff --git a/test/output-manager.test.ts b/test/output-manager.test.ts new file mode 100644 index 00000000..ec67e035 --- /dev/null +++ b/test/output-manager.test.ts @@ -0,0 +1,16 @@ +import { expect, it } from 'bun:test' +import { RingBuffer } from '../src/plugin/pty/buffer.ts' +import { OutputManager } from '../src/plugin/pty/output-manager.ts' +import type { PTYSession } from '../src/plugin/pty/types.ts' + +it('clamps negative read offsets before reporting pagination', () => { + const buffer = new RingBuffer() + buffer.append('line\n'.repeat(10)) + const result = new OutputManager().read({ buffer } as PTYSession, -40) + + expect(result.lines).toHaveLength(10) + expect({ offset: result.offset, hasMore: result.hasMore }).toEqual({ + offset: 0, + hasMore: false, + }) +}) diff --git a/test/pty-tools.test.ts b/test/pty-tools.test.ts index e15083ac..b47df8cb 100644 --- a/test/pty-tools.test.ts +++ b/test/pty-tools.test.ts @@ -389,15 +389,36 @@ describe('PTY Tools', () => { expect(buffer.readRaw()).toBe('') }) - it('should truncate buffer at byte level when exceeding max', () => { + it('should discard whole lines when exceeding the character limit', () => { const buffer = new RingBuffer(10) // Small buffer for testing buffer.append('line1\nline2\nline3\nline4') - // Input is 'line1\nline2\nline3\nline4' (23 chars) - // With buffer size 10, keeps last 10 chars: 'ine3\nline4' - expect(buffer.readRaw()).toBe('ine3\nline4') - expect(buffer.read()).toEqual(['ine3', 'line4']) - expect(buffer.length).toBe(2) + expect(buffer.readRaw()).toBe('line4') + expect(buffer.read()).toEqual(['line4']) + expect(buffer.length).toBe(1) + }) + + it('should keep the tail of the newest line when it alone exceeds the character limit', () => { + const buffer = new RingBuffer(10) + buffer.append(`${'x'.repeat(15)}\n`) + + expect(buffer.read()).toEqual(['x'.repeat(9)]) + }) + + it('should retain the last three whole lines at the line limit', () => { + const buffer = new RingBuffer(100, 3) + buffer.append('line1\nline2\nline3\nline4\nline5\n') + + expect(buffer.read()).toEqual(['line3', 'line4', 'line5']) + expect(buffer.readRaw()).toBe('line3\nline4\nline5\n') + expect(buffer.length).toBe(3) + }) + + it('should count UTF-8 bytes for non-ASCII output', () => { + const buffer = new RingBuffer() + buffer.append('á😀') + + expect(buffer.byteLength).toBe(6) }) }) })