fix(usage-ping): pin UTF-8 on the state reads and guard the class - #5160
Conversation
|
I reproduced it on a clean checkout of current The eight drift items are identical on Reproducing locally needs the repository npm dev dependencies (the test loads TypeScript semantics and I am deliberately not updating the manifest: #5141 already carries |
|
Follow-up to my earlier triage on this thread: the census drift is fixed on
I had opened #5163 to refresh it and have closed that: generated from |
huangruiteng
left a comment
There was a problem hiding this comment.
结论:APPROVE,未发现当前 exact head 的阻塞性代码问题。需要先处理与当前 main 的合并冲突,再对新 head 复验;本批准不是合并就绪证明。
动机
#4941 已证明结构化文件不能依赖 Windows 的 GBK 默认编码。usage-ping 新增的四个读取点重新引入了这一类问题,而且原有 subprocess guard 在基线直接报出 usage_ping.py:53。本 PR 的完整结果是修复这五个边界,并为文件文本 I/O 建立回归约束;不是宣称关闭整个 Windows issue。
改动思路
继续让 TypeScript 的 usage_statistics 模块拥有状态校验、consent、generation、锁和发送权限,Python 只把已由 Node 写成 UTF-8 的字节正确解码。与其增加第二个状态加载器或重写权限判断,直接修正原有调用点更小、更容易回滚。新增 AST guard 复用仓库 subprocess guard 的维护模式,限制在已出现的 pathlib/builtin open 形状,不增加运行时配置或自动加载的 Agent 指令。
具体改动
关键代码讲解
usage_ping.control(usage_ping.py:52):实际启动 Node,把 stdout 解码后交给 JSON parser;现在显式 UTF-8,并设置 replacement 错误策略。合法 Node JSON 输出没有观察到差异,损坏的任意子进程字节不在本次兼容证明之内。usage_ping.begin(:62):先读本机状态,再做 negative-only scheduling hint。只改读取 codec,notice、disabled、环境开关及 generation 的原有检查保持不变,最终授权仍由 TS 原子校验。observe_goal_execution(usage_goal.py:22)与observe_quota_cycle(:99):上下文检查点和 quota 观测都显式读取 UTF-8;失败仍不能替换业务返回结果,也没有新增同步 HTTP 等待。_dispatch_cycle(:154):脱离前台后的读取同样修正,generation mismatch/disabled 仍在生成观测前返回。测试文件新增的是编码 guard 和其负例 fixture,不是新的产品状态规则。
对主干的风险
我在不可变基线 420782f03725bf9b7603be481f2b0525beff5807 与 head 使用同一组真实文件及 Python→Node owner 用例:UTF-8/模拟 GBK、ASCII/合法 UTF-8 annotation、disabled/损坏 JSON、四个观测入口、非 ASCII invalid-policy 诊断和 disable 后独立 readback。head 36/36 通过;基线 30/36,GBK 下四个观测入口丢失观测/抛出 decode error、直接 disabled dispatch 的 decode error,以及 Node 控制回复解码失败均被复现。禁用状态未修改文件;ASCII、UTF-8、损坏状态及禁用恢复没有出现额外效果。模拟只替换默认 codec,读写真实文件、执行真实 Node owner;不是完整 Windows OS 资格证明。
仓库原生 focused pytest:head 61 passed;基线 58 passed/1 failed,唯一失败是现存 subprocess guard,本 PR 修复它。新增 guard 对基线精确报告四个文件读取点。TS usage 三个套件 33/33 通过;工作流的 Ruff、mypy(20 source files)和 diff check 均通过。没有查询或等待远端 CI;没有把作者关于红 CI 的说明当作独立归因证据。
当前已取回 main 74d372c4925c1043f1bb4725bea4fdc3e99790cc 含 bcde01707 的 UTF-8 control 回复修复,merge-tree 在 usage_ping.py 的 subprocess 行报告冲突。这是需 rebase 的集成条件,不是当前修改引起的测试回归。请保留 main 已有修复,合入四个读取点及 guard 后,对新 head 重跑上述检查;不要将本批准套到新提交。
我的整体评价
长期观测连续性和本机诊断体验改善,执行权限、业务状态、默认统计政策及关闭路线保持不变。复用既有 TS owner/现有状态词汇,没有新增 actor 或控制面协议;语义对齐为既有 UTF-8 边界修复。未来化简检查已做:五个调用点不值得再包装成一个 loader,两个静态 guard 保护不同 I/O 形状,当前无需泛化框架。静态扫描不是类型分析;别名 open、动态 codec 等覆盖限制应保留说明。未运行全量跨平台套件或任意损坏 stdout 的兼容测试,合并冲突与新 head 的复验仍需解决。
English verdict: APPROVE - exact head 7f4a005; UTF-8 decoding and existing disabled/recovery paths are independently validated. Rebase the current-main conflict and requalify the new head before merge.
The usage-ping state file is written by Node as UTF-8 and read back through `Path.read_text()` without a codec, and the control call reads the Node process with `text=True` and no codec. Both use `locale.getpreferredencoding(False)`, which is `cp936` on a zh-CN Windows host, so one non-ASCII character in the state either raises `UnicodeDecodeError` or is decoded as mojibake - the failure mode loopx-project#4338, loopx-project#4942 and loopx-project#4997 fixed elsewhere. `tests/test_runtime_subprocess_utf8.py` is red on current main for exactly this reason (`loopx/usage_ping.py:53`); it is green again with this change. The new guard covers the other half of the same class: text-mode file reads and writes under `loopx/` that do not pin UTF-8. It reports the four `read_text()` sites fixed here and nothing else, and its own fixture pins the narrowing that keeps a keyed `read_text("INSTALLER")` lookup from being reported as file I/O. Signed-off-by: kokokoXUY <13682395396@163.com> Rebased onto current main; the subprocess site was already pinned upstream.
7f4a005 to
be44e0e
Compare
Goal And Delivered Outcome
S12· #4941.The row states the gap directly: "main now has no bare
read_text()/write_text()underloopx/. The gap is that nothing stops the class from returning." Two things had happened sinceit was written: the class had returned in the usage-ping feature merged today, and there was
still no guard.
tests/test_runtime_subprocess_utf8.py::test_shipped_runtime_pins_utf8_for_every_text_mode_subprocess_callis red on current
mainwith['loopx\usage_ping.py:53']:subprocess.run(..., text=True)with no codec, decoding Node's UTF-8 stdout through
locale.getpreferredencoding(False).Path.read_text()calls read the usage-ping state JSON (written as UTF-8 by Node) with thesame locale codec:
loopx/usage_goal.py:29,:105,:156andloopx/usage_ping.py:71.cp936host: one non-ASCII character in the state either raisesUnicodeDecodeErroror yields mojibake, and a decoded-as-mojibakeconsent/generationvaluechanges what the scheduling hint decides.
encoding="utf-8", and the existing guard is green.loopx/without an explicit encoding —
tests/test_loopx_text_io_utf8.py. It reports zero offenders onthis branch and would have reported all four
read_text()sites before the fix.Scope And Continuation
decode bytes that were already UTF-8.
tests/test_runtime_subprocess_utf8.py, the sibling guard for text-modesubprocess reads, and modelled on its shape (module docstring stating the bug, a detector that takes
an optional package root, a repository assertion, and a fixture that pins the detector itself).
cp936hostand for [Bug]: Windows non-UTF-8 locale — Path text I/O decodes structured state files with the locale encoding #4941 to close. I ran this on a
cp936host, which is what surfaced the sites, but I am notthe reporter.
os.open/codecs.open/io.open. Those have zero occurrences underloopx/today, so adding themnow would be speculative surface with no call site to protect; the guard covers the two shapes the
class actually returned in.
Validation
unitpassedpython -m pytest -q tests/test_loopx_text_io_utf8.py tests/test_runtime_subprocess_utf8.py→ 6 passed. Before the source fix the same command was 2 failed, 4 passed (['loopx\usage_ping.py:53']from the existing guard, and the fourread_text()sites from the new one). Reverting only the five edits reproduces both failures.unitpassedpython -m pytest -q tests/test_usage_ping.py tests/test_usage_goal.py tests/control_plane/test_usage_summary.py→ 55 passed, so pinning the codec does not change the feature's behavior on a UTF-8 host.staticpassedloopx checkon the touched paths:errors=0. The guard's fixture pins its own narrowing, so a keyedread_text("INSTALLER")lookup (loopx/doctor.py:192) is not reported as file I/O.loopx/**/*.pywithastand checks the pinned codecvalue, not only the presence of the keyword, because an explicit
encoding="latin-1"reproduces thesame failure — the same reasoning
test_runtime_subprocess_utf8.pyalready documents.utf-8-sigis allowed and named in the module docstring: it is a UTF-8 codec that tolerates a BOM andis not locale-dependent.
proven UTF-8), and a non-
pathlibobject with aread_text/write_textmethod whose first argumentis a literal non-codec string is skipped by design.
Frontend / Visual Evidence
Not applicable: two runtime modules and one test file; no user-facing surface changes.