Skip to content

fix(quota): preserve explicit Todo selection gates and retry - #4332

Closed
LIHUA919 wants to merge 4 commits into
loopx-project:mainfrom
LIHUA919:codex/fix-todo-explicit-selection-4327
Closed

LIHUA919 wants to merge 4 commits into
loopx-project:mainfrom
LIHUA919:codex/fix-todo-explicit-selection-4327

Conversation

@LIHUA919

@LIHUA919 LIHUA919 commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

An explicit Todo request could be refused by a replan, workspace or write-scope gate, then reported as heartbeat_receipt_identity_conflict even when the Turn had no settlement identity. Preserve the TypeScript qualification result and return quota_action_selection_rejected or quota_action_selection_deferred, with deterministic guard re-entry and no receipt creation, upgrade or quota spend.

Recompute qualification after final workspace/scope guards while preserving their selected identity. Normal admitted selection, exact due-monitor obligations and committed-identity conflict rules remain unchanged. Failed preflight under --turn-envelope retains its typed packet and complete re-entry command.

中文:修复显式 Todo 选择把真实准入阻塞误报为回执身份冲突的问题。保留 replan、工作树、写入范围、能力和认领边界;最终 guard 改变准入时重新计算类型化资格,返回原 Goal/Agent/Turn 的重入路径,失败不写或升级回执、不扣额度。

Issue Or Task

  • Refs quota: explicit Todo selection can reject the canonical runnable successor #4327; covers the monitor-successor failure and a canonical add-after-pending-guard boundary failure.
  • Contributor task ID: explicit Todo selection admission and retry.
  • The reproduced monitor failure has a present canonical candidate but a deferred long-chain replan gate. The add-after-guard fixture reaches final workspace/scope repair. This PR reports those actual gates; it does not label an unverified source divergence as projection_stale or change the current-frontier admission policy.

Validation

  • Tested revision: df1c6889485cfffef5ecebfcfb01b5a30e0ccc96.
  • Run state: finished.
  • Input classes: synthetic, public_fixture.
Check kind Result Public-safe evidence / limitation
real_entrypoint passed 73 tests across explicit successor selection, quota settlement CLI, action portfolio, boundary selection, identity preconditions and error codes.
real_backend passed Disposable File and SQLite stores plus legacy state; actual monitor mutation, canonical Todo add, pending guard, selection, receipt readback and retry. Node 22.22.3 / SQLite 3.51.3.
regression_parity passed The same test_explicit_successor_selection.py fails on immutable main 7eb4b7bb1661bd5eff63a8725a33169792d5964b for both canonical long-chain and add-after-guard cases, returning the old identity-conflict code. Both pass on the candidate.
integration passed Existing managed Turn advisory-primary CLI test and two TurnEnvelope validation-failure tests pass. Host execution/model calls are not exercised. A system-installed tests package initially shadowed the repository namespace; the managed test passed with imports explicitly scoped to repository tests.
unit passed 17 TypeScript action-portfolio and quota-selection tests; their rule implementations are unchanged.
static passed Ruff, compile, diff checks and public/private scans.
integration passed Risk premerge: 4 direct checks and 18 selected checks; zero failures, warnings or skips. Includes CLI output budgets, monitor/scheduler contracts and maintainability.
integration passed Exact-head GitHub CI: Sign-off and merge-gate passed, including all four Python shards, Stage 2C correctness, installed-package, Node compatibility, packaged dashboard, and Windows checks. CI run. Publish/deploy jobs were skipped as expected for this PR.

Coverage and gaps: no store implementation, provider routing, promotion, score, persisted Todo schema or default capability change. PostgreSQL integration is not applicable to this CLI diagnostic/qualification projection change; File/SQLite are the reproduced entrypoints. A distinct case with an actually admitted default candidate and unavailable explicit source would need its own minimal fixture. No active Goal data or model/benchmark jobs were used.

Future-facing pass: moved existing explicit-selection validation/confirmation together with failed-preflight recovery into quota_action_selection.py; the main command handler passes the existing complexity ratchet. The TypeScript qualifier remains the semantic owner. #4061's fallback-source work is unchanged.

Frontend / Visual Evidence

  • UI impact: none. The changed user entrypoints are explicit quota CLI selection and failed --turn-envelope output. No setting, frontend control or configuration projection changes; managed Turn keeps the shared eligibility owner.
  • Before / After / Viewports: N/A.
  • Source data: synthetic.

Type of Change

  • Bug fix
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Host or runtime integration

Technical Direction

Shared-authority RFC fixture impact

N/A: this PR does not claim a provider migration or shared-authority promotion stage.

Boundary Checklist

  • No private state, credentials, raw traces, internal links or local machine paths in the diff or PR.
  • No duplicate maintainer benchmark work or new benchmark jobs.
  • Scoped to the linked issue's explicit selection failures.
  • UI impact marked none.
  • Every commit includes DCO sign-off; existing commits are preserved.

Signed-off-by: Lihua <1017343802@qq.com>
Signed-off-by: Lihua <1017343802@qq.com>
Signed-off-by: Lihua <1017343802@qq.com>
Signed-off-by: Lihua <1017343802@qq.com>
@huangruiteng

