fix(platforms,gateway,tools): add encoding='utf-8' to write_text() calls - #54240
fix(platforms,gateway,tools): add encoding='utf-8' to write_text() calls#54240AlexFucuson9 wants to merge 1 commit into
Conversation
Path.write_text() without an explicit encoding uses the platform's default encoding. On Windows this is typically cp1252 or mbcs, which corrupts non-ASCII characters (emoji, CJK text, accented characters) in user-facing content. Fixed the most critical locations where user/agent text is written: - Platform adapters (discord, telegram, feishu): temp file for reply text - gateway/run.py: agent response temp file - gateway/delivery.py: message content output files - tools/skills_hub.py: skill cache with ensure_ascii=False (Unicode data) This follows the existing pattern used throughout the codebase where read_text() calls consistently specify encoding='utf-8'.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Mechanical fix adding explicit encoding='utf-8' to write_text() calls across platform adapters and core modules (7+ files). Same rationale as #54241 for read_text -- ensures consistent UTF-8 encoding on Windows.
Strengths:
- Consistent application across all identified call sites
- No behavioral change on Unix
- Prevents encoding mismatches when writing response files, cron output, and skill cache
No concerns.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real Windows portability issue. Current main still has the six unencoded writes this PR targets, including gateway/run.py:9040, the Discord/Feishu/Telegram update-response paths, gateway/delivery.py:360/:373, and the three skills-hub cache writes.
Problems
gateway/platforms/qqbot/adapter.py:1204implements the same atomic.update_responseanswer-file contract as the three platform adapters, but still usestmp.write_text(answer)without an encoding. Please include it so the update-response fix covers all current adapters.- The PR has no regression coverage for a non-ASCII response/cache value or explicit UTF-8 writer behavior.
Suggested changes
- Add
encoding="utf-8"atgateway/platforms/qqbot/adapter.py:1204. - Add a narrow UTF-8 regression test for an update-response path and a skills-hub cache path.
Automated hermes-sweeper review.
| response_path = home / ".update_response" | ||
| tmp = response_path.with_suffix(".tmp") | ||
| tmp.write_text(answer) | ||
| tmp.write_text(answer, encoding="utf-8") |
There was a problem hiding this comment.
Please include the equivalent QQBot update-response writer at gateway/platforms/qqbot/adapter.py:1204; it uses the same tmp.write_text(answer) pattern and remains vulnerable on Windows.
|
Closing as resolved by PR #71078 (merged, commit d372fda): the class-wide close-out salvaged your #50655/#54241/#56385/#66856/#65440 series as the backbone (authorship preserved in git log) and swept the remaining sites, so every read_text/write_text call this PR touches is now guarded on current main — verified per-site. A CI linter rule in check-windows-footguns.py plus the AST guard test now prevent regressions. Your overlapping/split variants of the same series are being closed together; the credit for the class rests on your commits. |
Problem
Path.write_text()without an explicitencodingparameter uses the platform's default encoding. On Windows this is typicallycp1252ormbcs, which corrupts non-ASCII characters (emoji, CJK text, accented characters) in user-facing content.This is a cross-platform data corruption bug — content written on Windows with non-ASCII characters will be silently corrupted, and reading it back with
read_text(encoding="utf-8")(which the codebase consistently uses) will produce garbled output or raiseUnicodeDecodeError.Affected Locations
plugins/platforms/discord/adapter.py:6209plugins/platforms/telegram/adapter.py:4808plugins/platforms/feishu/adapter.py:2103gateway/run.py:8004gateway/delivery.py:281,294tools/skills_hub.py(3 locations)ensure_ascii=FalseFix
Add
encoding="utf-8"to eachwrite_text()call:This follows the existing pattern where
read_text()calls consistently specifyencoding="utf-8"throughout the codebase.Impact