Skip to content

fix: preserve UTF-8 across shell output chunks - #360

Merged
frostming merged 1 commit into
bubbuild:mainfrom
yunyv:fix/shell-output-utf8-boundaries
Oct 15, 2026
Merged

frostming merged 1 commit into
bubbuild:mainfrom
yunyv:fix/shell-output-utf8-boundaries

Conversation

@yunyv

@yunyv yunyv commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve UTF-8 characters when shell output splits them across reads. ShellManager._drain_stream() currently decodes each 4096-byte chunk independently with errors="replace". A valid multibyte character spanning two reads becomes replacement characters, so shell tool output can corrupt non-ASCII text and filenames.

Use a separate incremental UTF-8 decoder for each stdout/stderr reader, and flush it at EOF. Invalid bytes and incomplete trailing sequences still use replacement decoding. There are no new dependencies or changes to the shell API.

Reproduction

Run a shell command that writes b"a" * 4095 + "中".encode("utf-8") to stdout or stderr. On the base revision, the final character is returned as replacement characters. With this change, the original text is preserved.

Tests

  • Added real subprocess regressions for two-, three-, and four-byte characters crossing the read boundary on both stdout and stderr: all six fail against the original implementation and pass with the fix.
  • Added controls for empty output, invalid bytes, and an incomplete UTF-8 sequence at EOF.
  • make check: passed, including lock validation, all pre-commit hooks, and mypy (57 source files).
  • Full pytest suite with doctests: 594 passed, 20 skipped without optional tracing dependencies. A writable temporary HOME was used because this execution environment's default home is read-only.
  • With the optional trace extra installed: HOME=/tmp/bub-test-home OTEL_TRACES_SAMPLER=always_on .venv/bin/python -m pytest --doctest-modules: 613 passed, 1 skipped (PowerShell unavailable). The sampler override prevents the host’s inherited sampling setting from disabling the in-memory tracing assertions.
  • Python 3.12 tested locally; the repository CI covers Python 3.13 and 3.14.

@frostming
frostming marked this pull request as ready for review October 15, 2026 04:27
@frostming
frostming merged commit 0bb39f5 into bubbuild:main Oct 15, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants