fix(preflight): read desktop changelog as UTF-8#9718
Conversation
|
Fresh downstream proof from reviewing #9720 on native Windows: with |
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for tightening this up. I reviewed the diff and also exercised the PR head locally at 69bb6fab325b76db01cd48cfaaf6d9782fc9e376:
python3 .github/scripts/test_desktop_changelog.pypassed (2 tests)python3 -m py_compile .github/scripts/desktop-changelog.py .github/scripts/test_desktop_changelog.pypassedpython3 .github/scripts/desktop-changelog.py validatepassed
The change is narrow and looks directionally correct: explicitly pinning the desktop changelog read/write boundary to UTF-8 fixes the host-locale failure mode without changing product behavior, and the new contract test covers both the read and write paths. Registering it in the manifest is also appropriate for preventing regression.
I’m leaving this as a positive maintainer signal rather than a formal approval because it touches repo check/preflight manifest/tooling paths, which this automation treats as workflow-sensitive and leaves for a human maintainer to approve/merge.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
|
Hey @tianmind-studio 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Before your next PR, please skim:
If this was declined for direction or taste, maintainers should cite an invariant ID or open a proposed one — ask if that citation is missing. Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! |
Summary
Fixes #9717.
Root cause
desktop-changelog.pyrelied on the implicit encoding ofPath.read_text()andPath.write_text(). Native Windows Python uses the process ANSI code page when no encoding is supplied, so valid UTF-8 changelog content could raiseUnicodeDecodeErrorunder a GBK locale before validation started. The write path had the same host-dependent contract and could reject changelog text not representable in the active code page.Durable guard
The changelog boundary now always uses UTF-8 for both reads and writes. The regression tests provide path doubles that reject any omitted or non-UTF-8 encoding, and the shared check manifest runs those tests whenever the implementation or contract changes.
Real-path exercise
On native Windows Python, I cleared
PYTHONUTF8and ranpython .github/scripts/desktop-changelog.py validate. The old code failed with a GBKUnicodeDecodeError; the fixed command completed successfully against the repository's real changelog data.Product invariants affected
None. This changes repository tooling only and does not affect product behavior.
Validation
test_desktop_changelog.pyfailed both encoding tests before the production changepython .github/scripts/test_desktop_changelog.py-> 2 passedpython .github/scripts/desktop-changelog.py validatewith parentPYTHONUTF8unset -> passedpython .github/scripts/test_run_checks.py-> 9 passedpython -m py_compile .github/scripts/desktop-changelog.py .github/scripts/test_desktop_changelog.py-> passedmake preflightwith parentPYTHONUTF8unset -> all 9 selected checks passedgit diff --check origin/main...HEAD-> passed