fix(manager): dedupe Lark replies and diagnose Codex turn refusals - #4966
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Reviewed exact head: 5112c3873961a88feb2895a41a8d9d2f9936f2ac.
动机
同一 Lark Bot App 在本机有多个 profile 别名、或两个 Chat 服务同时消费事件时,可能产生重复管家回复;另一方面 Codex Turn 的结构化 HTTP 400 invalid_request_error 被笼统地呈现为 host_gate,让排障方向偏向额度或授权。目标是保持单次回复并给出不泄露上游原文的可操作诊断,同时不降低既有模型设置。
改动思路
LarkGoalTopicRuntimeService 用 Bot App 标识持有本机单消费者锁,待命别名不启动第二个事件流;活动别名扩大到同 App 的已绑定聊天,路由在同 chat 多候选时按 Topic root/manager 归属消歧,连接健康映射到活动监听者。Chat 的错误分类只读结构化状态/类型,不发布嵌套上游 message。这个方向复用现有事件处理和 ChatActionStore 决策权,没有新 UI 设置;但 App 级消费与每张卡记录的 profile 身份必须一同迁移。
具体改动
goal_topic_runtime_service.py 加入 App 范围的文件锁与 standby 状态;goal_topic_runtime.py 扩展同 App 目标匹配和歧义拒绝;goal_topic_connections.py 把待命别名显示为当前 App 的监听状态;team_plan_confirmation.py 扩大 review-card 回调流的 chat 过滤;chat_agent.py 与 manager_context.py 增加结构化拒绝类别和更中性的用户提示。相关 Lark/Chat 测试与一个 extension registry smoke 同步。
关键代码讲解
goal_topic_runtime_service.py:132的_poll_profile在事件流及其卡片回调流外持有 App 锁;只有一个别名会活跃。goal_topic_runtime.py:181的_target_for_profile_chat为活动别名寻找同 App 的已绑定目标,在相同 chat 下按 root 或唯一 manager 归属选择。team_plan_confirmation.py:105的active_profile_chat_ids让活动别名订阅同 App 其他别名的卡片回调 chat;而:673的handle_team_plan_review_callback仍要求回调处理者 profile 精确等于持久化投递 profile。chat_agent.py:103只从结构化 HTTP 400/type 推导upstream_invalid_request,不把上游 message 拼进回复。
对主干的风险
P1 阻塞:同 App 跨别名的 team.plan 卡片会变成不可确认。 假设卡片经别名 B 发出并记录 delivery.sender_profile=B,但 App 锁由别名 A 获得。新 active_profile_chat_ids 让 A 收到 B 聊天的回调;_poll_profile 却把监听别名 A 传入 handler,随后 handle_team_plan_review_callback 的 profile != delivery.sender_profile 抛 ActionConflictError("team plan callback delivery binding drifted"),在 ChatActionStore.decide_review_card 前终止。用仓库现有 proposal/card/event 测试辅助函数、真实 ChatActionStore 和同 App 不同别名调用该回调已复现此错误。原先 profile 各自消费时,B 的回调由 B 处理;新增跨别名订阅必须保留卡片投递身份而不是把活跃监听别名当作投递身份。请在仍验证 App、message、chat、操作者与卡片摘要的前提下,从已认证的投递记录解析发送 profile,补“两别名同 App、B 发卡、仅 A 监听、A 收回调、只 apply 一次并重放同一结果”的回归。
本地 Chat/Lark/连接/manager 聚焦套件 201 passed,卡片确认套件 4 passed,改动 Python 文件 Ruff 通过;现有测试没有覆盖上述交叉组合。没有在本轮访问真实 Lark 事件总线;当前 review 契约也不等待远端 CI。
我的整体评价
App 级单消费者和结构化错误诊断都解决了明确的问题,控制权仍留在原有 Chat action owner;但回调身份没有伴随消费范围扩展,是端到端用户旅程的实质回归。建议保留这个 bounded refactor,修正跨别名投递身份并跑负例后,在新 head 上重新 review;现在不能合并。
English verdict: REQUEST_CHANGES - head 5112c3873961a88feb2895a41a8d9d2f9936f2ac; the sole active alias receives another alias's review-card callback, but the unchanged delivery-profile check rejects it before one-time apply. This was reproduced with the real ChatActionStore despite 201 focused tests, 4 card tests, and Ruff passing.
One App-scoped consumer may handle several local profile aliases, so the alias that receives a review-card callback can differ from the alias that delivered the card. Resolve the sender identity from the authenticated delivery record (as the result-patch path already does) instead of requiring the listening alias to match, which rejected cross-alias team.plan cards before the one-time decision was recorded. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
…-single-reply-0924 Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
The authenticated delivery record is now the sole source of the card's sending identity, so the handler no longer needs the listening alias or its CLI binary. Removing them keeps the earlier listener-equals-sender coupling from being reintroduced. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head: c0c5250ebc024fd26850dd192bf0d5bf4bd684b4
This review addresses the single blocking finding from review 5296570996 on head 5112c3873961a88feb2895a41a8d9d2f9936f2ac.
动机
同一个 Lark Bot App 在本机可以有多个 profile 别名。改动引入 App 级单消费者后,待命别名会收到另一个别名投递的 team.plan 卡片回调;但回调处理仍然要求“处理者 profile 精确等于持久化投递 profile”,于是跨别名卡片会在写入一次性决策之前被 ActionConflictError 拒绝,端到端旅程实际回归。
改动思路
把“卡片发送身份”的唯一权威放回已认证的投递记录,与仓库既有的结果回填路径保持一致:投递记录已经保存 sender_profile/cli_bin,结果卡片回填本来就用它。因此不再把监听别名当作发送身份,而是从记录解析发送身份后继续做 App、message、chat 与卡片摘要校验。同时删掉 handler 上只代表监听别名的 profile/cli_bin 形参,让“监听别名等于发送身份”这一耦合无法被再次表达。
具体改动
loopx/extensions/lark/team_plan_confirmation.py:从投递记录解析delivery_profile/delivery_cli_bin,要求非空;绑定校验保留app_id、message、chat 与既有卡片摘要检查;卡片内容读取与租户成员校验改用投递身份。移除profile/cli_bin形参。loopx/extensions/lark/goal_topic_runtime_service.py:回调闭包不再把监听别名传入 handler,只传监听者 App 身份。tests/extensions/test_lark_team_plan_confirmation.py:新增“同 App、另一别名投递、单一 App 消费者收回调”的回归(只 apply 一次、重放同一结果、卡片水合走投递身份),以及 App/chat 漂移仍被拒绝的负例;投递 helper 支持覆盖sender_profile/cli_bin。
对主干的风险
修复已用判别性回归验证:把源码改动临时回退到被 review 的 head 时,新回归正好以 ActionConflictError: team plan callback delivery binding drifted 失败,说明该测试确实覆盖了原阻塞点。剩余边界与既有结果回填一致:跨别名回调依赖本地受信投递记录中的发送身份仍然可解析,且不新增任何权限或公开设置。App 级单消费者与错误分类诊断保持不变。
我的整体评价
App 级单消费者与结构化拒绝诊断仍然解决明确问题,控制权留在原 Chat action 决策 owner;本次修复让身份权威随消费范围一起迁移,跨别名 team.plan 卡片恢复为可一次性确认。建议保留该 bounded refactor 并合并。
Validation on this head: focused Lark/Chat/manager suite 207 passed, Ruff passed, capability-extension-registry smoke passed, loopx canary premerge --from-git-diff selected smokes and public-boundary scan passed, change-quality receipt cqr_d1d0e469352fea6a3556 valid. Remote CI was not awaited (wait_for_ci=false).
English verdict: APPROVE - head c0c5250ebc024fd26850dd192bf0d5bf4bd684b4; the cross-alias team.plan callback now resolves its sender identity from the authenticated delivery record, applies exactly once, and replays the same result, with the listener-only identity parameters removed so the original coupling cannot return.
Merge decision (author-owned PR; admin bypass)Requested change: addressed. The blocking P1 from review 5296570996 (cross-alias
Repair content: the review-card callback now resolves the card's sending identity from the authenticated delivery record ( Validation on the merged head: focused Lark/Chat/manager suite 207 passed; the two new regression/negative tests passed standalone; Ruff passed on the changed modules; Attributed or pre-existing failures: none on this slice. Remote CI was not awaited by request. Carried limits: cross-alias confirmation still depends on the locally persisted, trusted delivery record holding a resolvable sender identity; the change adds no new permission, setting, or public surface. |
Problem and result
A Lark Bot App with multiple local profile aliases, or two LoopX Chat servers on different ports, could consume the same event twice and send two manager replies. The runtime now holds one machine-scoped consumer lease per Bot App. The active consumer routes bound chats across aliases, fails closed on ambiguous ownership, and connection status resolves standby aliases to the active listener.
A separate Codex app-server failure was misreported as a generic
host_gate: an older selected CLI accepted the configured model when starting the thread, then returned a structured HTTP 400invalid_request_erroron the Turn. Chat now classifies that typed refusal without exposing upstream prose, and the Lark reply points to model, CLI, and account compatibility. The affected local service was moved to a compatible CLI; a real 6-sol xhigh Turn and the bound manager Session both answered successfully. No model downgrade is introduced by this PR.Validation
loopx canary premerge --from-git-diffpassed at the standard tier: 11 selected checks, no failures or manual holds.OKand cleared its previous error.No credentials, local Goal state, message content, or private runtime receipts are included. The web UI consumes the existing connection projection and needs no new control. The selected service executable is machine configuration outside this repository diff.