fix(quota): replay unleased monitor poll receipts - #5021
Conversation
Signed-off-by: huangruiteng <14976749+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)
动机
本次评审针对 dcbf1cf32b19964473527c7be75587106c32557e。发布前完整测试发现:一个仍在执行原始任务的 Turn 做了辅助 Monitor 轮询后,第一次观察成功,原样重试却被告知既有事务“缺少租约凭证”。旧式 Goal 根本没有租约;这会把正常的幂等重放误判为需要人工修复,阻断原任务继续。它是 v1.2.0 发布资格中的真实回归,不是为了让一个孤立断言变绿。
改动思路
公开 quota monitor-poll 仍经由既有的 CLI、Python 配额传输和 TypeScript 效果提交路径。事务回执与 canonical authority 各司其职:回执说明同一 effect 是否已有提交及其凭证,当前 authority 决定该 Todo 是否处于硬租约约束。补丁只在传输层区分“没有回执”和“有合法的无租约回执”;遇到硬租约模式或目标曾有租约时,缺少凭证仍拒绝。没有新增认领、续租或第二套写入规则。旧提交与新提交用同一个未修改的辅助 Monitor 用例对照,前者失败、后者通过;file/SQLite 的有租约重放在两边都通过。
具体改动
关键代码讲解
monitor_poll_lease_transport._receipt_proof现在返回“回执是否存在”与可选凭证。它仅对已知的无租约 provider plan 或写回 schema 接受空凭证;不存在的回执仍走当前租约发现,损坏的 effect 身份、未知状态和无效凭证仍报错。monitor_poll_lease_transport.current_monitor_lease_proof使用这个区分:旧式或软认领回执可原样重放;已有硬租约 authority 或目标租约时,空凭证不被提升为授权。原有已记录凭证的重放仍要求 canonical authority 存在。test_current_lease_cli_transport_keeps_soft_claim_compatible从首次成功扩展到同一命令的实际重放;新增负例移除硬租约回执内的凭证,验证传输层拒绝。未改动的辅助 Monitor 用例覆盖原始回归及其后续 Turn 继续条件。
对主干的风险
最危险的反例是把“没有凭证”一律当成安全,从而让硬租约观察绕过 owner 和版本约束。补丁只接受声明为无租约的既有回执,并在当前 canonical authority 已启用硬租约或目标存在租约时拒绝;负例实测拒绝。另一风险是误伤本来合法的有租约重放,file 与 SQLite 的公开 CLI 测试在旧 head 和本 head 各通过两例。旧式辅助轮询用例在旧 head 失败、本 head 通过,证明修的是可观察结果,而非只改测试预期。Ruff、mypy、TypeScript 类型检查、公开边界扫描、24 项聚焦测试和提交版 18 项 premerge 检查通过,零失败、零人工 hold。完整发布资格仍需在最终合并提交上重新执行;该 PR 的通过不等于版本已可发布。
我的整体评价
同意合入这个精确 head。 对长期任务,幂等轮询恢复后仍可继续原 advancement Turn;对操作者,重试不会再因不存在的租约收到虚假的阻断。机制局限于现有传输层两个函数和两条耐久测试,保留硬租约权限边界,没有新 CLI、schema 或自动启用能力。评审覆盖完整差异并对照了真实 CLI 的旧、新行为;发布整体验收与 tag 仍是后续独立关口。
English verdict: APPROVE - dcbf1cf restores idempotent unleased Monitor replay while preserving hard-lease rejection; 24 focused tests and 18 exact-head premerge checks passed.
|
Merge decision for exact head Validation: 24 focused cases, Ruff, mypy, TypeScript control-plane typecheck, Python compile, public-boundary scan, and 18/18 risk-based premerge checks passed. Failures: 0 on the head. Skips: 0 in these selected checks. Manual holds: 0. The exact-scope change-quality receipt and published exact-head review are valid; merge-readiness returned The owner explicitly authorized admin bypass for these release-blocking bug-fix merges. The bypass affects GitHub branch protection only; it does not waive the validation or exact-head review above. |
A
quota monitor-pollgenerated for a legacy or soft-claim Monitor could commit its observation, then reject an identical retry with “prior ... receipt lacks a valid lease proof.” This blocked idempotent replay of an auxiliary Monitor while the original advancement Turn was still open.The CLI transport now distinguishes an absent receipt from an existing receipt whose declared unleased plan/writeback has no proof. It replays that receipt without acquiring a lease, while rejecting proof-free replay after a hard-lease promotion or when the target has a lease. The existing file/SQLite leased path remains proof-bound.
Validation: 23 focused Monitor tests and one hard-lease missing-proof negative test passed; Ruff, mypy, TypeScript control-plane typecheck, Python compile, public-boundary scan, and diff checks passed. The managed change-quality receipt is valid for this committed diff. The risk-based premerge check is running on the exact commit; its result will be reported in the exact-head review before merge.