Repository navigation
fix(goals): fence deletion against canonical writes - #5854
huangruiteng merged 1 commit into
Conversation
Signed-off-by: Duang777 <duangjl007@gmail.com>
|
CI note: the failed |
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; runtime_reported; reasoning_effort=xhigh.
Exact head: df91bcc8ae1f44150e641b4a4985fb77ddb870c8; base: c0d519cfd3911129062653bf861b9d2f6f3277cc. 已读完整 3 个文件及其实际 writer/deletion/recovery 路径。
动机
停止并删除 Goal、或从中断删除恢复的本机用户,以及正在提交该 Goal 的 Todo 写入者。 之前一个 Todo 写入已通过来源校验、停在提交前,删除可能先返回成功,写入随后继续;现在删除和恢复先等待同一 Goal 的写入完成,再核验来源与停止状态,最后移除注册项。 在真实 File、SQLite 隔离数据上,删除与中断恢复四种竞争在旧版本都复现、新版本都通过,完成后注册项消失且已提交的 authority 数据保留。 不物理清除 Goal 文件或 authority 历史,不新增删除权限,不证明跨主机远程写入、Windows 或长期吞吐;没有修改运行中的真实 Goal。
这里 authority 是保留 Todo 数据与提交记录的权威存储。依据为本 PR 之前的 TypeScript 控制面迁移契约:canonical 写入与管理变更共享每 Goal maintenance guard,状态与 provider CAS 的权威保持;现有删除服务要求停止、在锁内复核来源/预览,只移除注册可见性、保留数据和恢复记录。逐条检查 Canonical maintenance serialization 和 Stopped Goal deletion/recovery existing contract:前者由共享 guard 和真实竞争验证,后者由原 source/stopped/backup/recovery 负例与保留数据回读验证。对应整体路线图 S2 的并发/恢复与 S10 的真实失败演练,不把这一修复当作远程 provider 或长期运行验收。
改动思路
复用现有 canonical maintenance guard;Python 只衔接既有注册表删除事务,TypeScript/provider CAS 继续拥有写入决策。 本 PR 完成本机删除与恢复的竞争修复,不新增删除权限、状态字段、CLI 参数或通用锁框架。
锁顺序为 maintenance → source registry transaction → target/global registry lock;真正写入和 readback 仍在原事务内。guard 从实际 target global registry 的父目录推导,和已准入 canonical writer 的 runtime root 对齐,不把来源工作区当作 authority root。等待后继续现有 stopped/source/route/preview 核验,等待本身不授予删除权。正常删除与匹配恢复记录的入口都覆盖,preview 不进入新锁。
具体改动
_canonical_writer_guard_path(deletion_service.py:272):复用shadow_maintenance_lock_target,一处派生共享路径。_execute_deletion(:1092):先取得跨 Python/TS 的 maintenance marker,再进入原 source/global 锁、备份、写入、补偿和回读。_execute_recovery(:631):在检查/应用恢复 preimage 与 postimage 前取得同一 guard,保留漂移拒绝与恢复回读。test_goal_deletion_waits_for_an_admitted_canonical_update:使用实际 TypeScript writer 和 File/SQLite commit,仅通过 barrier 控制时序;中断测试在 source registry 已写之后抛出模拟进程中断,再恢复。
源代码只增加 21 行;测试扩展 fixture 与三个 durable race;registry I/O manifest 仅刷新 13 个位置坐标。复核共享 Python/TS path hash、.ts-effect.lock marker 顺序、真实 target resolver 与 writer 持锁范围,未新增平行 Python 决策或 persisted knowledge。future-facing pass:两种事务有不同恢复/重核逻辑,抽象整个事务并无当前收益;一个小路径 helper 足以消除重复 owner 知识,不需要通用锁框架。
正路径:writer 已准入、真实 commit 暂停 → 请求删除 → 删除不能先完成 → release barrier → commit applied → 删除拿到 guard、重新核验、移除两个注册项 → provider 中仍能读到已提交内容。负路径:活跃 Goal、陈旧预览、恢复漂移、source 恢复/不安全 source lock 仍拒绝或回滚;新等待没有绕开这些规则,也没有物理清除数据。
对主干的风险
独立扩展私有验证增加了 File 恢复,与 File/SQLite 普通删除、SQLite 恢复合成四例:同一真实 provider 测试在不可变 base 四例全部复现“commit 仍暂停而删除已返回”,在当前 head 四例全部通过(4.29 秒)。测试末尾确认 source/global 项都消失、实际 commit 文本保留;不是 mocked provider 成功或只看锁计数。
当前扩展 Python 套件 101 通过、2 失败;两个失败是既有 File/SQLite test_terminal_lease_survives_public_cutover_and_original_recovery 在未改的 todo list 返回上访问 todos 导致 KeyError(line174)。已在不可变 base 原样独立复现,两个测试没有调用删除服务,故不归因于本 PR;保留为单独验证/合并提示,不把套件说成全绿。最初命令路径和私有 fixture import 的验证准备失败也保留,修正路径后才取得上述证据。
11 个 TypeScript lifetime/恢复测试通过;native premerge 13/13,含 diff/compile、maintainability、全树 semantic/I/O census、public/private 与风险 smoke,无 manual hold;changed-file Ruff、控制面 typecheck 与仓库配置的 19 个 mypy 文件通过。另外显式对 deletion module 跟随全部 imports 跑 mypy,出现 4601 个依赖树诊断;不可变 base 的诊断逐行多重集合完全一致(仅缓存遍历顺序不同),本模块只有原有 history.load_registry re-export 诊断。该附加检查不当作通过;仓库配置的 19 个类型文件通过。跳过 imports 的探针把导入类型擦成 Any,不能据此资格化新 helper。 按当前配置未抓取、轮询或等待 CI。
正确性 guard 是 execute 的默认保障,不是可选 capability:不增加字段/Skill/提示词、额度或热路径读取,preview 不变。成本是删除时一次锁操作及竞争时有界等待;没有做 throughput benchmark。未做 Windows、跨主机远程 store、soak 或安装态交互前端资格;这次没有变更前端/Lark/CLI 操作入口或 provider 存储实现。权威保存与中断恢复的本机 File/SQLite 实际路径已验证,不能推广成分布式删除认证。
我的整体评价
没有发现当前头的阻塞问题。对长程效果,这是有证据的本机恢复正确性改善:不再让用户面对“删除完成后仍有迟到提交”的矛盾状态。对体验,原来的一次合法删除动作保持,没有额外参数或确认;竞争时等待可以减少人工事后核对,但长期净效率、频率和吞吐未量化。保留既有权限与资料,有界修复值得进入主干;这是一项独立批准,合并仍交 maintainer,两个既有测试失败单独保留。
English verdict: APPROVE - df91bcc: the existing shared maintenance guard now serializes stopped-Goal deletion and interrupted recovery with real local canonical commits. All four File/SQLite base/head race pairs detect the old defect and pass at head; 101 Python tests,11 TS tests and13 premerge checks pass. Two unrelated retained-list tests fail identically at immutable base and remain a separate validation note. No remote/Windows/long-run or merge-authority claim.
Summary
Root cause
Goal deletion held the registry locks, but canonical Todo writers use a separate maintenance guard. A writer that had already passed source validation could pause before its provider commit, let deletion return, and then commit into the deleted Goal authority. Recovery from a partial registry deletion had the same race.
Deletion and recovery now acquire the same maintenance guard before entering the existing registry transaction and revalidation logic. This keeps the current stale-preview checks while making deletion mutually exclusive with canonical provider commits.
Verification
The full-tree Ruff baseline still reports an existing unused
Pathimport inloopx/control_plane/todos/active_state_editing.py; this change does not touch that file.