From 129d249b4ab86cf247a1840b1cefcbb063316721 Mon Sep 17 00:00:00 2001 From: PerryLink Date: Mon, 5 Oct 2026 23:33:51 +0800 Subject: [PATCH 1/2] fix(sleep): read and write state.json as UTF-8 --- skillopt_sleep/state.py | 4 +-- tests/test_sleep_state.py | 65 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/skillopt_sleep/state.py b/skillopt_sleep/state.py index 267adedd5..73eea093b 100644 --- a/skillopt_sleep/state.py +++ b/skillopt_sleep/state.py @@ -48,7 +48,7 @@ def __init__(self, path: str, data: Optional[Dict[str, Any]] = None) -> None: def load(cls, path: str) -> "SleepState": if os.path.exists(path): try: - with open(path) as f: + with open(path, encoding="utf-8") as f: data = json.load(f) merged = copy.deepcopy(DEFAULT_STATE) merged.update(data if isinstance(data, dict) else {}) @@ -60,7 +60,7 @@ def load(cls, path: str) -> "SleepState": def save(self) -> None: os.makedirs(os.path.dirname(self.path), exist_ok=True) tmp = self.path + ".tmp" - with open(tmp, "w") as f: + with open(tmp, "w", encoding="utf-8") as f: json.dump(self.data, f, ensure_ascii=False, indent=2) os.replace(tmp, self.path) diff --git a/tests/test_sleep_state.py b/tests/test_sleep_state.py index ef3f91510..1a6022c0a 100644 --- a/tests/test_sleep_state.py +++ b/tests/test_sleep_state.py @@ -1,5 +1,9 @@ """Isolation regressions for SkillOpt-Sleep's in-process default state.""" +import json +import subprocess +import sys + from skillopt_sleep.state import SleepState @@ -14,3 +18,64 @@ def test_fresh_states_do_not_share_nested_default_containers(tmp_path): assert second.last_harvest_for("/project/one") is None assert second.data["history"] == [] assert second.task_archive() == [] + + +def test_state_file_round_trips_non_ascii_as_utf8(tmp_path): + """state.json is a JSON document, so it must be UTF-8 on every platform. + + ``save()`` calls ``json.dump(..., ensure_ascii=False)``, so any non-ASCII + lesson or project path lands in the file verbatim. Without an explicit + codec the file is written in the *locale* charset -- GBK on a Chinese + Windows box -- while ``load()`` reads it back with the same locale codec. + The pair survives on one machine, but the file is then unreadable under + UTF-8 (WSL, containers, CI, a different box, or any interpreter with + PYTHONUTF8=1), and because ``load()`` swallows the decode error it resets + every field silently: night counter, harvest cursors, history and the + cross-night memory all disappear without a warning. + """ + lesson = "\u4e2d\u6587\u6559\u8bad" # CJK is representable in GBK, so save() does not raise + project = "/home/\u4e2d\u6587/repo" + path = tmp_path / "state.json" + + state = SleepState.load(str(path)) + state.set_last_harvest(project, "2026-08-15T00:00:00") + state.set_slow_memory(lesson) + state.save() + + # The bytes on disk must be UTF-8 even when the locale codec is not. + on_disk = json.loads(path.read_bytes().decode("utf-8")) + assert on_disk["slow_memory"] == lesson + assert on_disk["last_harvest"] == {project: "2026-08-15T00:00:00"} + + # ... and the round-trip must preserve it. + reloaded = SleepState.load(str(path)) + assert reloaded.slow_memory == lesson + assert reloaded.last_harvest_for(project) == "2026-08-15T00:00:00" + + +def test_state_io_does_not_fall_back_to_the_locale_codec(): + """Guard for the test above, which cannot fail on a UTF-8 CI runner. + + ``-X warn_default_encoding`` makes CPython emit ``EncodingWarning`` for + every ``open()``/``Path.open()`` that leaves the codec to the locale, so + promoting that warning to an error detects the defect on every platform. + """ + script = ( + "from skillopt_sleep.state import SleepState\n" + "import tempfile, os\n" + "with tempfile.TemporaryDirectory() as tmp:\n" + " state = SleepState.load(os.path.join(tmp, 'state.json'))\n" + " state.set_slow_memory('\u4e2d\u6587\u6559\u8bad')\n" + " state.save()\n" + " assert SleepState.load(os.path.join(tmp, 'state.json')).slow_memory == '\u4e2d\u6587\u6559\u8bad'\n" + ) + proc = subprocess.run( + [sys.executable, "-X", "warn_default_encoding", "-W", "error::EncodingWarning", "-c", script], + capture_output=True, + text=True, + encoding="utf-8", + ) + assert proc.returncode == 0, ( + "skillopt_sleep/state.py relied on the locale encoding for state.json:\n" + f"{proc.stderr}" + ) From 9dc00d49f3a7e42623d1c72147c87dc9518acc82 Mon Sep 17 00:00:00 2001 From: PerryLink <255665900+PerryLink@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:58:57 +0800 Subject: [PATCH 2/2] fix(sleep): migrate locale-encoded state.json instead of discarding it The previous revision read and wrote state.json as UTF-8. That is right for new files, but not for existing ones: a state file written by an earlier release on a GBK or cp1252 box holds those bytes, so the UTF-8 read raised and the bare `except Exception: pass` turned the failure into a fresh state. The next save() then wrote over the original, and the night counter, the harvest cursors, the history and the cross-night memory were gone with no warning. Read the bytes and decode as UTF-8 first; on a UnicodeDecodeError fall back to the locale codec -- the one the old writer used on the machine that produced the file -- and let the next save() rewrite it as UTF-8, so the migration completes on the first write. A file that decodes under neither is now reported as StateFileError rather than replaced. The message names the path and says the file was left untouched. Continuing to return a fresh state is what makes the loss silent, because a reset state is indistinguishable from a first run. Tests: a legacy GBK fixture carrying an unknown field is loaded, saved and read back as UTF-8 with every field intact; an unreadable file raises while its bytes stay on disk. Both fail on the previous revision -- verified by running them against it -- and the locale codec is injected so the migration test is meaningful on a UTF-8 runner rather than passing by accident. test_sleep_state.py: 5 passed. tests/test_model_change_warning.py and tests/test_split_hardening_2x3.py are unchanged (4 failures there are a missing optional `llm_client` import, present before this change too). --- skillopt_sleep/state.py | 79 ++++++++++++++++++++++++++---- tests/test_sleep_state.py | 100 +++++++++++++++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 11 deletions(-) diff --git a/skillopt_sleep/state.py b/skillopt_sleep/state.py index 73eea093b..155339547 100644 --- a/skillopt_sleep/state.py +++ b/skillopt_sleep/state.py @@ -13,10 +13,70 @@ import copy import json +import locale import os from typing import Any, Dict, Optional +class StateFileError(RuntimeError): + """An existing state.json could not be read, so it must not be overwritten. + + ``load()`` used to swallow every failure and hand back a fresh state. The + caller then ran a night against a reset night counter and empty harvest + cursors, and the first ``save()`` replaced the file -- the original history + and cross-night memory were gone with no warning. Reporting the failure is + what keeps the file intact; the message names the path and the fix. + """ + + +def _locale_codec() -> str: + """The codec ``open()`` picks when no encoding is given. + + Writers before this change left the codec to the locale, so a state file + produced on a GBK or cp1252 box holds those bytes. Kept behind a function + so the migration can be tested on a runner whose locale is UTF-8. + """ + return locale.getpreferredencoding(False) + + +def _read_state(path: str) -> Dict[str, Any]: + """Read state.json, migrating files written under the old locale codec. + + UTF-8 is tried first, so files written since this change are read directly. + A file that is not valid UTF-8 falls back to the locale codec -- exactly + what the old writer used on the machine that produced it -- and the next + ``save()`` rewrites it as UTF-8, completing the migration on first write. + + Anything that decodes under neither is reported rather than discarded. + """ + with open(path, "rb") as f: + raw = f.read() + + try: + text = raw.decode("utf-8") + except UnicodeDecodeError as utf8_error: + codec = _locale_codec() + try: + text = raw.decode(codec) + except (UnicodeDecodeError, LookupError): + raise StateFileError( + f"{path} is not valid UTF-8 and does not decode as {codec!r} either, so it " + f"cannot be read. It has been left untouched. Move or delete it to start " + f"from a fresh state; it will not be overwritten while it is unreadable." + ) from utf8_error + + try: + data = json.loads(text) + except ValueError as json_error: + raise StateFileError( + f"{path} could not be parsed as JSON ({json_error}). It has been left untouched. " + f"Move or delete it to start from a fresh state; it will not be overwritten while " + f"it is unreadable." + ) from json_error + + return data if isinstance(data, dict) else {} + + def _now_iso(clock: Optional[float] = None) -> str: # caller passes a timestamp; we avoid importing time at module import import time as _t @@ -46,16 +106,15 @@ def __init__(self, path: str, data: Optional[Dict[str, Any]] = None) -> None: # io --------------------------------------------------------------------- @classmethod def load(cls, path: str) -> "SleepState": - if os.path.exists(path): - try: - with open(path, encoding="utf-8") as f: - data = json.load(f) - merged = copy.deepcopy(DEFAULT_STATE) - merged.update(data if isinstance(data, dict) else {}) - return cls(path, merged) - except Exception: - pass - return cls(path, copy.deepcopy(DEFAULT_STATE)) + if not os.path.exists(path): + return cls(path, copy.deepcopy(DEFAULT_STATE)) + # Raises StateFileError rather than returning a fresh state: a state + # that silently resets is indistinguishable from a first run, and the + # save() that follows would overwrite the file it failed to read. + data = _read_state(path) + merged = copy.deepcopy(DEFAULT_STATE) + merged.update(data) + return cls(path, merged) def save(self) -> None: os.makedirs(os.path.dirname(self.path), exist_ok=True) diff --git a/tests/test_sleep_state.py b/tests/test_sleep_state.py index 1a6022c0a..03eea4571 100644 --- a/tests/test_sleep_state.py +++ b/tests/test_sleep_state.py @@ -4,7 +4,10 @@ import subprocess import sys -from skillopt_sleep.state import SleepState +import pytest + +import skillopt_sleep.state as state_module +from skillopt_sleep.state import SleepState, StateFileError def test_fresh_states_do_not_share_nested_default_containers(tmp_path): @@ -79,3 +82,98 @@ def test_state_io_does_not_fall_back_to_the_locale_codec(): "skillopt_sleep/state.py relied on the locale encoding for state.json:\n" f"{proc.stderr}" ) + + +def test_legacy_locale_encoded_state_is_migrated_not_discarded(tmp_path, monkeypatch): + """A file written before this change must survive the upgrade. + + The old writer left the codec to the locale, so a state.json produced on a + GBK or cp1252 box holds those bytes. Reading it as UTF-8 fails, and the + old ``load()`` swallowed that failure and returned a fresh state -- which + the next ``save()`` then wrote over the original, losing the night counter, + the harvest cursors, the history and the cross-night memory. + + The locale codec is forced here so the test is meaningful on a UTF-8 CI + runner, where the real locale codec would read the fixture by accident. + """ + lesson = "\u4e2d\u6587\u6559\u8bad" + project = "/home/\u4e2d\u6587/repo" + legacy = { + "version": 1, + "night": 42, + "last_harvest": {project: "2026-08-15T00:00:00"}, + "slow_memory": lesson, + "history": [{"night": 41, "accepted": 3}], + "task_archive": [{"id": "kept"}], + "last_model_key": "anthropic::claude", + "last_model_key_format": 2, + # A field this version does not know about: the upgrade must not drop + # it, or a newer release reading the migrated file loses its own state. + "field_from_a_newer_release": {"keep": [1, 2, 3]}, + } + path = tmp_path / "state.json" + path.write_bytes(json.dumps(legacy, ensure_ascii=False, indent=2).encode("gbk")) + # The fixture has to be the thing under test: bytes that are not UTF-8. + raw = path.read_bytes() + assert max(raw) > 0x7F + with pytest.raises(UnicodeDecodeError): + raw.decode("utf-8") + + monkeypatch.setattr(state_module, "_locale_codec", lambda: "gbk") + + state = SleepState.load(str(path)) + assert state.night == 42 + assert state.slow_memory == lesson + assert state.last_harvest_for(project) == "2026-08-15T00:00:00" + assert state.data["history"] == [{"night": 41, "accepted": 3}] + assert state.task_archive() == [{"id": "kept"}] + assert state.last_model_key == "anthropic::claude" + assert state.data["field_from_a_newer_release"] == {"keep": [1, 2, 3]} + + # load-then-save completes the migration: the file is UTF-8 afterwards and + # still carries every field, known and unknown. + state.set_last_harvest("/other", "2026-08-16T00:00:00") + state.save() + + on_disk = json.loads(path.read_bytes().decode("utf-8")) + assert on_disk["night"] == 42 + assert on_disk["slow_memory"] == lesson + assert on_disk["history"] == [{"night": 41, "accepted": 3}] + assert on_disk["field_from_a_newer_release"] == {"keep": [1, 2, 3]} + assert on_disk["last_harvest"] == { + project: "2026-08-15T00:00:00", + "/other": "2026-08-16T00:00:00", + } + + # And the migrated file reads back without the legacy codec at all. + monkeypatch.setattr(state_module, "_locale_codec", lambda: "ascii") + assert SleepState.load(str(path)).night == 42 + + +def test_unreadable_state_is_reported_and_left_untouched(tmp_path, monkeypatch): + """A file that decodes under no codec must not be replaced silently. + + Returning a fresh state here is what turned a decode failure into data + loss: the caller cannot tell "first run" from "could not read", and the + next ``save()`` overwrites the file. The read has to fail loudly, and the + bytes have to still be there afterwards. + """ + path = tmp_path / "state.json" + original = b'{"night": 7, "slow_memory": "\xff\xfe"}' + path.write_bytes(original) + monkeypatch.setattr(state_module, "_locale_codec", lambda: "gbk") + + with pytest.raises(StateFileError) as excinfo: + SleepState.load(str(path)) + + assert str(path) in str(excinfo.value) # the message names the file to move + assert path.read_bytes() == original # and the file was not touched + + # A corrupt JSON body is reported the same way rather than discarded. + path.write_bytes(b"{not json") + with pytest.raises(StateFileError): + SleepState.load(str(path)) + assert path.read_bytes() == b"{not json" + + # A genuinely absent file is still an ordinary first run. + assert SleepState.load(str(tmp_path / "absent.json")).night == 0