diff --git a/headroom/cli/learn.py b/headroom/cli/learn.py index 2b69d89c9..6d5a19981 100644 --- a/headroom/cli/learn.py +++ b/headroom/cli/learn.py @@ -236,9 +236,15 @@ def learn( click.echo(f"Path: {proj.project_path}") click.echo(f"{'=' * 60}") - sessions = plugin.scan_project( - proj, max_workers=max_workers, include_subagents=not main_only - ) + try: + sessions = plugin.scan_project( + proj, max_workers=max_workers, include_subagents=not main_only + ) + except Exception as exc: + # One unreadable agent/project must not abort the whole + # cross-agent run; skip it with a warning and continue. + click.echo(f" Skipping (could not scan sessions): {exc}") + continue if not sessions: click.echo(" No conversation data found.") continue diff --git a/headroom/learn/analyzer.py b/headroom/learn/analyzer.py index e939aea3c..110ba6046 100644 --- a/headroom/learn/analyzer.py +++ b/headroom/learn/analyzer.py @@ -198,7 +198,7 @@ def _build_prior_patterns_section(project: ProjectInfo) -> str: for label, path in candidates: if path is None or not path.exists(): continue - block = extract_marker_block(path.read_text()) + block = extract_marker_block(path.read_text(encoding="utf-8", errors="replace")) if block: parts.append((label, block)) @@ -486,6 +486,8 @@ def _call_cli_llm(digest: str, model: str) -> dict: input=prompt, capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=hard_cap, ) except FileNotFoundError: @@ -541,6 +543,8 @@ def _call_claude_cli_streaming( stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + encoding="utf-8", + errors="replace", bufsize=1, # line-buffered ) except FileNotFoundError: diff --git a/headroom/learn/plugins/claude.py b/headroom/learn/plugins/claude.py index 8cc5f7dcf..c790bfdf1 100644 --- a/headroom/learn/plugins/claude.py +++ b/headroom/learn/plugins/claude.py @@ -167,7 +167,7 @@ class ClaudeCodePlugin(LearnPlugin, ConversationScanner): msg_index = 0 try: - with open(jsonl_path) as f: + with open(jsonl_path, encoding="utf-8", errors="replace") as f: for line in f: try: d = json.loads(line) diff --git a/headroom/learn/plugins/codex.py b/headroom/learn/plugins/codex.py index 8b70ae748..4908aa74d 100644 --- a/headroom/learn/plugins/codex.py +++ b/headroom/learn/plugins/codex.py @@ -126,7 +126,7 @@ class CodexPlugin(LearnPlugin, ConversationScanner): def _scan_json_session(self, json_path: Path) -> SessionData | None: """Parse a single Codex session file.""" try: - with open(json_path) as f: + with open(json_path, encoding="utf-8", errors="replace") as f: data = json.load(f) except (OSError, json.JSONDecodeError) as e: logger.debug("Failed to read Codex session %s: %s", json_path, e) @@ -209,7 +209,7 @@ class CodexPlugin(LearnPlugin, ConversationScanner): msg_index = 0 try: - with open(jsonl_path) as f: + with open(jsonl_path, encoding="utf-8", errors="replace") as f: for line in f: try: entry = json.loads(line) diff --git a/headroom/learn/plugins/gemini.py b/headroom/learn/plugins/gemini.py index 7f8a1b901..15e589a68 100644 --- a/headroom/learn/plugins/gemini.py +++ b/headroom/learn/plugins/gemini.py @@ -143,7 +143,7 @@ class GeminiPlugin(LearnPlugin, ConversationScanner): def _scan_json_session(self, json_path: Path) -> SessionData | None: """Parse a Gemini JSON session file.""" try: - with open(json_path) as f: + with open(json_path, encoding="utf-8", errors="replace") as f: data = json.load(f) except (OSError, json.JSONDecodeError) as e: logger.debug("Failed to read Gemini session %s: %s", json_path, e) @@ -167,7 +167,7 @@ class GeminiPlugin(LearnPlugin, ConversationScanner): messages: list[dict] = [] try: - with open(jsonl_path) as f: + with open(jsonl_path, encoding="utf-8", errors="replace") as f: for line in f: try: entry = json.loads(line) @@ -314,7 +314,7 @@ class GeminiPlugin(LearnPlugin, ConversationScanner): def _detect_project_path(self, session_path: Path) -> Path | None: """Try to detect the project path from a session file.""" try: - with open(session_path) as f: + with open(session_path, encoding="utf-8", errors="replace") as f: data = json.load(f) except (OSError, json.JSONDecodeError): return None diff --git a/headroom/learn/writer.py b/headroom/learn/writer.py index ddad3e529..ade64a02a 100644 --- a/headroom/learn/writer.py +++ b/headroom/learn/writer.py @@ -26,6 +26,23 @@ _MARKER_PATTERN = re.compile( ) +def _read_text_tolerant(file_path: Path) -> str: + """Read an existing context file that we are about to rewrite as UTF-8. + + These files are predominantly valid UTF-8 but may carry a stray legacy + byte (e.g. a cp1252 em-dash ``0x97``). Strict UTF-8 decoding aborts the + whole ``--apply`` on a single such byte, so fall back to UTF-8 with + replacement: this preserves the valid UTF-8 content — a full-file cp1252 + fallback would instead turn every genuine UTF-8 em-dash into mojibake — + and the subsequent ``write_text(encoding="utf-8")`` self-heals the file. + """ + raw = file_path.read_bytes() + try: + return raw.decode("utf-8") + except UnicodeDecodeError: + return raw.decode("utf-8", errors="replace") + + # ============================================================================= # Abstract Writer # ============================================================================= @@ -153,7 +170,7 @@ def _merge_recommendations( """ if not file_path.exists(): return new_recommendations - prior = _parse_prior_recommendations(file_path.read_text(encoding="utf-8")) + prior = _parse_prior_recommendations(_read_text_tolerant(file_path)) if not prior: return new_recommendations new_sections = {r.section for r in new_recommendations} @@ -166,7 +183,7 @@ def _merge_into_file(file_path: Path, new_recommendations: list[Recommendation]) merged = _merge_recommendations(file_path, new_recommendations) section = _build_section(merged) if file_path.exists(): - existing = file_path.read_text(encoding="utf-8") + existing = _read_text_tolerant(file_path) if _MARKER_START in existing: return _MARKER_PATTERN.sub(lambda _match: section, existing) return existing.rstrip() + "\n\n" + section + "\n" diff --git a/tests/test_learn/test_plugin_encoding.py b/tests/test_learn/test_plugin_encoding.py new file mode 100644 index 000000000..6c15ea7ae --- /dev/null +++ b/tests/test_learn/test_plugin_encoding.py @@ -0,0 +1,51 @@ +"""Regression tests for #1202 — the ``learn`` session scanners must read agent +transcripts as UTF-8 with replacement, so a stray non-UTF-8 byte cannot abort +(or silently drop) a scan. + +``0x9d`` is undefined in cp1252 *and* an invalid UTF-8 start byte, so a bare +``open()`` fails on it regardless of the host locale. Before the fix this made +the Codex JSONL scanner raise ``UnicodeDecodeError`` (the scan caught only +``OSError``), aborting the whole cross-agent run, while the Claude scanner +caught it and silently dropped the session. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from headroom.learn.models import SessionData +from headroom.learn.plugins.claude import ClaudeCodePlugin +from headroom.learn.plugins.codex import CodexPlugin + + +def _stray_byte_line() -> bytes: + # A line that is neither valid UTF-8 nor decodable in cp1252. + return b"\x9d arrow \xe2\x86\x92 junk\n" + + +def test_claude_scan_recovers_session_with_stray_byte(tmp_path: Path) -> None: + jsonl = tmp_path / "session.jsonl" + valid = json.dumps( + {"type": "assistant", "message": {"usage": {"input_tokens": 5}}, "text": "em — arrow →"} + ) + jsonl.write_bytes(valid.encode() + b"\n" + _stray_byte_line()) + + result = ClaudeCodePlugin(claude_dir=tmp_path)._scan_session(jsonl) + + # Before the fix this returned None (session silently dropped); now the + # valid line is read and the stray-byte line is skipped, not fatal. + assert result is not None + assert result.session_id == "session" + assert result.total_input_tokens == 5 + + +def test_codex_jsonl_scan_does_not_crash_on_stray_byte(tmp_path: Path) -> None: + jsonl = tmp_path / "rollout.jsonl" + meta = json.dumps({"type": "session_meta", "payload": {"id": "abc"}}) + jsonl.write_bytes(meta.encode() + b"\n" + _stray_byte_line()) + + # Before the fix this raised UnicodeDecodeError and aborted the run. + result = CodexPlugin()._scan_jsonl_session(jsonl) + + assert result is None or isinstance(result, SessionData) diff --git a/tests/test_learn/test_writer.py b/tests/test_learn/test_writer.py index 80b98ad40..8e4ef956a 100644 --- a/tests/test_learn/test_writer.py +++ b/tests/test_learn/test_writer.py @@ -2,6 +2,8 @@ from pathlib import Path +import pytest + from headroom.learn.models import ProjectInfo, Recommendation, RecommendationTarget from headroom.learn.writer import ( _MARKER_END, @@ -9,6 +11,7 @@ from headroom.learn.writer import ( ClaudeCodeWriter, _merge_into_file, _parse_prior_recommendations, + _read_text_tolerant, extract_marker_block, ) @@ -308,3 +311,41 @@ class TestExtractMarkerBlock: block = extract_marker_block(content) assert block is not None assert block == f"{_MARKER_START}\n{_MARKER_END}" + + +class TestEncodingResilience: + """Regression tests for #1202 — ``learn --apply`` must not crash merging into + an existing context file that carries a stray non-UTF-8 byte (e.g. a legacy + cp1252 em-dash ``0x97``).""" + + def test_read_text_tolerant_preserves_valid_utf8(self, tmp_path): + path = tmp_path / "AGENTS.md" + path.write_text("Use em-dashes — and arrows →.", encoding="utf-8") + assert _read_text_tolerant(path) == "Use em-dashes — and arrows →." + + def test_read_text_tolerant_survives_stray_legacy_byte(self, tmp_path): + # Predominantly valid UTF-8 (genuine em-dash E2 80 94) plus one stray + # cp1252 em-dash byte (0x97) that strict UTF-8 cannot decode. + path = tmp_path / "AGENTS.md" + path.write_bytes("real em-dash — here\n".encode() + b"legacy \x97 byte\n") + + # The old strict read aborts the whole --apply on that single byte. + with pytest.raises(UnicodeDecodeError): + path.read_text(encoding="utf-8") + + text = _read_text_tolerant(path) + # Valid UTF-8 content is preserved (no cp1252 "â€" mojibake) and the + # stray byte is replaced rather than fatal. + assert "real em-dash — here" in text + assert "\x97" not in text + assert "â€" not in text + + def test_merge_into_file_applies_over_file_with_stray_byte(self, tmp_path): + path = tmp_path / "AGENTS.md" + path.write_bytes("# Notes — existing\n".encode() + b"stray \x97 byte\n") + recs = [_rec(RecommendationTarget.CONTEXT_FILE, "Environment", "- Use uv")] + + merged = _merge_into_file(path, recs) + + assert "Use uv" in merged + assert "Notes — existing" in merged