refactor(control-plane): own terminal Todo lifecycle in TypeScript - #4053
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
动机
评审 head:46cb2d22cb96b6409a98cb6252609d44cc27a338。本 PR 的方向正确:把 Todo complete/supersede/archive 的状态机、授权判定、completion validation、lease release 与 CAS 写回收敛到 TypeScript authority;Python 仅承担 provider 调用与 Markdown 投影。这个边界能减少 Python/TypeScript 双重真相,也与此前 claim/create 的迁移方向一致。
但当前 exact head 有两个阻塞项:新 wire adapter 在严格类型校验之前把布尔值和数字字符串强制转成 number,放宽了 lease/归档参数语义;同时 split-root fence 的既有公开异常契约没有被实现或测试一起迁移,导致 required CI 失败。二者都属于重构最容易漏掉的“看起来等价、实际语义变宽/变窄”问题,不能只凭主成功路径通过来接受。
改动思路
生产路径是 Python facade 经 provider_first_terminal_lifecycle 路由到 provider,TypeScript executeCoordinationTodoTerminalLifecycle 负责规范化输入、授权与完成策略,随后在同一 authority commit 中写 Todo/successors 并释放 lease;成功 receipt 再投影回 Markdown。promotion fence 已持久化后,provider 缺失会 fail closed,不会回退到 Markdown writer;archive 同样由 TypeScript 规范化请求并维护 legacy source index 顺序及 standing-decision receipt。
这套总体设计合理,默认行为/opt-in 隔离不适用,也没有新增更宽权限。不过 adapter 必须保持 TypeScript strict input contract,而不能先做 JavaScript/Python 的宽松数值转换。输入边界一旦接受 true 或 "1",后面的 safe-integer 检查就失去意义;对 expected lease version 而言,这甚至可能让非法 JSON 值命中版本 1 的 fence 并进入终态写入路径。
具体改动
关键代码讲解与阻塞项
-
loopx/control_plane/coordination/local_authority_runtime.ts:767-769在调用终态 lifecycle 前执行Number(input.lease_expected_version)。这会把 JSONtrue变成1、把"1"变成1,绕过下游optionalSafeInteger的原始类型约束。我在 exact head 上直接调用该入口并传lease_expected_version: true,请求继续到了 provider read,返回status=missing,而不是在输入边界拒绝,证明该语义漂移可复现。仓库现有 task-lease contract 明确防止“JSON true 变成 expected version 1”,这里不应产生第二套更宽规则。 -
相同模式还出现在
local_authority_runtime.ts:833-836的max_active_done: Number(...),Python adapterloopx/control_plane/todos/provider_terminal_lifecycle.py:58-60也会用int(...)预先吞掉部分非法类型。请在 wire 边界校验原始类型,或把原值交给唯一的 strict normalizer;补充 terminallease_expected_version与 archivemax_active_done对true、数字字符串、fraction 的负向测试,并保留合法整数的正向覆盖。 -
test_split_root_todo_writeback_fence.py::test_turn_validated_completion_blocked_when_override_root_is_fenced在当前 head 稳定失败:测试期望既有LegacyCoordinationWriterFenced,provider-first 路径在 durable promotion fence 后改为LocalCoordinationAuthorityUnavailable。fail-closed 本身是对的,但异常类别是调用方可见语义。请明确迁移后契约:若要保持兼容,应在 adapter 保留原可操作异常;若有意更改,应同步公开契约和回归测试,并说明受影响入口,不能留下 required CI 红灯。
本 PR 30 个文件、约 +4573/-172,体量较大,但主要由终态 authority、投影/validation sidecar、三臂 conformance 与迁移文档构成,主题仍然内聚。未来维护检查认为无需再加抽象层;当前更重要的是收紧现有 adapter 的 typed boundary,并把异常兼容决策落实到代码和回归矩阵。
对主干的风险
本地验证:TypeScript control-plane suite 709 passed, 1 skipped,TypeScript typecheck 通过;四个相关 Python 文件组合为 77 passed, 1 failed,失败与远端 test-shard (2)/最终 pytest 一致;变更 Python 文件 Ruff 通过,diff whitespace check 无告警。远端 DCO、dependency、build、Windows 等通过或按设计跳过,但 required Python CI 仍失败。
主要风险不是普通报错文案,而是 authority 输入语义:非法布尔值/字符串被转换后可能满足 lease fence 或改变 archive 保留数量。其次是调用方对 split-root fence 异常分类的兼容性。修复后应重跑完整 TypeScript suite、相关 Python authority/adapter/split-root 矩阵和远端 required checks;这类终态/lease 重构不能只用 mock 或单条 happy path 证明。
我的整体评价
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
方向和值得做的 bounded refactor 都成立,但当前不能合并。请先移除 terminal/archive wire boundary 的宽松数值 coercion,增加非法原始类型回归,并解决 split-root fence 的异常契约与 required CI 失败。完成后我会按新 exact head 复审。
English verdict: REQUEST CHANGES at 46cb2d22cb96b6409a98cb6252609d44cc27a338 (published as COMMENT because GitHub blocks formal self-review on an author-owned PR). The TypeScript terminal authority consolidation is cohesive, but the runtime adapter coerces booleans and numeric strings before strict validation (lease_expected_version, and similarly max_active_done), allowing invalid wire values to cross the typed boundary. The existing split-root fence contract also disagrees with the new exception and required CI is red. Remove the coercion or validate raw types, add bool/string/fraction negative coverage, reconcile the exception contract, and rerun the full relevant matrix.
38a1333 to
6f15858
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确复审 head:46cb2d22cb96b6409a98cb6252609d44cc27a338
动机
本 PR 把 Todo complete/supersede/archive 的终态状态机、授权判定、completion validation、lease release 与 CAS 写回收敛到 TypeScript authority,Python 只负责 provider 调用和 Markdown projection。这个方向与 claim/create 迁移一致,也有真实 public facade 调用点;但 authority refactor 必须保持原始 wire types 和既有 fail-closed 异常契约,不能把“转换后看起来相同”的值当成等价输入。
改动思路
Python 的 provider_first_terminal_lifecycle 在 durable promotion fence 生效后将公开 complete/supersede/archive 路由到 provider;TypeScript terminal lifecycle 负责 decode、actor/lease/policy admission、successor 构造和一次 authority commit,receipt 再投影回 Markdown。总体 ownership 合理,默认关闭/opt-in 不适用,也没有新增更宽 actor authority。
当前阻塞恰好位于 adapter boundary:TypeScript strict normalizer 之前先用 JavaScript Number(...),Python 侧也用 int(...),导致布尔值或数字字符串失去原始类型;同时 split-root writer fence 的公开异常类别发生变化,required CI 已稳定证明这一兼容裂缝。
具体改动
loopx/control_plane/coordination/local_authority_runtime.ts:769把lease_expected_version先转成Number(...)。JSONtrue和字符串"1"都会变成整数1,随后下游optionalSafeInteger()无法再拒绝原始非法类型。对 lease fence 而言,这可能让非法 wire 值命中真实版本 1 并继续终态事务。local_authority_runtime.ts:836对max_active_done重复同一 coercion;Pythonprovider_terminal_lifecycle.py:60的int(...)也会吞掉部分非法输入。应在 adapter 处验证原始值,或把原值直接交给唯一 strict decoder;增加true、数字字符串、fraction 的负例及合法整数正例。test_split_root_todo_writeback_fence.py::test_turn_validated_completion_blocked_when_override_root_is_fenced仍失败:既有 contract 期望LegacyCoordinationWriterFenced,provider-first 路径却抛LocalCoordinationAuthorityUnavailable。fail closed 是对的,但异常类型属于调用方可见语义;请明确并实现兼容策略,不能留下 required CI 红灯。- 当前 PR 与主干存在 merge conflict,必须在上述代码修复后 rebase,再对新的 exact head 重新验证。
对主干的风险
本地复现:相关 TypeScript policy/transaction/terminal decision 14 passed;相关 Python 集合 61 passed, 1 failed,唯一失败即 split-root fence contract。远端同一 exact head 的 required Python shard/聚合 pytest 也是 3225 passed, 13 skipped, 1 failed,失败位置和异常完全一致;DCO、dependency、build、Windows 等通过或按设计跳过。
主要风险是 authority 输入语义被放宽:非法布尔值/字符串可伪装成 lease version 或 archive limit。其次是升级后调用方无法按既有异常类别识别 writer fence。future-facing pass 不需要再造抽象层;最小而耐久的修复是让原始值只经过一个 strict typed decoder,并把公开异常兼容矩阵固化为 real-path regression。
我的整体评价
暂不批准。终态 authority 的收敛方向与整体 scope 可以保留,但必须先移除 terminal/archive adapter 的宽松数值 coercion、补齐非法原始类型测试、修复 split-root fence 异常契约和 required CI,并在 rebase 后按新 head 复审。由于作者自审限制,本结论以 COMMENT 发布。
English verdict: REQUEST CHANGES on exact head 46cb2d22cb96b6409a98cb6252609d44cc27a338 (published as COMMENT because GitHub blocks formal self-review). The provider-first terminal authority is cohesive, but its adapters coerce booleans and numeric strings before strict validation for lease_expected_version and max_active_done, widening the wire contract. The split-root fence path also raises a different public exception and fails required CI, and the PR is currently conflicted with main. Preserve raw types through one strict decoder, add bool/string/fraction negative coverage, restore or deliberately migrate the fence exception contract, rebase, and rerun the full relevant matrix.
|
Independent review from the claim/lease compatibility side — we authored the rejection exception contract this terminal path now inherits (#4020) and the claim-rejection message fidelity work (#4000), so I focused on the boundary the migration crosses rather than the consolidation itself. Reviewed at 46cb2d2 in a clean worktree; everything below was verified by running the suites, not just reading. Third-party verification (both of the owner's blockers reproduce, with one pointer and one mechanism detail): Blocker 1 confirmed. Blocker 2 confirmed and mechanism pinned. In my worktree Local matrix reproduction: the new Python domains (local_coordination_authority, completion_validation, completion_transaction_runtime, runtime_shadow_adapter, split_root fence) run 87 passed / 1 failed — the only failure is the fence contract above. F1 — the terminal path honors the #4020 rejection contract; nothing locks it. Verified the mechanism end to end: deterministic admission rejections surface as F2 — sidecar digest recovery is fail-closed and tested; two design asymmetries to consider. The direction is right and verified: the raw-argv sidecar is created 0600 with no world-readable window ( F3 — the committed fixture arm is genuinely reproducible; the baseline arm is not, and the fence is not at scale. The durable layer checks out: content is generated from public-safe seeds with fixed timestamps and a checked-in envelope ( Nits: the durable replay receipts are well designed (request sha excludes the volatile validation receipt, so post-effect re-entry replays identically, and commit readback verifies the receipt landed), but the Python adapter mints a fresh uuid4 Worth keeping as-is through the rework: one TS semantic owner for admission, lease release, successor creation, and settlement in a single CAS commit; the two-phase validation flow where TS decides, Python only executes the typed effect and returns a privacy-safe receipt; digest-bound sidecar recovery; the semantic-duplicate and successor-cycle guards; and standing-decision-preserving compaction. The consolidation direction has our support from the claim side — the ask is that the terminal surface adopt the same wire validation and rejection-contract test discipline claim already carries, since it now inherits that contract. |
46cb2d2 to
d5ec16a
Compare
|
Rebased onto current What changed:
Validation on the rebased exact head:
No self-merge: this authority-runtime PR remains for independent review. |
|
Added the production-scale fixture stewardship gate on exact head What changed:
The change-quality simplify pass removed duplicated rule text from the RFCs; they link to the testing guide as the single detailed owner. Validation:
No self-merge; this authority-runtime PR remains for independent review. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Reviewed head: 9dd6d8bd0e9209d17093b23b6d9b354d18ee8d54.
动机
本 PR 的价值成立:让 promoted Todo 的 complete / supersede / archive 在 canonical provider transaction 中闭环,统一终态决策,绑定 validation declaration,原子提交 Todo、successor、lease 和 receipt,并支持 replay / recovery。不能仅因净增代码就否定这个阶段。
但“保留旧存储”不等于“保留两份业务规则”。当前需要补一次有明确边界的收敛,而不是继续横向增加机制。
改动思路
已有的正确收敛值得保留:authority_core.py 的 complete / supersede 已调用 todo.terminal.decide;native 路径也使用 TS terminal decision,并复用已有 completion transaction reducer。因此不是“完全没删除 Python authority”。
真正缺口是 archive:旧 Markdown 路径仍自行选择归档对象,新 provider 路径又实现一遍。这可以在本 PR 内解决,无需强制迁移所有 Goal。
R1 — 合并前必须收敛 archive selection 的双重业务 authority。
对照以下两个仍然可达的实现:
- 旧路径 completed_archive.py:104–125:选择 done、保留 standing decision receipts、计算超出上限的数量、选择最早条目。
- 新路径 todo_terminal_lifecycle.ts:1088–1100:再次拥有同一组选择规则,另有
isStandingDecisionReceipt和archiveOrder。
这不是单纯“长得像”:两者面向同一个 Todo 归档契约,存储 promotion 不应改变 retention policy。即使现有样例全部一致,未来只修改其中一处的 standing scope、阈值或选择规则,两条公开调用路径仍会分叉;File / PostgreSQL conformance 都通过也不能排除它,因为它们共享的是新路径。
建议的最小修复:
- 在现有 Todo typed domain 内抽出一个纯 archive selector;接收归一化 Todo facts、明确的 ordering facts、role 和上限,返回选择的 ids 及必要计数。
- native transaction 与 legacy Markdown adapter 共用它;删除 Python 中对应的选择/保留规则。Markdown 解析、锁、block 搬运和写回继续留在 Python。
- 显式保留 imported record 的原始顺序和 native record 的 timestamp/id 顺序;不要为了“统一”静默改变旧顺序。
- 先以独立语义建立角色、standing receipt、limit=0、无需归档、顺序边界的 parity,再通过真实 CLI 的未 promotion / promotion 路径验证;至少让一个 retention 或 order 的故意错误变更使断言失败。跨语言调用应按一次归档批次发生,不要逐 Todo 调用。
这是本轮的架构阻塞,不是在声称已复现当前版本归档错误。
具体改动
本轮检查的完整改动面及保留判断:
- TS decision / transaction / runtime handlers:终态资格、验证结果、canonical CAS、lease/receipt/replay,是本阶段核心价值;保留。archive 的纯规则应从 1162 行 transaction 模块中收敛到现有领域边界。
- Python facade / adapter:decorator 根据 promotion 路由;未 promotion 返回 legacy,promoted head 缺失则明确失败,不是出错后偷偷回写 Markdown。兼容分支本身有真实调用者,不能整块删除。
- validation declaration / projection:私有命令保留本地、canonical 保存 digest;create、completion validation、投影渲染和 sidecar 读回一起调整,是实际执行及隐私边界,不应把所有新增 Python 行都叫“冗余 bridge”。
- shadow / schemas:validation digest 的契约与生成物、retired lease 的投影过滤,需要保留其一致性和负向覆盖。
- 验证与文档:provider conformance、真实 facade、split-root fence、validation、规模 fixture、三臂 rehearsal,以及双语 RFC、testing guide、PR template 的 fixture stewardship,服务于迁移契约;不要用删测试来制造净删行数。
R2 — 在当前 PR 补齐具体的 migration economics receipt,并落实可删除部分。
这是现有 TS RFC 的 payoff-phase 要求,不是新增一个大框架:
| 分类 | 本 PR 应交付的证据 |
|---|---|
| 现在删除 | R1 的 Python archive 规则;终态已委托 TS 后不再有有效调用契约的旧决策/helper,逐项核实后删除 |
| 明确保留 | Markdown parser/writer/lock/capture、私有 validation effect、仍有效的兼容入口,各写明实际调用者 |
| 不是迁移收益 | resolve_todo_state_path 从 todos.py 搬到新模块,属于 move,不能算消除了 lifecycle authority |
| 有界后续 | 确实不能删除的 terminal seam,给出具体退出条件,而不是笼统“以后全部迁移” |
| 运行成本 | 指定一条正常调用及 validation/retry 路径,记录实际 Python↔TS round trips;分别统计产品、纯 bridge、测试、生成物和文档 LOC |
当前整个 loopx/ 为 +3100/-111(含 3 行契约/生成字段),其中 Python +1313/-111;测试/fixture +1862/-32,rehearsal +432,文档/template +290/-26。这里应解释的是 产品层 +2989 的组成及本阶段删掉了什么知识,不是要求整个 PR 净负数。
顺带审计 _build_successors:其中 priority、capability binding、bound-agent、unblocks 的推导不只是序列化。请逐项区分“调用者提供的 intent”“对 canonical target 的业务推导”“纯 transport”。若与 legacy 完成/替代路径重复,优先共享当前终态领域 owner;不要仅把函数换目录,也不要求把整个 625 行适配器机械搬到 TS。对于仍有明确调用契约的 lease/fence 代码,不应仅凭文本搜索就删除。
对主干的风险
正路径静态追踪:公开 Todo facade → promotion routing → TS terminal decision / completion reducer → 必要时由 Python 执行 validation effect → TS 再验证并 CAS 提交 → receipt / projection settlement。
负路径静态追踪:promoted head 缺失不回退到 legacy;validation digest 不一致不授权终态提交;重放依赖原 operation identity。这些防线应保持,不能为了精简而弱化。
上一轮针对严格数值解码、writer fence、reason-code、sidecar 与重试身份的修订,以及本轮 fixture 文档追加,和这里提出的共享归档规则问题是不同维度;不把旧问题重新当成未修复 blocker。
**验证边界:**本次为 exact-head 代码/架构审阅,没有重新执行真实 CLI 三臂实验或 PostgreSQL integration;作者此前报告的测试不是本轮独立执行证据。最新 GitHub checks 中 Python shards / pytest 通过,但 stage2c (e2e)、stage2c (mutants) 及 aggregate 显示失败;整个 run 尚未完成,当前无法通过 gh run view --log-failed 读取完整日志,因此不在这里猜测归因。需解释失败并提供最终有效结果,不能把当前 head 描述为全绿。
R1 修改后,应在隔离合成数据上补真实 legacy/native CLI parity 与受影响的 provider integration;不使用活跃 Goal 做 promotion 或破坏性验证,也不只拿新 TS helper 单测替代真实入口。
我的整体评价
**REQUEST_CHANGES。**问题不是“大 PR 必然不好”,而是这次 terminal cutover 应同时完成一次可核验的语义收敛。
本 PR 内的完成标准:
- archive 的选择/保留规则只有一个 typed owner,两种存储入口共用,并删掉旧规则。
- 给出实际删除/保留/后续清单与 round-trip 证据,解释 terminal adapter 内业务推导的归属。
- 新增收敛有独立 oracle、真实调用路径及必要 backend 验证,解释并修复或证明当前失败 checks 的归因。
不要求本 PR 强制 promotion 全部 Goal、废除所有 Markdown I/O、迁移整个 lease 系统、引入通用 provider 框架,或删除有价值的验证来凑行数。目标是让下一次修改归档政策只改一个地方。
English verdict: REQUEST_CHANGES at 9dd6d8bd0e9209d17093b23b6d9b354d18ee8d54. The canonical terminal transaction is valuable, and complete/supersede already share a typed decision. Before merging, converge legacy/native archive selection under one typed owner, retire the replaced Python rule, and supply a concrete migration economics/retention receipt with real-path parity coverage. This is a source-backed architecture hold, not a claimed reproduced archive failure. CLI/PostgreSQL suites were not independently rerun in this review; current Stage 2C failures also need reconciliation.
huangruiteng
left a comment
There was a problem hiding this comment.
动机
精确复审 head:9dd6d8bd0e9209d17093b23b6d9b354d18ee8d54。
本 PR 的核心方向是对的:把 promoted Goal 的 Todo complete / supersede / role-scoped archive 的授权、hard-lease fence、completion policy、CAS、receipt 和 successor 写回收敛到 TypeScript authority;Python 只组装 provider-neutral 请求、执行声明式 validation effect,并投影 Markdown。此前复审指出的布尔值/数字字符串 coercion 已在当前 head 改为原始类型严格校验,旧 split-root 聚焦测试也已随既定异常语义迁移。
但当前 exact head 仍有三个阻塞面:可选 prose 字段被误用 actor/ID 的非空校验器,造成 --note "" 的真实兼容回归;与当前 main 合成后有 7 个 Stage 2C real-path parity 用例失败;mutation lane 在运行受保护回归前就因 locator drift 退出。这些都属于 authority refactor 必须捕获的细微语义/验证契约,当前不能批准。
改动思路
生产链路是 complete_goal_todo / supersede_goal_todo / archive_completed_todos 经 provider_first_terminal_lifecycle 先读取 canonical head,Python 构造 successor、registry facts 与 validation sidecar 请求,local_authority_runtime.ts 持有 per-goal canonical writer/maintenance lock,再由 todo_terminal_decision.ts 和 todo_terminal_lifecycle.ts 完成 typed admission、lease release、completion reduction 与单次 authority commit。receipt 成功读回后,Python 才更新兼容投影。显式 promotion 之前仍走 legacy,promotion 后 provider 缺失 fail closed;默认关闭隔离和 actor authority 没有被扩大。
这个 ownership 切分合理,但 adapter 只能解码 wire shape,不能把 field-specific 语义重新解释一遍。当前 claimAgentValue() 同时服务 agent identity 和 note / evidence / reason,在真正的 terminal semantic owner 之前建立了第二套更窄规则。验证设计也有同类缺口:已提交的 three-arm executable 只执行 archive;File/PostgreSQL 的 complete/supersede conformance 共用同一个新 TypeScript owner,因此不能独立证明 legacy parity。
具体改动
-
[P1] 保持可选 prose 字段的 legacy / provider 语义一致。
loopx/control_plane/coordination/local_authority_runtime.ts:830-835对note、evidence、reason使用claimAgentValue(),会拒绝空字符串和仅空白字符串;但下游唯一语义 ownertodo_terminal_lifecycle.ts:114-119明确把""规范化为null,legacyline_update.py:324-336也把空字符串视为“未提供”而继续完成。我用真实 public Python facade、真实 FileAuthorityStore 和 durable promotion fence 对照复现:相同的complete_goal_todo(..., note="", no_followup=True)在 legacy 路径返回成功并把 Todo 置为 done,promoted 路径却抛LocalCoordinationAuthorityUnavailable(code="invalid_local_coordination_todo_terminal_lifecycle_request"),原因是note must be a non-empty string。这会让升级前可成功的 CLI/API 调用在 promotion 后卡住终态。请为 prose 使用与 owning lifecycle 一致的 optional-string decoder,不要复用 agent/ID validator;增加None、""、普通文本以及明确决策后的 whitespace 行为的 legacy-vs-promoted public-facade parity,evidence和 supersedereason也要覆盖。 -
[P1] 先 rebase 并逐项收敛当前 Stage 2C 合成失败,不能只刷新快照。 当前 GitHub merge ref
45b32d741445839aef7890c7bc3ce2bc6bde57d6上,我本地重跑远端失败面得到7 failed:六个test_shadow_fence_caller_parity_e2e.py的 terminal/archive 行,以及test_source_snapshot_preserves_ordinal_mixed_case_lease_inventory。前者现在分别变成handoff_mode_requires_lease、成功 preview/执行等 provider-first 结果,而 fixture 仍声明 legacy fence envelope;后者的 source inventory 仍观察到三个 lease 文件,但runtime_shadow.py:118-134把它们全部从 projection 过滤成leases=[]。PR 文档确实声明了 orphan historical lease 不进入 live head,因此这里不是简单把测试改绿:请明确哪些是有意 contract migration、哪些是错误行为,更新 authoritative parity fixture/基线与 disclosure,或恢复旧语义,并保证 source inventory 与 canonical import contract 的证据关系仍然可验证。当前远端stage2c (e2e)和 requiredstage2c-correctness-e2e均为红灯。 -
[P1] 修复 mutation locator,恢复 maintenance-lock 负向证明。
examples/shared-goal-authority-e2e/mutants.py:132-141从updateLocalCoordinationTodo一直切到editLocalCoordinationTodo,而本 PR 把 terminal/archive 函数插在二者之间;该切片现在包含多个相同的withCanonicalWriter(...),replacement()因 count 不再等于 1 而报mutation locator drift。我在 exact head 和当前 merge ref 上都独立复现,mutation 尚未生成、更没有被 oracle kill。请把 locator 限定到 update 函数自身(例如可靠的函数边界/brace-aware 定位),再跑完整 mutant lane;否则本 PR 宣称的 native update maintenance-lock 防回归覆盖实际上已经失效。当前远端stage2c (mutants)也因此失败。 -
补齐与变更规模相称的独立语义对照。
authority-three-arm-rehearsal.py只 importarchive_completed_todos,provider 两臂也只调用executeCoordinationTodoArchiveCompleted;它没有执行 complete/supersede、终态 lease admission、validation sidecar、successor 或note/evidence/reason。请把 legacy-vs-promoted 的 terminal public operations 纳入 durable gate,或至少增加独立的完整 parity matrix;只让 File/PostgreSQL 共同执行同一套 TS 规则无法发现上述 adapter 语义漂移。
对主干的风险
当前 exact head 的聚焦结果:相关 Python authority/adapter/split-root/validation 集合 90 passed;相关 TypeScript terminal/decision/file-store 集合 29 passed;Ruff 与 git diff --check 通过。与此同时,真实空 note 对照稳定复现 1 个兼容回归;当前 merge ref 的 7 个目标 Stage 2C 用例全部失败;native_update_maintenance mutant 在 exact head 与 merge ref 都在生成阶段失败。远端 required stage2c (e2e)、stage2c (mutants)、聚合 stage2c-correctness-e2e 同样为红灯。
本 PR 共 36 个文件、约 +5684/-169。主体围绕 terminal authority、validation sidecar、provider projection、conformance 与迁移文档,主题仍算内聚,但规模意味着 happy-path unit suite 不足以证明等价。主要主干风险是 promotion 前后同一公开调用的输入语义变化、fence/preview envelope 变化和迁移时 lease history 的隐式丢弃。future-facing pass 不需要再加抽象层:复用已有 field-specific decoder、扩展现有 parity fixture/three-arm operation 集,并让 mutant locator 跟随具体符号即可。
我的整体评价
Request changes conclusion(作者自审限制下以 COMMENT 发布)
暂不批准。请先修复空 note(连同同类 evidence / reason)的 legacy/provider 语义差异,rebase 当前 main 并逐项解决 7 个 Stage 2C parity 失败,修复并跑通 mutation lane,再补齐 complete/supersede 的独立 legacy 对照。完成后按新的 exact head 复审。
English verdict: REQUEST CHANGES on exact head 9dd6d8bd0e9209d17093b23b6d9b354d18ee8d54 (published as COMMENT because GitHub blocks formal self-review). The TypeScript terminal-authority ownership is directionally sound and the earlier numeric coercion issue is fixed, but the local adapter now rejects optional empty prose that both the legacy facade and the owning lifecycle treat as absent; a real promoted-vs-legacy call reproduces --note "" succeeding before promotion and failing after it. The current merge ref also has seven reproducible Stage 2C parity failures, while the native-update maintenance mutant aborts on locator drift before testing anything. Rebase, reconcile each public contract deliberately, restore the mutant proof, and add independent legacy parity for complete/supersede rather than relying only on provider-provider agreement.
|
Addressed the two current review bodies on exact head Review-to-fix mapping:
Real-path validation on this head:
I also updated the PR description to replace the stale stacked-base narrative with the current review mapping and exact migration receipt. This PR remains open for independent review and will not be self-merged. |
|
Follow-up from the claim/lease contract side, verified on the current branch head [High] Promoted-goal Turn settlement always fails: the terminal envelope drops the keys [Medium-high] Drifted retries hard-fail forever where legacy is idempotent. The operation id is now fully deterministic over [Medium] Terminal decision rejections collapse into Happy to turn any of these into a patch branch if useful — the first one especially seems worth landing before the merge. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确评审 head:3ab3d03a209dc9d012aee11203e3b256e32ae224。
动机
这个 PR 要解决的问题是成立且重要的:Goal 完成、替换和归档在 canonical promotion 后,不能继续由 Python 与 TypeScript 各自持有一套决定与写入逻辑。理想边界应是 TypeScript 对 admission、lease、successor、Todo 和 receipt 做一次原子事务,Python 只负责兼容公共入口、执行 host-local validation effect,以及把 canonical 结果投影回现有状态文件。
我这轮没有从“这套迁移实现得是否完整”开始,而是先反向挑战“现在能否安全发”:沿着真实调用者分别验证了 Turn settlement、普通 Todo 重试、带 validation 的 Todo create、以及 decision rejection。结论是迁移方向正确,但当前精确 head 仍有四个公共语义回归。它们都能在作者的聚焦测试为绿色时复现,因此不能把 provider-local conformance、相同 reason code 或 schema 可读,视为公共兼容性已证明。
改动思路
正向路径的架构是清楚的:complete_goal_todo / supersede_goal_todo 通过 provider-first adapter 构造 typed request,todo_terminal_decision.ts 负责 actor、grant、claim/lease 与 gate admission,todo_terminal_lifecycle.ts 负责 validation plan、successor 检查、lease release、canonical CAS 和 receipt replay;todo_archive_selection.ts 统一 legacy 与 native archive 的资格和顺序;Python 最后执行 projection。这个 owner 划分符合现有 TypeScript control-plane migration RFC,也复用了既有 FileAuthorityStore、CAS/receipt 与 fail-closed promotion fence,而没有再造第二个 canonical authority。
这轮也按最新 PR-review capability 检查了新增/强化状态的必要性和真实生产者。Terminal receipt 是不能从最终 Todo head 安全推导的 authoritative fact,由生命周期事务自动产生,保留合理;私有 validation declaration 是不可从公开 digest 推导的 irreducible caller intent,私有/公开分离也合理。但二者的生产与维护边界尚未闭合:Python 给 receipt 的稳定 operation id 与 TypeScript 的 content-sensitive request hash 不一致;validation sidecar 又在 canonical create admission 之前发布。也就是说,“字段能写、reader 能读”并没有证明普通工作流中的生产、重试、拒绝与退役语义正确。
具体改动
整个 PR 共 47 个文件、+6542/-325:20 个 production 文件约 +3255/-145,18 个测试/示例文件约 +2956/-154,另有 7 个文档和 2 个生成契约文件。主要生产面包括 terminal decision/lifecycle、archive selector、effect-runtime handler、Python terminal/create adapter、completion-validation store/projection、runtime shadow 和 machine projection;测试面包括 Stage 2C 三臂 rehearsal、production-scale fixture、authority-store conformance、mutation 与 Python parity;文档补充了迁移边界、真实后端验证和 migration economics。
关键代码讲解
-
provider_terminal_lifecycle.py:450的 operation id 只绑定command/goal/todo/completion_turn_key-or-unscoped。它调用 TypeScript terminal transaction 后,在:545先写入规范化status="done",再用**payload覆盖结果,因而实际返回status="applied";同时返回体没有补回state_file和idempotent_replay。真实write_turn_validated_completion已复现:canonical Todo 成功提交,但结果为state_file=null、idempotent_replay=null。 -
todo_terminal_lifecycle.ts:387的terminalRequestSha还包含 note、evidence、reason、successors 等内容;:415的replayTerminal要求 receipt 中的 hash 完全一致。因此同一个稳定 operation id 下,仅 note 变化就被判定为coordination_operation_identity_mismatch。基线的同一公共调用则返回changed=false, idempotent_replay=true。 -
turn.py:501的todo_completion在公共完成调用后,必须从completion["state_file"]重新读取 durable Todo,并用changed或idempotent_replay决定是否可追加 settlement。当前 adapter 两个字段都没提供,所以这是“canonical 已完成、Turn 却无法结算”的失败;同请求 replay 仍不能自愈。 -
provider_create.py:77在调用 TypeScript canonical create 之前就执行persist_completion_validation_declaration。真实反例中,第二个同语义但不同 validation digest 的 Todo 被正确拒绝为todo_semantic_duplicate_conflict,canonical Todo 仍只有 1 个,但 sidecar 从 1 个变成 2 个,并保留了rejected-second的私有可执行声明。
归档 selector、optional prose 的 Unicode whitespace 归一化、promotion default-off fence,以及 TypeScript 对 actor/lease/successor 的 typed admission,本轮没有发现新的阻塞;聚焦测试覆盖也确实证明这些局部实现是可运行的。
对主干的风险
当前有四个 P1 blocker:
-
Turn terminal envelope 不完整。 触发条件是 promoted Goal 的 Turn 完成 Todo。TypeScript 已提交 canonical state,但 Python 返回体缺少 Turn 消费者要求的
state_file与 replay 语义,导致 settlementappended=false。最低修复是恢复 legacy-compatible envelope:durable state file、规范化 terminal status、从 typed terminal result 推导的idempotent_replay,并覆盖“canonical commit 后、journal append 前崩溃”的真实 replay。 -
稳定 operation id 与 content-sensitive hash 冲突。 同一 Turn/unscoped completion 在重试时只要 note/evidence/reason 或 successor 表达发生变化,就可能永久 identity mismatch。最低修复是让 logical operation identity 与 canonical request 内容使用同一个稳定定义,或在已完成状态上实现与公共契约一致的短路 replay;每个可变字段都要做 mutation/parity 覆盖。
-
被拒绝的 create 仍产生私有 side effect。 这违反 rejection/no-change 的无副作用不变量;并发同 ID create 还可能让 losing declaration 覆盖 winner,使 accepted Todo 后续因 digest mismatch 无法完成。最低修复是先暂存 intent,只在 canonical result 返回的 Todo identity/digest 匹配且被接受后发布,冲突/replay 时清理或 reconcile;至少增加 rejected duplicate 和 concurrent create 的零 orphan/零 overwrite 测试及单独 readback。
-
decision rejection 被折叠成 infrastructure unavailable。 同一 unregistered actor 调用,legacy 是可被
except ValueError捕获的 actionable rejection,promoted 变成LocalCoordinationAuthorityUnavailable(code=actor_not_registered)。Claim adapter 已有failure_kind=decision_rejection -> LocalCoordinationAuthorityRejection的邻近模式,terminal 应复用,而不是让调用者走错误的基础设施修复/重试分支。
验证方面,本轮在精确 head 上运行了 73 个 Python terminal/validation/archive 测试和 33 个 TypeScript decision/runtime/archive 测试,全部通过;Ruff 与 diff check 也通过。GitHub 必需检查为绿色,外部 SonarCloud 仍显示失败,但它不是本结论的依据。真正阻塞的是相同 synthetic fixture 经公共入口和真实 FileAuthorityStore 得出的基线—head 反事实。未对活跃 Goal、lease、registry 或生产状态做任何写入。
我的整体评价
这是一个有价值且 owner 放置基本正确的迁移,但目前还不能合并。尤其需要修正评审方式:不能只证明 TypeScript transaction 自洽,必须证明它经过 Python adapter 后,对真实 caller 的 accepted inputs、异常类型、完整 diagnostics、参数持久化/readback、replay 和 no-effects 仍保持契约。本 PR 净增约 3110 行 production、含约 732 行 bridge,问题本身值得做,但在这些核心边界仍回归时,当前完整机制的比例性也尚未证明;修完最新 finding 后不能沿用“接近批准”的轨迹,而要从原始问题重新跑整套 exact-head 评审。
请在当前 owner 边界内做最小修复,不需要另造 recovery framework:补全 adapter envelope、统一 logical identity、复用 typed decision-rejection 分类、把 sidecar publication 绑定到 accepted canonical create;然后用同一个公共入口 fixture 让上述四个反例从失败变为通过,并重新验证 File/PostgreSQL 与 Turn replay。
English verdict: REQUEST_CHANGES on exact head 3ab3d03a209dc9d012aee11203e3b256e32ae224. The TypeScript ownership direction is sound, and 73 focused Python plus 33 TypeScript tests pass, but real public-entry counterfactuals reproduce four blocking compatibility defects: the promoted terminal envelope cannot settle/replay a Turn, stable operation IDs conflict with content-sensitive request hashes, rejected validated Todo creation leaves an orphan private sidecar, and decision rejections change from ValueError-compatible rejection to infrastructure unavailability. Repair these boundaries and rerun the full exact-head review from the original problem; do not inherit approval from the previous round.
Review Summary(審查稿重點 · head
|
|
Reviewed exact head 1. A no-op archive on a promoted goal mints a new canonical transaction on every call. 2. A promoted goal whose Markdown has no user Todo section can never receive its compatibility projection. Validation: real CLI against |
3ab3d03 to
c9438dc
Compare
|
Addressed the owner review and @now-ing's follow-up on the rebased exact head Finite delivery target now met: promoted Goal Review-to-fix mapping:
Exact-head validation:
No new provider, daemon, promotion default, or recovery framework was added. This PR does not claim the full Todo migration or large net deletion; the RFC records 284 removed Python semantic-owner LOC, the bounded bridge cost, and the concrete facade exit conditions. No self-merge. The PR remains open for independent review. |
|
Final exact-head receipt after refreshing the RFC migration metrics:
The runtime code and its validation evidence are unchanged from the preceding review-reconciliation comment; only the two bilingual RFC receipt lines changed after the final main rebase. The PR body now reflects the final successor ownership, review fixes, exact merge-base metrics, and Stage 2C result (213/213 cases; four-worker final run 342.05s versus 552.11s at two workers, about 38% faster). Leaving this runtime-authority PR open for independent review; no self-merge. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确复审 head:4b195ea61d81ccc6e11d6525068f99753a4d01b1。
动机
这个 PR 要解决的问题仍然成立:promoted Goal 的 complete / supersede / role-scoped archive 不能让 Python 与 TypeScript 各自持有一套 terminal、successor、lease、CAS 和 receipt 语义。理想结果是 TypeScript 做一次 canonical transaction,Python 只适配现有公共调用、执行 host-local validation effect,并把 canonical 结果投影回兼容 Markdown。
与上次我在 3ab3d03a 的复审相比,原来的四个 blocker 已有实质修复:Turn terminal envelope 可以结算/重放;operation identity 与可变 prose 重试重新一致;validated create sidecar 绑定到 accepted canonical identity/digest;decision rejection 保持 ValueError 兼容分类。successor inheritance 也已经移到 TypeScript 单一 owner。当前 head 相对 c9438dc 只改了两行迁移文档,因此我重新按 whole-PR 而不是“只看修复点”的方式检查了 no-effect、default-off 和 projection counterfactual;结论仍是 REQUEST_CHANGES。
改动思路
正向链路清楚且总体符合仓库架构:公共 complete_goal_todo / supersede_goal_todo / archive_completed_todos 经 provider-first adapter 进入 todo_terminal_decision.ts 的 actor/claim/lease admission,再由 todo_successor_derivation.ts 推导 successor,todo_terminal_lifecycle.ts 对 Todo、successor、lease 与 receipt 做一次 File/PostgreSQL CAS;Python 最后交付 compatibility projection,Turn 从 readback envelope 完成 settlement。archive selection 也已收敛到 todo_archive_selection.ts,不再保留两份资格/顺序规则。
问题集中在三个“局部看起来合理、经过真实 caller 就改变语义”的边界:空 archive selection 仍被当成可提交事务;renderer 把缺失的空角色标题当成外部可重试前置条件;legacy complete 即使没有 successor,也无条件进入 successor actor validation。它们分别破坏 no-effect、projection convergence 和 explicit-promotion default-off parity。修复都应留在现有 owner 内,不需要再引入 abstraction 或 recovery framework。
具体改动
本 PR 全量是 55 个文件、+7982/-547:production +4050/-365,测试/示例 +3594/-156,文档 +335/-26,另有 3 行生成契约。主体包括 terminal decision/lifecycle、archive selector、successor derivation、effect runtime、Python facade、validation store/projection、File/PostgreSQL conformance、Stage 2C、mutation 和三臂 rehearsal;规模大,但仍围绕一次 terminal-authority migration,当前 blocker 是 correctness,不是主题散乱。
关键代码讲解
-
[P1] 空归档仍然制造 canonical transaction。
provider_terminal_lifecycle.py:508每次调用都生成todo-archive:<goal>:<role>:<uuid4>;todo_terminal_lifecycle.ts:1218-1240即使moved.length === 0,仍构造空 commit、附加 receipt 并调用commitAuthority。我在当前精确 head 上通过真实 public facade 与 FileAuthorityStore 连续执行三次:三次都返回status=applied, changed=false, moved_count=0,但 provider revision 从file:1依次推进到file:2/3/4。这会让周期性 maintenance 无限增长历史;真正移动过记录的 lost-response retry 也无法命中 replay。请在零选择时直接返回no_change,不写 receipt/transaction/revision;非空归档则使用与逻辑 intent/selection 绑定的稳定 operation identity。回归要验证三次 no-op 后 head/history 不变,以及一次 response-loss retry 只提交一次。 -
[P1] 合法 promoted Goal 可永久无法交付 Markdown projection。
machine_section_projection.py:417-422要求源 Markdown 同时具有所有TODO_SECTION_HEADINGS;但 shadow bootstrap/promotion 的 canonical projection 只保留 Todo records/roles,并不要求或记录空的 User Todo 标题。对只有## Agent Todo的合法 Goal,真实add_goal_todo已经提交 canonical Todo,随后返回projection_delivery=pending、reason_code=todo_projection_render_rejected、error_class=TodoSectionProjectionError;重复todo project-markdown仍会遇到同一个缺失标题,所谓 retryable 没有能改变前置条件的 owner。空角色标题是 machine-owned derived container,不是不可推导的 user intent。请由 renderer 在既定 Todo 区域确定性创建缺失空 section并保持 narrative bytes,或在 writer fence 生效前让 promotion 明确拒绝;对已有 promoted Goal,收敛式 renderer 更可恢复。回归应覆盖 canonical create、project-markdown、readback 与二次幂等。 -
[P1] default-off legacy complete 获得了 successor-only 注册门槛。
loopx/todos.py:1869现在无条件调用derive_successor_proposals;todo_successor_derivation.ts:249-253在确认successor_intents为空之前,就把actor_agent_id对照registered_agents校验。相同的 legacy Markdown Goal、无 successor intent、无显式 agent registry 在基线可完成,在当前 head 则公共 CLI 退出 1,payload 为ok=false/actor_agent_id is not a registered agent。这不是只有测试快照变了:当前 GitHub 两个 Python shards 共 5 个 unchanged-main 测试失败,我本地逐个重跑也是 5/5 失败,包括 periodic-report intent、settlement capability dispatch 和 unreadable optional machine-store recovery。最低修复是空 intent 时保留原 no-successor branch:不进入 successor derivation,或延迟 actor/completion-policy 的 successor-only 校验;同时让这 5 个现有测试原样转绿,并增加 typed empty-intent negative regression。
对主干的风险
这三个问题的共同风险不是“错误消息不漂亮”,而是状态效果和 authority boundary 发生了变化:idle archive 把无变化伪装成 canonical commit;canonical 成功后 compatibility projection 可能永久陈旧;最严重的是 feature-off/unpromoted 调用已经无法完成 Todo,直接阻断 post-writeback 与 settlement。第三项也解释了当前 required CI:shard 1 有 3 个失败、shard 2 有 2 个失败,aggregate pytest 和 merge-gate 因此失败;Stage 2C e2e、mutants、installed 与 Windows 均通过,所以不能用 Stage 2C 绿色覆盖 unchanged-main caller 的红灯。
正向验证是扎实的:我在隔离 PostgreSQL 16 上运行完整 TypeScript control plane,870 passed / 0 failed / 0 skipped;独立 PostgreSQL conformance 34/34;此前 focused Python terminal/validation/archive/split-root/Turn 集合 146 passed;TypeScript typecheck、changed-Python Ruff、git diff --check 和当前 origin/main merge-tree 都通过。反例同样来自真实边界:两个 FileAuthorityStore/public-facade 回归 2 failed,五个当前主干公共 CLI/post-writeback 用例 5 failed。这说明现有大套件对 transaction 正向自洽覆盖很好,但还没把 no-effect、projection completeness 和 feature-off parity 变成难以回归的独立 oracle。
我的整体评价
迁移方向、owner 放置与本轮已修的四个旧问题都值得肯定;完整变更的比例性也基本成立。但当前 exact head 仍有三个 P1,且其中一个已经使 required CI 和 explicit-promotion 的 default-off 承诺同时失效,不能批准。
请保持改动小而聚焦:空 archive 不提交;renderer 补齐 machine-owned 空 section 或在 promotion 前 fail closed;空 successor intent 不触发 successor-only actor gate。修复后需要在新 exact head 上重跑两个反例、五个 unchanged tests、完整 Python shards、Stage 2C 与真实 PostgreSQL,再做 whole-PR 复审,不能从这次结论自动继承 APPROVE。
English verdict: REQUEST_CHANGES on exact head 4b195ea61d81ccc6e11d6525068f99753a4d01b1 (published as COMMENT because GitHub blocks formal self-review). The prior four blockers are materially fixed and the TypeScript/File/PostgreSQL transaction architecture is sound, but three public-path regressions remain: empty archive execution writes a fresh canonical transaction on every call, a promoted Goal missing an empty User Todo heading cannot converge its compatibility projection after canonical success, and an unpromoted no-successor completion now fails an irrelevant registered-agent check. The last issue reproduces all five current Python-shard failures. Repair these no-effect, projection, and default-off boundaries in the existing owners, then rerun the whole exact-head review.
c9a7797 to
68cb8bb
Compare
|
Addressed all three findings from the latest review on exact rebased head
Exact-head evidence: affected Python matrix 84/84; TS 841 passed (plus one environment-gated PG skip); isolated PostgreSQL 34/34; Stage 2C 213/213; mutation lane 38/38; real read-only The rebased head is pushed and GitHub CI is running now. I will keep this open for independent review and will not self-merge. |
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
68cb8bb to
70c737e
Compare
|
Follow-up for the failed Windows lane is pushed on exact head The failure was a real retry-contract assertion gap, not terminal-transaction noise: on slow Windows filesystem I/O the first bootstrap request durably committed but its response missed the client window; the retry correctly returned Exact-head evidence: outbox suite 12/12; TypeScript typecheck; premerge 18/18; change-quality receipt |
Signed-off-by: huangruiteng <huangrt01@163.com>
|
CI follow-up is pushed on exact head The required Python/Windows/Stage 2C lanes were already green on the previous head. The remaining external Sonar failure was real evidence drift: Sonar analyzed the new TypeScript control-plane files but the workflow uploaded only Python This head fixes the evidence path without weakening the gate:
Local exact-path validation: 841 passed / 0 failed / 1 separately exercised PostgreSQL skip; TypeScript control-plane line coverage 83.06%; exact changed-TypeScript-line coverage 1,999/2,449 (81.6%); typecheck passed; premerge 18/18; public/private boundary passed. Change-quality receipt: I am waiting for this exact head's GitHub matrix and external Sonar result; no self-merge. |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这次按最新 pull_request_review_execution_contract_v2 对整个 PR 重新走了一遍真实调用链,并把审查绑定到 exact head ded61c4ec4ab5ed4b2e2d838d1219526cd9e1b36。把 Todo 终态决策、CAS 与 receipt 收拢到 TypeScript 的方向是对的;上一轮指出的空归档写 revision、缺失机器分区无法投影、空 successor 仍触发角色门禁三个问题也都已实质修复。
改动思路
我从公开 archive_completed_todos facade 追到 Python provider adapter,再到 TypeScript executeCoordinationTodoArchiveCompleted、AuthorityStore CAS/receipt 和兼容 Markdown 投影,并用真实 FileAuthorityStore 做了“提交已成功、投影阶段丢失响应、相同公开请求重试”的反事实验证。同时检查了 --note/evidence/reason 一类参数是否在真实 caller、operation identity 和 readback 中发生细微语义漂移。
具体改动
当前 head 相对 main 为 62 个文件、+8894/-584;其中产品运行时代码 24 个文件 +4108/-381,测试/示例 22 个文件 +3724/-175,公开文档 7 个文件 +335/-26,构建与 coverage 配置 9 个文件 +727/-2。最新提交增加了 TypeScript LCOV 上报,本地 coverage 运行通过 841 个测试、跳过 1 个,statement 86.48%、branch 77.32%。
阻塞问题(P1):provider_terminal_lifecycle._archive_operation_id 把调用前读到的 provider_revision 编进 operation id,但 archive transaction 没有把该 revision 作为 expected snapshot,也没有让一次丢失响应后的重试恢复原 operation id。第一次归档已提交并推进 canonical revision 后,如果兼容投影阶段抛错,第二次相同公开请求会基于新 revision 生成新 operation id,错过原 durable receipt,并返回 no_change / moved_count=0,而不是 replay 原 applied / moved_count=1。我通过真实公开 facade 和真实 FileAuthorityStore 稳定复现:第一次与重试的 operation id 不同,断言期望 replayed 实际得到 no_change。
建议做窄修:把实际 archive 执行绑定到已观察 revision,并保证响应丢失后的相同逻辑请求能够重用或恢复原 receipt;补一条至少归档一个 Todo、在 commit 后丢失响应、第二次调用读回同一 receipt 与原 moved result 的回归测试。无需引入通用 recovery 框架。
对主干风险
这不是内部字段差异,而是公开返回语义和幂等恢复语义丢失:canonical 状态已经变更,调用方却无法确认第一次操作结果;若据 no_change 判断没有发生归档,会造成错误审计和后续决策。当前远端 test-shard (2) 也失败:新增第 4 个 Sonar token guard 后,tests/test_sonarcloud_workflow.py 仍断言只有 3 个;其余已完成 required jobs 通过,aggregate pytest 尚在 pending。
验证结果:141 个 focused Python 测试通过;TypeScript control-plane 841 passed / 1 skipped;typecheck、Ruff、git diff --check 通过;隔离且一次性的 PostgreSQL 16 provider conformance 34/34 通过;上述 archive lost-response 反事实测试按预期失败并证明 blocker。
我的整体评价
结论是 REQUEST_CHANGES。这是作者自有 PR,GitHub 不允许正式 self-request-changes,因此以 COMMENTED review 明确记录阻塞。请先修复 archive receipt 的响应丢失重放语义,并修复红色 workflow regression;修复后需要基于新的 exact head 重新验证与复审。未来向检查已覆盖相邻 authority boundary;除该窄修与现有 coverage regression 外,没有理由扩展到更大的重构。
English verdict: REQUEST_CHANGES on exact head ded61c4ec4ab5ed4b2e2d838d1219526cd9e1b36. Archive lost-response retry derives a new operation id after the canonical revision advances, misses the durable receipt, and returns no_change/moved_count=0 instead of replaying the original applied result. The current remote test-shard (2) is also red because the Sonar workflow guard test was not updated for the added guarded coverage step.
|
Post-merge CI audit for
A narrow follow-up is prepared to make the test inspect every sensitive checkout/download/scan action and require the token guard semantically, instead of counting guard strings. Focused result: 3/3 passed; Ruff and diff checks passed. This is a test-contract fix only; the 80% Sonar threshold and analyzed source set remain unchanged. The failed Python shard prevented coverage aggregation, so external Sonar did not run on the merged head. The follow-up must verify both aggregation and external Sonar before closeout. |
|
@huangruiteng Both findings are confirmed fixed on One point I want to flag explicitly, because the archive case was an instance of a pattern that is still present. An authority
Reproduced on Consequences are the ones already accepted for archive: under Happy to open a tracking issue with the probe scripts if you would rather not carry this on a merged PR. |
|
@wchwawa Thanks for the concrete probes. I checked the shipped code and reran the real-file-provider/public-caller coverage on main
I would not implement either a blanket UUID ban or a parameter/current-revision hash. A permanent parameter hash can replay a stale result for genuinely new work, while a hash using the newly observed revision changes after a lost response and still cannot recover the original id. Empty archive selection has an explicitly different no-transaction contract; it is not a universal replacement for terminal no-change receipts. A follow-up should define the caller retry lifetime first, preserve explicit-key identity consumption and stale-writer fencing, and test response loss plus intervening writes before choosing durable attempt tracking or a lighter storage representation. Validation: 30 focused TS tests passed with real FileAuthorityStore (0 skipped), 33 Python local-authority/public-caller tests passed, and 5 managed-runtime retry/checkpoint tests passed (22 unrelated tests deselected). The additional update regression passes on the current implementation; it does not indicate a runtime fix. Documentation governance and diff/public-private scans also pass. No active goal or production provider was modified, and PostgreSQL was not rerun because this investigation changes no provider/runtime implementation. Product/architecture decision: preserve the current authority semantics; clarify the retry boundary and recovery limitation in the bilingual RFC and strengthen the no-change/intervening-write regression. No runtime patch or merge is being justified solely by the UUID pattern. Thank you for surfacing the storage cost and the missing cross-process recovery ergonomics; those are the useful boundaries for further work. |
|
Follow-up: #4130 is now merged. It clarifies the operation/retry and terminal no-change receipt contract in both RFC languages, and strengthens the update regression for replay after an intervening edit versus a new independent attempt. It does not change runtime semantics or claim to solve cross-process implicit recovery or receipt-history storage cost. Exact-head validation and the owner-authorized merge decision are recorded in that PR. |
|
The post-merge CI audit follow-up is complete in #4098 and merged as The brittle exact count of Sonar token guards was replaced with a semantic check over every sensitive checkout, coverage-download, and scan action. This keeps the token boundary and workflow behavior unchanged while allowing legitimate new guarded steps. Focused tests, Stage 2C collection, Ruff, diff/public-boundary checks, change-quality qualification, premerge checks, coverage aggregation, and external Sonar completed successfully. |
Summary
complete,supersede, and role-scopedarchivecommit Todo, successor, lease, and receipt mutations through one TypeScript-owned provider transaction.Review reconciliation
note,evidence, andreasonchanges remain legal retries, while a genuinely different successor intent is rejected as an operation conflict.ValueError-compatible error across legacy and promoted public entrypoints; storage/protocol failure remains an authority-unavailable error.unblocks_todo_id; Markdown/event paths call the same decision rather than reimplementing it.Migration economics
todo_terminal_decision.ts,todo_terminal_lifecycle.ts,todo_archive_selection.ts, andtodo_successor_derivation.tsown terminal admission, successor derivation, lease release, completion reduction, CAS/receipt replay, and archive selection.Stage 2C runtime
pytest-xdistworkers with--dist loadfile, preserving file-level isolation while running the 213-case real-process matrix in parallel.Validation on exact head
ded61c4ec4ab5ed4b2e2d838d1219526cd9e1b36loopx-metathree-arm rehearsal: 421 Todos and 58 leases; legacy/File/PostgreSQL heads and semantics matched, archive order and non-target state were preserved, receipts were present, and source state remained unchanged.applied,recovered, andreplayedsuccess.cqr_9b39fc8fefc6ac8456e9; fingerprint9b39fc8fefc6ac8456e9e43a2d1fc9b660a85b9bb95bbd76a564280bb0e6ee8c; 0 blockers.Scope boundary
This PR stops at the promoted terminal lifecycle closure. It does not add default promotion, a new provider/database/daemon, whole-CLI TypeScript migration, or a new recovery framework.
This runtime-authority PR remains open for independent review and will not be self-merged.