Skip to content

fix(shell): decode Windows output reliably - #28

Closed
h3c-hexin wants to merge 5 commits into
pinvou3-cleanfrom
fix/windows-shell-output-encoding
Closed

fix(shell): decode Windows output reliably#28
h3c-hexin wants to merge 5 commits into
pinvou3-cleanfrom
fix/windows-shell-output-encoding

Conversation

@h3c-hexin

@h3c-hexin h3c-hexin commented Aug 26, 2026

Copy link
Copy Markdown

Background

This 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 commit preserves @zhuowp as author, retains the original DCO trailer, and has a tree identical to #24 head 22ec868291922aa133197dd9736a0da31ae18329. Follow-up commits resolve the Clippy finding and add executable Windows coverage.

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

  • preserve strict UTF-8 decoder state across shell reads
  • fall back to the current Windows ACP only after genuine invalid UTF-8
  • share decoding semantics across synchronous output, snapshots, raw deltas, tails, and detached readers
  • seal detached streams without discarding bytes captured before the cutoff
  • derive completion events and evidence from one output cutoff
  • add decoder, shell-lifecycle, and forkguard_* regression coverage
  • remove a behavior-neutral loop whose branches all returned, resolving clippy::never_loop
  • make the incomplete UTF-8 unit test independent of the host ACP
  • run focused shell regressions on windows-latest in Pinvou fork CI
  • rerun the required contribution gate after PR head updates

Dependency advisory remediation remains separate in #27 because #24 does not change Cargo.lock.

No-Issue: continues #24 from a maintainable repository-owned branch while preserving contributor credit.

Testing

  • cargo fmt --all -- --check
  • git diff --check
  • cargo test -p codewhale-tui --lib tools::shell --locked locally (110 passed)
  • cargo test -p codewhale-tui --lib tools::shell::output::tests --locked locally after the Windows test-fixture fix (9 passed)
  • cargo clippy -p codewhale-tui --lib --locked --no-deps -- -A warnings -D clippy::never_loop
  • Exact original-commit tree comparison against fix(shell): decode Windows output reliably #24 head
  • Final-head Pinvou fork CI check on Linux (run 32929782904)
  • Final-head native windows-latest shell regressions (run 32929782904)
  • Full workspace Clippy (the public r10 baseline has unrelated Rust 1.98 warnings under -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 synchronize event. The current PR still needs its final gate regenerated after #27 lands and the combined tree is updated.

Advisory checks remain red until the independent lockfile fix in #27 lands; #28 must be evaluated again against the resulting combined tree before merge.

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes (not a UI change)
  • Harvested/co-authored credit uses a GitHub numeric noreply address (not applicable; the original signed commit is preserved)

zhuowp and others added 5 commits August 26, 2026 11:00
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

Copy link
Copy Markdown
Author

Temporarily closing while rebasing onto the newly merged #27 baseline. This avoids duplicate CI runs and lets the required contribution gate attach to the final head when the PR is reopened.

@h3c-hexin h3c-hexin closed this Aug 26, 2026
@h3c-hexin

Copy link
Copy Markdown
Author

Rebased onto pinvou3-clean after #27 merged. Reopening at final head 309cafb58d05969a0128c3d6aef7bb13fe0e395e to run the required gate, security audits, Linux checks, and native Windows shell regressions against the combined tree.

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