PTY output silently corrupts a UTF-8 character split across two collection windows

[ayaangazali]

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: main at 89c02c82, 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.

[fscfede-beep]

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]

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:

• hand-rolling the incremental UTF-8 validation is a dead end, and codecs.getincrementaldecoder("utf-8")("replace") with len(decoder.getstate()[0]) is the right primitive. Three review rounds found three separate malformed-prefix cases in my hand-rolled version;
• carrying the unfinished bytes back onto the shared deque races with teardown, because _watch_process_exit sets output_closed only after gathering the pump tasks while _finalize_pty_update pops the entry as soon as process.returncode is set. fix(sandbox): settle PTY output before cleanup #4738's collector-owned settlement, gating exit_code on output_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]

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]

@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_closed design 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:

• fix(sandbox): keep split UTF-8 characters intact across PTY output windows #4724 by @ayaangazali, closed by the author
• fix: keep a utf-8 character that straddles two pty collection windows #4745, mine, closed on @fscfede-beep's recommendation after he compared the pieces against fix(sandbox): settle PTY output before cleanup #4738 and found them already there

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 buffers ED 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 waiting
• a cancelled collection loses whatever it had already drained. once a previous window has handed a lead byte forward, that is corruption in the surviving session rather than a loss confined to the cancelled call
• the finalisers decide the session is over with a different predicate than the collector. unix_local reads process.returncode while collection waits for output_closed, so a session can be torn down while a pump still holds the rest of a character

the 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.