fix: autoapprove Tee buffer is unbounded and re-parsed from offset 0 on every write #462

New issue

Open

opened 2026-07-25 23:35:27 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:35:27 +02:00

Owner

Code-quality audit (P2). internal/autoapprove/tee.go:75-87 and :110-142.

func (t *Tee) Write(p []byte) (int, error) {
	n, err := t.W.Write(p)
	t.mu.Lock()
	t.buf = append(t.buf, p[:n]...)
	t.mu.Unlock()
	t.drain()          // re-splits the ENTIRE buffer, every write
	return n, err
}

drain calls splitSSELines(data) over the whole buffer and only advances consumed on a blank-line event terminator:

case raw == "":
	...
	consumed = pos + lineLen    // the only place consumed moves

Why it matters

Two failure modes on the SSE tee that feeds the auto-approve pipeline:

Unbounded growth. An upstream that never emits a blank line — a malformed stream, or one giant event — makes buf grow without limit. Nothing caps it.
O(n^2) parsing. Every Write re-splits the full buffer from offset 0. With a large pending event, per-token writes turn into quadratic work on the OpenCode event hot path.

The two compound: the bigger the un-terminated buffer gets, the more expensive each subsequent write becomes.

Suggested fix

Track a parse offset and split only from there, instead of re-splitting from zero.
Cap the buffer at a sane maximum (e.g. a few MB). On overflow, drop the buffer and resync at the next blank line rather than growing.

Acceptance criteria

Parsing cost per Write is proportional to the new bytes, not the buffer size.
A stream with no event terminator does not grow the buffer past the cap.
Test: feed N writes with no blank line — assert bounded memory and that parsing resyncs correctly once a terminator arrives.

Effort: M.

Code-quality audit (P2). `internal/autoapprove/tee.go:75-87` and `:110-142`. ```go func (t *Tee) Write(p []byte) (int, error) { n, err := t.W.Write(p) t.mu.Lock() t.buf = append(t.buf, p[:n]...) t.mu.Unlock() t.drain() // re-splits the ENTIRE buffer, every write return n, err } ``` `drain` calls `splitSSELines(data)` over the whole buffer and only advances `consumed` on a blank-line event terminator: ```go case raw == "": ... consumed = pos + lineLen // the only place consumed moves ``` ## Why it matters Two failure modes on the SSE tee that feeds the auto-approve pipeline: 1. **Unbounded growth.** An upstream that never emits a blank line — a malformed stream, or one giant event — makes `buf` grow without limit. Nothing caps it. 2. **O(n^2) parsing.** Every `Write` re-splits the full buffer from offset 0. With a large pending event, per-token writes turn into quadratic work on the OpenCode event hot path. The two compound: the bigger the un-terminated buffer gets, the more expensive each subsequent write becomes. ## Suggested fix - Track a parse offset and split only from there, instead of re-splitting from zero. - Cap the buffer at a sane maximum (e.g. a few MB). On overflow, drop the buffer and resync at the next blank line rather than growing. ## Acceptance criteria - [ ] Parsing cost per `Write` is proportional to the new bytes, not the buffer size. - [ ] A stream with no event terminator does not grow the buffer past the cap. - [ ] Test: feed N writes with no blank line — assert bounded memory and that parsing resyncs correctly once a terminator arrives. Effort: M.