Repository navigation
PTY output silently corrupts a UTF-8 character split across two collection windows #4744
Description
Please read this first
- Have you read the docs? Yes.
- Have you searched for related issues? Yes. fix(sandbox): preserve UTF-8 boundaries in Cloudflare SSE #4707 fixed this class in the Cloudflare SSE reader. Nothing covers the shared PTY collector. fix(sandbox): retain PTY output queued at timeout #4572 edits the same function for a different concern (draining output queued at timeout).
Describe the bug
collect_pty_output decodes each collection window with errors="replace". PTY output is not collected once, it is collected in repeated windows over one persistent chunk deque: pty_exec_start takes a window, then every pty_write_stdin takes another from the same entry.output_chunks.
So a multi-byte character whose bytes land either side of a window boundary is decoded as two separate partial sequences, and both halves become U+FFFD. The bytes are destroyed at that first decode, so no later window can reassemble the character. There is no error and no warning; the text is silently wrong.
This is not backend-specific. collect_pty_output is the shared helper behind the unix_local, docker, blaxel, daytona and modal PTY paths.
Debug information
- Agents SDK version:
mainat89c02c82, and v0.22.0 - Related library versions: none
- Python version: 3.14
- Operating system: macOS arm64; no platform-specific code involved
- Model and model provider: none, this is pure session-layer code
- Does the issue reproduce with the latest Agents SDK release? Yes
- Does the issue occur consistently or intermittently? Consistently for a given split position
Repro steps
import asyncio
from collections import deque
from agents.sandbox.session.pty_output import collect_pty_output
TEXT = "héllo wörld"
RAW = TEXT.encode("utf-8")
async def main() -> None:
# One persistent deque, exactly as _UnixPtyProcessEntry.output_chunks is reused
# across pty_exec_start and each pty_write_stdin.
chunks: deque[bytes] = deque()
lock = asyncio.Lock()
notify = asyncio.Event()
done = {"value": False}
async def window() -> bytes:
notify.set()
collected, _ = await collect_pty_output(
output_chunks=chunks,
output_lock=lock,
output_notify=notify,
is_done=lambda: done["value"],
yield_time_ms=1,
max_output_tokens=None,
)
return collected
split = 2 # after "h" and the first byte of the two-byte "e" with acute
chunks.append(RAW[:split])
first = await window() # producer still running
chunks.append(RAW[split:])
done["value"] = True
second = await window()
print("window 1:", first)
print("window 2:", second)
print("joined :", (first + second).decode("utf-8"))
print("matches :", (first + second).decode("utf-8") == TEXT)
asyncio.run(main())Output on main:
window 1: b'h\xef\xbf\xbd'
window 2: b'\xef\xbf\xbdllo w\xc3\xb6rld'
joined : h\ufffd\ufffdllo wörld
matches : False
The two U+FFFD replacement characters are where a single é should be. The ö later in the string is intact, because it happens to fall inside one window.
Expected behavior
A character split across two collection windows should survive, the same way #4707 made a character split across two SSE chunks survive. The joined output should equal the original text.
I opened #4724 for this and closed it myself, because the shape I chose was wrong rather than incomplete. Recording the reproduction here so the bug is not lost with the PR.
What I learned there, in case it saves someone time. Holding the unfinished bytes back on the shared deque races with entry teardown: _watch_process_exit sets output_closed only after gathering the pump tasks, while _finalize_pty_update pops and terminates the entry as soon as process.returncode is set, so a carried byte can land in a deque that is discarded and be lost entirely, which is worse than the replacement character. Hand-rolling the incremental UTF-8 validation was also a mistake; codecs.getincrementaldecoder("utf-8")("replace") plus len(decoder.getstate()[0]) gives the pending prefix length and rejects malformed prefixes correctly. My read is that a correct fix wants decoder state owned by the PTY entry and flushed on exit, which is per-entry state across five backend wrappers.
I'm a freshman in college, so I would rather leave the design call to you than guess at whether that spread is worth it. Happy to attempt it if you want it.
Activity
fscfede-beep commentedon Aug 29, 2026
Collision/triage update: #4738 predates this issue and directly covers the same shared PTY cross-window UTF-8/settlement area across all seven backends. #4745 was opened later from this issue and its author has now explicitly said that branch should give way to #4738 rather than keep two implementations alive.
I audited both branches. #4738 is the better consolidation target architecturally, but I found two cancellation-ownership gaps on its current head (
41be368c): shared carried bytes can be drained into a cancelled collector (3885258586), and Modal can consume provider bytes before they are committed to entry-owned state (3885258589). Both have deterministic repros and narrow transaction-style fixes posted in review.So I would keep this issue open until #4738 resolves those two findings (or another selected implementation does), then close it from the surviving PR rather than from the duplicate #4745 path.
ayaangazali commentedon Aug 29, 2026
Agreed, and the collision is mine to own: #4738 predates this issue by eleven hours and I should have found it before filing. I checked the open PR list that day and counted how many were seratch's without reading their titles, which is exactly the search that would have shown "settle PTY output before cleanup".
For whatever it is worth as corroboration rather than new information, I reached the same two conclusions independently on #4724 before closing it myself:
codecs.getincrementaldecoder("utf-8")("replace")withlen(decoder.getstate()[0])is the right primitive. Three review rounds found three separate malformed-prefix cases in my hand-rolled version;_watch_process_exitsetsoutput_closedonly after gathering the pump tasks while_finalize_pty_updatepops the entry as soon asprocess.returncodeis set. fix(sandbox): settle PTY output before cleanup #4738's collector-owned settlement, gatingexit_codeonoutput_closed, removes that race at the root rather than patching around it, which my approach could not do.So I have nothing to add to the implementation, and the blocked-pump ordering gap you identified is already with @HuzaifaChaudary to raise. Happy for this issue to be closed from #4738 whenever it lands, or now if you would rather not carry a duplicate tracker. I am not going to open anything else in this area.
mikemikimike commentedon Aug 29, 2026
I am taking this issue. I will add deterministic regression coverage for UTF-8 sequences split across PTY output windows and implement the smallest stateful decoding fix that preserves bytes until the sequence is complete.
HuzaifaChaudary commentedon Aug 29, 2026
@mikemikimike heads up before you sink more time into #4754, and this is not me holding ground on it, my own pull request here is closed.
#4738 by @seratch already covers this. it was opened 28 august 09:18, eleven hours before this issue was filed, and it touches the same eight files yours does. it also already has the collector owned settled
output_closeddesign across all seven backends, which is the part that takes longest to arrive at.two of us got here before you and both closed in its favour:
nothing links #4738 to this issue, which is exactly why all three of us missed it. an issue can be completely clean, no claim, no assignee, nothing cross referenced, and the fix still be in flight.
if you carry on anyway, these are the three that cost us rounds and are easy to miss:
codecs.getincrementaldecoder("utf-8")("replace")is the right primitive, but it still buffersED A0..BF, a surrogate lead no third byte can complete. i walked every one, two and three byte prefix it buffers, those 32 are the only ones that can never become a character. everything else it holds is genuinely waitingprocess.returncodewhile collection waits foroutput_closed, so a session can be torn down while a pump still holds the rest of a characterthe tested versions of all three are on
HuzaifaChaudary/openclaw... sorry, wrong repo,HuzaifaChaudary/openai-agents-python@78b5c4d, on the closed branch. take anything useful from it, no attribution needed.worth reading #4738's open review threads first either way, @fscfede-beep has two cancellation ownership findings on it that apply to any implementation of this.
seratch commentedon Aug 30, 2026
The split-UTF-8 problem is real, but the implementation and remaining lifecycle verification are already consolidated in #4738. Keeping a separate tracker is not necessary.
Closing this as a duplicate tracker does not mean the fix has shipped; completion should continue to be tracked in #4738.