Copy link
Copy Markdown
Collaborator

Thank you for the concrete reproduction and File/SQLite coverage. The canonical fix has now merged via #4335 as main b0f7aa0, and #4327 is closed. Before closing this duplicate, I ran your exact tests/control_plane/test_explicit_successor_selection.py against that merged main: 7/7 passed, including legacy/File/SQLite, the long waiting chain, deterministic retry/no duplicate receipt, and canonical add-after-guard behavior. Merging both implementations would now create a second CLI selection transport beside the existing TS qualification + quota wrapper owner. Closing as superseded, not as a rejection of the contribution: your reproduction and provider matrix materially strengthened the accepted boundary and post-merge verification.

@LIHUA919

Copy link
Copy Markdown
Contributor Author

Thank you for the clarification and for crediting the reproduction/provider coverage. I agree that the full #4332 transport should not be merged alongside #4335.

I independently checked the remaining delta in an isolated checkout pinned to merged main b0f7aa0d12eab1947808d5af3f46ac51d37ec794, with pytest and CLI helpers confirmed to load that checkout. Copying the unchanged tests/control_plane/test_explicit_successor_selection.py from df1c68894 (SHA-256 f15d292ecb967ca65b9545c56b7eaf8a427f44fc460e276f01ec0094efd4a78a) and running python3 -m pytest -q tests/control_plane/test_explicit_successor_selection.py produced 4 passed, 3 failed:

  • Two failures are only the intentional action_selection versus action_selection_qualification field difference; I am not treating those as functional blockers.
  • test_canonical_add_after_pending_guard_reports_final_boundary still exposes a narrow scope-guard recovery gap. In the real synthetic File fixture, the final decision is boundary_projection_repair, normal_delivery_allowed=false, and self_repair_allowed=true, while qualification remains qualified. The CLI then raises requested action selection qualification conflicts with its projection and returns quota_unexpected_collection_error plus heartbeat_receipt.status=write_failed.

The guarded early return in _resolve_agent_lane_delivery_route preserves the first-pass qualification instead of refreshing it after the final scope guard. The same fixture on #4332 returns typed deferred/control_repair without a fake receipt. This concerns the scope-boundary branch, separate from the workspace-repair recovery already added in #4335.

Could we retain this as a small follow-up on the merged implementation—keeping its inventory/TS owner and workspace-repair behavior, and adding only final scope-guard qualification plus the focused regression? No need to reopen or merge the full duplicate PR.

@huangruiteng

Copy link
Copy Markdown
Collaborator

嗯,感觉 sol 搞错了,这个我 reopen

@huangruiteng huangruiteng reopened this Sep 13, 2026
@huangruiteng

Copy link
Copy Markdown
Collaborator

我处理下吧

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

