fix(coordination): drop orphaned leases from the candidate head - #4358
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
结论:REQUEST_CHANGES
审阅绑定的 exact head:e8256e85cbc90957e2f20a7b665fa120ccd14d6e
动机
这个修复瞄准的是一个真实且高成本的 parity 缺口:在 hard_lease 流程中,lease 文件作为审计历史会继续留在磁盘上,但 Todo 被 archive-completed 移出当前图后,source projection 会过滤掉这条 lease;旧的 TypeScript candidate capture 却继续保留它,于是 coordination-shadow inspect 进入 shadow_projection_drift,后续 qualify / read-candidate 被阻断。修复方向是对的,而且没有通过删除审计文件或放宽 source 语义来掩盖问题。
改动思路
本 PR 在两个可能的写入顺序边界上对齐同一条 referential-integrity 规则:lease partition capture 根据调用方已经读取的当前 Todo id 集合过滤历史 lease;Todo partition fold 在 Todo 离开图时同步删除 candidate 中对应的 lease edge。task_lease_acquire 使用同一份 source-stable authority facts,lifecycle 在 authority facts 缺失时传 null,保留旧的严格行为而不猜测图状态。这两个边界都需要:只改 lease capture 会漏掉“先有 lease、后 archive”的 fold,只改 Todo fold又会漏掉后续 lifecycle capture 重新读入历史文件。
具体改动
关键代码讲解
readLeasePartition(local_authority_shadow_outbox.ts:177)新增activeTodoIds,扫描目录时跳过不在当前图中的已存 lease;planned lease 仍由当前已授权写入覆盖。这里使用精确 Todo-id 集合,而不是文本启发式。composeLocalAuthorityShadowHead(local_authority_shadow.ts:698)在 Todo partition 到达时重建 Todo id 集合,并过滤已有 candidate leases,保证无论分区到达顺序如何都不会留下 orphan edge。executeTaskLeaseAcquire/ lifecycle capture 将已经解码的 authority Todo 集合传到 outbox;缺少 authority 的 cleanup 路径继续采用null,避免不完整投影被误当成“没有 Todo”。
本地对 exact head 跑了两个相关 TypeScript 测试文件,30/30 通过;control-plane typecheck 与 git diff --check 通过;远端 25 个检查均成功。运行时实现和单元级 mutant sensitivity 看起来合理。
对主干的风险
仍有一个阻塞交付的问题:PR 正文声称“closes s2c2.archive_after_leased_completion_parity”,但仓库当前仍把这个 exact case 保留在 PENDING_ROWS / PENDING_ROW_IDS,PARITY_REQUIRED_WRITE_CLASSES 的注释也仍明确排除 todo archive-completed,README/correctness 文档继续宣告该缺口未关闭。也就是说,当前全部 CI 可以在从未执行 acquire -> fenced complete -> archive -> inspect/qualify 这条真实 CLI 链路的情况下变绿;两个 helper 测试不能覆盖 source snapshot、outbox drain、candidate compare 与 qualify 的组合。
最低修复是把 s2c2.archive_after_leased_completion_parity 从 pending declaration 转成 deterministic executable LadderRow,从 pending expectations 中移除,更新对应 README/correctness 说明,并证明删除 capture filter 或 Todo-fold filter 任意一个都会让该真实路径失败。这样“关闭 gap”才是仓库可持续验证的事实,而不是只存在于 PR 描述里的手工观察。
我的整体评价
实现本身小而聚焦,状态/权限边界也基本正确;我没有发现需要扩大生产机制的理由。当前不能批准的原因是 delivery completeness:这个 PR 要关闭的就是既有 Stage 2C2 gap,但它没有把 gap 变成可执行回归,甚至保留了“仍 pending”的机器事实。补齐真实 CLI row 和 stale declarations 后,我会按同一 exact head 重点复审该 row 的 baseline/head 与 mutant 证据。
English verdict: REQUEST_CHANGES — the runtime fix is well-scoped, but the exact Stage 2C2 archive-after-leased-completion case remains explicitly pending and unexecuted, so green CI does not yet prove the claimed gap is closed.
|
Additional exact-head blocker (distinct from the existing Stage 2C2 coverage finding), verified at
I reproduced this directly against the production function: passing Minimum repair: construct the candidate-head set from only |
e8256e8 to
fe6843f
Compare
|
Rebased onto current main and addressed both blocking points at new head
Two things I want to flag rather than overstate. First, mutation evidence, because it changed my understanding of the fix. I applied the patch to a clean upstream checkout and mutated each rule separately:
So the fold is load-bearing for the archive path, and the archived-row Second, your reproduction note about Verification at |
fe6843f to
3a5ac12
Compare
Archiving a Todo that still holds a released task-lease record left that lease in the candidate head while the source projection dropped it, so bounded qualification reported shadow_projection_drift and refused to qualify. Reproduced on main 9231d5b with the public CLI: a leased complete followed by archive-completed drifts, while an unleased control stays matched. The source projection models only the current Todo graph and treats the lease directory as append-retained history. Capture now applies the same rule in two places: the lease partition drops a lease whose Todo is absent from the supplied graph, and the Todo-partition fold drops leases that left the graph with it. A missing graph keeps the strict pre-existing behavior. Refs loopx-project#4315 Signed-off-by: song <liusongstep@gmail.com>
The Stage 2C2 gap `s2c2.archive_after_leased_completion_parity` was a declaration: `todo archive-completed` on a Todo holding a released lease record left that lease in the candidate head while the source projection dropped it, so bounded qualification reported `shadow_projection_drift`. Two rules had to agree with the source, not one: * The Todo-partition fold now derives the current graph from `archive_state === "active"`. A published Todo partition also retains archived rows for audit, so the raw record list is not the graph; folding against it re-admitted the lease the archive had just orphaned. * The lease-partition capture filters the retained lease directory against the caller's already-decoded current Todo graph, so a later lease write cannot inherit the archived Todo's retained lease. The declaration is replaced by a deterministic real-CLI row that drives `todo add -> task-lease acquire -> todo complete (fenced) -> add -> todo archive-completed -> add -> task-lease acquire` and asserts `matched` parity, no drift reason, a bounded qualification requiring the archive event kind, a qualified candidate read, and both retained lease files. Removing either rule fails the row. Verified: the row fails on the unfixed tree and passes with the fix; the TS suite is 30/30 on the two shadow files; the ladder file is 23 passed, 8 skipped; Ruff findings are unchanged from the upstream baseline. Signed-off-by: song <liusongstep@gmail.com>
3a5ac12 to
2edda66
Compare
|
Rebased onto No conflict, and I checked the interaction rather than assuming: #4348's canonical renew path reaches the outbox through the existing Re-verified on the new base: both scenarios still report |
songoow
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
审查 head:2edda6666cc5368e8e3882e89b3d6cc1de0b2904。结论:REQUEST_CHANGES。
动机
归档释放后的租约 Todo 不应破坏 shadow 候选头的引用完整性,更不能阻断后续 qualify/read-candidate。两处过滤确实修复了初次归档以及归档后再 acquire 的场景。相比删除审计文件或放宽 source 校验,在既有捕获/折叠边界修复是合理方向;但“后续租约生命周期仍可持续运行”的承诺尚未成立。
改动思路
完整路径为公开 Todo/task-lease CLI → authority facts → TS lease capture → outbox drain/partition fold → FileAuthorityStore → inspect/qualify。源侧当前图仍是权威,候选状态不参与执行授权。本轮相对上次评审已经加入 active membership 条件,并把 pending 行改成真实 CLI ladder 行;这些修改已核实。仍需检查没有 authority facts 的 fence-close 分支,它也是普通 Todo completion 的必经生产路径。
具体改动
运行时改动覆盖 local_authority_shadow.ts、local_authority_shadow_outbox.ts 与 acquire/lifecycle 两个调用方;ladder/row 实现、Python/TS 测试与四份 RFC/示例文档同步更新。没有改变 provider 默认开关或删除磁盘审计记录。
关键代码讲解
composeLocalAuthorityShadowHead在 todos 分区折叠时按 active Todo 图移除孤立 lease,初次归档验证通过。readLeasePartition在activeTodoIds != null时过滤历史文件,并用同一集合计算当前/之前分区摘要。captureLeaseWrite(task_lease_lifecycle.ts:523-525)在 authority 缺失时传null。上游 Python adapter 的needs_authority包含 terminal_verify/holder_verify,却不包含fence_close;因此此处的 null 是正常完成路径会走到的分支,而不只是异常环境兜底。- 新 ladder 行结束于 successor acquire,未继续完成 successor,恰好漏掉该分支。
对主干的风险
[P1] 归档后第二个 Todo 的 fenced completion 再次堵塞 outbox。
在此 exact head,用 disposable fixture 和真实 FileAuthorityStore/公开 CLI 执行:
- Todo A:acquire → fenced complete → archive-completed(
--max-active-done 0 --execute)。inspect 为 matched。 - Todo B:add → acquire。仍正常。
- Todo B:fenced complete,命令返回
ok=true。 - inspect 变成
drifted / outbox_pending;显式authority-shadow drain返回source_partition_continuity_unproved、pending_after=1;qualify 的ok和parity_matches均为 false。
原因是 B 的 fence-close 不带 authority,capture 又把 A 的历史租约读回之前分区,与此前已过滤的候选分区不一致。它不是 transient drain 延迟,显式 drain 也不能清掉。
最小修复:让 fence-close 获得与捕获时源状态一致、可验证的当前图,或在现有 TS owner 内统一派生该图;保持清理权限和源连续性守卫,不放宽摘要验证。把现有 ladder 扩展到 successor complete → drain → inspect/qualify。此处的有界后续重构应统一图规则的来源,而非继续增加独立补偿分支。
我的整体评价
本轮独立执行:相关 TS 30/30;新增 archive ladder 1/1;额外完整生命周期反例可复现。第一次 TS 调用的 Python 子进程落在旧解释器,换用 Python 3.11 的 PATH 后上述 30 项全过。远端此 head 的 checks 无失败,但测试成功未覆盖 successor fence-close。
实现范围合理,原评审修复确实落地;新的 P1 表明 #4315 尚不能宣告完整关闭。未执行十天 soak 或真实 PostgreSQL qualification,不把本次 FileAuthorityStore 证据扩展为 provider promotion。请修复上述生命周期缺口后重新审查。
English verdict: REQUEST_CHANGES at 2edda6666cc5368e8e3882e89b3d6cc1de0b2904. After archiving leased Todo A, acquiring and completing Todo B reintroduces A's retained lease through the authority-less fence-close capture. Explicit drain stops with source_partition_continuity_unproved; qualification fails. The 30 focused TS tests and new ladder row pass, but the real CLI continuation counterexample fails. Extend that row through successor completion and fix the shared capture boundary without weakening continuity checks.
Signed-off-by: song <liusongstep@gmail.com>
|
修复提交: 已修复归档后 successor fenced completion 再次阻塞 drain:shadow-enabled fence_close 使用 Todo 写入后的 source-stable 图,并在 TS capture 前复核 source receipts。默认关闭路径不新增图读取;现有 ladder 已覆盖 successor complete → drain → qualify/read-candidate。 验证:30 个 TS + 66 个 lease/CLI/fence 测试、扩展 ladder 和 LoopX premerge 通过。 已重写 PR 正文以反映当前实现与边界。全部提交带 DCO sign-off;新的 CI 已触发,尚未宣告合并或批准。请按此 head 复审。 Fix follow-up: the reproduced findings are addressed at this exact head with regression coverage and repository-native premerge validation. CI is rerunning; this is a fix report, not an approval. |
动机合并后审计:发现新的 P1 回归,需要后续修复。 审查 head 为 #4315 的实际问题是:归档一个已完成且保留 released lease 文件的 Todo 后,source 当前图已去掉它,candidate 却仍保留该 lease,导致 parity/qualification 失效。保留审计文件而修正候选投影,比删除 lease 文件或放宽 continuity 校验更合理。相对旧评审 head 改动思路修改沿用两个既有 owner:Todo partition fold 按当前 active 图去掉孤立 lease;lease outbox capture 用调用方已读取的 authority Todo 集合过滤历史文件。源事实继续决定执行,FileAuthorityStore candidate 只做旁路观察,文档没有宣告 provider promotion 或十天 soak 完成。 搜索并比较 base/head 的 正向真实链路为 add A → acquire → fenced complete → add anchor → archive(显式 具体改动关键代码讲解
全 diff 14 个文件 +272/-40,覆盖五个运行时文件、两个 ladder 实现文件、三个测试文件及四份文档。新机制主要是派生图过滤与 source 验证,没有新增持久 schema、CLI 或手工 lease 同步状态。 对主干的风险[P1] source receipt 失效后,fence-close 重试失去原锁,完成的 Todo 留下 active lease。 定位: 真实 public CLI 对照:启用 runtime shadow,创建并 acquire 一个 Todo,再执行 fenced complete。在 fence_close RPC 发出前,用一个调度钩子对临时 registry JSON 追加换行,模拟 snapshot 读取后发生等价的并发 registry 重写。没有修改 authority 决策事实,没有替换 RPC 返回值或磁盘 lease;同一注入程序分别运行 base/head:
原因是 revalidation 位于 最小修复是在既有 fence owner 内保持 source 失效重试所需的 fence 生命周期,例如在消耗 claim 前完成可重试复核,或提供安全的同一 held fence 重试;不能跳过 continuity/source 校验,也不能把 shadow 失败作为 primary lease 残留的正常结果。增加真实 CLI 并发/失效回归,断言 lease 的独立磁盘读回,而非只断言候选 matched。 验证:相关 TS 30/30;lease Python/CLI 35 passed;新增 archive ladder 与 vocabulary 2 passed;Ruff 改动 Python 文件、diff check 通过。另执行了上述 base/head × shadow on/off 对照,以及同一 archive harness 的 base 失败。未执行完整 provider 矩阵、真实 PostgreSQL 或十天 soak,不把本次 FileAuthorityStore 证据扩展到 promotion。 我的整体评价原 archive/fence-close gap 已经有可执行覆盖,生产修复方向和规模合理。但只读 shadow 增加的 source 检查现在改变了 primary fence 的失败恢复,仍需修复。该问题是本轮新复现的合并后发现,不重复旧评审已解决的问题;建议在既有 fence owner 中小范围处理重试并补上真实落盘断言。本审计不撤销既有合并,也不声称整体 provider qualification 完成。 English verdict: POST_MERGE_AUDIT_COMMENT at 86640c1. New P1: when the registry is equivalently rewritten after the fence-close authority snapshot, revalidation returns authority_source_changed after claiming the held lock; finally releases it, so retries return fence_token_invalid. Real CLI base/head comparison shows completed=true but released=false and an active on-disk lease only on head with shadow enabled. Both disabled paths release normally. Inspect/qualify can remain matched, so parity alone misses this primary-lifecycle regression. Preserve safe fence retry ownership without weakening source checks. The original archive/successor-completion row now passes; 30 TS tests, 35 lease/CLI tests, 2 ladder/vocabulary tests, Ruff and diff checks passed. Live providers/soak were not validated. The PR was merged by others during review; this is a new actionable post-merge audit. |
|
English verdict: POST_MERGE_AUDIT_COMMENT at 86640c1. New P1: when the registry is equivalently rewritten after the fence-close authority snapshot, revalidation returns authority_source_changed after claiming the held lock; finally releases it, so retries return fence_token_invalid. Real CLI base/head comparison shows completed=true but released=false and an active on-disk lease only on head with shadow enabled. Both disabled paths release normally. Inspect/qualify can remain matched, so parity alone misses this primary-lifecycle regression. Preserve safe fence retry ownership without weakening source checks. The original archive/successor-completion row now passes; 30 TS tests, 35 lease/CLI tests, 2 ladder/vocabulary tests, Ruff and diff checks passed. Live providers/soak were not validated. The PR was merged by others during review; this is a new actionable post-merge audit. |
Behavior
Fixes #4315. With shadow capture enabled, archiving a completed leased Todo no longer leaves orphan lease edges in the candidate head. Subsequent acquire and fenced completion also preserve inspect/qualify/read-candidate availability; retained lease files stay on disk for audit.
Implementation
fence_closereads fresh authority facts and revalidates their source receipts in TypeScript before capturing the lease release. It does not reuse the pre-write verification snapshot. Default-off cleanup adds no graph read.The bounded ownership pass keeps eligibility and persistence in existing TS owners; Python only collects and transports the existing source facts. No provider defaults, promotion policy, source continuity guards or audit retention rules are relaxed.
Validation
Entry points affected: Todo/task-lease CLI and shadow operator readbacks. No frontend configuration change is needed: the shadow opt-in and payload contracts are unchanged.