fix(chat): reject conflicting message ID replays - #5733
huangruiteng merged 1 commit into
Conversation
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent — gpt-6.1-sol (OpenAI); runtime_reported; reasoning_effort=xhigh
Exact reviewed head: 1cc69ee; immutable baseline: 9e78309
动机
通过消息 API 或委托回传保存结论的开发者和会话用户。
旧版复用消息标识并改变结论时，会返回旧消息而不保存新内容，看起来成功；此提交明确报告冲突并保留原消息，相同内容重试仍只保留一条，新结论换新标识后可正常保存。
六类字段冲突与两个独立进程竞争均由持久 store 明确处理；普通和精确实例回传的真实授权、终态及重启读取通过，合法回传的完整 transcript 读取从两次降为一次。
本次限定本地消息身份与回传一致性，不证明真实外部账户发送、付费模型采用、已安装 GUI 或各领域持续吞吐，也不增加跨会话和 Goal 权限。
改动思路
消息标识代表一次不可变消息，不能一边保留旧正文，一边把不同内容当成功。比较规则放到既有 Chat replay owner，store 在原 transcript 文件锁里同时比较与追加；两个 manager drain 只把这一结果适配为原 delivery 终态，删除锁外的独立比较。普通 generated-ID 消息不走重放判断，时间戳不影响身份；历史空附件仍能重放。消息一致性不授予会话授权，exact/legacy admission、receiver 和发送器的 owner 不变。
具体改动
依据 https://github.com/loopx-project/loopx/issues/5462#issuecomment-5967721942，固定版本 issuecomment-5967721942。1. Message identity：保留不可变消息与相同内容重试，排除生成时间，对不同语义载荷明确冲突且原消息不覆盖——本 head 已实现。维护者同一 frame 的 transport 和 channel 方向由已合并 #5554 保留，本 PR 不更改它们；现有正例与真实 exact/legacy 路径回归验证其采用。基线 App conversation RFC 的既有 reply identity/recovery 约束一致，不将其未来 promotion 当此次义务。
关键代码讲解
require_matching_message_replay（chat.py:83）比较 role/text/turn_id/origin/attachments/goal_draft；缺失和 None 附件统一 []，产生仅含字段名的 ChatMessagePayloadConflictError。append_message（chat_store.py:688）在已有 exclusive_file_lock 内执行检查：同内容返回原 row，冲突时没有追加。_drain_exact 和 drain 删除单独遍历 transcript 的 preflight，捕获 store 的 typed error，仍写 explicit_unverified/manager_return_payload_conflict，不伪造 delivered 或进入重试。6 文件 +143/-150；生产 +72/-114，两个测试文件覆盖六字段/并发及合法 generated-ID 场景，生成 manifest 只更新位置。现有 Python Chat 持久化/replay owner 被收敛，没有新增平行 TS/Python authority 或数据版本。
对主干的风险
未发现阻塞问题。独立同输入真实文件 store 对照 head14/14，base7/14；基线六字段均静默返回旧 row，两个 OS 进程竞争也都报告 appended，而 head 一成功一冲突且 disk 一条。冷 store 重启后同内容重试、新 ID 结论、另一 session 同 ID、生成 ID、历史 missing/None/[] 附件全部可用。完整真实 exact source_session_v1 的 deliver→acknowledge→report→drain 未 mock admission/authorization/settlement；正常返回一次，冲突原内容不变、终态不重试，重启第二 drain 为零。加入25条无关 transcript 后不换目标；合法 exact 与 legacy 回传的整段消息读取都从基线2次降为head1次，不把扫描次数当延迟或吞吐百分比。
193 项 owner/模式/精确实例测试及 40 项 HTTP、草稿、queue、steer、delegation 消费者测试通过；Ruff、5 direct +2 selected/executed canary、全树 semantic/IO census 与边界扫描通过，0 failure/warning。两次额外 pytest 命令误用了不存在的测试文件、未收集，已按真实清单修正；这是评审调用错误，未靠修改产品或断言得到通过。未读取 CI。真实外部 transport/provider 与模型效果未测；这些边界本次未更改，不能把合成回传当外部账户验收。
语义与 CI 对齐
新异常复用 ValueError 契约，回传继续复用已有 delivery 状态/错误，局部字段白名单成为唯一 replay 比较 owner，不用字符串包含来分类。默认行为确有变化：direct append 旧版忽略不同载荷，如今拒绝；PR body、store docstring、重新命名的 conflict tests 和本评审均披露，合法 same-ID 及 generated-ID 语义保留。没有 feature-off/opt-in 承诺，未引入新权限；拒绝是强制一致性规则，不能当建议忽略。空附件兼容有历史 row 读回，而不是以版本号推测兼容。
我的整体评价
APPROVE。long_horizon=improved：旧结论不会被假成功掩盖，冲突不进入无效重试，重启维持同一身份和终态；user_experience=improved：直接调用者能看到具体冲突字段，用新 ID 保存新结论的恢复路径明确。效果与效率在本地持久化、原会话回传边界上均正向；不能据此推断每个 Goal/domain 的长期收益。future-facing pass 已应用：删掉 manager 私有六字段与锁外比较，把下一次消息规则修改定位到一个 replay owner，同时少一次完整 transcript 读取。保持历史 row 兼容和既有两种 return settlement，无新增泛化框架；最强未测边界是实际外部发送与模型长期采用。运行时变更仍由维护者合并。
English verdict: APPROVE — 1cc69ee: changed semantic payloads now conflict atomically at the shared transcript owner while identical retries and historical empty attachments remain compatible. Independent real-file comparison improves 7/14 to 14/14; unmocked exact/legacy result-return and restart paths pass. Valid returns scan the transcript once instead of twice. 233 focused/consumer tests, Ruff and selected canaries pass. CI was not queried; external-provider and long-running model outcomes remain unmeasured.
The first loopx-project#5462 fix guarded only manager return delivery, so direct and other Chat callers still received an old transcript row when they reused an ID with changed content. Enforce semantic payload equality atomically in the shared message replay path. Identical retries remain idempotent, while manager return delivery maps the typed conflict to its existing terminal state. Refs loopx-project#5462 Signed-off-by: Duang777 <[email protected]>
1cc69ee to
78da491
Compare
3 participants
Goal
Close the remaining message-identity contract gap from #5462. The merged #5554 protects manager return delivery, but direct and other
ChatSessionStore.append_message()callers still receive the old row when they reuse an explicitmessage_idwith changed content.Root cause
append_message()checked onlymessage_idunder the transcript lock and returned the first matching row without comparing its semantic payload. A caller could therefore submit a new conclusion, receive a success-shaped result, and persist none of the new content.Changes
role,text,turn_id,origin,attachments, andgoal_draftmust match for an explicit-ID retry. Generated timestamps are excluded, and a missing attachment field remains equivalent to an empty list for stored-row compatibility.ChatMessagePayloadConflictErrorand leave the original row untouched.explicit_unverified/manager_return_payload_conflictresult remains unchanged, while the compare-and-append decision is now atomic.Regression evidence
Red on
f1efc22e1: the six changed-payload cases all failed withDID NOT RAISE ValueError; the public API repro returnedfirst conclusionfor a secondchanged conclusionappend and persisted only the first row.Exact head
78da4916fafter rebasing onto77d23b7c0:106 passed: focused store, active-turn, and manager-return suiteloopx canary premerge --from-git-diff --git-diff-base upstream/main: passed, including maintainability and semantic-vocabulary checksmessage_id conflicts with existing message payload field: text; disk retained only the original textThe same code diff passed 853 Chat/manager/attached/inbox tests on the preceding exact head before this no-conflict rebase. The rebase incorporates the mainline browser-fixture corrections from #5722/#5737 after the preceding CI head timed out on the stale fixture label. Its Windows failures were in unchanged Effect runtime permission tests.
Sibling sweep
The other stable-identity write paths inspected already compare the request or content before treating a retry as idempotent: Chat ingress, queued turns, attached completion, rollout events, and content-ops events. Lookup-only message-ID matches were left unchanged.
Closes #5462