评审对象是 exact head 4332@df1c6889485cfffef5ecebfcfb01b5a30e0ccc96。PR 的目标是修复显式 --todo-id 在 replan、workspace 或 write-scope 最终 guard 之后被误报为 heartbeat_receipt_identity_conflict 的问题。这个问题仍有真实增量:在当前 origin/main@0933d48efc058d9e1585bd30058942416aa258a9 上,Todo 在首轮 guard 之后新增、随后要求 loopx/** 写范围时,focused fixture 仍返回 quota_unexpected_collection_error,没有保留类型化资格。旧 PR 的测试在它自己的旧基线通过,但整个 seven-file diff 已与当前 main 的 #4335 运行时/协议 transport 重叠。

改动思路

入口是 quota should-run --codex-app --todo-id,资格的语义 owner 是 TypeScript action-selection reducer,should_run_packet.py 组装最终 delivery route,quota.py 负责 CLI 诊断与 heartbeat receipt。旧 PR 中 _resolve_agent_lane_delivery_route 的 final-guard ordering 修复是有价值的;但是新增 quota_action_selection.py、新的 action_selection 字段、协议文档和大批 monitor/retry 测试,与当前 main 已有的 _apply_requested_quota_action_selection_preflight、action_selection_qualification 和现有回执测试形成第二套 transport。rebase 到当前 main 时 quota.py 与 settlement 测试发生冲突,确认了这不是一个适合整批合入的增量。

正向路径应是:先从 canonical agent Todo inventory 得到 qualification,再冻结 selected identity 并返回最终 workspace/scope guard;deferred/rejected 只返回 typed diagnostic,已有 identity-less receipt 只 replay,不写新 settlement identity、不扣额度。最小修复是保留现有 owner,只移动 qualification 计算位置并加入一条 late-selection boundary regression。

具体改动

关键代码讲解

  • loopx/control_plane/quota/should_run_packet.py::_resolve_agent_lane_delivery_route(约 549 行):旧 main 在 guarded-agent-lane 分支直接 return,跳过 qualify_action_selection_from_inventory。PR 的最后一个 commit 将资格计算移到该 return 之前,保证 control_repair/delivery_not_allowed 仍能以 derived action_selection_qualification 输出,同时不改变 selected Todo identity。
  • loopx/cli_commands/quota.py::_apply_requested_quota_action_selection_preflight(约 390 行):当前 main 已是 CLI/receipt transport owner,负责把 deferred/rejected 转成 quota_action_selection_deferred/rejected、回放现有 receipt 并阻止 settlement。旧 PR 新增的 parallel helper 与 action_selection 字段不应再保留。
  • loopx/control_plane/work_items/action_portfolio.py::qualify_action_selection_from_inventory(约 134 行):继续作为 inventory-to-TS qualification adapter;没有新增状态源或 authority。
  • tests/control_plane/test_explicit_successor_selection.py 的 monitor cases 与 tests/control_plane/test_quota_settlement_cli.py 现有 guard cases 大量重复。真正缺少的是 late Todo + final boundary guard 的 canonical File fixture,应合并进现有 quota settlement suite。

对主干的风险

旧 exact head 直接合并会带来一个 P1 阻塞:它在当前 main 上是 stale/conflicting,并行引入第二个 explicit-selection transport,造成 action_selection 与 action_selection_qualification 两种公开字段和两套 retry owner;后续消费者可能把同一 deferred state 分流成不同错误码。该 batch 的 +522/-85 规模相对于当前只缺一个 ordering branch 是 disproportionate。

已验证的负向场景是:当前 main 的 early return 会让 late selection 退化为 quota_unexpected_collection_error;同一 real CLI/File-authority fixture 在把 qualification 前移后返回 quota_action_selection_deferred、reason=control_repair,现有 identity-less receipt 为 replayed 且 rollout event 数量不变。default/unselected、workspace repair、newly-due monitor、autonomous replan 和 user-gate 邻近场景均保持通过。没有新增 Todo authority、持久化 schema、quota spend、provider 或 capability activation。

请求修复(P1):将本 PR 收敛为现有 should_run_packet.py owner 中的 final-guard ordering 修复和一条 focused regression;删除/不合入已由当前 main 提供的 parallel CLI module、旧协议重复说明及重复 monitor 测试。对应回归为 tests/control_plane/test_quota_settlement_cli.py::test_selection_added_after_pending_guard_reports_final_boundary。

验证:旧 exact head 的 test_explicit_successor_selection.py 为 7 passed;当前 main 的 focused reproduction 在修复前复现错误码;收敛后的 two-commit patch 的 focused + 邻近 5 cases、完整 test_quota_settlement_cli.py(58 passed)、boundary/CLI suites(25 passed)及 Ruff/diff checks 通过。loopx canary premerge --from-git-diff 的 direct checks 和大多数 smokes 通过;quota-plan-smoke 在基线与修复分支均受 capability-memory effect-runtime fixture 失败影响,catalog-run-e2e-smoke 用项目 Python 直接运行通过但被 canary wrapper 报失败。这两项属于环境/基线证据,不是本 diff 引入的回归;应以推送后的 GitHub CI 为最终门禁。

我的整体评价

当前 #4332 exact head 不能合并,结论是 REQUEST_CHANGES。它包含一个正确且仍有价值的 final-guard ordering 修复,但其余改动在当前 main 上主要是重复实现/重复覆盖。收敛后的实现保持 TS qualification、Python quota transport 和 heartbeat receipt 的单一 owner,并以最小 real-path regression 锁定之前未覆盖的 boundary 分支。完成收敛、推送新 exact head 后应重新 review,不能继承本次结论或旧的 CI 通过状态。

English verdict: REQUEST_CHANGES for 4332@df1c6889485cfffef5ecebfcfb01b5a30e0ccc96. The final-guard qualification ordering is a valid incremental fix, but the seven-file head duplicates current main’s quota transport and exposes a second selection field/owner. Keep the existing TS/Python owners, retain the focused late-boundary regression, and drop the superseded module/docs/duplicate tests. Baseline reproduction shows quota_unexpected_collection_error; the focused repair returns typed quota_action_selection_deferred without receipt mutation.

huangruiteng added a commit that referenced this pull request Sep 13, 2026
Self-refined follow-up to #4332. Preserves existing TS/Python selection and receipt owners, fixes final-guard qualification ordering, and adds a real CLI regression. Local focused/full quota checks and all required GitHub checks passed; canary environment gaps are documented in the PR review.
@huangruiteng

Copy link
Copy Markdown
Collaborator

Superseded by the self-refined follow-up #4350, which rebased the remaining final-guard qualification fix onto current main and merged it after exact-head review and CI. The original seven-file transport remains closed because #4335 already supplied its overlapping owner/contract.

@LIHUA919

Copy link
Copy Markdown
Contributor Author

收到,谢谢重新核验并完成修复。看到剩余的 final-guard qualification 问题已通过 #4350 收敛并合并,这个处理方向我认同:沿用主干现有的 TS qualification 和 quota/receipt 处理,只保留 final guard 前刷新 qualification 的修复,以及 late-selection boundary 回归,避免引入重复的 transport、协议说明和测试。

原 #4332 由 #4335 和 #4350 接续覆盖后关闭没有问题。谢谢保留这条边界回归;后续如需我补验证或调整,直接 @ 我即可。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants