mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-11-02 13:12:50 +00:00
fix(check_framework_version): decode git diff output as UTF-8 (#511)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
f9ef82f169
commit
27a9b4a91d
3 files changed
+58
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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:
|
||||
|
||||
Reference in new issue
Block a user