mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
1baa04ef65
## Description On non-UTF-8 Windows locales (e.g. GBK/cp936 on zh-CN, cp1252 on Western installs) `headroom wrap codex` corrupts `~/.codex/config.toml`. Two root causes, both in how we read/write text: - `Path.read_text()` / bare `open()` default to the **system locale** encoding, so a UTF-8 config fails to decode as the locale codec (and a locale-written file fails to decode as UTF-8) — raising `UnicodeDecodeError`. - `Path.write_text()` / text-mode `open()` translate `\n` → `os.linesep` on write, so an existing `\r\n` becomes `\r\r\n`, which TOML parsers reject with *"carriage return must be followed by newline"*. This adds one small helper module and routes the unsafe config/text I/O through it. Closes #733 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - New `headroom/fsutil.py` with `read_text` / `write_text` / `append_text`: - `read_text`: decode UTF-8 → fall back to `locale.getpreferredencoding()` (for files a tool wrote in the locale encoding before this fix) → final UTF-8 with `errors="replace"` so it never raises on content. Line endings normalise to `\n`, so callers that search/rewrite the text see one ending and a later `write_text` can't re-double an existing `\r\n`. Supports `default=` for missing files. - `write_text` / `append_text`: UTF-8 with `newline=""` so the bytes written match the content exactly and existing `\r\n` endings are never doubled. - Routed the unsafe config/text I/O across the package through `fsutil` (or added an explicit `encoding="utf-8"` where only decode safety was missing): `mcp_registry/codex.py` (TOML read/write + `_load_toml` via `tomllib.loads`), `mcp_registry/opencode.py`, `mcp_registry/claude.py`, `cli/wrap.py`, `cli/mcp.py`, `cli/memory.py`, `install/providers.py`, `providers/anthropic.py`, `providers/openai.py`, `providers/opencode/config.py`, `providers/opencode/install.py`. - Tests: new `tests/test_fsutil.py` (CRLF preservation, no LF translation, CRLF normalisation on read, UTF-8 non-ASCII round trip, locale-decode fallback, never-raise replace fallback, missing-file default/raise, append preserves endings) and two `test_codex_registrar.py` regression tests (register doesn't double CRLF; non-ASCII values survive a register). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ pytest tests/test_fsutil.py tests/test_mcp_registry/test_codex_registrar.py -q tests\test_fsutil.py ......... [ 25%] tests\test_mcp_registry\test_codex_registrar.py ........................ [100%] 36 passed in 0.32s $ ruff check <changed files> All checks passed! $ ruff format --check <changed files> 14 files already formatted $ mypy headroom --ignore-missing-imports # (run with --python-version 3.12 to # parse the local numpy stub) Success: no issues in changed files ``` Note: locally, the two suites `tests/test_mcp_registry` + `tests/test_cli` share a pre-existing cross-test state leak that flakes `test_wrap_codex_..._serena...` and `test_dead_client_marker...`; both reproduce identically on `main` (changes stashed) and are unrelated to this PR. CI shards run them isolated. ## Real Behavior Proof - Environment: Windows 11, Python 3.13.11, `locale.getpreferredencoding()` = `cp1252` (a non-UTF-8 locale — the exact condition that triggers #733). - Exact command / steps: pre-seed a `~/.codex/config.toml` the way Codex writes it on Windows — CRLF endings plus a non-ASCII value `project = "比赛/机器人"` — then call `CodexRegistrar.register_server(headroom)` and re-parse with `tomllib`. - Observed result: register status REGISTERED, no doubled CRLF, `tomllib` parses, and the non-ASCII value is preserved. Full output: ```text python: 3.13.11 | locale preferred encoding: cp1252 register status: RegisterStatus.REGISTERED doubled CRLF present: False tomllib parsed OK: True non-ASCII project value preserved: True headroom in mcp_servers: True ``` Before this change the same flow produced `\r\r\n` and a `tomllib` "carriage return must be followed by newline" error. - Not tested: a real zh-CN GBK/cp936 Windows install (no such host available); the GBK-specific decode path is covered by `test_read_text_falls_back_to_locale_encoding` which monkeypatches the preferred encoding to `gbk`. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes - Purely-binary I/O and sites already using `encoding="utf-8"`+`errors="replace"` (e.g. `learn/analyzer.py`) and the ASCII-only PID file (`install/runtime.py`) were intentionally left untouched. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
73 lines
2.6 KiB
Python
73 lines
2.6 KiB
Python
"""Tests for headroom.fsutil — encoding- and newline-safe text I/O (#733)."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from headroom import fsutil
|
|
|
|
|
|
def test_write_text_does_not_double_existing_crlf(tmp_path):
|
|
"""A string containing \\r\\n must be written verbatim, never as \\r\\r\\n."""
|
|
p = tmp_path / "config.toml"
|
|
fsutil.write_text(p, 'model = "gpt-5"\r\nport = 8787\r\n')
|
|
raw = p.read_bytes()
|
|
assert b"\r\r\n" not in raw
|
|
assert raw == b'model = "gpt-5"\r\nport = 8787\r\n'
|
|
|
|
|
|
def test_write_text_does_not_translate_lf(tmp_path):
|
|
"""\\n must stay \\n on every platform (no \\r\\n rewrite)."""
|
|
p = tmp_path / "hook.sh"
|
|
fsutil.write_text(p, "#!/bin/sh\necho hi\n")
|
|
assert p.read_bytes() == b"#!/bin/sh\necho hi\n"
|
|
|
|
|
|
def test_read_text_normalises_crlf(tmp_path):
|
|
"""read_text returns universal-newline (\\n) text, so a round trip can't double CRLF."""
|
|
p = tmp_path / "config.toml"
|
|
p.write_bytes(b"a = 1\r\nb = 2\r\n")
|
|
text = fsutil.read_text(p)
|
|
assert text == "a = 1\nb = 2\n"
|
|
fsutil.write_text(p, text)
|
|
assert b"\r" not in p.read_bytes()
|
|
|
|
|
|
def test_read_text_roundtrips_utf8_non_ascii(tmp_path):
|
|
p = tmp_path / "config.toml"
|
|
fsutil.write_text(p, 'project = "比赛/机器人"\n')
|
|
assert fsutil.read_text(p) == 'project = "比赛/机器人"\n'
|
|
|
|
|
|
def test_read_text_falls_back_to_locale_encoding(tmp_path, monkeypatch):
|
|
"""A file a tool wrote in the locale encoding (e.g. GBK) still decodes."""
|
|
monkeypatch.setattr(fsutil.locale, "getpreferredencoding", lambda *_: "gbk")
|
|
p = tmp_path / "config.toml"
|
|
p.write_bytes('path = "模型"\n'.encode("gbk")) # not valid UTF-8
|
|
assert fsutil.read_text(p) == 'path = "模型"\n'
|
|
|
|
|
|
def test_read_text_replace_fallback_never_raises(tmp_path, monkeypatch):
|
|
"""When neither UTF-8 nor the locale encoding decodes, fall back to replace."""
|
|
monkeypatch.setattr(fsutil.locale, "getpreferredencoding", lambda *_: "ascii")
|
|
p = tmp_path / "config.toml"
|
|
p.write_bytes(b"\xff\xfe bad bytes")
|
|
# Must not raise; returns *something* decodable.
|
|
assert isinstance(fsutil.read_text(p), str)
|
|
|
|
|
|
def test_read_text_missing_returns_default(tmp_path):
|
|
p = tmp_path / "nope.toml"
|
|
assert fsutil.read_text(p, default="") == ""
|
|
|
|
|
|
def test_read_text_missing_raises_without_default(tmp_path):
|
|
with pytest.raises(OSError):
|
|
fsutil.read_text(tmp_path / "nope.toml")
|
|
|
|
|
|
def test_append_text_preserves_endings(tmp_path):
|
|
p = tmp_path / "AGENTS.md"
|
|
fsutil.write_text(p, "line1\n")
|
|
fsutil.append_text(p, "line2\n")
|
|
assert p.read_bytes() == b"line1\nline2\n"
|