fix(todos): restore coarse completion transaction - #4068
Conversation
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
这个 PR 处理的是 #4044 之后暴露出的 completion transaction seam:旧路径虽然先调用 todo.completion.reduce,但在 Python writer 已取得 mutation lock、actor authority 和 task lease 后,还要再通过 todo.completion_policy.resolve 做一次语义 IPC。这样会把同一次完成 admission 拆成两个 runtime 决策点,也让后续 TypeScript authority 迁移更难定位失败与重放。精确 head 60794190057236f1a082541187402096050c76f6 把既有 policy resolver 合入既有 completion reducer,不新增持久化状态或用户 CLI。
改动思路
最关键的设计约束是错误优先级:不能简单把 policy 异常提前抛出,否则“非法 actor / stale task lease / 非法 self-merge evidence”同时存在时,用户会先看到较低优先级的 policy 错误。当前实现让 TypeScript 在 reducer 内把 EffectRuntimeRequestError 转成 decision=policy_reject 的 typed data;Python 在锁内重新比对 Todo 与完整 policy source,依次执行 actor 和 lease fence,最后才由 completion_policy_from_transaction 恢复原有 ValueError。内部错误仍会抛出,不会被误归类成普通 policy rejection。
我也按最新 review capability 检查了状态来源:completion_policy_failure 是由 registry agent set、caller 参数、canonical successor rows 和既有 resolveTodoCompletionPolicy 确定性派生的一次性 projection,不是新增 annotation;没有人工 producer、更新/退休或分页完整性问题。相关开放 PR #4053 会扩展 terminal authority,且与本 PR 在 handler/validation/writer 文件上重叠;它当前仍被请求修改,后续应基于这个更小的既有 owner 修复重做,不能反过来让本 PR继承其未合入假设。
具体改动
完整 diff 为 11 个文件、+290/-127;生产 +143/-61,测试 +142/-64,RFC +5/-2。
关键代码讲解
completion_transaction.ts::evaluateCompletionPolicy复用既有 resolver,只把预期 request rejection 封装为loopx_todo_completion_policy_failure_v0;unexpected/internal failure 仍原样失败。reduceTodoCompletionTransaction新增policy_rejectdiscriminated union。terminal replay、非法 validation declaration 和 validation failure 都先于 policy;通过外部 validation 的第二次 reduction 才产生 commit 或 policy_reject。completion_validation.py::run_completion_validation_gate_with_source在首次和 validation receipt 回传后的 reduction 中都携带同一完整 policy source;锁内再从当前 registry/state 重建并比较,避免旧 validation 或旧 successor/agent facts 授权新状态。completion_policy_from_transaction删除 standalone IPC,将 typed reject 映射回原有 ValueError;effect_runtime_handlers.ts同步移除废弃 handler。todos.py继续把completion_state与metadata_updates作为 override 交给实际 writer;--note仍从 CLIargs.note贯穿到apply_todo_update_to_lines,不是由 reducer重新派生。
对主干的风险
最强反证是:提前计算 policy 可能改变 actor/lease 错误顺序,或者“coarse transaction”重构遗漏 --note 等未参与 admission 的写入参数。我用相同隔离 fixture 在 base 178c96e2d951e5313c48ac72ed0166a62f70bae2 与当前 head 走真实 complete_goal_todo + TypeScript runtime:两边都完成并持久化 note-boundary;blank self-merge evidence 的 ValueError 文本完全一致;wrong actor 与 policy error 重叠时仍先返回相同 claim 错误且 state byte-identical。唯一有意差异是 runtime call list 删除了 todo.completion_policy.resolve。精确 head 上 Python 49+2 个 focused tests、TypeScript 15 个 tests、Ruff 和 diff check 通过。
该分支显示 BEHIND,所以我另外把它无提交地合到当前 main@54681bccd5e5780dc03390488c5afcf217daf692:merge clean,相关 41 个 Python 选择测试与 15 个 TypeScript 测试再次通过。独立 Sonar quality gate 因 new coverage 56.3% 为红,其中 changed TypeScript lines 被报告为 0%,而 repository 的 non-blocking Sonar workflow 和实际 TS tests 均通过;这是保留的覆盖观测缺口,不构成已复现的语义 blocker。若作者 rebase,head 会变化,仍需按新 SHA 复审。
我的整体评价
无代码 blocker,结论等同 APPROVE;但这是 author-owned PR,GitHub 不允许正式自批准,所以发布 COMMENTED fallback。 这个版本把真正重复的 policy 决策收回现有 completion reducer,同时保留 Python host effect/lock/projection 边界与公共错误语义。规模与问题相称,也删除了旧 handler/helper。未来向审视未发现需要再加 framework;相反,#4053 等后续 terminal migration 应继承这个单一 owner,并继续用真实 public-entry 参数/诊断/持久化/readback 反证。
English verdict: Approval conclusion (COMMENTED because GitHub blocks formal self-approval) on exact head 60794190057236f1a082541187402096050c76f6. The change removes the locked standalone policy IPC while preserving public note persistence, actor/lease/policy error precedence, no-effects behavior, validation sequencing, and current-main integration. Focused Python/TypeScript suites, lint, diff hygiene, and a clean synthetic merge pass; a non-required Sonar coverage gate remains red and any rebased head needs fresh review.
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
这个 PR 修复的是 Todo completion 的重复语义边界:旧实现先调用 todo.completion.reduce,在 Python writer 取得 mutation lock、actor authority 和 task lease 后,又调用一次 todo.completion_policy.resolve。同一次完成 admission 因而跨两个 TypeScript 决策点,增加锁内 IPC、失败归属和迁移成本。精确 head 60794190057236f1a082541187402096050c76f6 复用既有 transaction 与 policy resolver,把 policy 接受或拒绝纳入一次 coarse reduction;目标不是改变 CLI、持久化格式或权限模型。
改动思路
TypeScript 仍是 completion state 与 policy 的纯决策 owner,Python 仍负责外部 validation、mutation lock、actor/lease fence 和最终写入。reduceTodoCompletionTransaction 在 replay、validation admission 和 completion metadata 之后评估 policy:成功返回 commit + completion_policy;预期的 EffectRuntimeRequestError 返回 typed policy_reject + completion_policy_failure;unexpected failure 继续 fail closed。Python 的 completion_policy_from_transaction 严格校验 reject shape,并且只在 actor 与 task lease 都通过后才把 summary 恢复为既有 ValueError,因此重构没有把较低优先级的 policy 错误提前暴露给非法 actor。
正向路径是 public complete_goal_todo 生成 pre-lock transaction,validation 如有需要在两次 reduction 之间执行,锁内重新比对 Todo 与完整 policy source,再经过 actor/lease 后消费 policy 并写回。负向路径中,source drift 返回无写入重试结果,terminal replay 不执行陈旧 validation/policy,wrong actor 与 invalid policy 重叠时仍由 actor fence 先拒绝。
具体改动
完整 diff 为 11 个文件、+290/-127:生产 +143/-61,测试 +142/-64,RFC +5/-2。
关键代码讲解
completion_transaction.ts::evaluateCompletionPolicy复用resolveTodoCompletionPolicy,仅把预期 request rejection 转成loopx_todo_completion_policy_failure_v0;它不吞掉内部异常,也不执行写入。reduceTodoCompletionTransaction增加policy_rejectdiscriminated-union 分支,并保证 accepted policy 与 failure 互斥;Python adapter 的_valid_result对两种 settlement shape 都 fail closed。completion_validation.py::run_completion_validation_gate_with_source在首轮与 receipt 回传后的 reduction 中携带同一份 policy source;locked_todo_completion_transaction再用当前 registry/Todo/successor facts 做 CAS,避免陈旧 validation 或 agent facts 授权新状态。completion_policy_from_transaction取代旧bind_completion_policy_to_transaction的第二次 IPC;effect runtime handler 同步删除废弃 method。complete_goal_todo保留现有 writer 参数边界;--note不由 reducer 派生,仍从 public caller 贯穿到 materialized/event writer 并在测试中做独立 readback。
对主干的风险
最强回归场景有两个:一是 coarse reduction 提前计算 policy 后改变 actor → lease → policy 的错误优先级;二是重构只覆盖 admission 字段而漏掉 --note 这类 writer-only 参数。当前精确 head 的真实入口测试同时监控所有 todo.completion* runtime calls、覆盖 validation/no-validation 路径,并从状态文件读回 note;adapter/TypeScript tests 还覆盖 malformed result、policy rejection、terminal replay 和 validation receipt。复核结果为 Python focused 49 passed、TypeScript focused 15 passed、Ruff 与 diff check 通过;对当前 origin/main@54681bccd5e5780dc03390488c5afcf217daf692 的 synthetic merge 无冲突。
远端 17 个功能/仓库检查成功。独立 Sonar coverage gate 仍为红,而仓库内 non-blocking Sonar workflow 成功;结合精确 changed-path tests,没有复现语义 blocker,但这是保留风险。任何 rebase 都会改变 head,必须重新跑 exact-head review。
我的整体评价
无阻塞问题,结论等同 APPROVE;由于这是 author-owned PR,GitHub 不允许正式自批准,因此发布 COMMENTED fallback。 最新 capability 要求的 repository reuse、observable base/head semantics、状态类型、authority、default-off、行为披露和 guidance/obligation lenses 均已执行:这个改动复用并收拢已有 owner,新增的 failure projection 是内部派生数据,不是第二份持久化状态;公开成功、错误优先级、无副作用、replay、validation 与 note readback 均有对应证据。未来向审视未发现需要再加抽象,当前删除旧 handler/helper 已是最有价值的简化。
English verdict: Approval conclusion (COMMENTED because GitHub blocks formal self-approval) on exact head 60794190057236f1a082541187402096050c76f6. The existing completion reducer now owns policy acceptance/rejection while Python retains lock, actor/lease and write authority. Public error precedence, replay, validation, no-effects behavior and --note persistence were revalidated; 49 Python and 15 TypeScript focused tests, Ruff, diff hygiene and current-main merge-tree checks passed. The standalone Sonar coverage signal remains red but no semantic blocker was reproduced.
Review Summary(審查稿重點 · head
|
Summary
todo.completion.reducepolicy_rejectdecision, instead of making a second Python-to-TypeScript leaf callValidation
todo.completion.reduce+todo.completion_policy.resolveto onetodo.completion.reduce; no live state was mutatedloopx canary premerge --from-git-diff --goal-id loopx-meta: 18/18 selected checks passed, 0 warningscqr_4a00b6d1884188707442; fingerprint:4a00b6d1884188707442fd666036ead713f9ae0471be8d3f33bcef2a0238d59dScope decision
This audit exposed a runtime call-shape invariant rather than a new production-data shape. The durable regression therefore spies on the real completion entrypoint and verifies persisted note readback; the production-scale semantic fixture does not need a new arm for this fix.
The change is based on
mainand intentionally removes a migration seam that later shared-authority transaction work should inherit. It is left open for independent review.