fix(leases): unify typed eligibility and canonical inspection - #4269
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
该 PR 试图消除 lease owner eligibility 的重复规则,并让 task-lease inspect 从同一 canonical revision 返回状态,避免旧 hint、Markdown 或本地残留复活失效租约。
改动思路
通过 task_lease_eligibility.ts 集中 typed owner 判定,acquire/lifecycle/native claim/Python adapter 复用同一决定;inspection 只读 Todo、lease 和 handoff mode,缺失或 provider 不可用时保持 canonical absence,不执行修复写入。
具体改动
关键代码讲解
task_lease_eligibility.ts成为 eligibility 规则拥有者。task_lease_acquire.ts移除 caller effective hint 的权威性。task_lease.py与authority_core.py适配 typed facts/result。test_canonical_lease_inspection.py、task_lease_eligibility.test.ts覆盖冲突 hint、缺失 lease 和 provider 场景。
对主干的风险
required windows-powershell check 当前失败,Windows 上的兼容性尚未得到可接受的回归证据;其余 TypeScript、Python、PostgreSQL provider conformance 与语义反例验证通过。请先定位失败并重跑该检查,再合并。
我的整体评价
REQUEST_CHANGES。设计边界和测试覆盖方向正确,但红色 required check 是明确的交付阻断,不能以其它平台通过替代。审阅 head: 5a6bc6582143713d4080ea7cf2fdd338bff61440。
English verdict: REQUEST_CHANGES — head 5a6bc65. Diagnose and rerun the failing windows-powershell required check before merge.
|
Request changes conclusion (author-owned PR; GitHub blocks formal self-review) 评审对象:#4269,exact head 动机PR 试图把 lease owner eligibility 和 canonical inspection 收回到同一套 provider-neutral 规则,避免 promoted goal 同时依赖局部 lease、Python 旧谓词和 canonical provider,产生 stale owner 或不一致诊断。 改动思路TypeScript 具体改动
正向路径:匹配 binding 的 registered actor 通过 typed eligibility,读取 canonical revision 并返回 lease owner。反向路径:foreign/excluded actor、canonical missing 或 provider unavailable 均拒绝/失败闭合,不写 Markdown、不返回 stale local lease。 对主干的风险
我的整体评价方向正确且范围合适,但当前 exact head REQUEST_CHANGES。最小修复是 rebase/解决冲突,并修复或明确 reconcile Windows required check 的 installer/quota skip contract;修复后以同一 exact head 重跑公共 lease acquire/inspect、provider-unavailable 和 required checks,再复审。 English verdict: REQUEST_CHANGES — #4269 exact head |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
这个 PR 的目标是把 lease-owner eligibility 的四份判断统一为一个 typed owner,并让 promoted inspection 从一个 canonical provider revision 同时读取 Todo、lease 和 handoff_mode。旧实现既存在 acquire/lifecycle/native claim 的语义漂移,也允许 acquire 参考 caller 提供的 effective hint;Python inspection 在 promoted 场景还可能把 canonical Todo 与过期 local lease 混在一起。新设计删除了这些平行决定源:eligibility 从 Todo、claim、exclusion、registration facts 推导,旧 hint 只兼容解码而不再有权威性;promoted 模式不回退 stale local lease,unpromoted 模式才保留明确的 legacy 兼容边界。
改动思路
task_lease_eligibility.ts 提供确定的 rejection precedence 和 schema task_lease_owner_eligibility_v0,authority_core 通过既有 effect runtime 调用它,acquire、lifecycle 和 native claim 共享同一结果;release 的 fence 仍是原有 transition owner。inspect_task_lease 在 promoted provider 下调用 read_canonical_todos_if_promoted(include_leases=true),从同一 revision 生成 projection,lease_path 为 null;若 canonical provider 缺失,保持 lease null 而不读旧文件。只有未 promoted 时才进入原来的 local reader。这个边界符合“TypeScript 负责控制面决策、Python 负责适配/展示”的仓库分层,也没有把 inspection 变成 grant authority。问题在于远程必需的 Windows installer/doctor/quota 路径目前没有通过,因此 Linux/TS 的语义证据尚不足以覆盖实际跨平台入口。
具体改动
- 新增
task_lease_eligibility.ts,集中 owner rejection precedence,覆盖 missing Todo、non-open、invalid/unregistered/excluded/conflicting claim 等条件。 authority_core.py删除 Python duplicate predicate,改为 effect-runtime schema/result 校验;task_lease_acquire.ts不再信任旧 wireeffectivehint。- lifecycle/terminal fencing 与
todo_claim.ts复用同一 eligibility owner,保留 release-after-eligibility-loss 的现有语义。 task_lease.pypromoted inspection 使用 canonical provider 的 Todo/lease/handoff_mode 快照;unpromoted path 显式保留 legacy local reader,避免兼容模式被误改。- 五份 RFC/protocol 文档记录 canonical reader closure 和剩余边界。
- 本地验证:canonical lease、task lease、local coordination 三组 Python 测试 79 passed(69.41s),TS eligibility 6 passed,Ruff 通过;这些测试证明 stale fallback、schema 和 rejection precedence 的预期行为。
- 但 exact head
5a6bc6582143713d4080ea7cf2fdd338bff61440的远程检查为 25/27 成功,必需windows-powershell与merge-gate失败。失败日志显示tests/test_windows_install.py::test_windows_installer_promotes_release_and_runs_doctor在 bootstrap/todo 后执行 quota 时返回 JSON statusskip、process returncode 1,断言quota.returncode == 0失败。Linux 等价流程返回 0,尚不能推断根因是代码还是 Windows 环境。
对主干的风险
在 Windows required check 未解释前合并,可能把 installer/doctor 的平台回归带入主干,使用户在安装后无法完成 quota/doctor 流程;merge-gate 也明确阻止了干净合并。该失败不等同于我凭空认定某一行代码错误,但它是 material unverified evidence,不能用本地绿色测试抵消。另有较小的边界风险:canonical snapshot 的注册 agent 列表和 provider completeness 需要继续保持一致,缺失 bounded snapshot 不能被解释为全局不存在;当前代码已 fail closed,但应保留 provider completeness 诊断。没有执行真实生产 lease promotion,也没有修改任何 active authority。
我的整体评价
架构方向是正确且比例合适:共享 typed predicate 确实消除了四处分叉,promoted/unpromoted reader 也把 canonical authority 与兼容读取分开;本地 79+6 测试和其它远程检查支持这一判断。但 required Windows/merge-gate 失败使 exact head 的 observable semantics 尚未闭合,尤其是用户可达的 installer/quota 入口。请先定位为何 Windows 路径把 quota 标为 skip 并以 code 1 退出,修复后重新运行 windows-powershell 和 merge-gate,再请求复审;在此之前不能合并。
English verdict: REQUEST_CHANGES
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
5a6bc65 to
2f06255
Compare
Signed-off-by: huangruiteng <huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
评审对象:#4269,exact head a12fe472454b7ff3adbb6284b8c6108fb6433dbd。
动机
本 PR 统一 lease owner eligibility 与 canonical inspection,避免 Python、TypeScript、旧 effective hint 和 stale local lease 形成多套状态判断。
改动思路
TypeScript typed eligibility 负责 owner/binding/exclusion 决策;promoted inspection 从同一 canonical revision 读取 Todo、lease 与 handoff mode,canonical 不可用或缺失时 fail closed。Windows 源代码安装路径的 UTC 默认值不再依赖本机 IANA 数据库,非 UTC 时区仍保持显式校验。
具体改动
- acquire、lifecycle、native claim 和 Python adapter 复用
task_lease_eligibility.ts的 typed 结果。 - promoted
inspect_task_lease不再合并 stale local lease;unpromoted 路径保留明确的 legacy reader。 - 补 canonical inspection、typed eligibility、跨语言身份和 Windows 诊断回归;RFC/protocol 文档同步边界。
- 未新增 provider、promotion policy 或 lease storage format;
tzdata尝试已撤回,避免破坏 no-deps/source-install CI。
对主干的风险
聚焦风险是非 UTC 时区仍需要平台 IANA 数据库;本修复只把内建 UTC 默认变成跨平台可用,不把未知时区默认为有效。canonical provider 缺失继续 fail closed,不会回退 Markdown 或 stale lease。
本地验证:TypeScript typecheck 通过;控制面筛选测试 1220 passed、1 skipped;Python lease/periodic-report 测试 39 passed;mypy 配置的 22 个源文件通过;Ruff 通过;PostgreSQL authority-store smoke 4 passed、真实连接测试因未提供 LOOPX_TEST_POSTGRES_URL 跳过。最新 PR CI run 34686284962 的 28 个检查全部成功,包含 windows-powershell、Stage2C、pytest、kernel-static-checks 和 merge-gate。
本机 premerge canary 的 17/18 检查通过;唯一失败是 hot-path-interface-budget-smoke 的既有 dashboard JSON 预算超限。用 origin/main 同样复现 18,930 > 18,500,故不是本 PR 引入,未扩大范围修改无关 dashboard;公共边界和 risk-profile smokes 全部通过。
我的整体评价
实现已把 eligibility 与 canonical read 收回正确 owner,兼容边界显式且可回滚;真实远程 Windows/Stage2C/merge-gate 证据闭合。基线 canary 的独立 dashboard 预算问题应另开修复,不阻断本 PR。
English verdict: APPROVE — exact head a12fe472454b7ff3adbb6284b8c6108fb6433dbd; required CI checks pass and the only local canary failure reproduces on origin/main.
Summary
effectivehint must neither replace an eligible retained lease nor keep an ineligible owner blocking.task-lease inspectread Todo, lease and handoff mode from one canonical revision. An absent lease remains absent; unavailable providers cannot revive obsolete local JSON or fall back to Markdown. Inspection never repairs or writes state.Issue Or Task
Owner-requested next cohesive TypeScript/shared-authority RFC implementation batch.
Semantics and scope
Intentional fixes: current-lease
effectiveis a backward-readable wire hint, not authority over the accompanying facts; promoted inspection returns canonical absence, mode and revision instead of combining different sources. Itslease_pathis nownull, with additive source/revision provenance.Preserved: owner registration, exclusion and conflicting-claim rejection order; terminal and version/idempotency/epoch fences; acquire CAS and replay; release remains a proof-fenced cleanup operation even after owner eligibility is lost. An unexpired raw
lease.status=activecan still produceactive=falsewith an executor constraint. No automatic ownership transfer, capability enablement or permission grant.Refactor rationale: these were duplicate domain rules, not merely similar code. The nearest existing work-items owner is sufficient; no capability, provider, generic framework or second Python policy is introduced. The public Python entrypoint only adapts typed facts/results. Current-lease precomputation is removed from its callers. Other-Todo overlap facts still come from the existing complete execution snapshot. Unpromoted storage and permanent Markdown rendering remain; Goal-channel lease display is a separate remaining consumer.
Validation
5a6bc6582143713d4080ea7cf2fdd338bff61440; final suites rerun after rebase onto37ca93ff7.effectivecounterexamples failed before the fix and pass after it. Shared precedence, release cleanup and production-scale synthetic fixture dimensions are covered by six new TS tests.37ca93ff7: 18,930 characters versus 18,500. This PR does not change that payload or relax the budget.Type of Change
LoopX Area
Technical Direction
mainShared-authority RFC fixture impact
productionScaleCoordinationFixture; no new persisted schema. New tests overlay claim/exclusion facts across its historical records.Boundary Checklist