fix(terminal): keep native Windows diagnostics readable

[fangliquanflq]

What does this PR do?

Windows users whose native programs emit the system ANSI code page now receive readable localized terminal output instead of irreversible U+FFFD replacement characters. The terminal still preserves UTF-8 output from MSYS tools, including streams where multibyte characters cross pipe-read boundaries.

Symptom

On a zh-CN Windows host using code page 936, localized output from a native program invoked through Git Bash was decoded as UTF-8 with replacement enabled. The terminal result contained replacement characters instead of the original message.

Impact

Affected Windows users lose diagnostic text from native commands, so error messages can become unreadable before the agent receives them. The verified failure affects foreground local execution and the equivalent raw-byte background reader path.

Bug Cause

Trigger: tools/environments/base.py:1164 / BaseEnvironment._wait_for_process when Git Bash forwards bytes from a native Windows child.

Causal chain:

1. A native Windows child writes localized output using the host ANSI code page while MSYS programs in the same shell continue to emit UTF-8.
2. Foreground, background, and byte-returning PTY readers feed all raw chunks to UTF-8-only incremental decoders.
3. Invalid UTF-8 byte sequences are replaced with U+FFFD before terminal output reaches the agent.

Why it is wrong: Git Bash can carry both UTF-8 MSYS output and native Windows code-page output, so one fixed UTF-8 decoder cannot represent every valid stream.

Working sibling / contrast: UTF-8 output from MSYS tools already decodes correctly and must remain unchanged; pywinpty string output is already decoded by its provider and does not need byte decoding.

Ruled out: The native process does not emit damaged text. Captured raw bytes decode correctly as code page 936, which isolates the loss to Hermes' raw-byte decoding step.

Fix

Add a shared bounded incremental decoder that prefers strict UTF-8 for each complete record and falls back to the host Windows ANSI codec only when UTF-8 is invalid. Use it for foreground pipes, background process pipes, and byte-returning PTYs so mixed encodings and multibyte sequences split across chunks are preserved without changing already-decoded string output.

Related Issue

Closes #89442

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/environments/base.py - detect the native Windows output codec, add bounded mixed-encoding incremental decoding, and use it for foreground raw-byte output.
• tools/process_registry.py - share the decoder across background pipe and byte-returning PTY readers.
• tests/tools/test_base_environment.py - cover UTF-8, code page 936, mixed records, prompt output, foreground execution, and chunk boundaries.
• tests/tools/test_process_registry.py - cover native Windows bytes and split multibyte sequences in background and PTY readers.

How to Test

1. On Windows 11 with ANSI code page 936, invoke a native PowerShell command that emits a localized error through the local terminal backend and confirm the text is readable with no U+FFFD characters.
2. Run an MSYS command that emits UTF-8 CJK text and confirm it remains unchanged.
3. Start the native-output command in the background and confirm ProcessRegistry returns readable output with no replacement characters.
4. Automated verification already run: 14 focused decoder, foreground, background, PTY, and timeout-output tests passed; Ruff passed for the changed files.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11, zh-CN, ANSI code page 936

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) - N/A, no user-facing configuration changed
• I've updated cli-config.yaml.example if I added/changed config keys - N/A, no configuration keys changed
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - N/A, no architecture or workflow changed
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide
• I've updated tool descriptions/schemas if I changed tool behavior - N/A, the terminal tool contract is unchanged

[yoggydev]

I have a ja-JP Windows box (ACP=932), so I ran the CP932 half of this. The numbers below are from that machine — harness and raw output: https://github.com/yoggydev/cp932-pipe-probe (MIT). Drafted with Claude; the measurements are mine.

Same failure reproduces on ja-JP.

A fixed 50-byte CP932 sequence pushed through a pipe, decoded two ways (Windows PowerShell 5.1):

decode path	chars	U+FFFD	stray \
UTF-8 with replacement	42	29	4
raw bytes → cp932	26	0	—

The four backslashes are the ja-JP-specific part. CP932 trail bytes are 0x40–0x7E and 0x80–0xFC, so 0x5C is a valid trail byte. A kanji whose second byte is 0x5C does not become U+FFFD — it leaves a \ behind. In the sample above those four were 十 (8F5C), 予 (975C), 構 (8D5C), ソ (835C): everyday characters, not rare ones. So a mangled ja-JP message can still look half-plausible to the agent, and the damage is easy to misfile as a path or quoting bug.

