fix(sandbox): keep a UTF-8 sequence split across PTY yield windows whole

[coderdailyone]

Summary

This pull request stops collect_pty_output() from turning a multibyte UTF-8 character into replacement characters when its bytes arrive in two different PTY yield windows.

Bug

collect_pty_output() (shared by the UnixLocal and Docker PTY backends) decodes whatever bytes it gathered before the yield deadline with errors="replace" and re-encodes the text. When a process has written only part of a multibyte sequence by the time the window closes, the partial bytes decode to U+FFFD in that update and the remaining continuation bytes decode to U+FFFD again in the next one, so the character is lost even though every byte was delivered.

Against a real UnixLocalSandboxClient session on main (1d471a47), with pty_exec_start at yield_time_s=0.1 followed by pty_write_stdin polls, the command printf '\\344\\270'; sleep 0.4; printf '\\255\\n' (U+4E2D written in two pieces) produced:

	bytes returned to the model
expected	中\n
on main	b'\\xef\\xbf\\xbd' then b'\\xef\\xbf\\xbd\\n', i.e. ��\n

Any command whose non-ASCII output straddles a yield boundary (CJK text, emoji, box-drawing progress bars, accented file names) can hit this; the pieces do not need a pause, only a window closing mid-sequence.

Fix

Collection and truncation are now separate steps in pty_output.py. collect_pty_output_bytes() gathers the window's raw bytes and, while the provider is still running, holds back an incomplete UTF-8 tail: the last three bytes are fed to Python's incremental decoder (final=False) and whatever it reports as pending is pushed back to the front of output_chunks under the output lock (re-checking is_done() there), to be completed by the next collection. Only what the decoder itself would wait for is held back; invalid leaders (0xC0, 0xC1, 0xF5 and up), ill-formed second bytes, and stray continuation bytes still decode to U+FFFD immediately. finish_pty_output() decodes and applies the token limit once. collect_pty_output() keeps its signature as the composition of the two for the other backends.

The UnixLocal and Docker backends now pass the raw bytes and max_output_tokens to _finalize_pty_update(). When it observes an exit code it drains anything still queued (drain_pty_output_chunks()) into the raw bytes before truncating, so a sequence held back just before the process exited is never stranded in an entry that is about to be removed, and the token limit and original_token_count cover the whole update.

Test plan

• tests/sandbox/test_pty_output.py: a two-collection test asserting the split sequence is withheld and then returned as 中\n, a done-flush test for a genuinely truncated tail, an invalid-leader test for the collector, a drain_pty_output_chunks() test, and a parametrized table for incomplete_utf8_tail_length() covering ASCII, complete sequences, 1 to 3 pending bytes, stray continuation bytes, invalid leaders, and ill-formed second bytes.
• TestUnixLocalPty::test_finalize_pty_update_flushes_bytes_held_back_when_the_process_exited reproduces the exit-before-output-closed window with a held-back sequence in the queue, and test_finalize_pty_update_applies_the_token_limit_after_flushing checks a one-token poll against truncate_text_by_tokens() of the combined text; both fail on the previous revision.
• tests/sandbox/test_unix_local.py::TestUnixLocalPty::test_pty_output_keeps_a_utf8_sequence_split_across_yields_whole runs the reproducer above through a real PTY session. On the previous revision it fails with AssertionError: assert '��\n' == '中\n'.
• tests/sandbox plus tests/extensions/sandbox/test_blaxel.py and the other sandbox extensions (2299 passed, 5 skipped), ruff format --check, ruff check, mypy, and pyright on the changed files pass locally on Linux / Python 3.10.

Issue number

None (found while exercising the UnixLocal PTY tools with non-ASCII output).

Checks

• I've added new tests, if relevant
• I've run the verification steps from .agents/skills/code-change-verification individually (format, lint, typecheck on changed files, tests)
• I've confirmed all verification steps pass (the repository-wide mypy src has pre-existing Python 3.10 errors outside this change)
• If using Codex, I've run /review before submitting this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02e9e9ac2e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Flush deferred bytes before removing an exited PTY

When a command exits after this collector drains a partial character but before its output pump sets output_closed, is_done() remains false and the drained bytes are placed back in the queue. Both UnixLocal and Docker can already observe the exit code in _finalize_pty_update() and immediately remove and terminate that entry, making the deferred bytes unreachable and returning incomplete output. This occurs on the supported quick-exit variant of the split-read scenario, so finalization must wait for output closure or explicitly flush the deferred bytes before removing the entry.

