fix(shell): decode Windows output reliably - #29
Merged
Conversation
Keep UTF-8 and legacy multibyte decoder state across background polls, use the Windows ANSI code page only after invalid UTF-8, and derive completion evidence from one output cutoff. Keep the Windows-only ACP mapping cross-platform tested without triggering dead_code in non-Windows library builds. This incorporates the behavior from upstream follow-up 0a85b13 without its policy-incompatible automated co-author trailer. Signed-off-by: zhuowp <zwp-1989@qq.com>
Return directly from the decoder match so Clippy no longer reports never_loop. This is behavior-neutral and preserves the original PR author commit unchanged. Signed-off-by: hexin <he.xin@h3c.com>
Run the focused shell test module on windows-latest so native output decoding changes receive executable Windows coverage without being blocked by unrelated r10 warnings in the upstream full-suite workflow. Signed-off-by: hexin <he.xin@h3c.com>
Exercise incomplete UTF-8 finalization without the host ACP so the assertion has identical semantics on Windows and Unix. Legacy fallback remains covered by its injected-encoding tests. Preserve the Windows build cache after failures to speed follow-up diagnostics. Signed-off-by: hexin <he.xin@h3c.com>
Include pull_request_target synchronize events so the required gate context is attached to the current head after contributors push follow-up commits. Signed-off-by: hexin <he.xin@h3c.com>
|
@h3c-hexin Pinvou/pinvou-agent#348 has now been realigned to the final merged maintenance head |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
This supersedes #28 after GitHub refused to reopen it because its head was rebased while temporarily closed. It continues #24 from a Pinvou-owned branch because the original head repository is no longer available for follow-up commits or workflow dispatch.
The first topic commit preserves @zhuowp as author and retains the original DCO trailer. Follow-up commits resolve the Clippy finding, make the test fixture deterministic, and add executable Windows coverage. The branch is rebased onto
pinvou3-cleanafter the dependency advisory fix in #27 merged.Windows-native tools may emit stdout or stderr using the active ANSI code page even when the surrounding shell is configured for UTF-8. The previous lossy, per-poll UTF-8 decoding could therefore produce mojibake and could corrupt a multibyte character split across background polls.
The reusable fix was first contributed upstream in Hmbown/CodeWhale #5602.
Changes
forkguard_*regression coverageloopwhose branches all returned, resolvingclippy::never_loopwindows-latestin Pinvou fork CINo-Issue: replaces #28 and continues #24 from a maintainable repository-owned branch while preserving contributor credit.
Testing
cargo fmt --all -- --checkgit diff --checkcargo test -p codewhale-tui --lib tools::shell --lockedlocally (110 passed)cargo test -p codewhale-tui --lib tools::shell::output::tests --lockedlocally after the Windows test-fixture fix (9 passed)cargo clippy -p codewhale-tui --lib --locked --no-deps -- -A warnings -D clippy::never_loopwindows-latestshell regressions before the dependency-baseline rebase (103 passed)-D warnings)Risks
ACP detection is deliberately a fallback after invalid UTF-8 and cannot infer OEM or arbitrary child-selected encodings. The change is confined to the shell byte-decoding lifecycle and does not modify sandbox, PowerShell, Python, or command-generation behavior.
The contribution gate previously ran only when a PR was opened or reopened, so strict branch protection lost the required context after every update. This branch adds the missing
synchronizeevent.Checklist