Fix generated monitor-poll commands under hard leases - #4995
Conversation
|
Self-review follow-up: added a separate File/SQLite test for a live Monitor lease owned by another agent. The read-only lease transport rejects it and leaves no quota-monitor-poll pending transaction (2/2 local tests passed). The public CLI has an earlier same-Turn receipt gate as well; the new test targets the owner fence directly so that both protections are covered without pretending the CLI reached the transport. The PR remains review-required. |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
已复核旧行为:生成的辅助 quota monitor-poll 命令没有传递已有硬租约的证明,所以 Agent 即使持有该 Monitor 租约,命令仍会在 canonical 校验处失败。本 PR 交付的是可独立验证的只读证明传递;它不申请或续租,也不解决所有 provider 拒绝后的 pending 清理(该边界由 #4991 处理)。
改动思路
在现有生成命令中加 --use-current-task-lease,在 quota preflight 前按精确 Goal/Turn/Todo/Agent 读取 canonical 活跃租约;同一 Turn 已有交易回执时优先复用原证明以维持请求身份。Python 只做发现和传递,最终版本 CAS、归属与变更许可仍由 TypeScript canonical Monitor 事务原子裁决。手动显式证明入口保留,soft-claim 和未提升的旧路径保持无证明行为。
具体改动
关键代码讲解
current_monitor_lease_proof:先按 exact effect 查 pending/prepared/committed 回执证明供重放;否则只接收一个活跃、归属匹配的 canonical 租约,过期或异主直接失败,不写入租约。record_quota_monitor_poll_for_decision:在 quota 预留前解析该证明,再沿既有 provider plan 传给最终 TS 校验;未启用 flag 的显式入口不变。_build_interaction_cli_channel:两条生成的辅助 Monitor 命令带上 flag,并在输入契约说明证明来源、不获取或续租。CLI 拒绝缺少 Turn/Todo/Agent 及与显式证明混用的请求。
对主干的风险
最强反例是读取租约后、canonical 提交前租约失效:TS 会拒绝过期证明,但若 #4995 先于 #4991 合并,独立运行时仍可能留下 provider_pending。因此这是明确的合并顺序约束:#4991 先合并,#4995 后合并,并复核集成后的确切 head,不是放宽租约验证。真实 File/SQLite CLI 31 项通过,覆盖活跃证明、过期/异主失败、soft-claim、进程死亡和释放后的重放;Ruff 与 diff check 通过。未跑真实 PostgreSQL、打包 UI 或全量套件;CLI-only 入口不新增前端/Lark 设置。没有等 CI,遵守 wait_for_ci=false。
语义与 CI 对齐
沿用现有 lease_proof 和持久 quota 回执,无第二个写入权威。新 CLI flag 对手动调用默认关闭,但生成的辅助命令有意启用;双语文档、CLI 帮助及投影测试披露了这一默认差别。
我的整体评价
未发现本 head 的阻断性实现问题;相对于每次硬租约监控都失败的旧行为,107 行局部只读 helper 与回归覆盖是相称的。未来相邻重构应继续让 TS 掌握租约准入,不能把 Python 发现逻辑提升为第二套裁决。当前结论不授权自行合并;请维护者按上述依赖顺序处理。
English verdict: APPROVE - head ff2ff7b. Merge only after #4991 and integrated exact-head recheck; maintainer merge required.
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
ff2ff7b to
717535c
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval). Approved exact head 717535c9cc7b3a3f40dfe1667f6464a64e60d610, rebased onto origin/main a6b79e666c25d933dc3febaae282ceba71db7860, which now contains the prerequisite #4991. My earlier review approved ff2ff7bb2 with a merge-order constraint (#4991 first, then this PR on an integrated exact head); that constraint is now satisfied, and the integrated head re-runs every check below.
动机
已复核旧行为:生成的辅助 quota monitor-poll 命令不带已有硬租约的证明,于是 Agent 即使正持有该 Monitor 租约,命令仍会在 canonical 校验处失败,Turn 拿不到可用的观测结果。本 PR 交付的是一条可独立验证的只读证明传递:它不申请、不续租,也不放宽任何租约校验。
对长期推进(long_horizon)而言,这个修复让「被分配到的监控工作」能够真正走完一次动作、落一个持久结果,并在后续 Turn 继续推进:修复前每次硬租约监控都在同一个校验处空转,反复重试不会累积任何进展;修复后同一 Turn 的首次提交与之后的幂等重放、租约释放后的重放都能落到同一份持久观测上,且 #4991 已合入,使「先缺证明被拒 → 同 Turn 补齐 → 一次提交」与「读取证明后租约才失效」两种路径都不会留下幽灵预留。对用户可感知的旅程(user_experience)而言,受影响入口是 loopx quota monitor-poll 与其生成命令:修复后失败信息明确指向「需要由本 Agent 持有的当前活跃租约」,成功状态可读回(ok 与 replayed),纠正方式是获取/续租或显式传证明,恢复路径经 #4991 的释放分支回到重新提交。两个维度都不是仅凭一次成功调用就断言,而是沿入口、持久结果与后续继续三段核对的。
改动思路
入口是生成命令里的新 flag --use-current-task-lease:quota 在预留前按精确 Goal/Turn/Todo/Agent 解析证明,先查同一 effect 的既有 quota 交易回执(pending/prepared/committed 三种形态各取对应字段),命中则复用原证明以维持请求身份;否则只接收一个活跃、归属匹配的 canonical 租约,过期或异主直接失败,绝不写入租约。解析结果沿既有 lease_proof 字段进入原 provider plan,最终版本 CAS、归属与变更许可仍由 TypeScript canonical Monitor 事务原子裁决;Python 只做发现与传递。
与既有实现相比,这里没有新增租约权威:复用 read_canonical_todos_if_promoted 的 promoted 读、lease_is_active 的活跃判定与既有回执形状;monitor_poll.py 在传递路径上跳过的 require_monitor_poll_source_available 只做同一次 promoted 读(fail-closed),而传递路径已经先做过同样的读,因此这是去掉重复探测而不是绕过闸门。soft-claim 与未提升的路径保持无证明行为,手动显式证明入口也原样保留。
具体改动
关键代码讲解
monitor_poll_lease_transport.py:current_monitor_lease_proof:先按effect_id定位.transactions/quota-monitor-poll/<hash>.json回执,provider_pending取provider_plan.lease_proof,prepared/committed取record.monitor_event.todo_writeback.lease_proof;两者都没有才回退到 live 租约,并要求恰好一个活跃且owner == agent_id的租约,否则抛出可读的QuotaCommandValidationError。整个过程只读。monitor_poll.py:721:use_current_task_lease为真时要求 Turn/Todo/Agent 三者齐备再解析证明,并把结果写回既有的task_lease_idempotency_key/expected_version参数;随后的 provider writeback 与异常处理(含 #4991 的provider_rejected释放)完全走同一条路径。quota_request.py:104:CLI 契约把新 flag 限定为「execute 的 monitor-poll + 必须有--todo-id/--turn-instance-id/--agent-id」,并禁止与显式--task-lease-*混用,避免出现「两套证明谁生效」的歧义。interaction_contract.py:1346,1380:生成的辅助 Monitor 命令加上该 flag,并在task_lease_proof里声明来源是canonical_lease_or_same_turn_receipt、acquires_or_renews_lease: false,让投影消费方能读到这个语义。test_leased_monitor_poll.py(新增用例):覆盖活跃证明提交、租约释放后重放、过期与异主拒绝且不留 pending、soft-claim 兼容、显式与自动证明混用被拒;test_auxiliary_monitor_poll_availability.py与test_work_lane_contract_core.py覆盖命令投影与工作通道契约。
对主干的风险
最强反例是「读取租约后、canonical 提交前租约失效」:这种情况下 TS 会拒绝过期证明,若此时没有 #4991 的负证明释放,运行时会留下 provider_pending。这正是上一轮 review 要求「#4991 先合并、再在集成后的确切 head 上复核」的原因;现在 #4991 已在 base 上,monitor_poll.py 的 provider 异常分支与其 provider_rejected 处理在 rebase 后保持单一实现,两个特性互不遮蔽。另一方向的反例是「越权放行」:传递路径只读不写,异主/过期/无活跃租约一律拒绝,显式与自动证明混用被 CLI 契约拒绝,因此不会出现「用别人的租约提交观测」。
验证:集成后 tests/control_plane/test_leased_monitor_poll.py、test_auxiliary_monitor_poll_availability.py、test_work_lane_contract_core.py 共 33 项通过(其中既有 #4991 的「缺证明被拒 → 同 Turn 补齐 → 一次提交 → 幂等重放」两臂,也有本 PR 的 6 个新用例);control-plane 全量 3068 项(3039 通过、0 失败、29 跳过);typecheck:control-plane、Ruff、git diff --check 与 goal 作用域 premerge(19 项检查、0 失败、0 人工挂起)通过,质量回执 cqr_40af5120ae8bb806ad1a 有效。opt-in 隔离成立:不带该 flag 的显式入口、soft-claim 与未提升路径行为不变,soft-claim 兼容用例即为配对的关闭态验证。未验证项据实记录:本 goal 不等待远端 CI,而上一 head 的红灯在干净 merge base 上同样复现(canonical snapshot 请求预算、runtime-shadow e2e、仓库级 mypy),因此该 head 的远端结果视为未验证;未跑真实 PostgreSQL 与打包 UI,因为这是 CLI-only 的只读传递。
语义与 CI 对齐
沿用既有 lease_proof 与持久 quota 回执词汇,没有第二个写入权威,也没有把「收到证明」当作新的持久业务状态。新 flag 对手动调用默认关闭,但生成的辅助命令有意启用,这一差别同时写在双语文档、CLI 帮助与投影契约里,属于已披露的默认行为差异,而不是静默改变。
我的整体评价
outcome_impact 结论:long_horizon 为 improved——修复前硬租约监控在每次 Turn 都因缺证明而失败,无法累积进展;修复后证明可由当前租约或同 Turn 回执提供,提交、重放与释放后重放都落在同一份持久观测上,且与 #4991 的释放路径组合后不会留下幽灵预留;user_experience 为 improved——失败信息可操作(明确指出需要本 Agent 的活跃租约),成功可读回,纠正与恢复路径都指向真实可完成的动作,而不是要求用户手工清理交易目录。
改动比例相称:107 行左右的只读 helper 加 CLI 契约与文档,换来硬租约监控从「必然失败」变成「可完成并幂等重放」,且没有把 Python 发现逻辑提升为第二套裁决。整体判断:达到可交付水平,未发现阻断项;依赖顺序已由 base 满足,集成后的确切 head 已复核。可交维护者合并。
English verdict: APPROVE - exact head 717535c9cc7b3a3f40dfe1667f6464a64e60d610; the read-only lease-proof transport plus the rebased integration with the merged no-effect release keeps execution authority in TypeScript, and the integrated focused suites, full control-plane suite and goal-scoped premerge are green.
Merge note — #4995 merged on the reviewed exact headMerged via admin bypass as Changed surfaces: the quota CLI entry point and its flag validation, the quota service branch, the new read-only lease-proof transport module, the interaction projection contract ( Why this head: the earlier review (id 5306346522) approved with the constraint that #4991 had to land first and this PR be re-reviewed on an integrated exact head. #4991 merged as Checks run on the reviewed head:
Skips and holds: remote CI was not awaited ( Residual risk: the transport is opt-in and inert for explicit-proof, soft-claim and unpromoted callers; the remaining monitor-lane work continues separately. |
Goal and gap
A managed agent can receive an auxiliary monitor-poll command that is Turn-bound but omits the proof required by canonical hard-lease mode. That command fails even when the exact Monitor lease is already held.
Delivered behavior
Entry points and verification
CLI and managed Turn command projection change. No frontend settings or Lark control changes: this is an execution proof for the already projected auxiliary command, not a new user option or configuration source. The shared interaction projection and CLI readback are tested; packaged frontend was not exercised because it does not render this command as a user control.
File and SQLite real CLI coverage verifies hard-lease commit, soft-claim parity, expired-lease rejection without provider-pending state, no-spend, and same-Turn recovery after business commit, process death and lease release. The pre-rebase monitor suite passed 27 tests; the post-rebase focused suite passed 12 tests; adjacent monitor runtime and CLI projection tests passed 9 tests. Ruff and diff checks passed. No PostgreSQL or packaged-UI test was run for this CLI-only transport.
Related: #4991 separately repairs direct missing-proof preflight cleanup. Suggested review order: #4991, then this PR. #4992 is independent. This is a control-plane change and is left for maintainer review and merge.