fix(prs): decode gh/git/claude output as UTF-8, not the Windows cp1252 codec#1980
Closed
LukeTheoJohnson wants to merge 1 commit into
Closed
fix(prs): decode gh/git/claude output as UTF-8, not the Windows cp1252 codec#1980LukeTheoJohnson wants to merge 1 commit into
LukeTheoJohnson wants to merge 1 commit into
Conversation
…2 codec `graphify prs` reads gh, git, and claude output via subprocess.run(text=True) with no explicit encoding=, so on Windows stdout is decoded with the cp1252 locale codec. gh emits UTF-8 JSON whose PR titles and logins routinely carry non-Latin1 bytes (emoji, or the Persian title in PR Graphify-Labs#1281), and cp1252 has no mapping for bytes such as 0x81. The capture reader thread raises UnicodeDecodeError, subprocess.run then returns stdout=None, and the caller dies one line later on json.loads(None) (TypeError) or None.splitlines() (AttributeError). Neither is caught by _gh's except clause, so the whole `prs` command aborts, with a spurious reader-thread traceback on stderr, against any repo that has non-ASCII PR metadata. Pass encoding="utf-8", errors="replace" to all five subprocess reads in prs.py so decoding matches Linux and macOS. This is the decode-side sibling of Graphify-Labs#1505, which applied the same change to the llm.py claude-cli subprocess. CI runs Linux only (a UTF-8 locale), so this path is never exercised there. Reproduced on Windows 11 / CPython 3.13 with a cp1252 locale: prs._gh("pr", "view", "1281", ...) crashed with TypeError before and returns the decoded title after. The added regression tests assert encoding="utf-8" on each call, mirroring tests/test_charmap_encoding.py, and are red without the fix.
LukeTheoJohnson
marked this pull request as ready for review
July 17, 2026 23:05
safishamsi
added a commit
that referenced
this pull request
Jul 18, 2026
Collaborator
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.
Problem
On Windows,
graphify prscrashes against any repo that has non-ASCII PR metadata.prs.pyreadsgh,git, andclaudeoutput throughsubprocess.run(..., text=True)with no explicitencoding=, so stdout is decoded with the locale codec, which is cp1252 on a default Windows install.ghemits UTF-8 JSON whose PR titles and author logins routinely contain non-Latin1 bytes (emoji, or the Persian title in this repo's own PR #1281), and cp1252 has no mapping for bytes such as 0x81.The failure is indirect. The capture reader thread raises
UnicodeDecodeError,subprocess.runswallows that and returnsstdout=None, and the caller then dies one line later:_ghrunsjson.loads(None)and raisesTypeErrorfetch_pr_files/fetch_worktreesrunNone.splitlines()and raiseAttributeErrorNeither is in
_gh'sexcept (TimeoutExpired, JSONDecodeError, FileNotFoundError), so the command aborts, and a spurious reader-thread traceback is printed to stderr as well.Root cause
text=Truewithoutencoding=useslocale.getpreferredencoding()for decoding (cp1252 on Windows). CI runs onubuntu-latestonly, where the locale is UTF-8, so this path is never exercised upstream.Fix
Pass
encoding="utf-8", errors="replace"to all five subprocess reads inprs.py. This is the decode-side counterpart of #1505, which applied the same change to thellm.pyclaude-cli subprocess;prs.pywas not touched then.Reproduction
Windows 11, CPython 3.13, cp1252 locale:
Tests
tests/test_prs.py::TestSubprocessOutputEncodingassertsencoding="utf-8"on each of the five calls, mirroring the existingtests/test_charmap_encoding.py. They are red without the fix (the four behavioural asserts fail withassert None == 'utf-8'). A guard test confirms the fixture bytes are genuinely cp1252-undecodable, so the tests exercise the real failure surface. A behavioural decode test can't be cross-platform here, since Linux CI decodes UTF-8 regardless, so the tests assert the contract instead.Scope
Five subprocess calls in
prs.py; no behaviour change on Linux or macOS, where UTF-8 is already what they decode with. Other modules have their owntext=Truecalls but decode ASCII-only output (git hashes, refs), so I left them out to keep this focused. Happy to harden those in a follow-up if you'd like.