From 27a9b4a91dd181b46e5e145a8e57e30400834e2a Mon Sep 17 00:00:00 2001 From: Timofei Erokhin Date: Fri, 2 Oct 2026 21:03:35 +0300 Subject: [PATCH] fix(check_framework_version): decode git diff output as UTF-8 (#511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(check_framework_version): decode git diff output as UTF-8 run_git() passed subprocess.run(text=True) with no explicit encoding, so it fell back to the host locale's default codec - cp1252 on Windows - instead of UTF-8. Any non-ASCII byte in a modified framework file's diff (e.g. an em dash in a profile edit) then raised UnicodeDecodeError before the gate could evaluate the change, instead of a pass/fail verdict. Reproduced on a real Windows checkout. Pin encoding="utf-8" on the subprocess.run() call. Added a regression test that patches subprocess.run and asserts run_git() always passes encoding="utf-8" - confirmed it fails against the pre-fix code and passes against the fix. Co-Authored-By: Claude Sonnet 5 * fix(check_framework_version): address review on UTF-8 decode fix Per MadsLorentzen's review on PR #511: - Add errors="replace" alongside encoding="utf-8" on the subprocess.run() call, matching the convention used elsewhere in this repo (robots_check.py). - Correct the docstring: an em dash does not crash the un-pinned decode (cp1252 maps it fine); the actual trigger is a byte cp1252 leaves undefined, e.g. Cyrillic Ё/ё, much CJK, or Á-class Latin. - Correct "profile file" to "framework file" - the tool only diffs the framework's own directories, not arbitrary profile files. - Add a CHANGELOG entry under [Unreleased] / Fixed naming the tool and test. --------- Co-authored-by: Claude Sonnet 5 --- CHANGELOG.md | 11 ++++++++ tests/test_check_framework_version.py | 39 +++++++++++++++++++++++++++ tools/check_framework_version.py | 9 ++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3603a57..36148725 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,17 @@ per-file diff commands. ### Fixed +- **`check_framework_version.py`'s git diff read no longer crashes on non-ASCII framework-file + content** (`tools/check_framework_version.py`) - `run_git()` called + `subprocess.run(text=True)` without an explicit `encoding`, so output decoded via the host + locale's default codec instead of UTF-8. On a real Windows checkout (cp1252 default) this + raised `UnicodeDecodeError` on any byte cp1252 leaves undefined - Cyrillic Ё/ё, much CJK, + Á-class Latin - appearing in a framework file's diff, crashing the version gate before it + ever evaluated the change. `run_git()` now passes `encoding="utf-8"` and `errors="replace"` + (matching the convention already used elsewhere in this repo, e.g. `robots_check.py`), + decoding deterministically regardless of host locale. Pinned by `RunGitEncodingTests` in + `tests/test_check_framework_version.py`, which fails against the original un-pinned call. + - **`/rank` still sweeps deadlines when there is nothing new to score** (`.claude/commands/rank.md` Step 1, `tests/test_rank_command.py`) - when `rank_state.py candidates` reported no eligible jobs, Step 1 said "Nothing new to rank" and stopped before Step 3's rule 6 expiry sweep ever diff --git a/tests/test_check_framework_version.py b/tests/test_check_framework_version.py index 95622333..54ee6b09 100644 --- a/tests/test_check_framework_version.py +++ b/tests/test_check_framework_version.py @@ -17,6 +17,7 @@ import subprocess import sys import tempfile import unittest +import unittest.mock from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent @@ -105,6 +106,44 @@ class FrameworkVersionGateTests(CheckerRepoFixture): self.assertEqual(result.returncode, 1, result.stdout + result.stderr) self.assertIn("missing 'framework_version'", result.stdout) +class RunGitEncodingTests(unittest.TestCase): + """run_git() must decode `git diff` output as UTF-8 explicitly, not via + whatever `subprocess.run(text=True)` falls back to on the host locale. + + On a real Windows checkout, the un-pinned call decodes as cp1252 (the + locale default there) and a byte cp1252 leaves undefined - e.g. + Cyrillic Ё/ё, much CJK, or Á-class Latin - in a framework file's diff + raises UnicodeDecodeError before the gate ever evaluates the change + (reported 2026-10-01, reproduced on Windows with Cyrillic Ё). An em + dash does not trigger this - cp1252 maps it fine. `LC_ALL`/`LANG` don't + influence this on Windows, so the regression is pinned directly against + the subprocess.run() call rather than by trying to simulate the OS + locale.""" + + def test_run_git_pins_utf8_encoding(self): + import importlib.util + + spec = importlib.util.spec_from_file_location( + "check_framework_version", SCRIPT + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + with unittest.mock.patch.object(module.subprocess, "run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=["git"], returncode=0, stdout="", stderr="" + ) + module.run_git(["diff", "-U0", "HEAD"]) + + _, kwargs = mock_run.call_args + self.assertEqual( + kwargs.get("encoding"), + "utf-8", + "run_git() must pass encoding=\"utf-8\" to subprocess.run() - " + "omitting it falls back to the host locale's default codec " + "(cp1252 on Windows), which crashes on non-ASCII diff content.", + ) + if __name__ == "__main__": unittest.main() diff --git a/tools/check_framework_version.py b/tools/check_framework_version.py index cfdea06c..01bf250d 100755 --- a/tools/check_framework_version.py +++ b/tools/check_framework_version.py @@ -23,7 +23,14 @@ if root_agents.exists(): FRAMEWORK_FILES.append(root_agents) def run_git(args: list[str]) -> tuple[int, str, str]: - res = subprocess.run(["git"] + args, cwd=str(ROOT), capture_output=True, text=True) + res = subprocess.run( + ["git"] + args, + cwd=str(ROOT), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) return res.returncode, res.stdout, res.stderr def get_base_commit() -> str | None: