fix(claude): bind completed-turn spend to the selected todo and refresh state - #3342
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审
审查对象:1451d748c4e83337f878646acfa9d3e441b3b892
动机
这个 PR 针对 #3341 的两个真实缺陷:Claude/Kunlun 共用的 complete_task 在 Todo 完成后调用未绑定 --todo-id 的 quota spend-slot,导致配额永远不增长;同时该路径没有执行 refresh-state,使 active-state 的下一动作长期陈旧。修复方向本身是必要的,影响的是 Claude Code/KunlunCode MCP adapter 的完成、记账和状态投影链路,而不是单纯的展示问题。
改动思路
实现把完成后的处理抽到 GoalModeMCPControlPlane._settle_completed_turn:先重新运行 quota should-run,从 selected_todo.todo_id 取出当前选择,再构造 spend-slot,最后运行 refresh-state。正向路径是“完成旧 Todo → 选择后继 Todo → 以其 id 记账 → 刷新状态”;无选择或 snapshot 无法解析时,则退化为无 --todo-id 的 spend。这个抽取让职责比原先内联命令清楚,但它没有复用控制面的 typed settlement 顺序与身份。
仓库现有机器契约不是提示性 guidance:settlement plan 明确要求 validation → durable_writeback → quota_spend,且同一个 Todo/turn identity 必须贯穿 writeback 和 spend;host integration contract 也明确写成 refresh-state 后再 quota spend-slot。当前实现却在 loopx/goal_mode_mcp.py:227-235 先 spend、后做一个不带 classification/outcome/todo/turn 的 quota-neutral refresh,因此没有满足 durable writeback obligation。
具体改动
complete_task:完成命令返回 typedok/completed/done后,改为调用_settle_completed_turn。这个入口仍保留了“只有完成成功才尝试记账”的正确保护。_settle_completed_turn:调用should_run_args获取 fresh snapshot;随后把 snapshot 中选中的 Todo 附加到 spend。但 fresh snapshot 发生在旧 Todo 已完成之后,所以这里绑定的是后继 Todo,不是刚完成工作的原始 settlement identity。_selected_todo_id:只把 JSON dict + 非空字符串视为选中项;它把“合法终态没有 selected Todo”和“CLI 失败/输出损坏”都压成None,随后走同一条无绑定 spend 路径。tests/test_kunluncode_goal_mode.py:新增/更新四组断言,覆盖完成后调用顺序、后继 Todo 绑定、无选择 fallback 和 malformed JSON fallback。它们完整覆盖了新 helper 的内部命令拼装,但 subprocess mock 对所有非 completion 命令一律返回{"ok": true},因此没有执行真实refresh-state/spend-slot的 receipt、身份和顺序校验;commands[-1] == refresh-state还把错误顺序固定成了预期行为。
对主干的风险
[P1] 当前顺序和绑定仍会让配额结算失去因果一致性,核心缺陷没有被可靠修复。 在 typed settlement 路径中,spend 会因为缺少同 identity 的 accountable refresh receipt 而 fail closed;在 legacy inference 路径中,它可能消费最近一条旧的 unspent accountable run,随后才写入本轮 state-only refresh,造成记账滞后一轮或错绑到后继 Todo。这样 spent_slots 仍可能不增长,或者增长却代表错误的工作项,配额 brake 与审计都会失真。
最小修复应当从 mutation 前的 authoritative selection/settlement packet 取得原始 Todo identity,并让 accountable refresh-state 与 spend-slot 使用同一个 Todo/turn;顺序必须是 writeback 后 spend,再做 Todo lifecycle closeout/必要的 quota-neutral projection refresh。优先复用现有 settlement plan/driver,不要在 adapter 中另造一套命令顺序。对应测试应至少有一条真实 CLI 或 contract-level seam,证明缺少 writeback receipt 时 spend 会失败、修复后同 identity 的 writeback/spend 成功,且后继 Todo 不会承担前一 Todo 的配额。
[P1] malformed snapshot 不能等同于合法终态并继续无绑定 spend。 这是状态分类规则,不应由 None 同时表达“terminal”与“invalid”。请使用 typed result(例如 selected / terminal-no-selection / invalid),对 invalid fail closed 并原样暴露 blocker;合法终态也应通过原始 settlement identity 完成已发生工作的记账,而不是无绑定猜测。
验证矩阵:精确 head 的 tests/test_kunluncode_goal_mode.py、tests/control_plane/test_quota_settlement.py、tests/test_ark_managed_agent_issue_fix_matrix.py 共 63 passed;远端 Sign-off、dependency-review、pytest、build、windows-powershell 均成功。未独立验证真实 Claude host 端到端运行;上述 P1 来自 exact-head 命令序列与仓库 typed settlement/host contract 的直接矛盾,而不是 CI 状态推断。
我的整体评价
改动规模(2 文件,199 additions / 16 deletions)集中,抽 helper 和补回归测试都具备可维护性;问题在于测试验证了实现自身,而没有验证控制面权威语义。当前不能合并。请先把原始 Todo/turn identity、accountable writeback、quota spend 和 lifecycle closeout 收敛到同一 settlement contract,并把 malformed snapshot 改为显式失败状态;修订后我会按新的 exact head 复审。
English verdict: REQUEST_CHANGES on exact head 1451d748c4e83337f878646acfa9d3e441b3b892: the adapter spends before an accountable refresh and binds the completed turn to the successor Todo; malformed snapshots also fall through to an unbound spend. Relevant tests pass (63/63) but mock away the production settlement guards, so the PR needs same-identity writeback-before-spend coverage before merge.
|
补充一个从历史演进和多 host 架构角度的判断,供这个 PR 继续探索。前一条 review 中的 same-identity / writeback-before-spend blocker 仍然成立;下面主要解释它为什么会出现,以及怎样避免只修 Claude 的一个症状。 根因不是单纯漏了
|
GH-C85 / huangruiteng#3082 follow-up: keep the review-approved real-control-plane negative regression proving that MCP complete_task inherits the declared-command validation gate. A failing command returns the typed validation_blocked_completion receipt, leaves the Todo open, and skips post-completion settlement. The branch is now test-only on main; positive settlement coverage remains with the blocked huangruiteng#3342 dependency instead of accepting failed receipts as success. Signed-off-by: 牛瑞博 <912906590@qq.com>
1451d74 to
3749ce9
Compare
Resolve and persist one turn-scoped settlement identity before the MCP completion mutates the Todo frontier. The provider-neutral control-plane path now validates lifecycle completion, records accountable refresh-state writeback, and spends quota in order against the original Todo. Fail closed on malformed, terminal-without-selection, mismatched, and incomplete receipts. Replays recover the original binding from the heartbeat receipt so a successor Todo is never charged and quota spend remains idempotent. Add real CLI coverage for missing-writeback rejection, successful writeback/spend, successor selection, and replay, while preserving the declared completion-validation gate. Fixes huangruiteng#3341 Signed-off-by: 牛瑞博 <912906590@qq.com>
3749ce9 to
6b35e19
Compare
|
已按 review 和后续架构建议重做,当前新 head 为 6b35e19。请按新 head 复审。 这次不再在完成后重新猜 selected_todo,也不再 fallback 到无绑定 spend:
本地验证:相关 MCP/quota/refresh/validation 套件 75 passed;全量套件首次 3641 passed,52 个环境型失败在固定 Python 3.12、PATH、端口/运行目录权限后逐项全部通过;Ruff 与 LoopX contract check 通过。全 host conformance / Codex-specific plan 泛化仍按建议保留为结构性 follow-up。 |
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审
审查对象:6b35e196241e06357293468a4dcdfdf781dc5015
未发现阻塞性问题。
动机
这个 PR 修复 Claude/Kunlun 共用 MCP complete_task 的两个实际结算缺陷:旧实现完成 Todo 后用未绑定 Todo/turn identity 的 quota spend-slot,在当前 quota contract 下会被拒绝;同时没有 accountable refresh-state,所以 quota brake 与 active-state 投影都可能停留在旧状态。受影响的是所有通过 GoalModeMCPControlPlane.complete_task 完成工作的 shipping host adapter。与旧 head 相比,新实现不再在 mutation 后从 successor 猜 identity,而是在 mutation 前取得原始 Todo 的 turn-scoped guard;完成后让同一 identity 贯穿 validation、writeback、spend 和可选 terminal closeout。这个问题不能靠只补一个 --todo-id 解决,因为真正需要保持的是整条 receipt chain 的顺序与因果身份。
改动思路
实现将 host adapter 的结算编排收敛到 provider-neutral 的 host_adapter_settlement 模块,MCP 层只负责把 goal/agent/todo、runtime profile 和原始 completion 参数翻译成 HostTodoSettlementRequest。权威输入是 mutation 前的 quota should-run guard;决策 owner 是 settle_host_todo_completion,它只接受与请求 Todo 一致的 selected state,并以稳定的 SettlementIdentity 执行 todo complete → accountable refresh-state → quota spend-slot。no_follow_up 先做普通 completion 以产生 validation receipt,writeback/spend 成功后才以同一 completion key 做 same_turn_terminal_closeout。
正向路径是:MCP 完成请求触发 guard,原始 Todo 被验证并完成,refresh-state 写入同 identity 的 durable receipt,quota 记一槽,最后返回一个 typed settlement envelope;有 successor 时它保持 open,且 successor 不承担前一 Todo 的消费。负向路径是:malformed guard、terminal-no-selection、Todo mismatch、effect mismatch、缺失 writeback receipt、spend 拒绝或 terminal closeout receipt 不完整时都在对应 stage fail closed,并返回 settlement_blocked_completion,不会退化为 unbound spend。
具体改动
loopx/control_plane/host_adapter_settlement.py新增 454 行 provider-neutral orchestration,集中保存 guard state、request schema、retry-stable turn id、identity matching、blocked payload 和完整结算顺序。loopx/goal_mode_mcp.py删除 adapter 内旧的 completion-success + unbound spend 拼接,改为调用共享结算入口;Claude 与 Kunlun 的 active call site 都走这条路径。loopx/cli_commands/project_lifecycle.py将已有completion_todo_id/completion_turn_key语义接到refresh-stateCLI,用当前 completion identity 识别“本次完成刚产生的 replan obligation”,避免把它误判为未完成的旧 replan。tests/test_goal_mode_mcp_settlement.py增加真实 CLI seam,覆盖缺 writeback 拒绝、同 identity 完整结算、successor 不扣额、幂等 replay,以及 terminal closeout 的四步 receipt。tests/test_kunluncode_goal_mode.py更新 MCP mock contract,并覆盖命令顺序、typed identity、terminal/invalid guard 的失败短路。
关键代码讲解
HostGuardState/classify_host_guard_snapshot(19、62 行)把selected、terminal_no_selection、invalid做成显式枚举;malformed payload 与合法终态不再共用None,降低错误 fallback 的误分类风险。host_adapter_turn_instance_id(55 行)从 goal/agent/todo 生成 retry-stable、public-safe turn id。重试时同一 Todo 会恢复同一 heartbeat receipt,避免 successor 或另一轮工作接管消费。settle_host_todo_completion(185 行)是行为主入口:223 行校验 guard,254 行执行 lifecycle completion,287 行写 accountable refresh,341 行记 quota,392 行以后完成同 turn terminal closeout;每一步都核对同一个 effect id 与 typed receipt。GoalModeMCPControlPlane.complete_task(201 行)现在只是 adapter translation boundary,不再复制 quota/writeback 的 host-specific 顺序。refresh-state的 hidden completion flags(project_lifecycle.py301、305 行)只把既有 semantic-replan 参数接入 CLI;真正的 completion identity 仍由 current Todo row 与 effect id 共同验证,不能靠 prose 绕过 obligation。
对主干的风险
当前 strongest regression scenario 是“Todo 已 durable complete,但后续 writeback/spend 的输出丢失或被拒”。新路径通过稳定 identity 与 idempotent receipt readback 允许安全重试;失败会暴露具体 stage,blast radius 限于该 Todo 的 quota/state settlement,不会把额度记到 successor。另一个风险是 output shape 从旧的拼接文本变为 typed JSON envelope;这是 MCP model-facing contract 的可观察变化,PR 描述与更新后的测试已经明确披露。
代码量方面,生产改动约 482 additions / 29 deletions,测试约 570 additions / 13 deletions;其中 454 行集中在一个新 orchestration 模块,后续仍可把 command builder 与 receipt predicate 再压缩到共享 settlement driver,但当前模块有 Claude/Kunlun 两个真实 caller、没有 speculative provider,且把原本散落的 identity/order/failure contract 收到一个可回滚边界,本 PR 不需要为了代码量再拆散行为闭环。
验证矩阵:exact head 的 Ruff(5 个 changed paths)通过;focused settlement/Kunlun/quota suite 在本地完整依赖环境中为 58 passed,另外 2 个 real-CLI fixture case 被本机 status-health adapter gate 拦在 settlement assertions 之前;在最小 exact-source 环境中这 2 个 case 单独为 2 passed。远端 clean-checkout 的 exact-head pytest、windows-powershell、Sign-off、dependency-review、build 均成功。直接对新模块做 strict mypy 时只触发 todos/contract.py 中 6 个既有 import-closure 错误;仓库配置的远端 mypy 已随 pytest job 通过。未独立运行真实 Claude host 的交互式端到端会话,残余风险主要是 installed host 的进程级重试体验。
我的整体评价
前一轮的两个 P1(successor identity、spend-before-writeback)以及 malformed snapshot fallback 都已在这个 exact head 上关闭。实现把强制 obligation 表达为 typed guard 与 receipt checks,核心错误文案保持 goal/domain neutral,默认行为变化也在 PR body 与 regression tests 中可见。整体设计比 adapter 内继续拼 CLI 更通用,真实 CLI seam 证明了 quota 与 state readback 的结果,不只是 mock 命令顺序。结论为 APPROVE;这只是复审结论,不授予或执行 merge authority,后续合并仍应遵循仓库门禁与 maintainer policy。
English verdict: APPROVE exact head 6b35e196241e06357293468a4dcdfdf781dc5015: the revision now preserves the original Todo/turn identity across guard, validation, accountable writeback, quota spend, and terminal closeout; malformed or mismatched state fails closed, focused coverage is substantive, and all remote required checks are green. No blocking finding remains; merge stays subject to repository policy.
Fixes both defects from #3341 in the Claude adapter's MCP
complete_tasksettlement path.Defect 1: every post-completion
quota spend-slotwas rejectedcomplete_taskbuilt the spend without--todo-id, so the control plane rejected it (quota spend requires --todo-id ... so the accounted turn is bound to the selected todo) on every completion andspent_slotsstayed 0 — the quota brake never engaged.The binding is order-sensitive: passing the just-completed id is also rejected (
binding mismatch: selected todo is X but --todo-id is Y), because after a completion the selection advances. The correct id is the todo a freshquota should-runreports as selected — exactly what the interaction contract's ownnext_cli_actionscommand does (interaction_contract.py: selected todo →--todo-idarg).complete_tasknow takes a post-completion should-run snapshot and binds the spend to the snapshot'sselected_todo.todo_id, mirroring the contract:spend-slot ... --source heartbeat --execute --todo-id <selected>→ok:true, slots increment--todo-id, still appended honestlyDefect 2:
refresh-statenever ran on this pathIt was only ever recommended via
next_cli_actions, so the active-state narrative went stale.complete_tasknow runsrefresh-state --goal-id <goal> --agent-id <agent>after the spend.Out of scope
The reporter's third rough edge (unregistered
agent_idfrom/loopxarming, hardcodedwaiting_on: codex) is explicitly left for its own issue per the report.Verification
tests/test_kunluncode_goal_mode.py: 34 passed (31 pre-existing + 3 new regressions; 1 updated for the new command order)tests/canary: 20 passed, no baseline changegoal_mode_mcp.py(one pre-existingno-untyped-defoncreate_fastmcp_server, unchanged)Fixes #3341