refactor(coordination): unify command recovery and isolate archive transactions - #4286
Conversation
|
Final validation for
Migration economics: product +366/-427 (net -61), validation +276/-8, docs +99/-4. No Python semantic deletion is claimed; repeated TS receipt authority is removed. Bridge code, RPCs and cross-runtime crossings added: zero. The future-facing pass separates archive retention from terminal validation/lease release and removes old internal exports. The compatibility editor and unpromoted Python writers retain their real callers; T1/T2 closure and D1–D3 promotion remain separate. No active Goal was promoted, and this receipt does not authorize merge or rollout. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
English verdict: REQUEST_CHANGES — exact head 4286@ff4c7bdeb57bdc48fc829dbb1bc00487ae22f45b. The required Python shard and merge-gate are failing on the authority-shadow crash-gap test; local suites pass, but the remote cross-process result is unresolved.
动机
PR #4286 的目标是把 Todo create/claim/update/terminal/Monitor 的 durable receipt、提交后 readback 和 response-lost recovery 收敛到 coordination owner,并把 archive retention 从 terminal lifecycle 中拆出。基线中这些命令各自维护 replay、identity、changed 和 readback 分支,容易出现错误码与历史 receipt 兼容性漂移。此 PR 保留命令自己的 planner、request hash、lease/terminal 规则,只共享 receipt 状态机;archive 仍复用既有 selection helper,没有新增 capability 或 authority。这个问题确实影响本地 coordination runtime 的多个生产调用点,但跨进程 primary/shadow 可见性仍需以完整远端检查为准。
改动思路
入口仍是各个 executeCoordinationTodo* 函数,当前 AuthorityStore head 仍是业务决策的权威输入,历史 receipt 只证明同一个 operation_id 的已提交决定。CoordinationCommandReceipt 负责精确 identity 比对、命令专属历史 payload 解码,以及一次 commit 后的 readback:found 映射到 applied/recovered,missing 与 applied 形成 readback mismatch,ambiguous 或 readback unavailable 则返回带同 operation id 的 recovery 指示。它不会重试写入,也不授予当前 actor、lease 或 remote write authority。业务资格、Todo/lease/continuation 规则仍由原模块拥有;todo_archive.ts 只负责 retention mutation 和原有选择器。这个边界比继续保留六套近似 replay helper 更容易维护,且没有把 provider 细节搬进通用层。
具体改动
本头涉及 18 个文件:新增 command_receipt.ts(106 行)和 todo_archive.ts(185 行),改造 create、claim、update、terminal lifecycle、Monitor poll 以使用统一 receipt owner,删除 terminal 内的 archive 实现,更新 local runtime 路由、authority rehearsal、conformance 测试及中英文 RFC。
关键代码讲解
loopx/control_plane/coordination/command_receipt.ts:30的project强制单 receipt 和完整 identity 匹配,调用命令自己的 decoder,再生成 schema、phase、changed、cursor/revision 与 projection delivery。loopx/control_plane/coordination/command_receipt.ts:59的commit只提交一次并做同 operation id 的 readback;响应丢失不会被伪装成无写入,应用结果缺 durable receipt 会返回coordination_commit_readback_mismatch。loopx/control_plane/coordination/todo_archive.ts:75的 archive executor 复用selectCoordinationTodoArchive,把 dry-run、no-change、projection mutation 和 archive receipt 组合成独立 retention 事务。loopx/control_plane/coordination/todo_claim.ts:409保留 claim-specific 的 todo/owner/lease 校验(包括旧 receipt 缺失changed的兼容语义),只把历史状态映射交给 common owner;terminal/update/Monitor 采用同样的窄适配方式。
对主干的风险
P1 blocker:GitHub Actions run 34701030754 的 test-shard (3) 和 pytest/merge-gate 失败,具体为 tests/control_plane/test_local_authority_shadow_cli_e2e.py::test_product_cli_loses_capture_between_commit_and_observer_then_refreshes_snapshot 报告 primary Todo commit did not become visible。这个测试在 observation lock 下启动真实 CLI 子进程,等待 primary commit 可见后模拟进程崩溃并验证后续 snapshot refresh;它覆盖的是 synthetic receipt 单测无法覆盖的跨进程 durability/observation 边界。当前本地 typecheck、1396 个 TS control-plane 测试(1395 pass、1 skip)和聚焦 Python 104 tests 通过,但一次本地通过不能抵消远端 required check 的失败,也不能证明这是单纯 flake。请复现并修复/稳定该时序边界,随后在新精确 head 上重新跑 pytest 和 merge-gate;不要绕过检查。默认关闭/authority 语义没有发现额外扩权,common error wording 也保持 domain-neutral。
我的整体评价
架构方向和拆分范围是 proportionate:receipt 机制被集中到一个窄 owner,archive retention 与 completion/lease 分离,且保留了各命令的业务语义。正向、identity mismatch、response loss、malformed receipt 等本地证据充分;但完整 exact-head 结果仍有一个远端跨进程 blocker,因此当前结论是 REQUEST_CHANGES。修复后需要读回新 head,确认 required pytest/merge-gate 全绿,再重新审阅这条 durability 边界。
ff4c7bd to
b038d23
Compare
|
Reply to the validation conclusion for exact head Root cause: the crash-gap E2E holds the observation lock while starting a real CLI subprocess, but its 5-second polling window was shorter than the subprocess timeout. On CI, Python import/startup could exceed that window, so the test reported “primary Todo commit did not become visible” before exercising recovery; this was a test-observation race, not a transaction-semantics failure. Fix:
Validation:
|
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
b038d23 to
83950ae
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
English verdict: REQUEST_CHANGES for exact head b038d23cedb63c97bd480654c806e26f0e93f80e. The receipt/archive refactor is internally well-covered, but the PR is currently CONFLICTING/DIRTY against current main; rebase, resolve the two integration conflicts, rerun the full evidence plan, and request a fresh review.
动机
本 PR 试图解决两个相互关联但应保持边界清晰的问题:多个 Todo coordination command 各自维护 receipt/replay/response-loss 逻辑,导致同一类持久化命令在重放、无变化、提交响应丢失时可能产生不同语义;同时 archive retention 与 terminal lifecycle 耦合,令归档选择和完成/租约 authority 难以独立演进。作者的目标是让 operation_id、request hash、CAS、readback 和 ambiguous recovery 由一个 provider-neutral owner 统一处理,并把归档变成独立 transaction。
我确认 exact head 的 focused negative paths 没有把 malformed receipt、身份不匹配、response loss、readback failure 或 stale lease 强行当成成功;这部分动机和可观测收益是成立的。当前阻塞不是抽象本身,而是分支停留在旧的 merge-base:PR head 与当前 main 无法形成稳定的可审查合并结果。
改动思路
CoordinationCommandReceipt(loopx/control_plane/coordination/command_receipt.ts:26)现在接收每个命令的 result schema、完整 identity、command-specific decoder 和 failure constructor。命令 owner 仍负责输入归一化、资格/租约/决策和 request hash;共享 owner 只负责 receipt identity 校验、历史 payload 投影、applied/recovered/replayed、no-change、readback mismatch 与 response-loss recovery。提交只执行一次,恢复必须沿用同一 operation_id,不会把不确定结果换成新的写入。
executeCoordinationTodoArchiveCompleted(loopx/control_plane/coordination/todo_archive.ts:76)从 terminal lifecycle 中独立出来。它从 canonical Todo projection 计算 retention selection,dry-run 不消费 durable identity,no-op 明确返回,真正变化通过一次 projection commit 写入 archive_state,再复用共享 receipt。这样 retention ordering 与 terminal completion/lease authority 的 change reason 分开,且仍由 AuthorityStore revision/CAS 保护。
我还检查了旧的 replay helpers、AuthorityStore codec、archive selection、local archive attempt、monitor poll 和所有相关 callers:旧的重复 receipt state machine 被删除,create/update/monitor 对历史 wire shape 的差异保留在各自 decoder 中,而不是强行把旧 payload 当成统一形状。这是合理的 reuse/separation 边界。
具体改动
关键代码讲解
command_receipt.ts:26-95:验证单一 receipt、捕获 commit response loss、随后 readback;found receipt 必须唯一且匹配 schema/operation/goal/request identity。applied但 receipt 缺失会返回coordination_commit_readback_mismatch,ambiguous 则返回同 operation id 的 recovery 指引。todo_archive.ts:40-185:验证 role、limit、日期和 IDs;读取并验证 projection,调用既有todo_archive_selection.ts,区分 preview/no-change/commit,并把 archive mutation 封装进 schema-versioned receipt。todo_claim.ts:364:保留 actor/lease/eligibility authority,只替换重复的 replay plumbing;对历史 claim receipt 缺少changed的兼容处理没有被泛化掉。todo_monitor_poll.ts:151:保留 monitor effect、writeback schema、goal 和 successor 的专属校验,使用共享 receipt 实现 poll 的原子 commit/replay;unchanged observation 不生成 delivery work。todo_terminal_lifecycle.ts:388,652:terminal receipt 通过共享 decoder 投影,但 complete/supersede、decision、lease、successor authority 仍在 lifecycle owner;归档实现已从此文件移除。
RFC/reference 文档同步记录 command recovery、crash-gap、receipt replay 和 archive separation。测试面新增 shared receipt 单测、跨 provider conformance,并更新 rehearsal/shadow fixtures;exact merge-base 的 PR delta 是 19 个文件、758 行新增、443 行删除。
对主干的风险
P1 阻塞:当前分支无法作为主干合并后的 exact review target。 GitHub 在我最后回读时确认 head 仍为 b038d23cedb63c97bd480654c806e26f0e93f80e,但 mergeable=CONFLICTING、mergeStateStatus=DIRTY。PR 分支从 fbd8fef0 分出,而当前 origin/main 是 7eb4b7bb;本地 merge-tree 在以下两个同一 authority surface 上报告真实 content conflict:
examples/control_plane/authority-three-arm-rehearsal.pytests/control_plane_ts/authority_store_conformance.ts
因此,分支 CI 全绿并不能证明合并后仍保留 receipt/archive invariant;PR body 还引用了更旧的 bfd1ec8db baseline 并写 18 个文件,而当前 packet 的真实 merge-base delta 是 19 个文件。最低修复是 rebase/retarget 到当前 main,解决这两个冲突,确认没有丢失 conformance/rehearsal 覆盖,再对新 exact head 重新执行完整 review 和 hosted checks。此头不应合并。
验证结果:exact-head 的 shared receipt/conformance、archive attempt、update、completion transaction、terminal decision、monitor poll 和 authority conformance 聚焦套件共 62 个 TypeScript tests 全部通过;Python shadow CLI smoke 5 个通过;GitHub hosted checks(含 stage2c、correctness e2e、merge-gate、pytest、DCO、build 和 test shards)全部成功。当前本地完整套件不能作为额外正面证据:工作树缺少 tsc/pg,系统 Python 为 Apple CLT 3.9;这些是环境限制,不是把失败归因给 PR 的依据。由于没有在冲突消除后的同一合并结果上完成 baseline-vs-head real-path parity,observable semantics 仍是 not_yet_proven。
我的整体评价
在隔离的 exact head 上,这个改动是有明确 shipped callers、合理 owner 边界和足够 focused negative coverage 的;没有发现另一个已确认的命令语义回归,也没有看到新增 actor/merge/repository authority。共享 receipt owner 与独立 archive owner 的机制是 proportionate 的。
但 review 必须针对可实际合并的完整 diff。当前 head 的主干冲突和 stale baseline 使最终代码、fixture 与文档组合尚未确定,且 baseline/current-main parity 尚未完成,所以我不能批准。请先 rebase 并解决上述两个冲突,重新跑完整 evidence plan(尤其是 62 个 focused TS tests、Python shadow smoke、hosted required checks 和 merge-readiness),然后以新 exact head 请求复审。
|
Self-refinement for exact head 冲突已解决:rehearsal 同时保留连续 journal scan 与 receipt replay 断言;conformance 注册同时保留 scan 和 coordination-receipt 覆盖。生产 authority、receipt、archive 语义未改变。 Validation:
Remote required checks for this exact head are green (DCO, dependency review, builds, Node minimum/forward, all test shards, stage2c, Windows PowerShell, kernel static checks, dashboard acceptance, pytest, checks, stage2c-correctness-e2e, and merge-gate). SonarCloud remains non-blocking. The earlier request-changes review targeted the superseded pre-rebase head; the crash-gap fix and the full exact-head checks now address that blocker. Proceeding with authorized admin self-merge. |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
English verdict: APPROVE for exact head 83950ae01b21a227cd41fa1e8720915850956159. The shared command-receipt recovery owner and isolated archive transaction are correctly scoped, the rebased branch is mergeable against current main, baseline parity and focused negative paths pass, and all required hosted checks/merge-gate pass. SonarCloud is non-blocking.
动机
本 PR 解决两个真实的控制面维护问题:create、claim、update、monitor、terminal lifecycle 等 Todo 命令过去各自维护 receipt/replay/response-loss 逻辑,同类 durable operation 的重放、无变化和提交响应丢失语义容易漂移;archive retention 又与 completion/lease lifecycle 混在一起,令归档选择和终端 authority 的演进彼此耦合。作者希望以统一的 operation_id、request hash、CAS、readback 和 recovery 语义减少重复 authority knowledge,并把 retention 变成独立 transaction。
我先审了旧 merge-base,再审 rebased exact head 83950ae0。前一轮发现的主干冲突已由作者解决;当前 PR 已基于 main,GitHub 报告可合并。这个重基不是只改元数据:新增的 journal scan conformance 和 rehearsal readback 断言也验证了 provider 的完整持久化观察。
改动思路
CoordinationCommandReceipt(loopx/control_plane/coordination/command_receipt.ts:26)是新的共享边界。每个命令提供 result schema、完整 receipt identity、payload decoder 和 failure constructor;命令自身继续拥有输入归一化、资格/租约/决策、request hash 与 projection mutation。共享 owner 只做 found receipt 的唯一性/身份检查、历史 payload 投影、applied/recovered/replayed/no-change,以及 response-loss、readback mismatch 的分类。durable write 只调用一次,恢复沿用同一个 operation id。
executeCoordinationTodoArchiveCompleted(loopx/control_plane/coordination/todo_archive.ts:76)从 terminal lifecycle 迁出。它读取并验证 canonical Todo projection,调用已有 todo_archive_selection.ts 选择需要保留/归档的记录,区分 dry-run、no-op、CAS commit,再通过同一 receipt owner 投影结果。这样 archive ordering/retention 与 complete/supersede、lease、successor authority 具有清楚的 change owner。
我搜索并比较了旧 replay helpers、AuthorityStore codec、archive selection、local archive attempt、monitor poll、command callers 及 conformance。旧的重复 state machine 被删除;create/update/monitor 对历史 wire shape 的差异留在 command-specific decoder,而不是为了表面统一而丢掉兼容约束。这个分离与复用边界是合理的。
具体改动
关键代码讲解
command_receipt.ts:26-95:found receipt 必须唯一并匹配 schema/operation/goal/request identity;malformed result 不会被转成成功;applied 缺 receipt 返回 readback mismatch;ambiguous response 输出同 operation id 的 recovery 信息。todo_archive.ts:40-185:验证 role、非负安全整数、日期和 IDs,读取/校验 projection,区分 preview/no-change/commit,并把 archive mutations 与 version/cursor 放进 versioned receipt。todo_claim.ts:364:保留 actor/lease/eligibility authority,只删除 command-local replay 重复逻辑;历史 claim receipt 缺少changed的兼容读取仍是显式规则。todo_monitor_poll.ts:151:保留 monitor effect、writeback、goal 和 successor 的专属身份检查;unchanged observation 不凭空创建 delivery work,poll commit/replay 走共享 owner。todo_terminal_lifecycle.ts:388,652:complete/supersede、decision、lease、successor 仍由 lifecycle owner 决定;terminal receipt 复用共享投影,archive 实现不再隐藏在此模块中。
RFC/reference 文档同步记录 command recovery、crash-gap、receipt replay 和 archive separation;authority-three-arm-rehearsal.py 与 authority_store_conformance.ts 还补充了 journal page、provider revision、receipt 和最终 projection 的精确 readback。当前基于 main 的 PR diff 为 19 个文件、758 行新增、443 行删除。
对主干的风险
本轮没有发现可复现的 blocking defect。最重要的风险是未来有人在 command-specific decoder 中放宽 identity 或把 ambiguous 当 no-op;这会影响所有 Todo durable writes。但 exact head 的 63 个 TypeScript 聚焦测试覆盖 cross-command/cross-goal/stale-intent identity、malformed changed、response loss、readback failure、archive snapshot drift、dry-run/no-op、stale lease 和 monitor writeback,未观察到该问题。
验证详情:
- current
main上相同的 lifecycle/update/terminal/monitor baseline harness:43 passed;exact head:同样 43 passed。 - exact head 的 receipt/archive/scan/conformance 增量路径:合计 63 个 TypeScript tests,0 failed、0 skipped。
- Python shadow CLI smoke:5 passed。
- GitHub hosted required checks 全部通过,包括 DCO、build、kernel static/typecheck、stage2c 两个 e2e、stage2c correctness、mutants、四个 test shards、pytest、Windows 和 merge-gate;SonarCloud 仅为 non-blocking,最后回读时仍在运行。
本地 npm run typecheck:control-plane 因 review worktree 没有安装 tsc 无法启动;这项缺口由 hosted kernel/typecheck 与测试矩阵覆盖,不能归因于 PR 代码。没有新增 opt-in capability、actor lifecycle、merge/repository authority 或 domain-specific obligation;共享 helper 也没有自动加载的 prompt/skill 表面。
我的整体评价
这是一个有真实生产 callers、边界清楚且规模合适的 control-plane refactor。共享 receipt owner 去除了重复的协议知识,同时没有吞并 create/update/monitor 的历史 payload 兼容责任;独立 archive owner 让 retention 不再与 terminal authority 纠缠。rebase 后 exact head 与当前 main 可合并,baseline parity、positive/negative real-path evidence、hosted required checks 和 merge-gate 都已验证。
因此我对 exact head 83950ae0 给出 APPROVE 结论(因作者自有 PR,GitHub 以 COMMENTED 形式记录)。这只是 review 结论,不授予 merge 权限;合并前仍应保持该 exact head 不变并遵循仓库的 merge-readiness gate。
A canonical Todo command could commit successfully and then return an ordinary read failure when receipt transport disconnected. Callers lost the distinction between a rejected write and a possibly durable operation. Seven command operations now share one typed receipt/recovery owner, with one commit attempt and exact historical readback.
Changes
ambiguouswith the original operation id; a conclusive CAS rejection survives failed diagnostic readback. Malformed receipt payloads fail closed.todo_archive.ts, update every internal caller and remove its old export from the terminal lifecycle module. Completion validation, ownership/grants, lease release, field planning and Monitor generation remain in their existing domain owners.--update-operation-idrecovery key and remaining T1/T2/D1–D3 boundaries.Validation
npm run typecheck:control-plane: passed.npm run test:control-plane: 1,472 passed, 0 failed, 0 skipped; PostgreSQL 16.15 ran on a disposable local server, with separate synthetic tenants. File and SQLite used real local storage; NoKV used its conformance transport, not a live NoKV deployment.bfd1ec8dbversus candidate: all seven operations have matching initial state, successful responses, replay responses and persisted heads; only opaque per-store provider revision tokens were normalized.cqr_1cad545449d2a9b46b92: pass, verification valid; 18 files, one authorized safe-fix pass, zero blockers/warnings/advisories. Fingerprint:1cad545449d2a9b46b927508762c522402d9f23f28c637c2c63253612121e6a0.rufffor the changed Python rehearsal andgit diff --check: passed. Sensitive-pattern scan clean; public-boundary check has zero errors and only pre-existing local-state warnings.Migration economics and limits
Baseline:
bfd1ec8db846bca3af47e559aa5fe7e515e57370(main). Runtime/product: +366 / -427 lines, net -61, including the archive move; validation: +276 / -8; docs: +99 / -4. The 106-line shared owner replaces repeated TS receipt decisions, not Python business writers. Bridge code added: 0. RPC/handler count and cross-runtime crossings: unchanged (native Todo update remains one request; receipt lookup/recovery stays in-process).Existing hashes, receipt schemas, successful payloads, no-op consumption and default provider selection retain compatibility. Fault diagnostics intentionally change as documented; common identity/missing-receipt messages are standardized, while their reason codes remain stable. Claim preserves its historical omitted-change decoder and existing invalid-receipt code. No active Goal promotion, new provider default, lease/gate authority expansion, model job, release or merge is included. Other command protocols and the compatibility editor retain their own recovery contracts.
Future-facing pass applied at the same boundary: archive retention no longer shares a thousand-line terminal transaction module. No new provider framework, generic command registry or Python transport facade was added. T1 metadata/effect closure, T2 leased Monitor work and D1–D3 durability qualification remain follow-up scope.