fix: bind checkpoint-only recovery to a fresh decision basis - #4882
xyx2002OvO wants to merge 5 commits into
Conversation
Signed-off-by: Tartar <xiaoyx67@mail2.sysu.edu.cn>
Signed-off-by: Tartar <xiaoyx67@mail2.sysu.edu.cn>
Signed-off-by: Tartar <xiaoyx67@mail2.sysu.edu.cn>
Signed-off-by: Tartar <xiaoyx67@mail2.sysu.edu.cn>
Signed-off-by: Tartar <xiaoyx67@mail2.sysu.edu.cn>
huangruiteng
left a comment
There was a problem hiding this comment.
1. 结论与目标对齐
我按 exact head 29334935c8b7b723cc98aabce4ff754c1263644b 复核了两阶段 checkpoint-context 协议、CLI/MCP 接入、持久化边界以及未改动的 canonical provider 写路径。把“先读、后判断、提交前重验”做成 typed receipt 是正确方向,普通 stale/replaced receipt 也有覆盖;但当前实现尚未兑现 PR 与文档声明的 File/SQLite 本地原子新鲜度保证,因此不能批准。
2. 实际调用链与边界
提交路径是 read_checkpoint_context 生成 receipt,checkpoint_commit_guard 在 _source_guard 内重新执行 _source_facts,随后把锁保持到 refresh run append。问题在于 _source_guard 只持有 shadow-maintenance、legacy Todo 与 state-file 锁。
真实 promoted Todo 写路径在 provider_update.py 中先于投影同步执行 effect_runtime_result("coordination.local_authority.todo_update", request);canonical provider commit 已在这里完成,之后才调用 settle_canonical_todo_projection(...)。后者即使会等待 shadow-maintenance lock,也无法撤销已经落下的 provider commit。
因此存在如下合法交错:checkpoint 做完最终 _source_facts 读取 → canonical File/SQLite provider update 提交 → projection settlement 等待 shadow lock → checkpoint append 旧 basis → projection settlement 恢复。receipt 对旧 snapshot 自洽,但并不新鲜。
3. 验证证据
node --test --experimental-strip-types tests/control_plane_ts/checkpoint_read_context.test.ts tests/control_plane_ts/host_todo_completion.test.ts tests/control_plane_ts/refresh_recovery.test.ts:25/25 通过。uv run --extra test pytest -q tests/control_plane/test_checkpoint_read_context.py tests/control_plane/test_refresh_checkpoint_isolation.py tests/control_plane/test_refresh_checkpoint_recovery.py tests/test_host_vision_recovery.py:60/60 通过。git diff --check origin/main...HEAD:通过。loopx pr-review --check-result:结果契约通过,verdict 为REQUEST_CHANGES。- 按 review packet 的
wait_for_ci=false,没有把 CI 状态作为本次判断依据。
这些测试证明 receipt 机械和直接锁竞争成立,但现有隔离用例没有通过生产 public canonical writer 制造竞争,因而没有覆盖上述 provider-commit-before-projection 的窗口。
4. 阻断问题
[P1] commit guard 没有 fence canonical provider 写入。
checkpoint_commit_guard 的最终比较与 append 之间,真实 provider 写入仍可提交;这会让 checkpoint 在 canonical Todo/acceptance 已变化后仍记录旧 decision basis,直接违背文档中“参与写者不能在 comparison 与 append 之间改变 basis”的承诺。
最小修复不是再加一层 projection lock,而是把 append 绑定到 authoritative provider boundary:要么持有真实 provider transaction/write fence 直到 append,要么在 append 时用 provider-owned revision 做原子 CAS/校验;如果 promoted provider 暂时无法提供这个边界,则应先 fail closed。请同时添加 File 与 SQLite 回归:在最终 basis read 后暂停 checkpoint,通过真实 public provider update 入口提交 mutation,再证明 checkpoint append 必须失败并要求 reread。PostgreSQL 可以按现有声明另行处理,但不能用“不保证 PostgreSQL”来豁免本地 provider 竞态。
5. 未来维护性与最终判定
本次 future-facing refactor 检查的核心边界是“谁拥有 freshness”。最小、长期稳定的方向是让 canonical provider revision/transaction 成为唯一真相,receipt 负责携带和验证它;不建议让每个 projection/legacy writer 都去猜测并参与新的全局锁协议。
修复后请保留现有 receipt/stale/replaced 覆盖,并补上真实 writer 的负例。我会基于新的 exact head 复核。
English verdict: REQUEST_CHANGES - the final checkpoint guard does not fence the production canonical provider commit, so a local File/SQLite write can land after the last basis read and before append.
huangruiteng
left a comment
There was a problem hiding this comment.
动机
我按 exact head 29334935c8b7b723cc98aabce4ff754c1263644b 复核了两阶段 checkpoint-context 协议,重点判断它是否真正兑现 PR/文档对 File/SQLite 的本地原子新鲜度承诺:最终 decision-basis 比较完成后,任何参与的 canonical writer 都不能在 checkpoint append 前提交新状态。typed receipt、stale/replaced 分类和 host/CLI 接入本身方向正确,但真实 provider writer 仍在该原子边界之外。
改动思路
当前提交路径是:read_checkpoint_context 生成 receipt,checkpoint_commit_guard 在 _source_guard 内重读 _source_facts,然后把它掌握的 shadow-maintenance、legacy Todo 与 state-file locks 保持到 refresh run append。这个方案只在所有 authoritative writer 都参加同一锁协议时成立。
实际 promoted Todo 更新不是这样:provider_update.py 先通过 effect_runtime_result("coordination.local_authority.todo_update", request) 提交 canonical provider mutation,随后才调用 settle_canonical_todo_projection(...)。后者会等 shadow lock,但 canonical commit 已经发生,所以 projection lock 无法成为 provider transaction fence。
具体改动
- PR 新增 TS/Python typed checkpoint receipt、读取/提交两阶段 API、CLI/MCP/host 适配与本地持久化。
- exact-head 验证:三组 TS recovery/host 测试 25/25 通过;四组 Python focused recovery 测试 60/60 通过;
git diff --check origin/main...HEAD通过。 loopx pr-review --check-result对本轮结构化结果返回契约有效,verdict 为REQUEST_CHANGES。- 按 packet 的
wait_for_ci=false,本次没有使用远端 CI 状态作为判断证据。 - 这些用例证明 receipt 机械、直接文件替换和已纳入
_source_guard的锁竞争能被检测;它们没有通过 production public provider update 入口制造 canonical commit,因此不能覆盖下面的竞态。
对主干的风险
[P1] checkpoint_commit_guard 没有 fence canonical provider 写入。
存在合法交错:checkpoint 完成最终 _source_facts 读取 → File/SQLite provider update 提交 canonical Todo/acceptance mutation → projection settlement 等待 shadow-maintenance lock → checkpoint 使用旧 basis append → projection settlement 恢复。receipt 对旧 snapshot 内部自洽,却已经不新鲜;系统不会报 stale,也会持久化错误 continuation basis。这直接违背 PR 声明的 local atomic-freshness guarantee,属于 correctness blocker。
最小修复应位于 authoritative provider boundary,而不是再增加 projection lock:要么让 append 持有真正的 provider transaction/write fence,要么把 append 原子绑定到 provider-owned revision/CAS;若某个 promoted provider 暂时不能提供该边界,则 checkpoint 应 fail closed。请增加 File 与 SQLite 的 production-path concurrency regression:在最终 basis read 后暂停 checkpoint,通过真实 public provider update 入口提交 mutation,并证明 append 被拒绝且要求 reread。
future-facing pass 的结论也是同一件事:freshness 应由 canonical provider revision/transaction 单点拥有,receipt 携带并验证它;不要让每个 projection/legacy writer 继续扩张成一套易漂移的全局锁协议。PostgreSQL 可以按 PR 明示的范围另行验证,但不能据此豁免本地 provider 竞态。
我的整体评价
两阶段 receipt 是有价值的机制,API/host 覆盖也较完整;但当前 exact head 的核心原子性承诺仍被真实 canonical writer 绕开。现有绿测不能证明最关键的生产交错,因此我继续请求修改。修复 provider-owned fence/CAS 并加入 File/SQLite 真实 writer 负例后,再基于新 exact head 复核。
English verdict: REQUEST_CHANGES - head 29334935c8b7b723cc98aabce4ff754c1263644b; the final checkpoint guard still does not fence the production canonical provider commit, so a local File/SQLite write can land after the last basis read and before append.
A checkpoint-only retry could confirm an unchanged direction after another writer changed its task or acceptance basis.
checkpoint-contextnow stores the read manifest and returns an opaque receipt. The agent rejudges that basis and echoes--checkpoint-read-context; supplementation compares the selected Todo, dependency results, Goal/acceptance, current vision and source binding while holding the existing writer locks through checkpoint append. A reread replaces the previous receipt for that Turn; exact committed retries remain idempotent.MCP
review_task_visionuses the same two-step protocol: call with only Todo/Agent to read, then submitread_context_idwith one decision. Tool schema and Claude host instructions expose the protocol. Missing, stale or replaced receipts cannot silently confirm an old judgment. Recovery never repeats completion or spends again.This changes admission for missing-checkpoint supplements, including historical writebacks. First writebacks and non-Turn vision authoring retain their contract. Content-based revisions include native revision fields; mutable external bytes, raw writes ignoring locks and unobserved legacy ABA remain outside the guarantee. No PostgreSQL service authority is introduced.
Validation:
29334935c8b7b723cc98aabce4ff754c1263644b: all four Python shards and the pytest aggregate passed, together with real PostgreSQL authority, Windows, Node compatibility, static/output-budget checks, builds, Dashboard, Stage 2C and Python 3.11/3.13 adapter contracts. The original failing test lanes are green.Placement/future-facing review: keep typed decision authority in Goal vision and the existing host transaction planner. The Python source/receipt module is explicitly named
checkpoint_context_io; it only collects facts, locks and persists receipts. Existing architecture budgets remain unchanged. No additional capability/provider or speculative host state layer was added. Runtime changes remain a draft pending maintainer review; no merge authorization is implied.