fix(chat): scope attached completion messages to turns - #4353
huangruiteng merged 2 commits into
Conversation
f734ad1 to
b624ed8
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
结论:REQUEST_CHANGES。当前 head b624ed866f71361fe3190e283af10a5ee57a8a27 修复了跨 Turn 复用 completion_id 的原始碰撞,但 recovery predicate 只按 role + turn_id 匹配,扩大成了错误的“任意 agent 消息都可证明 attached-host completion”。
阻塞项(P1):finalize_attached_turn_completion() 在 loopx/chat_store.py:1098-1117 取同 turn 的第一条 role=agent 消息;若文本等于 reserved response,就跳过 canonical origin=attached_host append,继续发布 terminal events、release session 并把 turn 标 completed。它没有校验 origin,也没有限制为 legacy attached.<completion_id> 或新 attached.<turn_id>.completed identity。
我用真实 ChatSessionStore、真实 JSONL/lock/finalizer 做了 crash-recovery 负向复现:先让 turn 停在 completing,再插入同 turn、同文本、origin=other_runtime、message_id=other.runtime.message 的 agent row,然后调用 recovery。结果 turn 变为 completed,但 transcript 中唯一 agent row 仍是 other_runtime,没有任何 attached_host completion。这里文本相等不能替代来源/receipt 语义;否则 unrelated output 会被静默提升为 attached host 的完成证据。
最小修复:兼容扫描必须至少要求 origin=attached_host,并只认可该 turn 对应的 legacy/new canonical message identity;若发现多条或内容冲突,应确定性拒绝而不是取“第一条 agent 消息”。新增 equal-text 与 different-text 的 wrong-origin 负向测试,证明它们都不能满足 attached recovery。
动机
原问题成立:旧 attached.<completion_id> 把 host receipt 当作全局 transcript identity;不同 Turn 合法复用同一 completion id 时,append_message 会返回旧 row,导致第二个回答缺失但 terminal state 已完成。把 idempotency key 改为 canonical turn 是正确方向。
改动思路
新写入使用 attached.<turn_id>.completed,与 managed sibling 的 managed.<turn_id>.completed 一致;同时为 crash 后已落盘的旧 attached.<completion_id> row 保留兼容识别。问题只在兼容识别把“旧 attached receipt”放宽成了“任何 same-turn agent row”,破坏了现有 origin=attached_host 边界。
具体改动
chat_store.py:改为 turn-scoped message id,并在 finalize/recovery 前扫描历史消息。- 测试:当前分支 focused attached-session 21 passed;完整
tests/test_chat*.py167 passed;Ruff、git diff --check、DCO 通过,远端 required checks 全绿(release/presentation 的预期 skip 除外)。 - 正向用例能让旧实现失败,证明跨 Turn collision 的修复有效。
- 现有 legacy recovery 测试通过同一个 attached append 构造旧 row,所以没有挑战 wrong-origin / wrong-identity 的假阳性;上述真实 store 反例补出了这个证据空洞。
对主干的风险
当前风险是 provenance 与 terminal-state 一致性:聊天消费者看到 completed,却无法证明可见回答来自拥有该 Turn 的 attached host;同文本时静默吞错,异文本时则用普通 ValueError 把 recovery 卡住。线性扫描的性能目前是非阻塞残余风险;代码注释按可测量历史再建索引是合理的,无需为这次修复引入新状态或框架。
我的整体评价
范围、价值和 turn-derived identity 都合适,修复不应被扩大。只需在现有 finalizer 中把 legacy compatibility predicate 收紧到 attached-host canonical identity,并加入两条负向回归;修完后可以快速复审。
English verdict: REQUEST_CHANGES — the turn-scoped id fixes the original collision, but recovery can treat any same-turn agent row as attached-host completion; require attached-host origin/canonical identity and add wrong-origin negative coverage.
b624ed8 to
248947e
Compare
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
248947e to
a41a057
Compare
|
Addressed the requested provenance fix at head
Please re-review the current head when available. |
huangruiteng
left a comment
There was a problem hiding this comment.
结论:APPROVE。本次复审绑定 exact head a41a057edbccc636ac77b81692c995d13f4cccd1。上一轮指出的 provenance 缺口已经在真实 transcript/recovery 边界修复;我没有发现新的阻塞项。
动机
原问题不是普通的重复消息,而是把 host 提供的 completion_id 错当成跨 Turn 唯一的 transcript identity。两个合法 Turn 复用同一个 receipt id 时,旧 append_message 会把第二次写入当作第一次的重放,于是 Turn 已发布 terminal event、释放 active claim 并持久化 completed,但用户只看得到第一条回答。上一版 head 已把新写入改成 Turn 派生 identity,却在恢复时只用 role + turn_id + text 判断“已有回答”,使同文本的 other_runtime 消息也能被提升为 attached-host completion proof。本轮的目标因此是同时守住两个不变量:每个 canonical Turn 必须有独立可见回答;恢复证据必须真的来自该 attached host 的 current/legacy identity。
改动思路
当前实现没有增加第二套状态或 completion ledger,而是复用现有 Turn JSON、messages.jsonl、Turn file lock、terminal event 和 active-Turn release owner。complete_attached_agent_turn 仍先在锁内把 response/receipt 预留为 completing,随后由 finalize_attached_turn_completion 统一发布可见效果。新的纯函数 resolve_attached_completion_replay 只负责判定:零条 same-Turn agent row 时返回 attached.<turn_id>.completed;恰好一条时,只有 origin=attached_host、message id 为当前 Turn identity 或精确 legacy attached.<completion_id>、且文本相同才算合法 replay;wrong origin、wrong id、冲突文本或多条 row 一律在 terminal effects 前失败。
我同时比较了 origin/main、上一轮 reviewed head b624ed866f71361fe3190e283af10a5ee57a8a27 和当前 head。相同真实 File/JSONL fixture 下,main 丢失第二个复用 receipt 的回答;上一版保住两个回答但把 wrong-origin row 错判成完成;当前版既保住两个回答,又在 wrong-origin 情形返回显式 identity conflict、保留 completing、不产生 turn.completed。这覆盖了上一轮 finding,而不是只让 mock 返回期望值。
具体改动
生产改动集中在两个既有 owner:loopx/chat.py 增加 40 行纯 replay/identity 判断,并接收从巨大 store 模块移出的通用 request replay equality helper;loopx/chat_store.py 以该决定替换 completion-id 写入和“第一条 agent row”扫描,净减少代码;tests/test_attached_session_broker.py 增加 159 行 real-store 覆盖。没有 schema version、CLI、权限、调度、provider 或 managed-session 默认行为变化。
关键代码讲解
resolve_attached_completion_replay:以完整 session journal、canonicalturn_id、storedcompletion_id和 reserved response 为输入。它用字段精确相等和 row cardinality,而不是 substring/自然语言启发式;legacy identity 只读兼容,不再有 writer。ChatSessionStore.finalize_attached_turn_completion:在持有 Turn lock 时先调用 resolver,只有返回 current id 才 append;合法 replay 则跳过 message append。之后才写 terminal events、释放 active Turn、把completing落成completed,因此 provenance 冲突不会留下半真 terminal state。test_attached_completion_recovery_rejects_ambiguous_agent_messages:真实创建/bind/claim/reserve store 状态,只用 monkeypatch 放置 crash 点;后续的 JSONL、lock、finalizer、event 和 state readback 都是真实路径。四个 case 覆盖 equal/different wrong-origin、attached origin 但 wrong id、以及 current+legacy 多 row。
相关 future-facing pass 已落实:identity 决策从 I/O 中抽成单一纯函数,而不是继续在 store 里堆第二组条件;现阶段没有必要再引入索引或框架。完整 journal 的线性扫描是残余性能风险,但 finalization 不是 display-cap 视图,正确性优先;只有真实 session 历史证明它成为热点时再加所属 index。
对主干的风险
最强负向场景是:response 已预留、进程崩溃,另一个 runtime 在相同 Turn 下写入同文本 agent row,然后 startup recovery 继续。上一版会静默完成;当前版明确拒绝并让 Turn 保持 completing。这会把已损坏/竞态状态从“自动追加第二条回答并完成”收紧为“显式修复后重试”,属于有意的完整性变化;错误可观察、没有 terminal side effect,恢复 owner 仍是同一个 finalizer。
验证结果:focused attached-session suite 25 passed;完整 tests/test_chat*.py 218 passed;Ruff 和 git diff --check 通过;三版本 real-store counterfactual 得到预期差异。远端 exact head 的 DCO、依赖、构建、Windows、Node、dashboard、test shards 和 E2E/installed 等检查已成功或按设计 skip;剩余 merge-gate 处于 review-dependent queued,Sonar 是 non-blocking queued。发布前已再次确认 remote head 未变化。
我的整体评价
这个 PR 的问题价值、机制和范围匹配:它修复一种会把“已完成”与“可见回答”永久分离的静默数据完整性缺陷,只在既有 attached completion/recovery owner 内增加 Turn-derived identity 和严格 legacy predicate。上一轮 blocker 有可失败的历史 head 证据,也有当前 head 的 real-path readback;没有未核验的架构、默认关闭或 authority 语义。建议按正常合并门禁推进。
English verdict: APPROVE — exact head a41a057edbccc636ac77b81692c995d13f4cccd1 preserves distinct responses when completion ids are reused and now rejects wrong-origin/ambiguous recovery evidence before terminal effects; 25 attached-session tests, 218 chat tests, exact three-revision real-store probes, Ruff, diff checks, DCO, and applicable remote suites passed.
Summary
Validation
python -m pytest -q tests/test_chat*.py(167 passed)python -m ruff check loopx/chat_store.py tests/test_attached_session_broker.pypython examples/docs-governance-smoke.pyloopx canary premerge --from-git-diff --format jsongit diff --checkRisk
Low and localized to attached-session completion transcript finalization. No authority or external-effect behavior changes.