Skip to content

fix(sleep): preserve config limits across encoding upgrades - #304

Open
KennySimpson (KennyMcSimpson) wants to merge 2 commits into
microsoft:mainfrom
KennyMcSimpson:fix/sleep-config-utf8
Open

KennySimpson (KennyMcSimpson) wants to merge 2 commits into
microsoft:mainfrom
KennyMcSimpson:fix/sleep-config-utf8

Conversation

@KennyMcSimpson

@KennyMcSimpson KennySimpson (KennyMcSimpson) commented Oct 6, 2026 •

Copy link
Copy Markdown

Summary

Sleep's default-encoding reader can drop UTF-8 settings under a non-UTF-8 locale. Reading only UTF-8 fixes that case but breaks existing GBK/cp1252 files: a configured 1,000-token limit can silently become 400,000.

Read UTF-8 first. On a decode failure, try the system's legacy encoding and warn that the file should be saved as UTF-8 before moving to another locale. The configuration file is left unchanged.

If an existing file cannot be read, decoded or parsed, has a non-mapping root, or requires unavailable PyYAML, raise a configuration error instead of discarding it. The CLI reports the path and how to fix it, then exits with code 2 before starting a cycle. Missing files still use defaults; explicit CLI flags take precedence over loaded values.

Validation

  • 30 focused configuration tests passed, including six legacy upgrade cases and six CLI precedence cases across GBK/cp1252 and JSON/YAML/YML.
  • 26 new regressions fail on the previous UTF-8-only commit (522f3f72b601) and pass with this update.
  • python -m pytest -q: 1,588 passed, 12 skipped; 359 subtests passed.
  • Ruff checks passed for all three changed Python files.
  • python -m mkdocs build --strict passed.

The legacy-codec cases simulate codecs on Linux; they are not native Windows test results. The existing UTF-8 subprocess controls also remain green.

Related: #302 concerns the separate state.json reader and writer.

@Yif-Yang

Copy link
Copy Markdown
Contributor

The UTF-8 configuration positive path is useful, but the October 6 review of 522f3f72b601 found a compatibility regression for existing non-UTF-8 configurations.

JSON/YAML/YML files that the baseline successfully reads under GBK or cp1252 fail after the UTF-8-only change. The swallowed decode failure silently discards explicit settings: in synthetic tests a 1000-token limit falls back to 400000, a two-task limit to 40, and explicit preferences/target_skill_path disappear. No paid calls were made and the file itself was not overwritten; the problem is running with unintended defaults.

The positive slice passed 16 tests. The independent upgrade slice had 6 failures across the two codecs and three formats, with 3 UTF-8 controls passing. This is a Linux codec simulation, not a native Windows claim.

Before merge, provide an explicit legacy migration/compatibility path or stop with an actionable configuration error. An explicitly requested file must not silently lose its budget and target restrictions. Please cover these upgrade cases as well as CLI precedence. #302 needs the equivalent protection for existing state; neither change should be merged with this regression left for a later follow-up.

@KennyMcSimpson KennySimpson (KennyMcSimpson) changed the title fix(sleep): read config files as UTF-8 fix(sleep): preserve config limits across encoding upgrades Oct 7, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants