Skip to content

fix(chat): bind idempotency keys to requests - #3981

Merged
huangruiteng merged 2 commits into
loopx-project:mainfrom
Duang777:codex/fix-core-bug-4
Sep 6, 2026
Merged

huangruiteng merged 2 commits into
loopx-project:mainfrom
Duang777:codex/fix-core-bug-4

Conversation

@Duang777

@Duang777 Duang777 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reject reused Chat client IDs when their request payload differs
  • cover managed Turns, queued Turns, and live-steering ingress receipts at the shared persistence boundary
  • preserve idempotent replay for matching requests

Root cause

Chat replay lookup used only client_turn_id or client_ingress_id. Reusing an ID with different message, origin, attachments, or mode returned the old record as a successful replay and silently discarded the new request.

Validation

  • focused regression reproduced twice before the fix
  • python -m pytest -q tests/test_chat_session_active_turn.py::test_chat_idempotency_keys_reject_different_requests
  • python -m pytest -q tests/test_chat_session_active_turn.py tests/test_attached_session_broker.py (40 passed)
  • python -m pytest -q tests/test_chat_*.py (73 passed)
  • ruff check loopx/chat_store.py tests/test_chat_session_active_turn.py
  • loopx canary premerge --from-git-diff (passed)
  • npm audit --omit=dev (0 vulnerabilities)
  • git diff --check

Scope

No schema migration is needed: persisted records already contain the request fields required for exact replay validation. No authentication or permission behavior changes.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@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.

动机

发现一个阻断问题:[P1] create_turn 校验了没有写入 Turn 的 attachments,导致相同带图请求重试失败。 精确 head:8e2833ae7bad1d808c9421bce36de2357e0416f3。位置:loopx/chat_store.py:590-597,尤其第 596 行;对应创建记录的 payload 在 612 行开始,没有 attachments。

PR 解决的问题真实且值得修:旧实现仅凭 client ID 命中就返回旧记录,换正文、origin 或 ingress mode 仍可能被当作成功重放,实际新请求被静默丢弃。个人 Workspace 的网络重试、重复点击与 live steering 都需要“相同请求复用、不同请求拒绝”的稳定语义。但保护冲突不能牺牲合法重试,当前附件路径恰好破坏了这一半合同。这里只需要在既有持久化边界补齐比较,不需要新的幂等服务或数据库迁移框架。

改动思路

实现把 request matching 放进已有 store 锁内:managed Turn 比较 message/origin/attachments,queued Turn 比较 message/origin,ingress receipt 比较 mode/message。共享 _require_matching_replay 是合理的小边界,统一“同 key 必须同请求”的拒绝语义;实际调用来自 ChatRuntimeController.submit_turn、排队路径和 steer_active_turn,不是未使用的工具函数。

正向路径核查:无附件 managed 请求第一次创建 Turn,第二次相同 ID/正文/origin 返回原 Turn 和 created=false,不再启动 worker;queued 与 ingress 的字段确实存在于各自持久化记录中,比较不同正文会抛 ValueError。锁的位置也复用原来的 Session/receipt 排他锁,没有新增竞争窗口。

负向路径核查:用经过正式 normalize_chat_image_attachments 校验的 1×1 PNG 创建 managed Turn,再以相同 ID、正文、origin、附件重试;turn_for_client 读出的是 Turn JSON,其 attachments 缺失,existing.get("attachments") 恒为 None。于是合法请求也被判为不同请求。重启 store 后同样复现。反向去掉附件重试,None 又等于请求中的 None,反而错误返回旧 Turn。因此不仅仅是附件内容变化漏检,而是比较基准选错了。

具体改动

完整 diff 为 2 个文件、+62/-0:loopx/chat_store.py 增加 29 行生产逻辑,tests/test_chat_session_active_turn.py 增加 33 行回归;没有新模块、依赖、CLI、schema ID 或生成资源。

关键代码讲解

  1. loopx/chat_store.py:89 的 _require_matching_replay 用逐字段值相等实现 request binding,差异直接抛出带 identity 名称的错误。它有三个生产调用方,拥有真实的幂等不变量,不是为减少几行代码而增加的转发层。风险在于它默认 existing 包含全部 request 字段,而 managed Turn 的附件不满足这个前提。
  2. create_ingress_receipt 的 existing 分支比较经过同样规范化的 mode/message,再返回持久化 receipt;创建分支确实保存二者。steer_active_turn 在继续 provider delivery 之前调用这里,因此比较必须位于现有 receipt 锁内,当前位置正确。
  3. create_turn 的 existing 分支新增 message/origin/attachments 检查;但创建分支只把 message/origin 放入 Turn payload,附件在稍后的 append_message(..., attachments=attachments) 中写进用户 transcript。ChatRuntimeController.submit_turn 确实传入规范化图片,问题会进入 Workspace 正式带图聊天路径,而非只影响直接测试 store 的用户。
  4. create_queued_turn 比较 message/origin,二者均保存到 queued Turn;它没有附件参数,attached queue 的附件仍由 runtime 明确拒绝。不能把 managed 的附件比较缺陷泛化为 queue 也必须增加图片能力。
  5. 新测试仅覆盖三个入口“改正文要报错”,遗漏同附件重放与删除/替换附件的对照。因此现有 focused suites 和 CI 通过,仍不能支持 PR body 中“persisted records already contain the request fields”的完整声明。

