fix(lark): reuse the established goal topic root on reconnect - #3983
huangruiteng merged 7 commits into
Conversation
Reconnecting a Goal Channel whose binding predates the multi-connection schema re-derived the topic idempotency key with the new connection_id component, so the provider no longer deduplicated the send: every reconnect (any config edit) re-announced a duplicate Goal Topic message and overwrote the stored root, leaving the old topic silently disconnected. Resolve the existing connection through the typed binding reader with the current provider target before sending: when the stored connection is enabled, its target still resolves to this chat, and its topic root is a valid message id, reconnect now adopts that root and skips both the send and the readback. A binding that points at a different target or chat still sends a fresh topic, and same-schema reconnects keep the established root regardless of provider idempotency retention. Verified with three regressions: legacy v0 reconnect without resend and with the legacy root preserved, mismatched target_ref still sending a new topic, and same-schema reconnect without a second send. Signed-off-by: now-ing <now-ing@users.noreply.github.com>
The reconnect root-reuse block grew goal_topic_connections.py past the module-line ratchet (1524 > 1500). Extract the resolved-connection check into reusable_goal_topic_root() beside the binding reader it consumes, leaving the connect path one typed call. Signed-off-by: now-ing <now-ing@users.noreply.github.com>
Signed-off-by: now-ing <now-ing@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
结论:REQUEST_CHANGES,当前 head ed73486fbaef4c421e6c8595fa06eacc74ea4734 有两个需要修复的问题,不建议直接进入 v1.0。
- [P1] 复用路径跳过 provider readback,却仍生成新的已验证回执。
goal_topic_connections.py:580-582直接采用旧 root,之后仍写verified_at=now_iso()并返回readback_verified=True、external_write_performed=True。用空 receipts 的 legacy binding 和一个会拒绝读取该旧消息的 provider runner 复现:零消息读取/发送,仍得到 connected + verified。旧 root 被撤回或不可读时,用户会看到成功但回复仍指向不可用话题。最小修复是验证复用 root 的存在及归属,失败显式反馈;若产品允许仅复用历史证据,应保留原 receipt 时间并明确 cached/unverified,不能制造本次 readback 成功。新增“根消息不可读/不存在”和“零发送但真实验证”回归。 - [P2] 另一个 Agent 的不同 target 会让当前合法 root 复用失败。
goal_channel_contracts.py:743-750把整个连接集合交给带单个 provider_target 的binding_for_goal;其内部bindings_for_goal先解析所有连接,再筛选 connection_id。只要 sibling 的 target_ref 不同,就抛 ValueError,被新 helper 当成当前 root 无效,进入发送路径。独立双 Agent fixture 中,alpha 的合法om_existing实际返回空串。它保留了此 PR 想消除的重复发送/旧根被覆盖风险。应先选择准确 connection,再只对该连接做目标校验;补两个 Agent 位于不同群/target 的回归,不能因为 sibling 改变就放弃已建立的 root。
PR 的原始动机是正向的:多连接升级把 connection_id 加入发送幂等键,旧版本已经建立的话题无法再靠相同 provider key 去重;保存连接配置时会重复公告并换掉路由根。稳定复用已建立的话题能保护升级用户和长期会话。问题在于“本地字符串可复用”“该远端消息仍存在”“本次已验证”是不同事实,当前实现混在一起。
改动思路
生产入口来自 chat_lark_api.py 的连接保存请求,调用 connect_lark_goal_topic。它仍先校验 Agent、枚举化路由、app readiness、群访问和 bot membership,执行授权及 preview 边界未改。执行路径读取已有绑定,经 reusable_goal_topic_root 判断后分成复用或发送/readback,再保存连接和返回结果。
正向路径已经由现有测试证明:单个 legacy v0 绑定 → reader 派生稳定 connection_id → target 相符且 root 形状有效 → 不重发公告 → 保存为连接集合,原 root 保持。新格式重复连接也只发送一次。负向的 target mismatch 仍走新发送并 readback。但 sibling resolution 和根消息实际可读性没有被这三个新增回归覆盖。
复用现有 typed binding reader 和 transport pattern 比新增一套迁移规则更合理;不过复用前必须确认 reader 的真实解析顺序,不能把“函数接收 connection_id”误认为“只验证这一个连接”。
具体改动
整个 PR 是 3 个文件,+283/-61:两个生产模块、一个测试模块,没有新 CLI、配置 schema、生成资产或可选能力。生产部分相当一部分是原发送/readback 块增加分支后的缩进移动,而非新增业务规则;测试部分有少量无关格式调整。
关键代码讲解
goal_channel_contracts.py:723的reusable_goal_topic_root:新增纯读取判定,调用绑定 reader,检查 enabled、root 的消息 ID 格式及解析后 chat_id。返回 root 或空串,由调用者决定是否发送。实际调用在goal_topic_connections.py:562,不是仅测试使用的抽象。这里的 ValueError 降级受到第 2 项问题影响。goal_topic_connections.py:363的connect_lark_goal_topic:权限、preview 和群成员前置检查保留;580 行新增复用分支。新发送分支保留原 send failure 和 readback mismatch 返回。但后续共同 closeout 没有区分复用与真实写入/读回,导致第 1 项回执不实。- 未修改的
goal_channel_contracts.py:174_resolve_goal_binding/ 211 行binding_for_goal是关键依赖:前者校验目标引用、provider 并合成 channel/identity;后者在完整集合解析后才选择 ID。因此 helper 的边界必须缩到选中的连接,不能把目标参数应用给所有 Agent。 tests/extensions/test_lark_goal_topic_connections.py:新增 legacy payload/target fixture、升级重连不发送、不同 target 发送、新格式重连保持 root/receipt 三个测试。30 项文件级测试通过,但assert connection.get("receipts")仅证明字典非空,不能证明历史验证证据被保留;当前实际上生成了新的 verified_at。
对主干的风险
这是既有 Lark Goal Channels 连接行为的默认修复,并非新 opt-in 能力。preview 在新增逻辑之前返回,未打开未授权消息发送;原 routing enums、Agent 注册约束及能力边界保留。没有新增 actor 权限语义、字符串状态分类、机器义务或领域专用核心错误。PR body 已披露重连从 send/readback 变成直接复用;但返回 receipt 语义没有同步,不能把它当作无行为变化的重构。
验证:本地该测试文件 30 passed,完整 extensions suite 606 passed,三个变更文件 Ruff 和 diff hygiene 通过;两个独立 synthetic probe 2 failed,分别验证跨 Agent target 隔离和 readback 真实性。测试使用实际绑定读写和生产连接函数,provider 用可控 runner,不访问真实群或账号。精确 head 的远端执行检查成功,不能替代缺失的负向证据。未运行 live Lark 验证。
最低修复保持同一主题:限定选中连接的解析范围、区分历史复用和本次 provider 证据,补针对性测试。不需要引入大规模迁移、索引或新的连接框架。现有旧记录无需批量改写;不可读 root 的恢复路径需显式选择,而不是静默确认连接健康。
我的整体评价
方向值得保留,规模总体与升级兼容性问题相称;维护成本主要来自一个 49 行 helper 和三项测试,发送块移动可接受。已完成邻近简化审视:应修正 reader 的单连接解析边界并复用它,不另建重复 target 规则;receipt 应复用原证据而不是仅复制其存在形状。当前两个缺口使“稳定复用且真实报告结果”尚未成立,因此暂缓。修复后按新 exact head 复审,本次不合并。
English verdict: REQUEST_CHANGES at ed73486fbaef4c421e6c8595fa06eacc74ea4734. P1: cached-root reuse skips provider readback but manufactures a fresh verified receipt and reports a write/readback. P2: resolving every sibling connection against one selected target invalidates an otherwise reusable root. The 30 topic-connection tests, 606 extension tests, Ruff and diff hygiene pass; two independent synthetic production-path/contract probes fail. Preserve the upgrade fix, narrow target resolution, and make reuse evidence truthful before merging. No live-provider validation performed.
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
… baseline Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
结论:原 P1/P2 已修复,完整复审未发现阻断项。精确 head:b15ed0e35ba9e5c4985e2eb682500cc8420aceb1。维护者已明确授权直接修复此 fork 分支并自合并。
保留原作者的升级兼容修复:Goal Channels 引入 connection_id 后,旧绑定重新连接不应因 provider 幂等键变化而重复发公告、替换已有话题根。本次补齐两个边界:已缓存的 root 不是当前 provider 验证证据;另一个 Agent 的目标不应使本连接的 root 失效。
改动思路
复用现有 binding reader、单连接 target resolver 和 transport readback,而不是新增绑定格式或验证器。reusable_goal_topic_root 先选 connection_id,再只对选中连接解析 target。连接函数不重复发送已建立的话题,但发送和复用都进入同一个 readback 分支;复用时验证 Goal 标记及实际 chat_id。通过后才刷新验证回执,且 external_write_performed=false;验证失败时不替换原绑定、不自动重发。
正向:HTTP 保存连接 → 原权限/群成员检查 → 精确连接的旧 root → 真实读取消息并核对 Goal/群 → 保留 root、写当前验证回执、返回 connected,零公告发送。负向:消息不存在、群不符或 Goal 不符 → readback_mismatch → blocked,旧绑定及其 receipts 不变。不同 Agent 位于其他 target 时,当前连接仍能正常复用,sibling 不被修改。
具体改动
关键代码讲解
goal_channel_contracts.py:保留原 PR 的reusable_goal_topic_root,复用binding_for_goal选择当前连接,再用_resolve_goal_binding验证它的 target;不再把一个 target 应用到整个连接集合。现有 legacy v0 到连接集合的读取契约保持。goal_channel_transport.py:message_readback_verified增加可选 expected_chat_id,复用已有字段匹配逻辑;未传该参数的已有调用者保持原行为。它仍负责实际 provider 读取而非仅判断本地格式。goal_topic_connections.py:connect_lark_goal_topic将 readback 收敛到发送/复用共享的一处。复用不要求话题标题等于当前 objective,因此正常修改标题不会误触发新话题;但要求 Goal 标记及群一致。成功和注册失败分支的 external_write_performed 与是否发送一致。权限、preview、路由枚举和 provider 发送幂等键不变。tests/extensions/test_lark_goal_topic_connections.py:保留原升级、新格式重连和 target mismatch 回归;新增缺失消息、错误群、错误 Goal、跨 Agent target 隔离的用例,明确验证零发送、真实读回、失败后绑定不变及 sibling 不变。旧 fixture 补齐 provider 返回的 chat_id 和旧 root 文本,不用凭空的 receipts 充当验证。examples/lark-goal-topic-connection-smoke.py:修复原先直接索引旧单绑定结构和未指定 connection_id 的断开请求,改用已有 reader/HTTP 合同;加入 HTTP 重连的 readback=true、external_write=false 断言。仍是一个真实本地 HTTP/持久化边界 smoke,没有新增大型测试框架或私有素材。examples/capability-extension-registry-smoke.py:一行合并验证前置修复,补入已经交付的reliability-diagnostics。依据其既有 catalog entry、README 和合入历史核对,不改 capability 实现、发现或激活策略;原硬编码清单遗漏该项使标准 premerge 失败,已修复并独立重跑通过。
对主干的风险
默认行为明确变化:正常重连从“重发”改成“复用并验证”;失效旧根从“假报 connected”改成明确 blocked,不自动替换。旧标题仍可用,缺失或错误归属的消息需用户恢复话题后重试。没有增加权限、actor 生命周期、配置 schema、可选能力或核心义务;现有 Agent 注册/execute/群成员检查都在复用之前。可选 expected_chat_id 默认 None,既有 readback 调用保持兼容,完整扩展测试验证相关消费者。
验证:完整 extensions suite 610 passed;聚焦话题测试与两个独立故障探针共 36 passed;真实本地 HTTP topic connection smoke 和 human-gate delivery smoke 通过;Ruff、编译及 diff hygiene 通过。HTTP smoke 起初暴露旧迁移断言和断开 selector 错误,已按现有接口修复并重跑通过。所有 provider 响应使用合成 runner,未访问真实 Lark 群,不能声称 live provider qualification。标准 premerge 和精确质量凭据结果见本次合并验证记录。
我的整体评价
最终合并门:标准 premerge 10/10 checks passed,另含 diff 与编译检查;0 failures、0 manual holds。精确质量凭据 cqr_1f95548753e3332479c9 验证 valid,6 文件,0 blocker/warning/advisory。此前 registry smoke 失败已修复并通过完整重跑,未绕过门禁。
Approve。核心五文件范围仍围绕一个连接重试契约:复用稳定 root,同时不伪造验证事实且隔离其他连接;第六文件仅补一条已有能力的测试期望以修复合并门。相关重构采用一处共享 readback,未新建摘要、迁移字段、验证器或 generic 框架;HTTP smoke 的小修复直接保障受影响的真实入口。质量门通过且无 manual hold 后可依据维护者授权自合并。推送后的远端完整 CI 尚在运行,不将旧 head 的绿灯当作最终 head 已通过,也未把本地 synthetic transport 当作 live Lark 证据。
English verdict: APPROVE at b15ed0e35ba9e5c4985e2eb682500cc8420aceb1. Both prior findings are resolved: target resolution is isolated to the selected connection, and reused roots undergo real readback with Goal/chat matching before fresh verification receipts; reuse reports no external write. Missing or mismatched roots fail without replacing the binding. 610 extension tests, 36 focused/probe tests, HTTP and human-gate smokes, Ruff, compile and diff checks passed. A one-line stale registry-smoke baseline was repaired against the already-shipped capability catalog and rerun successfully. Full remote CI is pending; no live Lark qualification claimed.
- 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>
Summary
Fixes an upgrade regression introduced by the multi-agent Goal Channels work (#3969, merged as
8a2c89c8):connect_lark_goal_topicstarted including the newconnection_idin the topic-root idempotency key, so for any Goal Channel binding written by a pre-#3969 release, the first reconnect after upgrading (saving any configuration edit) computed a key the provider had never seen. The provider no longer deduplicated the send, and the group received a duplicate "LoopX Goal Topic" announcement while the storedroot_message_idwas overwritten — leaving the old topic silently disconnected with no routing to it.The fix resolves the existing connection through the typed binding reader (
binding_for_goalwith the current provider target) before sending:_resolve_goal_bindingcontract), and its topic root is a well-formed message id, reconnect adopts that root and skips both the send and the readback;connectcalls.Issue Or Task
No open issue; the regression was found by code review of #3969 while auditing the multi-agent Goal Channels merge (
8a2c89c8) and is reproducible onmainwith a legacy v0 binding fixture (duplicate+messages-sendobserved, stored root overwritten). Happy to file a tracking issue if preferred.Validation
tests/extensions/test_lark_goal_topic_connections.py(30 passed, was 27):test_reconnect_after_upgrade_reuses_legacy_topic_root_without_resend— legacy v0 binding reconnects with zero+messages-sendcalls and the legacy root (om_legacy_root) preserved in the migrated connection settest_reconnect_with_mismatched_target_ref_sends_new_topic— a binding pointing at a different target still sends a new topic (fail-safe)test_set_connection_reconnect_reuses_root_without_resend— same-schema reconnect does not send a second message and keeps the first rootpytest tests/extensions/ -q→ 606 passed (full extensions suite)reusable_root = ""turns both reuse regressions red; reverting restores 30 passedruff checkclean on both changed files;git diff --checkcleanType of Change
LoopX Area
Technical Direction
Boundary Checklist
.loopx/, credentials, private traces, raw sessions, or local machine paths committedSigned-off-bytrailer (git commit -s)