Skip to content

octo: preserve UTF-8 text and LF bytes on Windows - #195

Open
jscjscjscjscjsc wants to merge 1 commit into
OctoSense-org:mainfrom
jscjscjscjscjsc:codex/octo-utf8-lf-20261014
Open

jscjscjscjscjsc wants to merge 1 commit into
OctoSense-org:mainfrom
jscjscjscjscjsc:codex/octo-utf8-lf-20261014

Conversation

@jscjscjscjscjsc

@jscjscjscjscjsc jscjscjscjscjsc commented Oct 14, 2026 •

Copy link
Copy Markdown

octo relies on the OS default encoding for several UTF-8 templates, manifests, listings and source files. On Windows with CP936, an app containing 晨报📰… can fail to decode, get rewritten incorrectly, or produce CRLF instead of LF. Publishing a Unicode manifest also fails, and re-reading a generated UTF-8 workflow can incorrectly defeat the idempotence check. Signed manifests should reach the existing sealed-release rejection rather than a decoding error.

Specify UTF-8 for format-defined text reads and add write_utf8() using Path.open(encoding="utf-8", newline="\n"), compatible with Python 3.9. Preserve signed bundle bytes during checking, and preserve the refusal to run sealed releases. Decode the JSON-compatible SDK path string in one existing TOML test assertion so a Unicode workspace is compared correctly.

Validation on Windows / Python 3.12.10 against d0344682:

  • Four new real-file regression tests simulate default CP936 and CRLF only when the caller omits the encoding/newline. Before: 1 failure and 3 errors. After: all 4 pass.
  • Coverage: Unicode source byte preservation in new, UTF-8/LF workflow generation and repeat-publish idempotence, checking without restamping signed manifests, and sealed-release refusal before starting a host.
  • Broader CLI suite in an exported Unicode-path fixture: 39 tests; the same two release-shell fixture failures and six skips before and after using the corrected SDK assertion. The full suite is not claimed green.
  • Combined with the separate publisher-root guard PR octo: refuse nested publisher layouts before generating workflows #194: 4 encoding tests pass; publishing suite 16 tests, OK (skipped=3) (Windows symlink privileges). The two independent PRs touch adjacent lines in cmd_publish_github; the tested resolution keeps validate_publisher_root(app) followed by the UTF-8 manifest read. One PR needs that small rebase after the other merges.
  • git diff --check passes. No native host build, GUI validation, real Actions workflow execution, tag, release, source-pin or signing changes are claimed.

Team / 项目归属

Submitted by the 共创队(晨报卡 / Morning Brief;报名 ID:小鞠老师), via GitHub account jscjscjscjscjsc. Project repository: jscjscjscjscjsc/morning-brief. This contribution comes from issues encountered and fixes developed collaboratively while building the Morning Brief hackathon project.

本 PR 为晨报卡共创团队的上游贡献,关联项目仓库及已有修复记录;提交账号是团队贡献的提交入口。团队归属不代表主办方已经确认加分,上游合并状态以本 PR 为准。

@BH3GEI BH3GEI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together — a clean, tightly scoped fix for a real Windows authoring gap.

Verified on macOS against d0344682 with Python 3.12.13 (CI's version):

  • Regression honesty: base + the new test file → FAILED (failures=1, errors=3), exactly the before-state described; PR head → 4/4 pass. The CP936/CRLF simulation is faithful: pathlib's read_text/open resolve io.open/io.text_encoding as module attributes at call time, so both patches intercept, and the wrapper only injects defaults when the caller omits encoding/newline. Deleting just newline="\n" from write_utf8 makes the LF assertion fail, so that argument is load-bearing, not decorative.
  • Real CLI differential (no mocks, PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 LC_ALL=C → US-ASCII default): octo publish-github on a Unicode manifest dies with "'ascii' codec can't decode byte 0xe6" on base (exit 1) and installs the workflow with LF-only bytes on head (exit 0); a second run prints "already current" and leaves the mtime untouched. octo run on a Unicode signed manifest now reaches "card-host cannot run a sealed release" instead of a codec error, before any binary lookup.
  • Full tools suite: base 50/50 OK (6 skips), head 54/54 OK (same 6 skips) — no regressions. Every format-text read/write in tools/octo now carries explicit UTF-8/LF; the one remaining bare text open (octo:467, card-host log) is subprocess stdout only and is read back with explicit UTF-8 + errors="replace".
  • The test_octo.py assertion change is correct: sdk_dependency writes the path JSON-escaped, so decoding it with json.loads is the right comparison (and the only workable one for Windows backslash paths).

Two non-blocking observations for possible follow-ups (neither blocks this PR):

  • subprocess.run(..., text=True) sites (octo:557, 571, 1041; wasm_component.py:776) still decode hub/cargo output with the locale encoding, so on CP936 Windows a non-ASCII crate path can still mojibake or raise in octo check/wasm build. Pre-existing and outside this diff's declared scope.
  • cmd_check reads the manifest/listing (octo:552, 584) outside try/except; a bundle authored in the machine locale that isn't valid UTF-8 now ends in a raw traceback. Such a bundle could never pass hub check anyway (the Rust side parses JSON as UTF-8), so failing early is defensible — wrapping those reads in the usual die() would just be friendlier.

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