AGENTS.md reference: AGENTS.md:L104-L104

Useful? React with 👍 / 👎.

[coderdailyone]

Fixed in 7e0211a4: _finalize_pty_update() in both UnixLocal and Docker now drains whatever is still queued (via the new drain_pty_output_chunks()) whenever it observes an exit code, and appends it to the update before the entry is removed, so bytes a collection held back are never stranded. TestUnixLocalPty::test_finalize_pty_update_flushes_bytes_held_back_when_the_process_exited reproduces the window (process exited, output not yet marked closed, the split sequence sitting in the queue) and fails on the previous revision.

[chatgpt-codex-connector]

Reject invalid UTF-8 leaders instead of buffering them

When a running child emits an invalid UTF-8 byte such as b'\xc0' or b'\xc1' (for example, printf '\300'; sleep 30), this mask classifies it as an incomplete two-byte sequence. Every poll therefore pushes the byte back and returns no output until another byte arrives or the process exits, rather than immediately returning the replacement character as the existing behavior and this helper's contract require. Restrict the buffered prefixes to genuinely valid UTF-8 prefixes; the analogous broad checks also admit invalid four-byte leaders such as 0xF5–0xF7.

AGENTS.md reference: AGENTS.md:L92-L94

Useful? React with 👍 / 👎.

[coderdailyone]

Fixed in 7e0211a4: incomplete_utf8_tail_length() no longer classifies by bit masks. It feeds the last three bytes to codecs.getincrementaldecoder("utf-8") with final=False and reports the decoder's pending state, so only what CPython itself would keep waiting for is held back: 0xC0, 0xC1, 0xF5 and above, and ill-formed second bytes such as E0 80, F0 80, and F4 90 are returned as U+FFFD immediately (parametrized cases added, plus a collector test for b"\xc0"). The one case CPython still keeps pending in non-final mode is a surrogate second byte (ED A0); it is flushed at exit like any other tail.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e0211a4c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reapply the token limit after draining exit output

When max_output_tokens is set and the process exits after collection has applied truncate_text_by_tokens(), this appends all deferred or newly queued bytes after truncation. For example, a one-token poll that collected four ASCII bytes and held back a split UTF-8 character can return both the ASCII and completed character, exceeding the requested limit while leaving original_token_count as None; a larger final queued burst can bypass the bound by much more. Combine the deferred bytes before truncation, or truncate the combined output and recompute its count; the identical Docker path needs the same correction.

AGENTS.md reference: AGENTS.md:L102-L102

Useful? React with 👍 / 👎.

[coderdailyone]

Fixed in 3f8fb954: collection and truncation are now separate steps. collect_pty_output_bytes() returns the raw window (with the hold-back), the UnixLocal and Docker _collect_pty_output() return those bytes, and _finalize_pty_update() receives raw_output plus max_output_tokens: when it observes an exit it drains whatever is still queued into the raw bytes and only then calls finish_pty_output(), so the limit is applied once to the whole update and original_token_count reflects it. collect_pty_output() keeps its signature (collect + finish) for the other backends. test_finalize_pty_update_applies_the_token_limit_after_flushing checks a one-token poll against truncate_text_by_tokens() of the combined text and fails on the previous revision. The hold-back decision is also re-checked under the output lock, so a provider that finishes in between flushes the tail instead of parking it.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f8fb95483

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Wait for output pumps before draining exited PTYs

When a quick-exiting process has buffered output but its stdout/stderr pump has not completed its final read, process.returncode can already be non-None; this drains only the bytes currently queued and then _terminate_pty_entry() cancels the pump tasks, so the update returns process_id=None while permanently omitting the unread bytes. Fresh evidence after the prior fix is that the new drain still occurs before pump completion; the Docker path has the same issue when a reader-thread append is scheduled but not queued before the drain. Wait for output_closed or otherwise finish the pumps before the final drain and removal.

AGENTS.md reference: AGENTS.md:L104-L104

Useful? React with 👍 / 👎.

[seratch]

Thanks for working through the UTF-8 and token-limit cases. I recommend consolidating this work into #4738, which already addresses split UTF-8 and carries a settled output-closure fact into finalization. This head still drains the currently queued bytes before the Unix/Docker pumps finish, then removes the entry; an exited process can therefore lose unread final output. Maintaining a second collector/finalizer implementation would duplicate the same lifecycle work. I would close this PR in favor of #4738 and retain any distinct regression coverage there.