refactor(todos): move completion policy into TypeScript - #4044
Conversation
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确复审 head:e438c6afe1af6d0e1cb4cf7ea08744e768b592cf
动机
把 Todo completion policy 从 Python 迁到 TypeScript 是正确方向:状态机与 effect authority 应该由已有的 typed TypeScript 边界统一持有,Python 只负责读取事实、调用和适配结果。这也能减少完成、排除、successor ownership 等规则的双重实现。
改动思路
本 PR 新增 loopx/control_plane/todos/completion_policy.ts,由 Python completion_policy.py 组装 registry、linked successors 等事实;TypeScript 决定 registered-agent、exclusion、self-merge evidence、same-agent successor ownership 和 successor selection;completion_transaction.ts 在提交点重新评估并返回 typed result;Python adapter 再在 mutation lock 内 CAS completion_policy_source,最终由 complete_goal_todo 实际调用。整体 ownership、active call site 和事务重检路径是合理的,typed enums/messages 也保持了 domain-neutral,没有把 guidance 混写成 obligation;此变更没有 default-off 开关面。
具体改动
阻塞问题:新 completion_policy.ts 自己实现的 normalizeAgentId() 用了 JavaScript 的 \s:.trim().split(/\s+/u).join(" "),但 Python authority 的 compact_todo_text() / normalize_todo_claimed_by() 依赖 str.split()。这两套 Unicode whitespace 语义并不相同,而且本 PR 正在把身份/ownership 判断迁到 TypeScript,因此差异会直接改变 completion 的接受/拒绝结果。
在此精确 head 上可复现:
claimed_by在agent与one之间放入 U+0085、registered agent 为agent-one:Python 归一化为agent-one,TypeScript 却拒绝为未注册/非 public-safe id。claimed_by在agent-one前放入 U+FEFF:TypeScript 会 trim 后接受为agent-one,Python 则不会把 U+FEFF 当空白并返回None。
这不是纯展示差异:它位于 completion policy 的注册身份与 successor ownership 判定边界,可能把本来合法的 successor 卡住,也可能让 Python 不承认的 identity 被 TypeScript 接受。更关键的是,主干刚通过 #4020 在 loopx/control_plane/coordination/todo_agents.ts 引入了与 Python 精确对齐的 29-codepoint whitespace class 和可复用 normalizeTodoAgent();这里重新写一套 \s 正是会再次产生语义漂移的重复 authority。
最小修复建议:复用或提取 #4020 的共享 normalizeTodoAgent()(并让 registered-agent 输入走同一归一化/校验边界),不要再补一条局部正则;增加覆盖 Python 29 个 whitespace codepoint、U+FEFF 反例、registered/claimed/successor 三类身份的 data-driven parity/mutation tests,并至少有一条通过真实 Python adapter + TypeScript runtime 的路径,而不只直接测 TS function。
对主干的风险
当前风险为 P1:迁移后的 typed policy 在少见但合法的 Unicode 输入上与现有 Python contract 不一致,属于权限/ownership 决策的语义回归。现有测试只覆盖 ASCII 变体,因此 8 个 focused TS、50 个 focused Python、public todo-continuation-policy-smoke.py 和 typecheck:control-plane 全部通过仍无法捕获它。
其余范围是成比例的:它删除重复 policy authority、接入真实 caller,并覆盖 commit-time re-evaluation;没有发现 product-specific wording、prose-only classification、未披露默认行为变化或无调用点的未来脚手架。future-facing pass 的结论是必须把 agent-id normalization 收敛到共享 typed helper;更大的无关重构不需要放进本 PR。
我的整体评价
方向和边界设计值得保留,但当前 head 不能批准。请先统一 Unicode normalization authority,并用跨语言 parity + 负例证明 Python/TypeScript 对相同事实给出相同身份和完成决策;修复后我会按新 head 复审。
English verdict: Request changes on exact head e438c6afe1af6d0e1cb4cf7ea08744e768b592cf. The TypeScript completion policy reimplements agent-id normalization with JavaScript \s, which is not semantically equivalent to Python str.split() (for example U+0085 and U+FEFF), so registered-agent and successor-ownership decisions can diverge across the adapter boundary. Reuse the Python-parity normalizer already landed by #4020 and add data-driven cross-language positive and negative coverage through the real adapter/runtime path.
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确复审 head:e438c6afe1af6d0e1cb4cf7ea08744e768b592cf
动机
本 PR 要把 Todo 完成时的 registered-agent admission、排除、same-agent successor ownership 和 existing-successor selection 从 Python 迁到既有 TypeScript completion transaction。这个方向正确:状态机与 effect authority 应由 typed TypeScript 边界统一持有,Python 只投影 registry/source facts、执行 CAS fencing,并适配 legacy persistence。
改动思路
真实入口是 complete_goal_todo,Python build_completion_policy_request() 投影 claimed_by、registered agents、successors 等事实;completion_transaction.ts 在提交点调用 resolveTodoCompletionPolicy();Python adapter 再校验 typed result 并在 mutation lock 内比较 completion_policy_source。正向路径可以在一次 coarse transaction 中决定完成与 successor;负向路径应对 Python 与 TypeScript 看到的相同 agent identity 给出完全相同的接受或拒绝结果。
当前 head 的问题是 completion_policy.ts::normalizeAgentId() 自己使用 .trim().split(/\s+/u),而 Python authority 的 normalize_todo_claimed_by() 依赖 str.split()。JavaScript \s 与 Python Unicode whitespace 集合并不等价,因此迁移后出现第二份 identity normalization authority。
具体改动
全量 diff 为 14 文件、+957/-299:新增 TypeScript completion policy/transaction glue,收窄 Python policy adapter,更新真实 transaction/validation caller,并补充 TypeScript、Python 和 public CLI smoke。范围本身围绕同一个完成事务,active caller、typed result 和 commit-time re-evaluation 都存在;没有无调用点的未来模块,也没有 default-off、权限扩张、product-specific obligation 或 guidance/obligation 混淆。
阻塞复现仍成立:Python 把 agent\u0085one 归一化为 agent-one,TypeScript 当前 head 却以 “must be a public-safe registered agent id” 拒绝;反向地,Python 对 \ufeffagent-one 返回 None,TypeScript 会 trim 后接受为已注册的 agent-one。这会直接改变 claimed_by、next_claimed_by 和 next_excluded_agents 的注册/ownership 决策。主干已有 todo_agents.ts::normalizeTodoAgent() 对齐 Python 的 29-codepoint whitespace contract;这里不应再维护局部正则。
独立验证:Python completion transaction/validation 48 passed;TypeScript policy/transaction 8 passed;npm run typecheck:control-plane、public todo-continuation-policy-smoke.py、Ruff 和 git diff --check 均通过。远端必需 checks 通过(独立 SonarCloud quality gate 仍显示 failure)。这些绿色测试只覆盖 ASCII identity,不能关闭上述跨语言反例。
对主干的风险
阻断:[P1] 跨语言 agent-id 归一化会改变 completion authority。 触发条件是合法 Python whitespace(如 U+0085)或仅被 JavaScript trim 接受的字符(如 U+FEFF);错误路径经过 requireRegisteredAgent(),可能把合法 successor 卡死,或接受 Python contract 不承认的 identity。爆炸半径覆盖 Todo 完成、same-agent continuation、successor ownership 和 exclusion;错误可见为 misleading 的注册身份拒绝,也可能直到完成时才暴露。
最小修复是复用/提取已落地主干的 Python-parity normalizeTodoAgent(),让 registered-agent 输入和 claimed/successor/excluded identities 走同一 typed boundary;增加 29 个 Python whitespace codepoint、U+FEFF 负例以及 claimed/next/excluded 三类字段的 data-driven parity tests,并至少保留一条真实 Python adapter → TypeScript runtime 回归。无需新的 normalization framework。
我的整体评价
暂不批准。迁移边界、事务重检和代码体量是成比例的,future-facing pass 也只需要把 agent-id normalization 收敛到现有 typed owner;但在这个 identity/ownership 决策点上,细微 Unicode 语义差异就是行为回归,不能由常规绿色测试豁免。修复共享 normalizer 并补齐跨语言正负例后再按新 head 复审。
English verdict: REQUEST_CHANGES on exact head e438c6afe1af6d0e1cb4cf7ea08744e768b592cf. The TypeScript completion policy reimplements agent-id normalization with JavaScript \s, which is not equivalent to Python str.split() (U+0085 is rejected by TypeScript while U+FEFF is accepted only there). Reuse the Python-parity typed normalizer already present in todo_agents.ts and add data-driven cross-language positive and negative coverage through the real adapter/runtime path.
e438c6a to
38a1333
Compare
|
Addressed the completion-policy parity review on the rebased branch.
Validation on the exact final diff:
Fixture decision: the live corpus exposed no additional semantic class beyond the empty-value and Unicode normalization gaps now captured by focused durable tests. I did not copy live identities or operational state into a public fixture, and I did not widen this PR with redundant production-scale fixture rows. Commit: |
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
精确复审 head:38a1333445073f4402411a4f2682335a23e22c7f
动机
本 PR 把 Todo complete 时的 registered-agent admission、same-agent successor ownership、排除规则、existing-successor selection 迁入既有 TypeScript completion transaction。目标是让 typed TypeScript 边界拥有完成策略,Python 只投影 registry/Todo facts、执行外部 validation、做 lock 内 source CAS 并适配 legacy 写入。这个方向正确,也有真实 complete_goal_todo 调用点。
当前新提交已修复上轮指出的 claimed_by/registered/next/excluded agent-id Unicode 归一化差异,并额外补上空 next_agent_todo 与 Python truthiness 的一致性;但复审发现相同的跨语言空白语义仍留在 continuation policy 和 --self-merged evidence,两者会在真实 adapter 路径产生相反授权结果,因此不能批准。
改动思路
正向路径是:Python 从 registry、当前 Todo 和 linked successors 生成 completion_policy_request,TypeScript reduceTodoCompletionTransaction() 在 commit decision 中调用 resolveTodoCompletionPolicy(),返回 normalized owner/exclusions/successor;Python 在 mutation lock 内重算 policy source 并 CAS,随后消费 typed result 写回 Todo 和 successor。带 validation 的路径由 TS 先产出 effect,Python 执行 caller-approved command,再把 receipt 送回第二次 reduction。负向路径应在任何同一输入上保持 Python 旧 contract 与新 TS contract 的接受/拒绝、owner 和诊断一致。
新 head 复用了 todo_agents.ts::normalizeTodoAgent(),这是正确的单一 typed identity owner;29 个 Python whitespace codepoint 和 U+FEFF 反例对 claimed/registered/next/excluded identities 已覆盖。问题在于 completion_policy.ts 的另外两个 policy 字段仍各自使用 ECMAScript trim(),所以“修复共享 normalizer”没有覆盖完整决策面。
具体改动
关键代码讲解
completion_policy.ts:63-85:agent id 已改用共享normalizeTodoAgent()/normalizeRegisteredTodoAgents(),并把 typed protocol error 适配为 runtime request error。这个修复关闭了旧 head 的 U+0085/U+FEFF identity blocker。completion_policy.ts:185-191:continuationPolicy()仍以String(...).trim().toLowerCase()判断same_agent_non_delivery。Python 旧 authority 使用str(...).strip().lower(),两者的 Unicode whitespace 集合不同。completion_policy.ts:241-245:self-merge evidence 仍以 JavaScripttrim()判断是否为空,和旧 Python 的str(evidence or "").strip()不一致;这里不是展示问题,而是--self-merged审计证据的 machine gate。completion_validation.py:232-303/todos.py:1644-1737:policy 已进入 pre-lock validation transaction,并在 lock 内重算 source 后,再执行 lifecycle mutation authority。source CAS 能防止 validation 期间 registry/successor 漂移,但 policy 错误现在会先于 actor authority 和 task-lease fence 返回,这也是旧实现未披露的诊断顺序变化。
阻塞复现通过真实 Python adapter → TypeScript runtime 完成:
next_continuation_policy两端包 U+0085(NEL)时:Python 旧 contract 归一化为same_agent_non_delivery,应继承agent-a;当前 TS 回退到independent_handoff,返回effective_next_claimed_by=null。- 同字段包 U+FEFF 时结果反向:Python 回退 independent,TS 却 trim 后接受 same-agent 并继承
agent-a。 self_merged=true且 evidence 仅含 U+0085 时:Python strip 后为空并拒绝;当前 TS 接受且返回self_merged=true。仅含 U+FEFF 时又反向拒绝。也就是说非法的“视觉空 evidence”可以跨过新 authority gate。
最小修复不是再复制一条局部正则,而是从现有 Python-whitespace contract 提取通用 typed strip/blank helper,让 continuation normalization 与 evidence presence 共用;增加 29 个 codepoint、U+FEFF 的 data-driven 正负例,并至少通过公开 loopx todo complete 或 complete_goal_todo 证明 owner 与 self-merge gate 的真实结果。另请明确 policy-before-actor/lease 的错误优先级是否有意;若不是,恢复旧顺序,若是则披露并补 unauthorized/stale-lease 负例。
对主干的风险
风险为 P1。same-agent policy 的误判会把应继续由原 agent 承接的 successor 变成未认领,或把旧 contract 认定为 independent 的输入错误绑定给原 agent;self-merge evidence 的误判会让没有有效证据的完成记录带上 self_merged=true。爆炸半径覆盖 completion、same-agent continuation、quota settlement 后继以及自合并审计,不是罕见字符的纯文案差异。
全量 diff 为 14 文件、+1102/-299:TypeScript policy/transaction、Python adapter/CAS、真实 CLI smoke、四组测试与三处迁移文档,范围内聚且有 active caller;default-off 不适用,没有新增更宽 actor authority、产品专属文案或 guidance/obligation 混写。public smoke 303 行仍是耐久的真实完成边界,不是一次性样例。
独立验证结果:focused TS 10/10、focused Python 49/49、完整 TS control-plane 719 passed/1 skipped,typecheck、Ruff、public todo-continuation-policy-smoke.py 和 diff check 通过;远端 Python/DCO/build checks 通过或按设计跳过。独立 SonarCloud quality gate 仍为 failure(new-code coverage 43.5%、D reliability;另含 complexity/sort 告警)。其中 .sort() 的修复不能盲用 locale-dependent 比较而破坏 Python ASCII id 排序,应采用 parity-safe 处理或有依据地抑制。
我的整体评价
REQUEST CHANGES。当前 head 确实解决了上轮 exact-head 的 agent-id blocker,且架构、事务 source CAS 与改动体量总体合理;但同一 completion policy 内仍有两个可复现的 Python/TypeScript whitespace 语义裂缝,其中一个直接放宽 --self-merged evidence gate。请把共享 parity 扩展到所有迁移字段,并固定 policy 与 actor/lease 的错误优先级后再复审。
English verdict: REQUEST CHANGES on exact head 38a1333445073f4402411a4f2682335a23e22c7f (published as COMMENT because GitHub blocks formal self-review). The prior agent-id normalization blocker is fixed, but continuationPolicy() and the self-merge evidence gate still use ECMAScript trim() instead of Python-compatible whitespace semantics. Through the real Python-to-TypeScript adapter, U+0085 makes Python choose same-agent while TypeScript chooses independent, U+FEFF reverses that result, and U+0085-only evidence is accepted as self_merged=true even though the legacy Python gate rejects it as blank. Reuse one generic Python-whitespace helper across all migrated policy fields, add data-driven real-path parity tests, decide and cover the new policy-before-authority/lease error ordering, and clear or explicitly resolve the remaining Sonar quality gate.
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
38a1333 to
6f15858
Compare
|
Addressed the latest completion-policy review on a branch rebased onto current
Validation on exact head
The production-shaped corpus remains ignored/local; no live identity or operating state was added to the public fixture surface. |
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
精确复审 head:5605961e5dcf1d339493171be5ebc83705b6f130
动机
本 PR 把 Todo completion 的 registered-agent admission、same-agent successor ownership、排除、自合并证据和 existing-successor selection 从 Python 迁入既有 TypeScript authority。方向正确:typed TypeScript 边界拥有策略,Python 只投影 registry/Todo facts、执行外部 validation、做锁内 source CAS,并适配 legacy 持久化。这样能消除 completion policy 的双重真相,同时保留真实 complete_goal_todo 入口。
改动思路
Python 在 validation 前投影 policy source,TypeScript completion transaction 先决定 completion state;进入 mutation lock 后重新读取 Todo、registry 和 linked successors,比较 policy source,随后按 legacy 顺序完成 actor authority、task-lease admission,最后才调用纯 TypeScript policy reducer并消费 typed result。带 caller validation 的路径仍只执行一次外部命令,policy 不会在 Python 重新决策。
前两轮发现的细微语义差异已按同一 owning boundary 收敛:agent id、continuation policy 与 self-merge evidence 统一复用 Python 29-codepoint whitespace contract,明确把 U+FEFF 保持为非空白;空 next_agent_todo 仍表示未提供,而非空的纯空白字符串仍按 Python bool(str) 视为已提供;actor、lease、policy 的错误优先级也恢复为旧 contract。
具体改动
completion_policy.ts使用stripPythonWhitespace、hasPythonNonWhitespaceText、normalizeTodoAgent和normalizeRegisteredTodoAgents,不再维护 ECMAScripttrim()/\s的第二套 authority;claimed/registered/next/excluded、continuation 与 evidence 均在同一跨语言规则下处理。completion_validation.py捕获 completion policy source 并在锁内重算,覆盖 validation 窗口内 registry/successor 漂移;completion_policy.py只在 actor/lease admission 之后调用 TypeScript reducer并做结果形状适配。- 数据驱动测试覆盖 Python 全部 29 个 whitespace code point、U+FEFF 反例、空字符串与 whitespace-only successor、self-merge evidence、actor/lease 错误优先级及真实 Python adapter → managed TypeScript runtime。public smoke 已从 helper 级断言升级为公开
loopx todo complete路径。 - 最新提交只把 serialization test 的 monkeypatch 从旧
exclusive_file_lock改到当前 canonicalexclusive_cross_runtime_file_lock,与主干 writer ownership 一致,没有改变生产语义。
对主干的风险
独立验证通过:TypeScript typecheck;完整 control-plane suite 807 passed, 1 skipped;focused TypeScript 10 passed;completion transaction/validation/mutation/serialization Python 集合 111 passed;公开 continuation smoke、Ruff 与 git diff --check 均通过。远端 DCO、dependency review、build、Windows、两个 Python test shard 和聚合 pytest 已通过。独立 SonarCloud quality gate 仍因 new-code coverage 52.3% < 80% 报 failure,但 reliability、security、maintainability、duplication 与 hotspot 条件均为 green;该聚合覆盖率不计入完整 TypeScript 路径,且本 PR 的迁移决策面已有 29-codepoint/U+FEFF、truthiness、authority ordering、source CAS 与真实 CLI/adapter 回归,因此把它记录为非阻塞覆盖信号而非未验证的语义缺口。
本次重点不是只看 happy path,而是逐字段对照旧 Python truthiness、Unicode strip、注册身份、排序、linked-successor selection 与错误优先级,并检查真实 caller 与锁内重读。没有发现 substring/prose-only classification、product-specific wording、guidance/obligation 混淆、权限扩张或未披露 default behavior change。future-facing pass 的相关收益已经通过共享 whitespace/identity helper 和单一 TypeScript policy owner实现;再加框架会超出本 PR 的 bounded scope。
我的整体评价
批准此 exact head。此前 U+0085/U+FEFF、空 successor 以及 policy-before-authority/lease 的阻塞都已在共享 typed boundary 修复,并有跨语言负例和真实公开路径回归;当前 diff 内聚、可回滚,验证与风险相称。由于作者自审限制,本结论以 COMMENT 发布,但语义等同于 approve。
English verdict: APPROVE exact head 5605961e5dcf1d339493171be5ebc83705b6f130 (published as COMMENT because GitHub blocks formal self-approval). The refactor now preserves the legacy Python contract across all migrated policy fields: the shared 29-codepoint whitespace rules cover identities, continuation, and evidence; U+FEFF remains a negative counterexample; empty versus whitespace-only successor semantics are explicit; and actor/lease errors retain priority over policy admission. Full TypeScript, focused Python, real adapter/CLI smoke, lint, diff, and current required remote test evidence are green.
|
Post-merge audit finding on exact head 动机这次按“重构后必须重新证明旧语义、真实入口和事务形状”的标准,从原始目标重新复审,而不是沿用前几轮“blocker 已修复,所以接近批准”的轨迹。原始目标是把 Todo completion policy 并入既有 coarse TypeScript completion transaction,由一个 typed owner 决定 policy,同时不增加 leaf bridge; 改动思路最终代码确实把 registered-agent、successor ownership/exclusion、continuation 与 self-merge evidence 的语义实现收敛到了 TypeScript,并用 Python 做 registry/Todo fact projection、source CAS、actor/lease admission 和 legacy persistence。前几轮发现的 Unicode whitespace、空 successor truthiness 与 actor/lease 错误优先级已经修好。 但为恢复 actor → lease → policy 的旧错误顺序,最终提交没有再把 policy request 交给 具体改动[P1] 最终实现为每个非 replay completion 新增了第二次、且位于 mutation lock 内的跨 runtime 调用;文档和测试把它误报成一次 coarse reduction。
语义复核也做了反证:我对旧 Python policy 与最终 TypeScript policy 跑了 2,547 个确定性组合(claimed/next owner、空值与 Python 29-codepoint whitespace、U+FEFF、continuation、exclusion、self-merge evidence、linked successor),结果为 0 mismatch;真实 对主干的风险这条额外 IPC 位于共享 Todo writer lock 与 task-lease fence 内,会让每个普通 completion 的临界区承担 managed-runtime 请求、冷启动/重启和超时风险;同一 Goal 的并发 Todo 写会因此受到额外阻塞。即使 policy failure 仍能在写前 fail closed,新增 failure point 和 lock hold time 也与本迁移“减少 bridge chatter、完成一笔 transaction cutover”的收益假设相反。更重要的是,当前测试会继续把任何新增非 最小修复建议:把 policy evaluation 重新折回 我的整体评价这是一个需要跟进的 post-merge P1 finding。最终 policy 语义本身经过差分与真实 note 路径复核,没有发现新的接受/拒绝或 English verdict: POST_MERGE_AUDIT finding on exact head |
|
Post-merge audit follow-up is now in #4068. The fix folds completion-policy evaluation into the existing Evidence on the exact head includes 110 focused Python tests, 808 passing TypeScript control-plane tests (1 expected skip), 30/30 isolated real PostgreSQL tests, an unchanged read-only live-goal semantic projection with the runtime call sequence reduced from two methods to one, 18/18 premerge checks, and change-quality receipt The follow-up is intentionally left open for independent review. |
Summary
Validation
npm run typecheck:control-planenpm run test:control-plane— 691 passed, 1 existing skiploopx todo add/completereal-Markdown smoke passedloopx canary premerge --from-git-diff --goal-id loopx-meta— 18/18 passedcqr_29c93d4a2a391263cefd29c93d4a2a391263cefd31b43ab84d1aec5e9870ba2e8e60de95bc832341f538Scope / review
This is a bounded TypeScript-control-plane cutover that prepares shared-authority work without introducing a new leaf runtime handler. The future-facing pass was applied only at the adjacent completion-policy boundary. A maintainability-ratchet safe-fix extracted the lock-comparable fact projection and kept the hot Python module within its ceiling.
Please review independently; this PR will not be self-merged.