mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
Ran an adversarial review before opening the PR. It found the truncation notice added alongside the buffer cap didn't actually work: prefixing execOutputTruncatedMessage into drainString()'s text meant the notice always sat ~2MiB before the end of the combined output, but the tool's own downstream truncateExecOutput() keeps at most a 400KB tail (even at the maximum allowed max_output_tokens) — so the notice was reliably swallowed, or chopped into an unreadable fragment at smaller budgets. Verified with direct reproductions against the real functions. Fixed by moving the signal out of the text stream entirely: - execOutputBuffer gains consumeTruncated() (read+reset, called once per collect()) and peekTruncated() (read-only, for status views) instead of injecting a marker into the drained bytes. - collect() now returns (string, bool); both call sites thread the bool through to execToolResult, which sets meta["output_buffer_truncated"] and appends the notice to the response body *after* truncateExecOutput runs, so it can never be re-truncated or land in a discarded region. - Also flagged: a session torn down without ever being polled again after truncation loses the signal entirely, since nothing drains/logs it on teardown. Full audit-trail logging felt like overkill for a soft transparency signal on top of a hard memory-safety fix, but exposed peekTruncated() on ExecSessionSnapshot and surfaced it in the /ps row (background_terminals.go) so it's visible any time a still-running session is checked — covers the common case (the review's own scenario: a long-lived dev server nobody's polling but that's still alive) even though a session reaped before ever being looked at is still a gap. New tests: reworked TestExecOutputBufferCapsUndrainedData for the consume/peek API, and TestExecToolResultSurfacesBufferTruncationOutsideByteBudget reproducing the review's exact failure case (max_output_tokens=1) and proving the notice now survives it.