From 522f3f72b60141e28e162c0279c9c7401995cc3c Mon Sep 17 00:00:00 2001 From: KennySimpson <70093673+KennyMcSimpson@users.noreply.github.com> Date: Tue, 6 Oct 2026 16:14:55 +0800 Subject: [PATCH 1/2] fix(sleep): read config files as UTF-8 --- skillopt_sleep/config.py | 4 +- tests/test_sleep_config_encoding.py | 60 +++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 tests/test_sleep_config_encoding.py diff --git a/skillopt_sleep/config.py b/skillopt_sleep/config.py index d4a008d1f..d502dd53f 100644 --- a/skillopt_sleep/config.py +++ b/skillopt_sleep/config.py @@ -204,11 +204,11 @@ def _load_file(path: str) -> Dict[str, Any]: if path.endswith((".yaml", ".yml")): try: import yaml # optional - with open(path) as f: + with open(path, encoding="utf-8") as f: return yaml.safe_load(f) or {} except Exception: return {} - with open(path) as f: + with open(path, encoding="utf-8") as f: return json.load(f) diff --git a/tests/test_sleep_config_encoding.py b/tests/test_sleep_config_encoding.py new file mode 100644 index 000000000..cd73a9d13 --- /dev/null +++ b/tests/test_sleep_config_encoding.py @@ -0,0 +1,60 @@ +"""Sleep configuration stays readable independently of the process locale.""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from skillopt_sleep import config + + +@pytest.mark.parametrize("suffix", ["json", "yaml", "yml"]) +def test_utf8_config_survives_a_non_utf8_locale(tmp_path: Path, suffix: str) -> None: + values = { + "backend": "codex", + "preferences": "Use résumé examples and 中文说明.", + "target_skill_path": "技能/SKILL.md", + } + path = tmp_path / f"config.{suffix}" + if suffix == "json": + text = json.dumps(values, ensure_ascii=False) + else: + yaml = pytest.importorskip("yaml") + text = yaml.safe_dump(values, allow_unicode=True) + path.write_text(text, encoding="utf-8") + + script = """ +import json +import sys +from skillopt_sleep import config + +config.HOME_STATE_DIR = sys.argv[1] +expected = json.loads(sys.argv[2]) +loaded = config.load_config() +for key, value in expected.items(): + assert loaded.get(key) == value, (key, loaded.get(key), value) +assert set(expected) <= set(loaded.get("_user_config_keys")) +overridden = config.load_config(backend="claude", preferences=None) +assert overridden.backend == "claude" +assert overridden.preferences == expected["preferences"] +""" + env = { + **os.environ, + "LC_ALL": "C", + "PYTHONUTF8": "0", + "PYTHONCOERCECLOCALE": "0", + } + root = str(Path(config.__file__).resolve().parent.parent) + env["PYTHONPATH"] = os.pathsep.join(filter(None, [root, env.get("PYTHONPATH")])) + result = subprocess.run( + [sys.executable, "-c", script, str(tmp_path), json.dumps(values)], + env=env, + capture_output=True, + text=True, + encoding="utf-8", + timeout=10, + ) + assert result.returncode == 0, result.stderr From 2a4099b6c18b69f27a65f533fad25ffc7e0e9cf6 Mon Sep 17 00:00:00 2001 From: KennySimpson <70093673+KennyMcSimpson@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:46:28 +0800 Subject: [PATCH 2/2] fix(sleep): preserve legacy config limits and reject unreadable settings --- docs/sleep/README.md | 14 +++ skillopt_sleep/__main__.py | 8 +- skillopt_sleep/config.py | 46 +++++++-- tests/test_sleep_config_encoding.py | 152 ++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 13 deletions(-) diff --git a/docs/sleep/README.md b/docs/sleep/README.md index ba84923c3..662c0d592 100644 --- a/docs/sleep/README.md +++ b/docs/sleep/README.md @@ -179,6 +179,20 @@ One engine, thin per-agent shells (see [`plugins/`](https://github.com/microsoft | **Devin** | [`plugins/devin`](https://github.com/microsoft/SkillOpt/tree/main/plugins/devin) | register `plugins/devin/mcp_server.py` as an MCP server | | **OpenClaw** | [`plugins/openclaw`](https://github.com/microsoft/SkillOpt/tree/main/plugins/openclaw) | adapt the reference wrapper and paths for your installation | +### Configuration files + +Sleep reads `~/.skillopt-sleep/config.json`, falling back to `config.yaml` or +`config.yml` when the earlier names are absent. YAML requires PyYAML. +Save new files as UTF-8. Existing files written with the system's legacy +encoding (for example, GBK or cp1252) are still readable under that locale: +Sleep tries UTF-8 first, then the system encoding, and warns when the latter is +used. Open the file with its original encoding and save it as UTF-8 before +moving it to a machine with a different locale. Sleep does not rewrite it. + +An existing file that cannot be decoded, parsed, or read stops the command +with a configuration error and exit code 2; it does not discard your settings +and run with default budgets. Explicit CLI flags still override loaded values. + ### VS Code GitHub Copilot Chat Use `--source copilot` to harvest local VS Code GitHub Copilot Chat sessions. diff --git a/skillopt_sleep/__main__.py b/skillopt_sleep/__main__.py index e3b7794c6..a825aba37 100644 --- a/skillopt_sleep/__main__.py +++ b/skillopt_sleep/__main__.py @@ -33,7 +33,7 @@ from typing import Any, Dict from skillopt_sleep.backend import CursorBackendError -from skillopt_sleep.config import load_config +from skillopt_sleep.config import ConfigError, load_config from skillopt_sleep.cycle import _one_line_display_text, run_sleep_cycle from skillopt_sleep.harvest_sources import harvest_for_config from skillopt_sleep.mine import mine @@ -227,7 +227,11 @@ def _cfg_from_args(args, task_meta: Dict[str, Any] | None = None) -> Any: overrides["progress"] = True if getattr(args, "auto_adopt", False): overrides["auto_adopt"] = True - return load_config(**overrides) + try: + return load_config(**overrides) + except ConfigError as exc: + print(f"[sleep] configuration error: {exc}", file=sys.stderr) + raise SystemExit(2) from exc def cmd_run(args, dry: bool = False) -> int: diff --git a/skillopt_sleep/config.py b/skillopt_sleep/config.py index d502dd53f..64814979d 100644 --- a/skillopt_sleep/config.py +++ b/skillopt_sleep/config.py @@ -12,7 +12,9 @@ from __future__ import annotations import json +import locale import os +import warnings from dataclasses import dataclass, field from typing import Any, Dict, Optional @@ -200,16 +202,35 @@ def _user_config_path() -> Optional[str]: return None +class ConfigError(ValueError): + """An existing configuration could not be loaded safely.""" + + def _load_file(path: str) -> Dict[str, Any]: + try: + with open(path, encoding="utf-8") as f: + text = f.read() + except UnicodeDecodeError: + # Older versions read using the process locale. Preserve that + # upgrade path without guessing unrelated encodings on a new machine. + encoding = locale.getencoding() if hasattr(locale, "getencoding") else locale.getpreferredencoding(False) + with open(path, encoding=encoding) as f: + text = f.read() + warnings.warn( + f"Sleep configuration {path!r} was read using the legacy {encoding} encoding; save it as UTF-8.", + UserWarning, + stacklevel=2, + ) if path.endswith((".yaml", ".yml")): - try: - import yaml # optional - with open(path, encoding="utf-8") as f: - return yaml.safe_load(f) or {} - except Exception: - return {} - with open(path, encoding="utf-8") as f: - return json.load(f) + import yaml # optional + data = yaml.safe_load(text) + else: + data = json.loads(text) + if data is None: + return {} + if not isinstance(data, dict): + raise ValueError("configuration must be a mapping") + return data def load_config(**overrides: Any) -> SleepConfig: @@ -218,11 +239,14 @@ def load_config(**overrides: Any) -> SleepConfig: path = _user_config_path() if path: try: - file_data = _load_file(path) or {} + file_data = _load_file(path) user_keys.update(file_data.keys()) data.update(file_data) - except Exception: - pass + except Exception as exc: + raise ConfigError( + f"Cannot load sleep configuration {path!r}. Check its format and permissions, " + "and save it as UTF-8 (or restore the original system locale). YAML requires PyYAML." + ) from exc for key, value in overrides.items(): if value is not None: data[key] = value diff --git a/tests/test_sleep_config_encoding.py b/tests/test_sleep_config_encoding.py index cd73a9d13..2b2ec00d7 100644 --- a/tests/test_sleep_config_encoding.py +++ b/tests/test_sleep_config_encoding.py @@ -1,6 +1,8 @@ """Sleep configuration stays readable independently of the process locale.""" +import builtins import json +import locale import os import subprocess import sys @@ -15,6 +17,8 @@ def test_utf8_config_survives_a_non_utf8_locale(tmp_path: Path, suffix: str) -> None: values = { "backend": "codex", + "max_tokens_per_night": 1000, + "max_tasks_per_night": 2, "preferences": "Use résumé examples and 中文说明.", "target_skill_path": "技能/SKILL.md", } @@ -58,3 +62,151 @@ def test_utf8_config_survives_a_non_utf8_locale(tmp_path: Path, suffix: str) -> timeout=10, ) assert result.returncode == 0, result.stderr + + +@pytest.fixture(params=[(codec, suffix) for codec in ("gbk", "cp1252") for suffix in ("json", "yaml", "yml")]) +def legacy_config(request, tmp_path, monkeypatch): + """Simulate the old locale codec on Linux; this is not a native Windows test.""" + codec, suffix = request.param + text = "中文规则" if codec == "gbk" else "résumé rules" + values = { + "backend": "mock", + "max_tokens_per_night": 1000, + "max_tasks_per_night": 2, + "preferences": text, + "target_skill_path": f"skills/{text}/SKILL.md", + } + path = tmp_path / f"config.{suffix}" + if suffix == "json": + content = json.dumps(values, ensure_ascii=False) + else: + yaml = pytest.importorskip("yaml") + content = yaml.safe_dump(values, allow_unicode=True) + path.write_bytes(content.encode(codec)) + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + monkeypatch.setattr(locale, "getencoding", lambda: codec, raising=False) + monkeypatch.setattr(locale, "getpreferredencoding", lambda _do_setlocale=True: codec) + + # Also reproduce default open() for an unchanged, pre-UTF-8 loader. + def locale_open(file, mode="r", **kwargs): + if kwargs.get("encoding") is None and "b" not in mode: + kwargs["encoding"] = codec + return builtins.open(file, mode, **kwargs) + + monkeypatch.setattr(config, "open", locale_open, raising=False) + return path, values, codec + + +def test_legacy_config_preserves_explicit_limits_and_targets(legacy_config, recwarn): + path, values, codec = legacy_config + original = path.read_bytes() + loaded = config.load_config() + for key, value in values.items(): + assert loaded.get(key) == value + assert set(values) <= set(loaded.get("_user_config_keys")) + assert path.read_bytes() == original + assert any(codec in str(w.message) and "UTF-8" in str(w.message) for w in recwarn) + + +def test_cli_overrides_legacy_config_without_losing_other_limits(legacy_config, monkeypatch, recwarn): + from skillopt_sleep import __main__ as cli + + path, values, _codec = legacy_config + original = path.read_bytes() + target = path.parent / "override" / "SKILL.md" + captured = [] + + # Use the real argument parser and config assembly, without starting a cycle. + def capture_run(args, dry=False): + captured.append(cli._cfg_from_args(args)) + return 0 + + monkeypatch.setattr(cli, "cmd_run", capture_run) + assert cli.main([ + "dry-run", "--max-tasks", "1", "--preferences", "CLI rules", + "--target-skill-path", str(target), "--backend", "mock", + ]) == 0 + loaded = captured[0] + assert loaded.max_tasks_per_night == 1 + assert loaded.max_tokens_per_night == values["max_tokens_per_night"] + assert loaded.preferences == "CLI rules" + assert loaded.target_skill_path == str(target) + assert set(values) <= set(loaded.get("_user_config_keys")) + assert path.read_bytes() == original + + +@pytest.mark.parametrize("suffix", ["json", "yaml", "yml"]) +@pytest.mark.parametrize("failure", ["encoding", "syntax", "shape"]) +def test_unreadable_config_does_not_fall_back_to_defaults(tmp_path, monkeypatch, suffix, failure): + path = tmp_path / f"config.{suffix}" + if failure == "encoding": + content = b"\xff\xfeinvalid" + elif failure == "syntax": + content = b'{"max_tokens_per_night":' if suffix == "json" else b"max_tokens_per_night: [" + else: + content = b"[]" if suffix == "json" else b"- not a mapping\n" + path.write_bytes(content) + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + monkeypatch.setattr(locale, "getencoding", lambda: "ascii", raising=False) + monkeypatch.setattr(locale, "getpreferredencoding", lambda _do_setlocale=True: "ascii") + with pytest.raises(ValueError, match="Cannot load sleep configuration") as exc: + config.load_config(max_tasks_per_night=1) + assert str(path) in str(exc.value) + assert "UTF-8" in str(exc.value) + assert path.read_bytes() == content + + +@pytest.mark.parametrize("suffix", ["json", "yaml", "yml"]) +def test_cli_stops_before_a_cycle_on_unreadable_config(tmp_path, monkeypatch, capsys, suffix): + from skillopt_sleep import __main__ as cli + + path = tmp_path / f"config.{suffix}" + content = b"\xff\xfeinvalid" + path.write_bytes(content) + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + monkeypatch.setattr(locale, "getencoding", lambda: "ascii", raising=False) + monkeypatch.setattr(locale, "getpreferredencoding", lambda _do_setlocale=True: "ascii") + + def unexpected_cycle(*args, **kwargs): + pytest.fail("a configuration error must stop before running a cycle") + + monkeypatch.setattr(cli, "run_sleep_cycle", unexpected_cycle) + with pytest.raises(SystemExit) as exc: + cli.main(["run", "--backend", "mock", "--max-tasks", "1"]) + assert exc.value.code == 2 + stderr = capsys.readouterr().err + assert str(path) in stderr + assert "UTF-8" in stderr + assert "Traceback" not in stderr + assert path.read_bytes() == content + assert list(tmp_path.iterdir()) == [path] + + +def test_missing_yaml_dependency_is_not_silently_ignored(tmp_path, monkeypatch): + path = tmp_path / "config.yaml" + path.write_text("max_tokens_per_night: 1000\n", encoding="utf-8") + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + real_import = builtins.__import__ + + def without_yaml(name, *args, **kwargs): + if name == "yaml": + raise ImportError("PyYAML unavailable") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", without_yaml) + with pytest.raises(ValueError, match="PyYAML"): + config.load_config() + + +def test_absent_config_keeps_defaults_and_explicit_overrides(tmp_path, monkeypatch): + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + loaded = config.load_config(max_tasks_per_night=1) + assert loaded.max_tasks_per_night == 1 + assert loaded.max_tokens_per_night == config.DEFAULTS["max_tokens_per_night"] + + +def test_unreadable_config_path_does_not_fall_back_to_defaults(tmp_path, monkeypatch): + (tmp_path / "config.json").mkdir() + monkeypatch.setattr(config, "HOME_STATE_DIR", str(tmp_path)) + with pytest.raises(ValueError, match="Cannot load sleep configuration"): + config.load_config()