fix(quota): keep deferred receipt-bound Turn from selecting a successor - #4992
Conversation
136fa88 to
d9987e3
Compare
|
Validation update after rebasing onto main d290e53: the deferred receipt-bound Turn regression and fresh-Turn P1 fallback both pass; the complete tests/control_plane/test_quota_settlement_cli.py suite passes (71/71), Ruff lint passes, and this PR no longer introduces oversized-decision-function findings. The remaining Lark module-metric ratchet failure reproduces on current main; the Goal Acceptance scope-required e2e failure also reproduces on current main and is being handled in the separate acceptance-scope task. This control-plane PR remains review-required; no bypass or self-merge requested. |
|
Exact-head update for 0486013: moved the deferred effective_action assignment back to the declared quota producer, fixing the architecture semantic-production failure. The targeted deferred receipt regression, nearby successor tests, architecture check, Ruff, and syntax checks pass locally. The earlier oversized TypeScript Effect request failure also reproduces on current main da4acd4 in the file variant of test_real_rpc_keeps_budget_and_cli_recovers_complete_display; the Lark module-metric maintainability-ratchet failure likewise reproduces on that main. These two failures are baseline debt, not evidence of a #4992 regression. Fresh CI for this head is still running. This PR remains review-required; no bypass or self-merge requested. |
|
Exact-head validation update for 5d132da: the same-Turn deferred receipt regression now exercises legacy, promoted File, and promoted SQLite authority through the public quota CLI and canonical todo update path; all three variants pass. The pre-extension quota settlement CLI suite completed 71/71 on the unchanged production code, and focused TS settlement/interaction checks passed 51/51. Ruff, py_compile, and diff checks pass. The new test commit adds only canonical-authority coverage; the PR remains review-required and the full expanded 73-case suite has not yet been rerun. |
|
Correction to the latest validation note: the expanded exact-head quota settlement CLI suite completed with 72 passed and 1 failed. The failure was the existing test_prior_host_closeout_reads_archived_exact_todo[file] at the later begin-turn assertion; that test passes when rerun alone (1/1). The newly added legacy/File/SQLite deferred-receipt cases still pass (3/3). This is an unresolved full-suite/order-sensitive validation failure, not a green suite claim. Further diagnosis is required before a formal approval conclusion; this control-plane PR remains review-required. |
|
Exact-head validation update for 5d132da: the complete tests/control_plane/test_quota_settlement_cli.py suite now passes 73/73 when run under an isolated TMPDIR (636.74s). The earlier order-sensitive 5s Effect RPC timeout disappeared when the test process used its own runtime directory, confirming cross-worktree test-runtime contention rather than a deterministic same-Turn selection assertion failure. This is not final merge readiness: the branch is behind current main, and the Goal Todo currently points its required validator at a non-existent test_quota_same_turn_receipt_projection.py rather than the committed cross-authority regression in test_quota_settlement_cli.py. Rebase and canonical validator correction await release of the overlapping hard lease held by the independent #5000 P0 repair. No quota spend or Todo completion is claimed. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Exact head: 5d132da5d0ad1b9d0e50e6e11ce2465e1d56618b.
动机
这个 PR 要修复已提交的 heartbeat Turn 在绑定 P0 Todo 后,P0 变为 deferred 时又选中独立 P1 的问题。现有回执的结算身份必须保持不变,P1 只能在新 Turn 里获得自己的身份。这个目标值得修复:旧 Turn 错绑后会让后续执行与记账无法可靠对应。
改动思路
准备阶段从原有 Todo 来源识别“回执绑定且已 deferred”的 P0,生成不要求执行的 work-lane;配额决策把旧 Turn 变为无支出 quota_skip,并在送入 TypeScript delivery router 前过滤不属于该回执的 fallback。再次校验 router 的结果,使同一 Turn 不能偷换成 P1;新 Turn 仍可按现有选择机制处理 P1。这里复用了原有回执身份和路由所有权,没有新增持久状态。
具体改动
关键代码讲解
should_run_prepare.py::_deferred_receipt_bound_work_lane从规范 Todo 状态识别精确绑定的 deferred P0;work_lane.py::receipt_bound_deferred_work_lane输出must_attempt_work=false、绑定 Todo id 和等待动作。should_run_packet.py::_resolve_agent_lane_delivery_route在 router 前后都核对 Todo id,避免独立候选变成旧回执的执行目标。settlement_precedence.py::deferred_receipt_bound_skip_fields生成 no-spend、DONT_NOTIFY 的配额与 heartbeat 建议;should_run_packet.py::_resolve_quota_should_run_route将执行路径关掉并设置quota_skip。- 新增的
test_same_turn_bound_p0_does_not_project_p1_after_p0_becomes_deferred用 legacy/File/SQLite 三种 authority 走公开 quota CLI,覆盖旧 Turn 回放、冲突 Todo 拒绝和新 Turn 选 P1。不过它未断言顶层recommended_action,正好漏掉以下反例。
对主干的风险
[P1,阻塞] 同一响应仍在推荐独立 P1。 触发条件是 P0 回执已提交、P0 被 deferred、P1 保持 open;新增分支设定 effective_action=quota_skip 且 selected_todo=P0,随后第 941 行附近的 selected_recommended_action_from_work_lane 又从开放 Todo 摘要产出“[P1] Advance the independent alternative delivery.”。我在 legacy 和 File 的实际 quota should-run --codex-app 读回中看到了这个矛盾,机器执行标志虽然为 false,但公开下一步建议可能误导其它消费方在错误 Turn 尝试 P1。请让 deferred 分支同时投影回执绑定的等待建议,并在三种 authority 的回归测试中断言顶层推荐不含 P1,同时保留新 Turn 选 P1 的断言。重跑该参数化测试及实际 CLI 读回。
这也违反现行同一 Turn 保留已提交回执身份的契约;不是通过更改优先级或引入新语义可以解释的差异。分支还落后当前 main,#4993 已触及相邻选择投影;修复后需在新 base 上重新核对交互。远端 CI 按本轮 capability 的 wait_for_ci=false 未查询,不能把它当作证据或障碍。
我的整体评价
方向和机制规模基本合适,三个新用例本轮重跑通过(3 passed,70 deselected),已看到 no-spend 与旧身份保护;但端到端公开 readback 尚不自洽,既影响用户/Agent 可理解的下一步,也无法证明长期回放不会把独立工作重新卷入旧回执。因此此 exact head 暂不宜合并。修正推荐字段、补充跨 authority 反例并复测当前 main 组合后再审;不需要扩成新的状态框架。
English verdict: REQUEST_CHANGES - head 5d132da5d0ad1b9d0e50e6e11ce2465e1d56618b; same-Turn quota_skip still recommends independent P1. Three targeted authority cases pass, but real legacy/File CLI readback exposes the missing assertion.
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>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
5d132da to
3978af7
Compare
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)
Exact head: 3e0e6ea801ca30801f8f3cae9400e19c9bbe22e5; base: a6b79e666c25d933dc3febaae282ceba71db7860.
动机
PR #4992 修复已绑定 Todo 的 heartbeat Turn 在该 Todo 被 deferred 后,重新借用独立任务身份的问题。验收依据是已有回执身份不可变,以及上一轮 5d132da5 审查指出的公开建议矛盾。主干上的真实 CLI 对照表明,绑定 P0 后将它 deferred,再读旧 Turn 会触发 receipt identity conflict;此前 PR 虽然返回 quota_skip,顶层 recommended_action 仍建议执行 P1。反复重试既不能修好身份,也会误导执行者。
当前完整改动让旧 Turn 保留 P0,返回一致的无执行、无支出等待;新 Turn 仍能选择 P1。这个结果关闭本 PR 的具体问题,不代表更大的调度或长期协作路线图完成。修复不增加权限、存储迁移、能力开关或用户配置。
改动思路
权威来源仍是规范 Todo 状态和已提交的 heartbeat settlement identity。准备阶段识别绑定任务已 deferred,派生等待 work-lane;quota 路由禁用本 Turn 的工作与支出,再复用该 work-lane 的等待动作作为公开建议。传入 TypeScript delivery router 的 fallback 及其返回值都不能用独立任务替换旧身份。TS 路由和原有 receipt preflight/writer 继续拥有各自决策与效果,没有引入第二个调度器。
只修改建议文本不足以修复主干的身份冲突;直接重绑回执会破坏已有结算契约;把 deferred 当作 settled 则会伪造完成事实。当前方案复用现有边界,并明确区分这三种状态。实际观察到旧 Turn 重放保留一个回执、零支出,而新 Turn 可以继续独立任务,因而没有把防止越权变成阻断后续工作的全局等待。
具体改动
关键代码讲解
should_run_prepare.py::_deferred_receipt_bound_work_lane从完整规划来源匹配规范 Todo id 和TODO_STATUS_DEFERRED。它只根据明确的状态事实生成等待,不从缺失的展示行推断任务不存在。work_lane.py::receipt_bound_deferred_work_lane输出selection_binding=heartbeat_receipt、绑定任务 id 和must_attempt_work=false,并解释独立工作应使用新 Turn。该结果是派生投影,不需要额外持久标记。should_run_packet.py::_resolve_agent_lane_delivery_route在 router 前后保持已提交身份;_resolve_quota_should_run_route使用deferred_receipt_bound_skip_fields关闭执行路径,随后让绑定等待动作覆盖普通建议,并跳过独立 frontier 的建议覆盖。_resolve_external_evidence_observation将原有“未到观察窗口或观察未变化则抑制”的判断整体提取到同一 quota 文件。原函数达到 93 条语句,超过已有 90 条维护性预算;提取后原预算检查通过。参数、调用时机和返回语义保持不变,未提高阈值,也未新增通用框架。test_quota_settlement_cli.py用 legacy、File、SQLite 的实际 CLI 链路覆盖绑定、defer、旧 Turn 重放、显式冲突、新 Turn 选择;新增断言检查顶层建议等于等待动作且不再投影独立 P1。
正向路径是 P0 绑定→deferred→旧 Turn 等待→新 Turn 选择 P1。负向路径是在旧 Turn 显式请求 P1,原有 preflight 仍拒绝且不追加回执。另以 80 个后来新增的任务和行顺序变化施压:旧绑定不受展示数量影响,新任务在新 Turn 可选。主干与最终版本的完整冲突错误响应相同。
对主干的风险
主要风险是共享 quota 路由误抑制正常工作、改变 monitor/replan 优先级,或只修复执行标志却继续泄漏建议。正常绑定、重复读取、显式冲突和新 Turn 选择的同输入对照保持一致;deferred 分支的改变是本 PR 明确要求的修复。#4993 的已合并选择投影已包含在当前 base,相关 CLI 回归一起验证。
验证:完整 quota settlement CLI 套件 74 passed;另有 12 项 Python quota/work-lane 测试、70 项 TypeScript settlement/readback/interaction 测试通过。外部观察 smoke 在不可变主干和最终源码均通过;同一回放脚本比较普通、deferred 和重排行的 15 次真实调用。新增推荐断言在修复前确实失败,修复后跨三种 authority 通过。最终 canary premerge --from-git-diff --goal-id 的 18 项风险检查、维护性检查、Ruff、23 个配置文件的 mypy、编译、diff hygiene 和五个变更文件的公开边界扫描均通过。质量回执已绑定最终 diff。
保留失败记录:一次验证将 TMPDIR 放在 Git checkout 内,使“非 Git 目录”用例错误继承仓库身份;该轮在 39 passed / 1 failed 时中断,改用仓库外隔离目录后完整重跑通过。此前维护性检查也真实失败,已通过上述职责提取消除。没有把两次失败隐藏成通过。边界扫描附带两个与本 diff 无关的运行状态告警,文件扫描本身无问题。依已解析的 wait_for_ci=false,未查询或等待远端 CI;本地结果不冒充远端 CI 结果。
受影响入口是 CLI 与 managed Turn 交互投影。已检查 dashboard 的 recommended_action 消费和 Lark 通用投影路径;没有新增控件、schema 或配置项,修复由现有字段传递,因此无需前端/Lark companion 改动。不声称视觉验收或 PostgreSQL provider 迁移验收;本 PR 未改变存储实现。若需回退,可独立回退 PR,无回执格式迁移。
我的整体评价
本次重新审阅覆盖整个 base-to-head diff,而非沿用上一轮结论。上一轮阻塞项已经用能先失败再通过的公开 CLI 回归修复;持续工作和用户读回都得到验证。规则使用规范状态与身份判断,没有 prose denylist、特定业务措辞或把强制限制称为建议。未来维护性检查已落实为一段同职责的行为保持提取,剩余机制与问题规模相称,无未解决的代码阻塞项。
结论为 APPROVE。所有者已明确要求本 PR 修复后自合并;该请求是此次控制面 PR 的具体合并授权。发布此 exact-head review 后,仍需紧邻合并重新读取远端 head 并通过 capability merge-readiness。若 GitHub 因必需审查/检查策略阻挡,将在该明确授权及本地资格证据下使用 maintainer bypass;它不代表 CI 已通过,也不豁免 exact-head 审查。
English verdict: APPROVE - head 3e0e6ea801ca30801f8f3cae9400e19c9bbe22e5; deferred receipt replay now preserves identity and a consistent no-spend recommendation while a fresh Turn can select independent work. Real CLI regression, baseline/head comparison, 74 CLI tests and 18 premerge checks passed; remote CI was not consulted under the configured policy.
Summary
When a committed heartbeat Turn's Todo becomes deferred, replay now keeps that Todo identity and returns a consistent no-execution, no-spend wait. A fresh Turn can still select independent work. Previously, main could substitute a successor and fail receipt writeback; the earlier PR also left a contradictory successor recommendation.
The fix uses the existing canonical Todo source, work-lane projection and receipt owner, filters successor substitution around the existing delivery router, and aligns the top-level recommendation with the wait. A bounded extraction keeps external-observation window resolution in the same quota owner without changing behavior or raising maintenance budgets.
Validation
User entry points and merge
CLI and managed Turn readback change through existing fields. Dashboard/Lark consumers need no new settings, schema or controls; no visual acceptance or storage-provider migration is claimed.
The owner explicitly requested repair and self-merge for this PR. Merge remains conditional on a published review of the final head, a valid exact-scope quality receipt, and an immediately refreshed capability merge-readiness result. No live Goal state or quota ledger was changed for testing.