GBK has 0x5C trail bytes too — 118 of them against 52 in CP932 — but they are mostly low-frequency. The CP932 set includes 表 十 能 予 構 貼, which is why ja-JP hits this constantly rather than occasionally.

Two things that may matter for the fix:

1. Python's cp932 table is the more permissive one. It recognises 9,604 two-byte sequences; .NET's recognises 9,206 (same on Windows and Linux — .NET carries its own table rather than deferring to the OS). A Python-side cp932 fallback is therefore not the narrower option.

2. The replacement output is not stable across runtimes. The identical 50 bytes produce 29 U+FFFD on .NET Framework 4.8, and 30 on both .NET 8 and CPython 3.11. So errors="replace" doesn't just discard the original bytes — the wreckage it leaves isn't consistent either, which rules out treating it as a reliable marker for "this output was mangled". (Related, not a diagnosis: UTF8 Encoding isn't consistent with .Net Framework dotnet/standard#1679.)

Happy to run anything else against ja-JP — that's the environment I have.

[fangliquanflq]

Thank you for validating this on an ACP 932 machine and for documenting the 0x5C trail-byte failure mode. I checked both observations against the current PR tip:

• The fallback codec is resolved from the host ACP through locale.getencoding() and normalized by Python's codec registry, so a ja-JP ACP 932 host selects cp932. Each complete record is decoded as strict UTF-8 first, then the original raw bytes are decoded with that host codec only when UTF-8 is invalid.
• The implementation does not inspect replacement characters or their count, so the runtime-dependent U+FFFD behavior does not affect codec selection. It also does not depend on Python's cp932 table being narrower than another runtime's table.

I additionally exercised 表十能予構貼\n (955c8f5c945c975c8d5c935c0a) through the PR decoder one byte at a time with a cp932 fallback. The output matched exactly and contained no U+FFFD, including all six 0x5C trail bytes. The focused foreground/background/PTY decoder suite also passed: 10 tests, 0 failures; the PR's Windows-only CI job is passing.

This is therefore informational validation rather than a required code change, and it materially strengthens the cross-locale evidence for the fix. Thanks also for offering access to the ja-JP environment.

[yoggydev]

That all matches what I see, and locale.getencoding() is the right pick specifically — it ignores Python UTF-8 Mode, unlike getpreferredencoding(False). So a ja-JP host launched with PYTHONUTF8=1 still resolves to cp932 instead of quietly reporting utf-8 and defeating the fallback. With requires-python = ">=3.11" there is no version gap either.

Agreed the U+FFFD count is not load-bearing for the fix. I raised it to rule out the tempting shortcut of detecting mangled output downstream by counting replacement characters, not to influence codec selection.

Your six bytes are the right set to have picked — there are 50 in the full CP932 0x5C trail-byte space if you ever want a wider fixture. Happy to run it on the ja-JP host whenever it is useful.

[fangliquanflq]

Thanks for confirming the locale.getencoding() behavior under Python UTF-8 Mode and the CP932 trail-byte details. I rechecked the current tip: _windows_output_encoding() calls locale.getencoding(), the project requires Python 3.11+, and _IncrementalOutputDecoder attempts strict UTF-8 before applying the host-codec fallback to the original bytes. The implementation does not use U+FFFD counts for detection. The existing split-chunk, mixed-record, foreground, background, and byte-returning PTY tests cover that contract, and the current head has all required checks passing. No additional code change or wider fixture is needed for this feedback; the ja-JP measurements are useful cross-locale validation.

[monerostar]

Native Win11 review (en-US ACP 1252, PYTHONUTF8=1)

Host: Windows 11 10.0.26200.9168, Hermes venv Python 3.11.15. Approve is blocked for this fork-scope token.

This box is the complementary locale to the existing zh-CN / ja-JP comments: ANSI code page 1252, and this process has PYTHONUTF8=1.

Codec selection (this is the useful bit)

PYTHONUTF8                 = 1
locale.getencoding()       = cp1252
locale.getpreferredencoding(False) = utf-8
_windows_output_encoding() = cp1252

So locale.getencoding() is the correct API here: under UTF-8 mode, getpreferredencoding(False) would report utf-8 and disable the fallback entirely. The PR already does this; this host confirms it.

Decoder probes (live, default host fallback = cp1252)

input	result	U+FFFD
caf\xe9\n (cp1252)	café	0
UTF-8 café\n	café	0
UTF-8 工具\n one byte at a time	工具	0
error \x97 failed\n (cp1252 em dash)	error — failed	0

UTF-8 still wins when it is valid; host ACP is used only when it is not.

Tests

pytest tests/tools/test_base_environment.py tests/tools/test_process_registry.py -k "decoder or Incremental or windows_output or encoding or 936 or native" -o addopts=

8 passed, including the cp936 fixtures, mixed UTF-8/ACP records, split-chunk UTF-8, foreground wait, background reader, and byte-returning PTY path.

CI on the PR is already green (including Windows-only tests). I did not re-run a zh-CN native child through Git Bash; this host is ACP 1252.

Looks correct from the en-US + UTF-8-mode side.

[fangliquanflq]

Thank you for the native Windows ACP 1252 and PYTHONUTF8=1 validation. I checked the review against the current head (c735a03): _windows_output_encoding() uses locale.getencoding(), normalizes the host codec, and _IncrementalOutputDecoder tries strict UTF-8 per complete record before falling back to the original bytes with that codec. The focused tests cover split UTF-8 chunks, native-code-page bytes, mixed UTF-8/ACP records, foreground pipes, background pipes, and byte-returning PTYs. The current head also reports all required checks passing. Your cp1252 probes therefore confirm the intended cross-locale behavior and do not identify a code change; no additional patch is needed for this review.

[jackulau]

@fangliquanflq - this PR and my #89465 are the same fix, opened two minutes apart (mine 2026-08-18T20:45Z, yours 20:47Z). Neither of us could have seen the other; I only found it today with a sweep that looks for PRs opened after mine that cite the same issue or touch the same files. Flagging it rather than letting two heads sit on tools/environments/base.py indefinitely.

I have read both diffs properly, and I think yours should be the base. Reasoning, so it is checkable rather than polite:

Where yours is better, and it is not a small margin

You wired it into the background readers; I did not. process_registry.py's _reader_loop and _pty_reader_loop both had a bare codecs.getincrementaldecoder("utf-8")(errors="replace"), and they have exactly the same defect as the foreground path. Mine covers _wait_for_process and nothing else, so a user hitting this through a background session would still get U+FFFD after my PR merged. That is a real coverage hole and yours does not have it.

And your ASCII fast-path is what makes that possible. This is the part I want to be explicit about, because it is the thing I got structurally wrong:

ascii_end = 0
while ascii_end < len(self._buffer) and self._buffer[ascii_end] < 0x80:
    ascii_end += 1

My decoder buffers to a newline unconditionally. I justified that in the docstring on the grounds that _wait_for_process renders collected output only after the drain thread joins, so nothing observes the stream mid-command - which is true for the foreground path, and is exactly why my design does not generalise. Dropped into _reader_loop, which is a streaming reader whose whole job is to hand text to a live session, unconditional line buffering turns into latency on any output that never ends a line. Your fast-path is correct there and mine would have needed rewriting to get where yours already is.

Three things from mine worth folding in

Offered as patches to your branch, not as a reason to prefer my head:

1. The fallback should be gated per environment, not on os.name. _windows_output_encoding() keys on the host being Windows, and _wait_for_process lives on BaseEnvironment - shared by the remote and container environments, not just LocalEnvironment. So on a Windows host driving a Linux container, bytes produced inside that container get a cp936 retry. In practice the blast radius is small (only bytes that already fail strict UTF-8 reach the fallback, and from Linux those are usually genuinely binary), but the reasoning is the same reasoning the bug is made of: the codepage of the machine reading the bytes is not the codepage of the machine that wrote them.

Mine handles it with a hook - _output_fallback_encoding() on the base class returning None, overridden in LocalEnvironment to return the host ANSI codepage only on Windows - so the policy attaches to "this output came from a local Git Bash child" rather than to "this process is running on Windows". That drops onto your decoder as a constructor argument you already have (fallback_encoding=), so it is a small change.

2. A NUL-byte guard before the fallback. If a record contains \x00 it is binary, not text in the host codepage, and retrying it in cp936 produces confident garbage where U+FFFD was at least honest about being lossy. Cheap and it only ever fires on the already-failed path.

3. \r as a record boundary, not just \n. Progress output (a spinner, a percentage counter, pnpm's installer) redraws with a bare CR and may never emit a newline. Your ASCII fast-path covers the common case of ASCII progress bars, but a localized non-ASCII progress line buffers to _PROBE_LIMIT before anything is emitted. Mine treats 0x0A and 0x0D both as boundaries.

What I would like to do

Close #89465 as a duplicate of this one once you have had a chance to say whether you want those three folded in. I would rather send them as a patch against your branch than have two heads at the same seam - say the word and I will open it against your fork, or just take the descriptions above and write them yourself, whichever you prefer.

Either way the tests in mine that might be useful to you are test_a_pure_utf8_stream_matches_the_old_decoder_exactly (asserts byte-for-byte agreement with the old decoder on pure-UTF-8 input, which is the invariant that makes this class of change safe to merge) and the split-multibyte-across-chunks case, which you also cover.

cc @maintainers for the disposition: two open PRs, same seam, same bug, opened two minutes apart. My recommendation is this one.

[fangliquanflq]

Thanks for the careful comparison and for coordinating the duplicate. I agreed with all three suggested changes and pushed them to this branch:

• Foreground fallback selection is now environment-scoped: BaseEnvironment defaults to no fallback, while LocalEnvironment opts into the host Windows ANSI codec. The local background pipe and byte-PTY readers retain their host-local default.
• Records containing NUL stay on UTF-8 replacement behavior, including when the NUL and invalid bytes arrive in separate reads or span a bounded-buffer flush.
• Both CR and LF now terminate decoder records, so localized carriage-return progress output is emitted promptly.

The bounded decoder also retains one selected codec for an over-limit record, preventing a long ACP record from switching to UTF-8 after a flush. I added behavioral coverage for environment opt-in, CR boundaries, NUL records, split chunks, bounded flushes, and long-record codec consistency.

Verification:

• scripts/run_tests.sh tests/tools/test_base_environment.py -k IncrementalOutputDecoder — 12 passed
• scripts/run_tests.sh tests/tools/test_process_registry.py -k reader_loop — 8 passed
• Ruff on the related source and test files — passed

The full two-file run also exposed unrelated native-Windows failures in pre-existing /bin/bash and POSIX-only tests; all decoder and reader-loop tests relevant to this change pass. Your recommendation to use this PR as the shared base makes sense; no patch against the fork is needed now.

[yoggydev]

Native ja-JP review of the four commits added since my last comment.

Host: Windows 11, ACP 932, OEMCP 932, locale.getencoding() = cp932, Python 3.12.10.

What I went after. _flush_long_buffer now pins the codec for the rest of an over-limit record:

if exc.end == len(raw) and exc.reason == "unexpected end of data" and exc.start:
    ...
    self._record_encoding = "utf-8"

CP932's two-byte lead range includes 0xE0-0xEF, which is exactly UTF-8's three-byte lead range — 2,353 characters in the table on this host. So a CP932 record ending mid-character in that range presents to a strict UTF-8 decode as truncated-but-valid, and if that branch fired at a flush it would pin the whole remainder of the record to UTF-8. That is the failure this commit's guard has to avoid, so it is where I pushed.

It cannot fire. decode() strips the leading ASCII run out of the buffer before it tests _PROBE_LIMIT, so the buffer at flush time always begins with a non-ASCII byte. exc.start is then 0 and the branch is skipped. I went in expecting to break this and found the ordering was already doing the work — I had not read that ASCII-strip carefully enough the first time.

Measured, driving _IncrementalOutputDecoder from this branch directly:

	cases	mismatches
randomized CP932 records, 1-5 random chunk splits	4,000	0
4096-boundary: every 0xE0-0xEF lead x trail samples x pad 4094/4095/4096/4097	256	0

Records were terminated with \n and \r at random, so the CR-boundary change in 6ded5ac is exercised rather than assumed. Character pool: 7,336 CP932 characters.

The other thing I checked. With _fallback_encoding now defaulting to None on BaseEnvironment (abc3a4e), self._record_encoding = self._fallback_encoding can leave it None, and the next statement is codecs.getincrementaldecoder(self._record_encoding). That path is unreachable: decode() early-returns to the plain UTF-8 incremental decoder when _fallback_encoding is falsy, and __init__ normalises a utf-8 fallback to None. Worth stating because the two commits that create the situation are separate.

What this does not cover. The harness forces fallback_encoding="cp932" and drives the decoder in isolation, so it exercises the codec logic and not the foreground / background / byte-PTY plumbing around it. The host line at the top is separate evidence that _windows_output_encoding() resolves correctly here.

Looks correct from the CP932 side. If you want a different fixture — a wider trail-byte set, or a live Git Bash child instead of the decoder alone — I can run it on this host.

(Measured on my ja-JP host. Drafted with Claude.)