refactor(quota): make TypeScript own prior-Turn closeout recovery - #4472
Conversation
Validation comment — migration economics receiptRe-validated on the current base after rebasing onto the release-1.0.5 head
Check matrix
Coverage and gaps: the changed authority surface (the closeout decision) is covered |
d988ce9 to
2403e45
Compare
Premerge canary result (commit 2403e45)
One real finding from the gate, now fixedThe first canary pass failed Two earlier canary failures ( Environment disclosureThe canary executes already-committed repository smokes; no benchmark job was |
CI result (2403e45)All required checks pass on this head:
Failures: none. Manual holds: none. Merged state is left to reviewer decision — this is a runtime semantic cutover, not a docs/cleanup batch. |
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
这个 PR 解决的是"同一套 prior-host-Turn closeout 策略在两个语言里各有一份权威"的问题。改动前，loopx/control_plane/quota/heartbeat_receipt.py 的 prior_closeout_required_heartbeat_receipts 和 unsettled_host_turn.py 里的判定一起决定"上一个 Turn 是否还欠一次 host closeout、哪些 kind 算已经结账"，而 TypeScript 侧的 settlement readback 又自己读同一份 rollout-event log、并另外保留了一份 receipt identity 与路径解析的拷贝。后果有两层：一是新增一种可接受的 closeout 或改一条 identity 规则时，必须两处同时改，漏改就会让"当前该做什么"在两个命令之间分叉；二是 Python 那份是权威，但真正能执行这条规则的 effect runtime 是 TypeScript 那一侧。
改动思路
把决策整体搬到 typed owner，Python 退化成 transport。放置依据是我在 loopx/control_plane/quota/ 里核对的既有边界：closeout 与结算类规则由这个目录拥有；rollout-event log 的路径与读取规则归 rollout_receipt_log.ts；receipt identity 规则归 heartbeat_receipt_identity.ts，并且被 settlement readback 复用——这一步是去重，不是新增第二份权威。
值得肯定的是，这一版没有把 Python 侧保留成"另一半决策"：unsettled_host_turn.py 只负责编码请求、调用 effect runtime、校验返回形状；策略字符串在 Python 侧没有留下副本（我按 accepted_closeouts 做了全仓扫描，Python 侧 0 命中）。同样，unsettled_host_turn_contract.py 从原先的 binding_task_class 改成消费 typed 的 repair，把"渲染命令"和"判定事实"分开，这个方向是对的。
具体改动
- 新增
loopx/control_plane/quota/unsettled_host_turn_recovery.ts（505 行）：ACCEPTED_CLOSEOUTS（:57）给出唯一的已结账词表，reduceUnsettledHostTurnRecovery（:424）做判定，preflightPriorHostTurnCloseout（:201）负责读取前一步的 receipt 事实。 - 新增
loopx/control_plane/quota/heartbeat_receipt_identity.ts（182 行）：selectEffectiveHeartbeatReceipt（:163）、heartbeatReceiptFactFromEvent（:82）、heartbeatReceiptBinding（:106）把 receipt 的 identity 归一化收成一份；settlement_readback.ts:295原来的本地实现被替换为调用它。 - 新增
loopx/control_plane/rollout_receipt_log.ts（99 行）：goalRolloutEventLogPath（:39）与readGoalHeartbeatReceipts（:65）统一路径规则与"畸形行跳过、不制造也不抹掉 receipt"的读取容忍度；scheduler/heartbeat_followup.ts的本地路径/读取实现被删除。 - Python 侧：
unsettled_host_turn.py的apply_unsettled_host_turn_recovery_if_required（:250）保留为入口，heartbeat_receipt.py删除prior_closeout_required_heartbeat_receipts（-45 行），unsettled_host_turn_contract.py改按 typedrepair渲染。 effect_runtime_handlers.ts:470注册quota.prior_host_turn_closeout.preflight与quota.unsettled_host_turn_recovery.reduce；tsconfig.control-plane.json登记三个新模块；测试为tests/control_plane_ts/unsettled_host_turn_recovery.test.ts（20 例）加test_quota_settlement_cli.py、test_effect_turn_live_quota_decision.py的新用例。
关键代码讲解
unsettled_host_turn_recovery.ts:57 ACCEPTED_CLOSEOUTS：四元as const元组就是"什么算已结账"的全部词表，AcceptedCloseout类型由它派生，因此词表与类型不可能分叉。不在这个元组里的 kind 永远不会被当成已结账，方向是保守的。heartbeat_receipt_identity.ts:163 selectEffectiveHeartbeatReceipt：一个 goal/agent/Turn 下允许多条 receipt，这里规定"最新的带 binding 的那条胜出；同一个 Turn 上出现两个互相冲突的 binding 则报错，而不是挑一条"。这条规则原先在settlement_readback.ts里有一份，现在两处消费者共用。unsettled_host_turn_recovery.ts:424 reduceUnsettledHostTurnRecovery：真正的决策点，输入是 preflight 后的 receipt 事实与候选 Turn 的 binding、Todo/replan 事实，输出recovery_required与恢复 packet；没有 binding 事实时拒绝判定，而不是猜一个 binding。rollout_receipt_log.ts:39 goalRolloutEventLogPath：把日志位置和"goal_id 必须是单一路径段、解析结果必须留在 runtime_root 内"的约束收成一处，三个读者（preflight、readback、scheduler follow-up）不再各写一套。unsettled_host_turn.py:250 apply_unsettled_host_turn_recovery_if_required：transport 的边界很清楚——形状对不上就RuntimeError("TypeScript recovery result shape mismatch")，Python 不会退回自己判定，这保住了"单一权威"的承诺。
对主干的风险
P1（阻塞，必须先修）：这个 head 已经不能合入 main。 我在 2403e45ae42c84b66974d99e36071f721e59a2f4 上跑 git merge-tree --write-tree HEAD origin/main，结果是两处冲突：
CONFLICT (modify/delete): loopx/semantics/inventory_v0.json——main 在 #4494 已经退役这份提交进仓库的清单（现在.gitignore忽略它，改为scripts/generate_semantic_inventory.py按需生成），而本 PR 仍在往里加条目；CONFLICT (content): tests/control_plane/test_quota_settlement_cli.py。
需要的不只是 rebase：inventory 这两个改动在 rebase 之后没有落脚点。按 main 现在的语义治理模型（新的 carrier 自动发现、只有注册过的词表需要 owner/registry 同步），新符号应当走按需生成器加 examples/semantic-vocabulary-drift-smoke.py 这条路径重新安放，而不是把已退役的快照改回来。另外 rebase 后要对 main 版本的 test_quota_settlement_cli.py 重跑本 PR 新增的用例。
证据缺口（不是本 PR 的回归）。 test_quota_settlement_cli.py 的两个 sqlite 参数化用例在本 head 与本 PR 的 merge base c3cee4064 上都同样失败，报的是 local_authority_provider.ts:257 的资格校验：本机 PATH 上的 Node 是 25.5.0 / SQLite 3.51.2，不是仓库要求的 Node 22.22.3 / SQLite 3.51.3 资格组合（/opt/homebrew/opt/node@22/bin/node 是 22.22.0 / 3.51.2，同样不达标）。所以这两例是环境缺口，我在两边都复现了相同输入，不能算成本 PR 引入的红灯。
P3（不阻塞）。 合并日志路径守卫时，错误码从 invalid_scheduler_receipt_path 改成了 invalid_rollout_event_log_path（rollout_receipt_log.ts:39）。这个码没有任何测试或调用方引用，且该分支几乎不可达（goal_id 在前面已按单一路径段校验过），但它确实是一处静默改掉的机器可读契约，建议在 PR 描述里写一句，或给 scheduler 调用点保留旧码。
我的整体评价
方向我赞成，实现也基本干净：这次是真的把权威收到 typed owner 上了，顺手把三处日志读取和一处 identity 拷贝合并掉，Python 只留 transport 和一个形状守卫。本轮结论是 REQUEST_CHANGES，唯一的阻塞项是 P1 的合入冲突：head 2403e45ae42c84b66974d99e36071f721e59a2f4 落后 main 69 个提交，且它修改的 loopx/semantics/inventory_v0.json 已被 main 退役——rebase 后请按按需生成清单的流程重新安放这两个新符号，并在 main 版本的结算 CLI 测试模块上重跑新增用例。除此之外没有需要改设计的地方；我正在跑的证据是：node --experimental-strip-types --test tests/control_plane_ts/unsettled_host_turn_recovery.test.ts 20 passed / 0 failed，相邻四个 TS 套件 44 passed，tsc -p tsconfig.control-plane.json --noEmit 干净，pytest tests/control_plane/test_effect_turn_live_quota_decision.py tests/architecture/test_semantic_inventory.py 26 passed，pytest tests/control_plane/test_quota_settlement_cli.py 87 passed / 2 failed（即上文那两例环境缺口）。
English verdict: REQUEST_CHANGES — the relocation of prior-host-Turn closeout policy from Python to the typed TypeScript owner at head 2403e45 is the right ownership fix and the dedup of the three rollout-log readers plus the receipt identity rule is real, with 20 new TypeScript cases and the neighbouring suites green. It cannot merge as it stands: git merge-tree against origin/main reports a modify/delete conflict on loopx/semantics/inventory_v0.json — a snapshot main retired in #4494 in favour of on-demand generation — plus a content conflict in tests/control_plane/test_quota_settlement_cli.py. Rebase, re-home those inventory entries through the converged semantic-vocabulary flow, and re-run the new settlement-CLI cases on main's version of that module. Two secondary notes: the two sqlite-parameterized cases fail here for an environment reason (local Node 25.5.0 / SQLite 3.51.2 is below the qualified profile) and I reproduced the identical failure at merge base c3cee40, and the consolidated path guard silently renamed the internal error code invalid_scheduler_receipt_path to invalid_rollout_event_log_path.
Move the prior-host-Turn closeout transaction behind one TypeScript request/response boundary so the Python coordinator keeps only transport and public-payload projection. The typed owner now decides which prior must-attempt Turn still needs a closeout, whether its settlement already validates, which closeout is accepted, and what the recovery obligation is. Python reads exactly the two provider facts the preflight names - the bound Todo and the committed monitor-poll receipt - and projects the typed verdict. Two requests, not one per receipt: a fail-closed preflight that reads the persisted guards and the settlement readback itself, then one final reduction over the checkpointed bound facts. The preflight reads the rollout log from runtime_root because a real goal persists 3.25 MiB of receipt facts for its busiest Agent, and the runtime bridge rejects any request over its 2 MiB bound. Semantics tightened while sharing the rule with the settlement readback: - the heartbeat receipt identity rule has one owner instead of one TypeScript and one Python copy; - a receipt that declares a Todo binding the settlement authority cannot address now fails the read instead of minting a recovery obligation for an identity no other reader can reproduce; - a receipt that names a settlement effect without a Todo or autonomous replan binding keeps failing closed, and the declared effect id is no longer silently replaced by the derived one. Python deletion: the closeout policy, the receipt-selection rule, the identity rule, and the settlement-readback transport for this path. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
2403e45 to
e6b6376
Compare
Self-merge validation (e6b6376, rebased onto main @ 8c55247)Owner-authorized self-merge for this transaction cutover. Changed surfaces: Checks run:
Failures and skips, named:
Why the coverage is enough: the changed authority surface is the closeout decision itself, covered by the typed unit corpus (20 cases), the real CLI integration path (65 cases), and a parity replay of the pre-change Python rule against the typed preflight over an owner-authorized read-only production goal's persisted receipts (zero mismatches). The bridge bound is evidenced rather than assumed: the preflight reads the rollout log in-runtime because a real goal's busiest Agent exceeds the runtime payload bound, and the change takes that path from 6085 cross-runtime round trips to 2. Rebase notes: the regenerated semantic inventory is dropped (main made it a local, uncommitted report), and both added tests at the one conflict site are kept. |
CI result (e6b6376) — and why the four failing shards are inheritedGreen:
They are inherited, not introduced here. Run on a pristine checkout of So the merge uses the maintainer bypass on a main-level baseline failure, recorded here rather than papered over. Fixing those goal-channel and ratchet failures belongs to their own owners; they are not part of this transaction. |
1 participant
Summary
Cut over the prior-host-Turn closeout transaction to TypeScript, following the
bounded order both RFCs already prescribe rather than inventing a new migration plan.
docs/architecture/rfcs/shared-goal-authority-state-provider-v0.md§11.3("TypeScript-first burden-reduction order"): characterize, move one delivered
ownership transaction at a time into an existing TS boundary, qualify against
file/NoKV/real isolated PostgreSQL, and only then touch binding/migration/canary.
The RFC explicitly says this order is not a licence for a big-framework rewrite.
docs/architecture/rfcs/typescript-control-plane-migration-v0.md§3.2 (payoffphase) and Stage 2B already name this exact transaction as the next one: prior-host-Turn
recovery must move receipt selection, exact Todo lifecycle observation,
settlement validation, and recovery/continuation selection together before the
Python coordinator can retire, and must keep crowded-inventory, provider-failure,
identity-conflict, and same-Turn no-spend recovery coverage.
This PR is that one transaction. The typed owner decides which prior must-attempt
Turn still needs a closeout, whether its settlement already validates, which
closeout is accepted, and what the recovery obligation is. Python keeps only
transport and public-payload projection.
Semantics changed and fixed
is now the same rule the settlement readback uses, instead of a second copy.
declares a Todo binding the settlement authority cannot address now fails the
read. Previously it minted a recovery obligation for an identity no other
reader can reproduce.
naming a settlement effect without a Todo or autonomous-replan binding keeps
failing closed, and its declared
settlement_effect_idis preserved rather thanoverwritten by the derived one.
normalizes to a typed error instead of producing a phantom recovery obligation.
differently, or a settlement effect bound to two different identities, raise
heartbeat_receipt_identity_conflictinstead of picking one.values literally (for example
donewith no trimming and no successortolerance), and an open lifecycle only closes out when it carries a resume
condition and a successor.
repairfield instead of re-deriving the lane frombinding_task_class == "continuous_monitor", so the lane can no longer driftfrom the decision that produced the obligation.
from the runtime root and sends only the named provider facts. A real goal's
busiest Agent persists several MiB of receipt facts, which exceeds the runtime
bridge payload bound, so a "send the whole log" design was not viable.
Duplication deleted
quota/settlement_readback.tsand one Python copy inquota/heartbeat_receipt.py(_receipt_settlement_identity,_effective_heartbeat_receipt,prior_closeout_required_heartbeat_receipts)quota/heartbeat_receipt_identity.ts; settlement readback and the Python coordinator both call itjoin(goalRoot, …)read in settlement readback,receiptLogPath+ an inline loop inscheduler/heartbeat_followup.ts, and a Pythonload_rollout_events(rollout_event_log_path(...))rollout_receipt_log.ts(goalRolloutEventLogPath,readGoalHeartbeatReceipts); Python keeps only the writerKnown but deliberately not in this PR (bounded follow-up, tracked as a
coordination-result retirement todo): the remaining Python/TS knowledge pairs under
todos/(completion policy, decision scope, resume planning, quota selection,standing decision, completion fence, frontier revision, completion transaction),
work_items/interaction contracts, the coordination shadow modules, and theno-production-caller
coordination/file_provider.py,head.py,executor.py,goal_state_shadow.py.Migration economics receipt
quota/unsettled_host_turn.pyowned closeout policy and recovery payload. After: TSquota/unsettled_host_turn_recovery.tsowns both; Python retains transport and projection only. No dual authority: Python has no branch that can produce a decision the TS reducer did not return.quota/unsettled_host_turn.py; 45 lines (prior_closeout_required_heartbeat_receipts) out ofquota/heartbeat_receipt.py; 82 lines of duplicated identity rule and local log-path handling out ofquota/settlement_readback.ts; 51 lines of duplicated log read and path building out ofscheduler/heartbeat_followup.ts.quota/unsettled_host_turn.py(48 preflight transport, 76 reduce transport, 97 projection, remainder module wiring), plus 182 lines for the shared identity owner and 99 lines for the shared rollout-log owner. The two TS owners are not transport: they are the single copy of rules that already had two copies.tests/control_plane_ts/unsettled_host_turn_recovery.test.ts, 20 cases) and the CLI suite carries the end-to-end path.quota/unsettled_host_turn.pyis a facade over the typed transaction, not a second authority. It is deleted once thequota should-runmain decision moves into TS, at which point the preflight/reduce transport and the PythonHeartbeatReceiptIdentityConflictErrormapping have no remaining caller.Validation
main@ 8c55247)staticpasseduv run --extra test ruff check loopx/control_plane tests/control_plane— clean;npm run typecheck:control-plane— clean; mypy adds no new error to the touched files (the one finding inquota/unsettled_host_turn.pyis byte-identical onmain)unitpassednpm run test:control-plane— 1656 pass / 0 fail / 3 skipped (1659)integrationpassedtests/control_plane/test_quota_settlement_cli.py— 65 passed, exercising the real CLI entrypoint through the typed preflight/reduce transaction, including a prior Turn with several receipts recovering exactly onceunitpassedtests/control_plane/test_effect_turn_live_quota_decision.py+test_heartbeat_receipt.py— 20 passed, including the identity-conflict fail-closed caseregression_paritypassedcloseout_requiredturns): zero mismatches. The same baseline reports "settlement already validated" for the two lanes that carry real closeout traffic, and the typed owner agrees.real_backendpassedreal_backendblockedpostgresql-authority (real server)job covers the shared surfaces against a real server.manualnot_runloopx canary premerge --from-git-diff:status: passed,merge_gate_passed: true,self_merge_allowed: true,manual_holds: 0, changed files 13, surfacescontrol_plane, python; 10/10 catalog canaries and 8/8 risk-profile smokesexecuted with zero failures. One catalog entry is reported as
advisory_inherited_failure:examples/control_plane/control-plane-maintainability-ratchet-smoke.pyfails onmaintoo with the identical finding (module_metric_budget:loopx/chat_runtime.py,1560 lines against a 1502 ceiling), on a file this PR does not touch, so it is
recorded here and not treated as this diff's regression.
loopx/semantics/inventory_v0.jsonfrom the earlier revision is dropped, becausemainnow treats that report as a local, uncommitted artifact and--checkrequires an explicit--output; (2) at the one conflict site both sides had added a test, so both are kept — this PR's multi-receipt recovery case andmain's#4542recovery-binding case; (3)#4542's change lives incli_commands/quota.py, which this PR does not touch.