对主干的风险

P1 的触发条件是 managed Chat 请求携带非空附件并复用 client_turn_id,属于合法重试路径。结果是相同请求报冲突;删除附件却可被接受为旧请求。影响范围为带图 Chat 幂等性,不是认证绕过,也不是所有普通文字请求失效。

最小修复:为附件比较选定真正持久化的 canonical 请求身份。可以复用按 Turn 关联的原始用户消息附件,或在首次预留时持久化有明确兼容策略的规范化请求摘要;不要简单把“缺失字段”解释为没有附件,也不要仅删除附件校验而继续接受不同请求。已有 Turn 的附件原本在 transcript,因此需要说明历史记录如何比较,修正“不需任何兼容处理”的假设。避免为了比较再复制一份大型 data URL。

要求回归:同附件重试成功且 created=false;替换/删除附件报冲突;同样用重启后的 store 测试;保留无附件、origin、mode 与正文对照。所有测试使用合成图片和临时真实文件 store,不接触个人历史。

实际验证:仓库 attached broker、active-turn、image-attachment suites 共 45 passed,Ruff 与完整 diff hygiene 通过;独立语义探针 2 failed、1 passed,分别对应同图重试误拒绝、删图重试漏拒绝及无图重试正对照。有效图片探针复核仍得到相同失败。远端 exact-head 执行检查成功,不覆盖这两个反例;没有把 CI 绿色当成正确性的替代品。

没有默认关闭/opt-in 宣称,没有新增 actor authority 或 work-lane obligation;精确字段比较也没有 prose heuristic。错误文案保持 Chat 领域内中立。预期默认行为变化已在 PR body 说明,但附件结果与声明相反,需修复后再评审。

我的整体评价

REQUEST_CHANGES,当前不建议纳入发布。 方向与范围都是正向、适度的:在现有锁与持久化模型处校验请求比另造幂等系统更合适。阻断原因不是架构过大,而是持久化事实与比较字段不一致,直接破坏合法带图请求重放。请补最小修复及上述对照测试,不需要扩大成通用请求框架。

与 #3977 互补:#3977 处理完成阶段的恢复,本 PR 处理创建/ingress 阶段的重放。两者不是替代关系,也不能用 #3977 的通过结果给当前附件分支背书。修正后值得优先复审;本次不修改代码、不合并。

English verdict: REQUEST_CHANGES at 8e2833ae7bad1d808c9421bce36de2357e0416f3. P1: managed Turn replay compares attachments against a Turn record that never persists them. Identical image retries are rejected, while removing the attachment is accepted as a replay. 45 existing tests and lint/diff checks pass, but independent real-store probes fail both attachment invariants. Compare a durable canonical request identity and add restart-aware matching/conflicting attachment tests before merging.

Signed-off-by: huangruiteng <huangrt01@163.com>

@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.

动机

结论:原 P1 附件重试问题已修复,本次完整复查没有发现阻断项。审核精确 head:862bb30d5ff43604a9852985253c4ac1d331a3b3。

PR 原本解决聊天幂等键被不同请求复用时静默返回旧结果的问题。原实现直接比较 Turn 中不存在的 attachments,导致相同带图请求重试失败、删除图片的重试反而成功;此次维护者修复保留请求身份约束并消除该回归。

改动思路

复用 ChatSessionStore.messages() 中已有的 user 消息及 turn_id 关联作为附件权威来源,不给 Turn 再复制图片、不加摘要字段或迁移协议。普通文本仍从 Turn 中比较 message/origin,附件从对应持久化消息比较。旧正常记录无需迁移即可重放;缺少原始消息的损坏/中断记录明确拒绝,不猜测它是无附件请求。

生产路径为 HTTP _handle_create_turn 的附件归一化 → ChatRuntimeController.submit_turn → ChatSessionStore.create_turn。存储层在现有锁内判断身份;一致时返回原 Turn 且 created=false,调用者不启动第二个 worker;冲突由 ValueError 明确拒绝。

