fix(turn): fail closed on key/owner-mismatched settlement replay (GH-C83 M7.1 parity) - #3193
Conversation
The Turn settlement adapter accepted a journal committed under one effect identity and replayed it under a different plan identity: it re-attributed the committed validation/writeback/spend receipts to the new effect id and skipped the effects, without any typed failure. The quota adapter fails closed with SettlementFailureKind.IDENTITY_MISMATCH for the same identity drift, so this was a contract parity gap in the Turn adapter (RFC M7.1, same direction as PR huangruiteng#3074). - execute_turn_driver_settlement gains an optional committed_effect_id cross-check; mismatch fails closed at the validation step with IDENTITY_MISMATCH, keeping legacy/direct callers unchanged when no journal provenance is supplied. - the executor wires the journal's committed effect id on resume via _journal_committed_effect_id; legacy journals without a typed settlement plan skip the check. - new M7.1 parity fixtures pin key-mismatched and owner-mismatched replay, the same-key idempotent control, the opt-in seam, and the provenance helper. Fixes huangruiteng#3190
The GH-C83 validation command referenced tests/test_loopx_turn_journal.py which does not exist; the M7.1 parity fixtures live in tests/test_loopx_turn_settlement_parity.py.
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审：Turn 结算 replay 的 key/owner 失配 fail-closed（PR #3193）
评审目标 head：bee3b68814bec11a18229a212b9e1e042ea7858d（base: main，2 个 commit，4 个文件，+264/-1）
动机
Turn settlement 适配器此前存在一个契约缺口：如果一份 journal 是在某个 effect identity 下提交的，而恢复时调用方用另一个 plan identity 去 replay，适配器会把已提交的 validation/writeback/spend receipts 重新归属到新的 effect id 并跳过 effects，却不产生任何 typed failure。quota 适配器对同样的 identity drift 会用 SettlementFailureKind.IDENTITY_MISMATCH fail-closed（RFC M7.1、与 #3074 同方向），所以这是 Turn 侧的 parity 缺口。本 PR 的目标是让"换 key/换 owner 的 replay"与 quota 侧行为一致：先证明 journal 的已提交 receipts 属于当前 plan 的结算身份，否则在 validation 步直接失败，而不是悄悄重挂账。
改动思路
改动采用 opt-in 的 provenance 交叉校验，而不是无条件收紧所有调用：
execute_turn_driver_settlement新增可选参数committed_effect_id；只有提供该值时才比较它和当前 plan 的effect_id，不相等即返回IDENTITY_MISMATCH（step=VALIDATION），且不产生任何 receipts、不调用 writeback/spend。- executor 在 resume 时通过新的
_journal_committed_effect_id(journal)从 journal 存储的plan.transaction.settlement_plan.identity.effect_id读取已提交身份；没有 typed settlement plan 的 legacy journal 返回None，从而跳过检查、保持旧恢复行为。 - 新增 217 行 M7.1 parity fixtures：key 失配、owner 失配、同 key 幂等控制、无 provenance 的 opt-in 保留、provenance 读取 helper 的 typed/legacy 两种路径。
- CONTRIBUTOR_TASKS.md 把 GH-C83 的验证命令从不存在的
tests/test_loopx_turn_journal.py指向真实的 parity 测试文件。
具体改动
loopx/control_plane/turn_driver/executor.py（+25）：新增_journal_committed_effect_id，并在_typed_settlement_stage的execute_turn_driver_settlement(...)调用处传入committed_effect_id=_journal_committed_effect_id(journal)。loopx/control_plane/turn_driver/settlement.py（+21/-1）：execute_turn_driver_settlement增加尾参committed_effect_id: str | None = None，在_settlement_effect_id通过后立即做失配检查。tests/test_loopx_turn_settlement_parity.py（+217）：7 个测试函数覆盖全部语义分支。CONTRIBUTOR_TASKS.md（+1/-1）：修正 GH-C83 验证路径。
关键代码讲解
_journal_committed_effect_id（executor.py）：沿journal.plan.transaction.settlement_plan.identity逐层做Mapping类型检查，读取effect_id并 strip；任一环节缺失即返回None。我对照了真实 journal 结构：build_loopx_turn_transaction_plan序列化的SettlementIdentity.as_dict()确实包含effect_id，且_settlement_effect_id在结算时读取的也是同一个字段，因此该 helper 在生产 journal 上能拿到真实已提交身份，不是只在 fixture 里自洽。execute_turn_driver_settlement的committed_effect_id检查（settlement.py）：位置在_settlement_effect_id(transaction_plan)成功之后、_seed_result之前，保证任何 receipt 组装和 effect 调用都不会发生；返回的 failure 携带IDENTITY_MISMATCH、VALIDATIONstep 和包含两个 effect id 的 reason。Noneprovenance 走原路径，直接调用者完全不受影响。- executor 的接线：resume 分支读的是 journal 里的存储 plan（写入时的身份），而当前执行用的是新 plan（请求时的身份），这正是需要交叉校验的两侧；检查通过后才允许同 key 幂等重放。
- 测试断言的关键点：失配时
result.receipts == ()且 writeback/spend 计数为 0（不允许"跳过 effects 却重挂 receipts"）；同 key 时三个 receipts 的effect_id全部等于 committed id；committed_effect_id=None时保持旧契约。
对主干的风险
- 阻塞项（CI 红）：GitHub Actions 的 pytest job 在其 lint 步骤失败——
tests/test_loopx_turn_settlement_parity.py引入了两个未使用导入TurnSettlementCheckpoint、TurnEffect（ruff F401），CI 日志明确报Found 2 errors并以 exit 1 结束。这是本 PR 引入的、阻塞合并的失败；修复只需删除这两个导入（如顺手整理 import block 排序）。 - 功能本身在本机 exact-head 验证通过：
tests/test_loopx_turn_settlement_parity.py+test_loopx_turn_transaction.py+test_loopx_turn_executor.py共 48 项全部通过；语义与 quota 侧 precedent 一致，legacy opt-in seam 有测试固定。 - 风险面控制得当：默认参数放在签名末尾，所有现有直接调用者不变；只有提供 journal provenance 的恢复路径才启用新检查，不会误伤预览或首次执行。
我的整体评价
这是一个语义正确、范围收敛的 parity 修复：失配时在 validation 步 fail-closed、不产生 receipts、不跳过 effects，测试从契约而非实现输出推导，provenance helper 与真实 journal 结构一致。唯一阻塞是新增测试文件里的两个未使用导入导致 CI lint 失败，属于一行级修复。结论：REQUEST_CHANGES（仅 lint blocker；修复后即可 re-review）。
English Verdict (PR #3193)
- Exact head reviewed:
bee3b68814bec11a18229a212b9e1e042ea7858d - Verdict: REQUEST_CHANGES
- Key finding: The functional change is sound — replaying a journal under a different settlement effect now fails closed at validation with
IDENTITY_MISMATCH, no receipts are re-attributed, and legacy/no-provenance callers are unchanged (48 focused tests pass locally, including parity fixtures). However, the PR breaks CI: the pytest workflow's lint step fails on two unused imports (TurnSettlementCheckpoint,TurnEffect) intests/test_loopx_turn_settlement_parity.py(ruff F401). - Required fix: remove the two unused imports (and tidy the import block); then re-run CI.
Remove TurnSettlementCheckpoint and TurnEffect from the parity test imports (ruff F401); the inline callbacks do not annotate with them.
|
已按评审修复：删除了 英文版：Fixed per review — removed the two unused imports ( |
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审（r2，exact head 3225ca7）：Turn 结算 replay 的 key/owner 失配 fail-closed（PR #3193）
评审目标 head：3225ca7ccd61217de17df998bcf22fe5241fb72c（base: main，3 个 commit，4 个文件）
动机
首轮评审（head bee3b688）确认功能语义正确，但 CI 被新增测试文件里的两个未使用导入（ruff F401：TurnSettlementCheckpoint、TurnEffect）拦红。作者已按评审意见提交第 3 个 commit 3225ca7c 移除这两个导入。本轮 r2 在 exact head 上重新验证：原功能修复（journal 身份失配时 settlement replay fail-closed）保持不变，阻塞项已解决。
改动思路
整体设计不变：execute_turn_driver_settlement 增加可选 committed_effect_id，只有提供 journal provenance 时才做身份交叉校验；失配即在 VALIDATION 步返回 IDENTITY_MISMATCH，不产生 receipts、不调用 writeback/spend。executor 通过 _journal_committed_effect_id(journal) 读取已提交身份；legacy journal（无 typed settlement plan）返回 None 并跳过检查。新 commit 只删除了测试文件中的两个未使用导入，并把多行 import 收敛为单行，不触碰任何产品代码。
具体改动
tests/test_loopx_turn_settlement_parity.py（本轮 -4 行）：移除TurnSettlementCheckpoint、TurnEffect导入（inline 回调不做类型标注，确属未使用）。- 产品代码与其余测试与首轮一致：
executor.py（+25）、settlement.py（+21/-1）、CONTRIBUTOR_TASKS.md（+1/-1）、parity 测试主体（7 个用例）。
关键代码讲解
- 修复后的 import 块：
from loopx.control_plane.turn_driver.settlement import execute_turn_driver_settlement——只保留实际调用的符号，F401 消除；TurnSettlementCheckpoint/TurnEffect在_settle的 inline callback 中通过类型推断即可，不需要显式标注。 - 功能核心（首轮已验证，本轮复核未变）：
committed_effect_id与当前 planeffect_id不一致时SettlementResult.failed(kind=IDENTITY_MISMATCH, step_kind=VALIDATION)；_journal_committed_effect_id读取真实 journal 中plan.transaction.settlement_plan.identity.effect_id（SettlementIdentity.as_dict()序列化包含该字段），legacy 路径 opt-in 保留。
对主干的风险
- 原阻塞项已解决：exact head 上
ruff check tests/test_loopx_turn_settlement_parity.py通过（All checks passed!），pytest tests/test_loopx_turn_settlement_parity.py + test_loopx_turn_transaction.py + test_loopx_turn_executor.py共 48 项全部通过。 - CI：本轮 head 的 build 与 dependency-review 已绿；pytest job 在我评审时仍在运行（in_progress）。本机复现与 CI 前两步均通过，剩余以 CI 终态为准。
- 风险面：产品代码与首轮完全相同，没有引入新行为；只读的 import 清理不改变任何执行路径。
我的整体评价
作者按评审意见精确修复了唯一阻塞项（删除两个未使用导入），功能语义、测试覆盖与 legacy opt-in 边界均保持良好。r2 exact head 上本机 lint 与聚焦测试全绿。结论：APPROVE。
English Verdict (PR #3193, re-review)
- Exact head reviewed:
3225ca7ccd61217de17df998bcf22fe5241fb72c - Verdict: APPROVE
- Key finding: The only blocking finding from the first review is resolved — the two unused imports (
TurnSettlementCheckpoint,TurnEffect) flagged by ruff F401 intests/test_loopx_turn_settlement_parity.pyare removed. The fail-closed identity cross-check (journal provenance vs current settlement effect id) is unchanged and correct. - Validation at exact head:
ruff checkon the parity test file passes; 48 focused tests pass (parity + transaction + executor); CI build and dependency-review are green, with the pytest job still in progress at review time.
2 participants
What
GH-C83 (claim issue #3190): M7.1 parity fixtures for the fenced Turn settlement vertical slice, plus the small production change the fixture exposes.
Characterization showed five of the six M7.1 fault classes were already covered in tests/test_loopx_turn_executor.py (partial execution, retry, cancellation, permission denial, budget rejection). The missing parity is owner/key-mismatched replay: the Turn settlement adapter accepted a journal committed under one effect identity and replayed it under a different plan identity — it silently re-attributed the committed validation/writeback/spend receipts to the new effect id and skipped the effects, with no typed failure. The quota adapter (resolve_heartbeat_settlement_identity) fails closed with SettlementFailureKind.IDENTITY_MISMATCH for the same identity drift, so this is a contract parity gap (RFC M7.1), in the same direction as PR #3074 ("permit only owner/key-matched terminal replay").
Changes
Validation
Scope notes
Closes #3190