fix(scanner): decode rg/git output as utf-8 with replacement#55
Merged
Conversation
The benchmark surfaced a real cross-platform crash: subprocess.run(text=True) decodes rg/git output with the platform default encoding (cp1252 on Windows, strict utf-8 on Linux), so the CLI raised UnicodeDecodeError on any repo containing a file with non-decodable bytes (hit on assafelovic/gpt-researcher). The clean-ASCII fixture never triggered it. Fix: every output-capturing subprocess.run now passes encoding='utf-8', errors='replace' (8 call sites). Added a regression test that scans a file with raw non-UTF-8 bytes and asserts no crash. Fixture unchanged (36); 27 tests pass. Surfaced during PR-16 benchmark prep.
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
1 participant
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.
The bug
subprocess.run(..., text=True)decodes child output with the platform default encoding — cp1252 on Windows, strict UTF-8 on Linux. Any repository containing a file with non-decodable bytes made the CLI raiseUnicodeDecodeErrorand abort the scan. The clean-ASCII fixture never exercised this, so the self-test stayed green while real repos crashed.Surfaced by the PR-16 benchmark run — the scan of
assafelovic/gpt-researcherhit byte0x8fand died mid-scan.The fix
Every output-capturing
subprocess.run(rg + git; 8 call sites) now passesencoding="utf-8", errors="replace", so undecodable bytes are replaced rather than fatal. Behaviour is otherwise unchanged.Added a regression test (
test_scan_survives_non_utf8_bytes) that writes a file with raw non-UTF-8 bytes and asserts the scan completes without crashing.Scope
cli/dsgai_scan.py— 8 call sitestests/test_runner.py— +1 regression testNo rules, schema, or report changes. Split out from the PR-16 benchmark branch so this fix can be reviewed and merged on its own.