具体改动

  • loopx/chat_store.py:整个 PR 共增加 39 行生产代码。_require_matching_replay 统一做字段精确比较;create_ingress_receipt 绑定 mode/message;create_queued_turn 绑定 message/origin;create_turn 绑定 message/origin 及原 user 消息附件。本次仅补足 managed replay 的存储来源,未改变队列、权限或 provider 执行规则。
  • tests/test_chat_session_active_turn.py:整个 PR 增加 93 行回归测试。覆盖三个入口的不同请求拒绝;新增参数化带图/无图和重启/不重启组合,证明相同附件重放、添加/修改/删除附件拒绝、origin 冲突拒绝、不重复追加消息;通过中断 transcript append 验证不完整旧记录明确失败。
  • 正向:带图首次请求写入 Turn 与 transcript → 重建 store → 相同请求返回原 turn_id,不追加第二条消息。
  • 反向:相同 key 但删除或改动附件 → 持久化 user 消息对比失败 → 调用者收到冲突,不启动错误的新执行;原消息缺失时返回 original request is unavailable。

对主干的风险

这是默认开启的幂等校验修复,不是可选能力:此前不同请求可以静默命中旧结果,现在会拒绝;损坏旧记录缺少原始 transcript 时也会明确失败。未增加 actor/authority 协议、配置、状态机或 prose 分类规则。错误保持通用,且不披露原始请求内容。

主要剩余代价是重试时读取已有会话 transcript;它复用现有读取边界,首次创建不增加扫描。未扩展成新的索引/存储迁移,避免为窄修复增加长期维护负担。原本 Turn 与 transcript 的多文件写入不是原子事务,本 PR 不承诺恢复该历史中断窗口;现在对此明确失败而非错误接受。未来若需恢复中断写入,应单独验证完整 closeout 契约。

验证:93 项聊天模块与 attached broker 测试通过;独立有效 PNG 的真实文件存储探针 3 项通过(原两项失败现已通过);loopx-chat-store-smoke、Ruff、编译、diff hygiene 和公开边界扫描通过。标准 canary premerge 通过(diff/compile/maintainability checks);精确质量凭据 cqr_3ac0eadbc3697f3f39ba 校验 valid,2 个文件,0 blocker/warning/advisory,无 manual hold。没有运行 live provider 调用或本地全仓 pytest。推送后新 head 的远端完整 CI 尚在运行,不能把旧 head 的绿灯当作新 head 验证。

我的整体评价

Approve。改动规模与问题匹配,保留原作者的幂等修复价值,并复用现有消息存储消除附件回归。已做相关的简化审视:保留共享比较 guard,不新增重复附件持久化、摘要字段或通用框架;当前没有必要扩大重构。依据维护者明确授权,在上述风险定向验证及质量门通过后可自合并;远端全量 CI 未完成的事实保持披露。

English verdict: APPROVE at 862bb30d5ff43604a9852985253c4ac1d331a3b3. The attachment replay blocker is fixed by comparing the durable user transcript, without duplicating image payloads. Identical requests replay, changed/removed images reject, and incomplete legacy records fail explicitly. 93 chat tests, 3 independent normalized-PNG store probes, file-store smoke, Ruff, compile, exact quality receipt and standard premerge passed. Fresh remote full CI is still running; no live-provider validation claimed.

@huangruiteng
huangruiteng merged commit 8ed49d7 into loopx-project:main Sep 6, 2026
9 checks passed
yanfeng98 added a commit to yanfeng98/nano-loopx that referenced this pull request Sep 6, 2026
- 6fc4723 lark goal topic reconnect (loopx-project#3983)
- 9d70e1e reward-memory 外发召回指引 (loopx-project#3968)
- 8ed49d7 chat idempotency keys (loopx-project#3981)
- 62a799c chat attached completion closeout
- 92b03ac status contract refresh / 7bb1eb1 workspace index bound
- 中继 merge commits

文档融合(中文唯一):
- periodic-report-v0: 上游英文化+新增 limit/offset 有界窗口说明 -> 中文并入
- agent_turn_recall README: 以上游英文全文为源重译(新增 Turn 契约/使用/Freshness)
- reward_memory README/OUTBOUND: 以上游官方全中文版(README.zh-CN.md/
  OUTBOUND.zh-CN.md)升为主文件,删除 zh-CN 文件与英文原版
- repair-patterns: 上游仅整文件英化(内容逐行对应无新增)-> 保留中文版
- lark-goal-topic-connection-smoke: 采用上游实现(连接嵌套/connection_id)

Co-Authored-By: Claude Code <noreply@anthropic.com>
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