fix(events): reject non-integer planner order at the replay adapter - #5033
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval). Reviewed exact head 76df0bb4e8b8c0eb20d1ad6a175f0e0cafd0f306, based on origin/main d42adb874d042a039e5846ea19c4e3acba04b655. This closes the one still-open finding from the earlier REQUEST_CHANGES review of #5014 (head 552654d0e), which the rebase-only integration of that PR did not repair.
动机
已复核该 finding 在合并后的 main 上仍然成立:build_state_projection 在把事实交给 typed fold 之前先做 int(order),于是 planner_order=1.5 被截断成 1——fold 按 1 排序,投影却仍把 1.5 报给调用方。当前 main 上的真实入口复现结果:
{"outcome": "accepted", "order": [["todo_fractional", 1.5], ["todo_integer", 1]]}
这与 #5014 ledger 里「非法数值事实拒绝」的承诺、以及 TS 侧 nullableInteger 的负向测试直接矛盾。修复后同一输入 fail closed:StateEventError: planner_order must be an integer。
改动思路
不新增 schema、不做第二套数值规则:沿用本文件已经写好的整数惯用法(append_sequence 用的正是 isinstance(value, bool) or not isinstance(value, int)),把 planner_order 的强转与宽 except 一起删掉,换成一个显式谓词。这样 Python 适配器不再先于 typed fold 修改调用方数据,两者语义一致。
具体改动
loopx/event_sourced_state.py:build_state_projection的事实构造处改为if order is not None and (isinstance(order, bool) or not isinstance(order, int)): raise StateEventError("planner_order must be an integer"),删除int()强转与try/except,并留下注释说明为何不能先强转。tests/control_plane/test_event_replay_integrity.py:新增经真实build_state_projection的小数拒绝用例、对True/"1"/[1]/{"value": 1}的参数化拒绝用例,以及「整数与缺省仍可投影」的对照用例。
对主干的风险
唯一行为变化是「原先被静默截断的值现在被拒绝」。这是 fail-closed 方向,且我核实了历史形态:真实本机 Goal 源里 418 个 todo_added 事件的 planner_order 全部是 int,并且同一份真实源(961 事件、191 agent + 227 user Todos)在 base 与本 head 上的投影摘要逐字节相同(sha256=31289710…,源摘要 sha256=19bd138a… 不变),所以没有观察到任何合法日志会被这条规则拒掉。若确实存在手工编辑或损坏的日志携带小数/字符串,现在会在读取时给出可操作错误而不是静默错序,修数是操作者动作,本 PR 不引入迁移。门禁现状:canary premerge --goal-id loopx-meta 只选中 5 项检查,除继承的 semantic-vocabulary-drift-smoke(模块对预算 44 > 43,在干净 origin/main 上同样失败)之外全部通过;本目标不等待远端 CI,打包前端与 PostgreSQL 车道未被本 diff 触及。
我的整体评价
没有发现阻断项。这条 31 行的改动把 #5014 遗留的真实缺陷补齐了:typed 承诺现在在真实入口成立,同时用真实源等值证明没有把正常历史拒之门外。验证覆盖正例、负例与真实路径:40 个 replay/append/事务 Python 用例、真实源 base/head 投影摘要等值、严格 mypy、ruff 与类型检查全部通过;变更质量回执 cqr_8a4db95d6fe7d89699e7 对精确指纹有效。建议按维护者流程合并。
English verdict: APPROVE - exact head 76df0bb4e8b8c0eb20d1ad6a175f0e0cafd0f306; the replay adapter no longer truncates a fractional planner_order before the typed fold can reject it, the fixed entrypoint now fails closed, and the live Goal source projects to an identical digest at base and head so no observed producer is rejected. The only failing check is the inherited module-pair vocabulary budget.
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
76df0bb to
1f6ea38
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval). Reviewed exact head 1f6ea38081c58f23d764dbda25214da4ac2ffcde, rebased onto origin/main 542c3cb0e785f2a39e8c5cc862ecd8d50af92971 (which now also contains the module-pair consolidation #5030). This closes the one still-open finding from the earlier REQUEST_CHANGES review of #5014 (head 552654d0e), which the rebase-only integration of that PR did not repair.
动机
已复核该 finding 在合并后的 main 上仍然成立:build_state_projection 在把事实交给 typed fold 之前先做 int(order),于是 planner_order=1.5 被截断成 1——fold 按 1 排序,投影却仍把 1.5 报给调用方。当前 main 上的真实入口复现结果:
{"outcome": "accepted", "order": [["todo_fractional", 1.5], ["todo_integer", 1]]}
这与 #5014 ledger 里「非法数值事实拒绝」的承诺、以及 TS 侧 nullableInteger 的负向测试直接矛盾。修复后同一输入 fail closed:StateEventError: planner_order must be an integer。
改动思路
不新增 schema、不做第二套数值规则:沿用本文件已经写好的整数惯用法(append_sequence 用的正是 isinstance(value, bool) or not isinstance(value, int)),把 planner_order 的强转与宽 except 一起删掉,换成一个显式谓词。这样 Python 适配器不再先于 typed fold 修改调用方数据,两者语义一致。
具体改动
loopx/event_sourced_state.py:build_state_projection的事实构造处改为if order is not None and (isinstance(order, bool) or not isinstance(order, int)): raise StateEventError("planner_order must be an integer"),删除int()强转与try/except,并留下注释说明为何不能先强转。tests/control_plane/test_event_replay_integrity.py:新增经真实build_state_projection的小数拒绝用例、对True/"1"/[1]/{"value": 1}的参数化拒绝用例,以及「整数与缺省仍可投影」的对照用例。
对主干的风险
唯一行为变化是「原先被静默截断的值现在被拒绝」。这是 fail-closed 方向,且我核实了历史形态:真实本机 Goal 源里 418 个 todo_added 事件的 planner_order 全部是 int,并且同一份真实源(961 事件、191 agent + 227 user Todos)在 base 与本 head 上的投影摘要逐字节相同(sha256=31289710…,源摘要 sha256=19bd138a… 不变),所以没有观察到任何合法日志会被这条规则拒掉。若确实存在手工编辑或损坏的日志携带小数/字符串,现在会在读取时给出可操作错误而不是静默错序,修数是操作者动作,本 PR 不引入迁移。门禁现状:canary premerge --goal-id loopx-meta 在本 head 上选中 5 项检查并全部通过——此前那条继承的模块对预算红灯已由 #5030 把 blocked_retry 的 TS 断言并回既有 owner 后回落(smoke 现报 independently_maintained=43/43)。本目标不等待远端 CI,打包前端与 PostgreSQL 车道未被本 diff 触及。
我的整体评价
没有发现阻断项。这条 31 行的改动把 #5014 遗留的真实缺陷补齐了:typed 承诺现在在真实入口成立,同时用真实源等值证明没有把正常历史拒之门外。验证覆盖正例、负例与真实路径:40 个 replay/append/事务 Python 用例、真实源 base/head 投影摘要等值、严格 mypy、ruff 与类型检查全部通过;变更质量回执 cqr_ba9043726cfa2f57128b 对精确指纹有效。建议按维护者流程合并。
English verdict: APPROVE - exact head 1f6ea38081c58f23d764dbda25214da4ac2ffcde; the replay adapter no longer truncates a fractional planner_order before the typed fold can reject it, the fixed entrypoint now fails closed, and the live Goal source projects to an identical digest at base and head so no observed producer is rejected. The goal-scoped premerge gate is fully green on this head, including the module-pair vocabulary smoke that #5030 restored to 43/43.
Merge note — #5033 merged on the reviewed exact headMerged via admin bypass as Why this PR exists: the earlier Changed surfaces: Checks on the reviewed head
Failures, skips and holds: none on this head. Remote CI was not awaited ( Residual risk: a corrupted stored payload carrying a non-integer |
Problem and result
The typed replay fold (#5014) promises that illegal numeric facts are rejected, but the Python adapter truncated
planner_orderfirst:int(1.5)became1, so the fold sorted the Todo by1while the projection still reported1.5. Reproduced on currentmainthrough the real entrypoint:This closes that gap: the adapter now rejects any non-integer
planner_order(bool, float, string, collection) instead of coercing it, using the same idiom the codec already applies toappend_sequence. After the change the same input fails closed:Scope and continuation
loopx/event_sourced_state.py:build_state_projectionvalidates the fact before sending it, so the typed promise holds at the real caller, not only for direct TypeScript calls.tests/control_plane/test_event_replay_integrity.py: a fractional-order rejection throughbuild_state_projection, a parametrized rejection for bool/string/list/dict forms, and a case proving integer and absent orders still project.todo_addedin the real local Goal source (418 events) carries anintplanner_order, and the full real-source projection digest is byte-identical between base and this head (sha256=31289710…, 961 events), so no observed producer writes a form this rule rejects. A hand-edited or corrupted log carrying a float now fails the projection read with an actionable message instead of silently mis-ordering; repairing that payload is an operator action, and no migration is introduced here.Validation and limits
uv run --extra test python -m pytest tests/control_plane/test_event_replay_integrity.py -q— 14 passed.d42adb874and this head produce the identical excluding-timestamp projection digest on the same live Goal source, with an unchanged source checksum.npx tsc --project tsconfig.control-plane.json --noEmit, configuredmypy, changed-pathruffandgit diff --checkpass; the goal-scoped premerge gate is reported in the review.