feat(lark): confirm team plans across manager and Goal cards - #4814
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
这个 PR 解决的是一个真实且完整的用户路径:远程 manager 生成的一份 team.plan,应该同时在 manager 会话与目标 Goal 会话中可确认,但两张卡不能演化成两份 action authority。旧路径只有 typed proposal,没有两个 Lark 受众之间共享的原子决策与结果回读。
我认同当前 exact head 的核心边界:ChatActionStore 保存唯一 proposal/confirmation,ChatActionService.apply 仍是唯一 Todo effect owner,Lark 只负责外部投影、认证输入和结果卡回读。这个设计比“两个会话各建一份 proposal”小,也避免了第二套团队计划执行权。
不过当前实现还没有完成正常的动态绑定生命周期。新 Goal 复用已运行的 Lark profile 时,卡片能按最新 binding 发出去,callback consumer 却仍订阅启动时的旧 chat 集合,所以这条用户路径会暴露一个可点击但实际无响应的卡片。
改动思路
正向链路是:manager Turn 的 team_plan.projected 事件给出 proposal id;manager reply delivery 在文本回读后调用双卡投递;prepare_review_card_delivery 先冻结 manager 与 goal:<id> 两个受众;每张卡走现有 GoalChannelMessageDeliverySession 的发送、去重与原生回读;只有两个 delivery 都持久化后,source inbox 才 ACK。回调侧再用精确 message/chat/app/card/fingerprint、tenant membership 与原始 sender principal 校验,由 decide_review_card 在锁内记录首次决定;confirm 才调用 ChatActionService.apply,后续反向点击只读取同一个 canonical result。
复用关系也合理:operation 与 team-plan 保留各自状态机,只把 Lark callback hydration、membership、result update/patch/readback 抽到 card_callback.py;没有把 provider transport 变成第二个业务 owner。
阻塞点在 worker 配置身份:start_team_plan_review_callback_stream 把启动 snapshot 的 active_profile_chat_ids 固化为 --jq filter,但 LarkGoalTopicRuntimeService.refresh 只比较 profile 名字集合。profile 仍存在时,即使 chat 集合改变,也既不 stale、也不 missing,因此运行中的 callback subscription 不会更新。
具体改动
生产代码新增/调整 12 个文件,测试 6 个文件;goal_channel_operation.py 与 goal_topic_runtime.py 的大部分删除是 callback helper 和 worker lifecycle 的机械抽取。行为性新增主要集中在 chat_action_store.py 的 review_card 状态、team_plan_confirmation.py 的双卡 delivery/callback、TypeScript review-card result frame,以及 server 对 action service 的注入。
关键代码讲解
deliver_team_plan_review_cards:按当前 registry 解析 manager/Goal binding,先冻结完整 audience set,再逐一发送并记录 exact delivery;返回的双卡 receipt 是 manager source ACK 的前置条件。ChatActionStore.decide_review_card:要求 delivery set 与 expected audience set 完全相等,并匹配 provider/message/chat/app/card/fingerprint/principal;首次 confirm 进入applying,首次 reject 进入rejected,后续点击只返回已有 confirmation。handle_team_plan_review_callback:在 provider 认证后读取 canonical decision,用 proposal 级 dispatch lock 保证 apply/recovery 串行,再将同一个 result card patch 到所有尚未完成结果回读的 audience。LarkGoalTopicRuntimeService.refresh+start_team_plan_review_callback_stream:前者目前只以 profile name 判定 worker 是否需要重启,后者却用一次性 snapshot 构造 chat filter;这两个局部合同没有对齐。
对主干的风险
[P1] 同 profile 新增 Goal chat 后,新卡 callback 会被旧 filter 丢弃
触发方式:profile shared 已为 chat A 启动 worker;运行中连接 chat B,并仍使用 shared;随后 manager 向 B 对应 Goal 投递团队计划卡并点击。
代码路径:refresh() 的 stale/missing 仅比较 profile 集合(goal_topic_runtime_service.py:275-313),因此保留原 worker;该 worker 在启动时把 snapshot 传给 start_team_plan_review_callback_stream(goal_topic_runtime.py:427-438),后者将当时的 chat ids 固化进 --jq(team_plan_confirmation.py:144-160)。普通 message 路由和 delivery 会读取更新后的 snapshot/binding,但 card.action.trigger 仍只接收 A。
我用源码入口复现:第一次启动记录 chats_at_start=[oc_one];加入第二个 binding 后再次 refresh(),current_chats=[oc_one,oc_two],但 worker_starts 仍只有一次且只有 oc_one。现有 100 个 focused tests 全绿,是因为“复用同 profile worker”的测试没有改变 chat 集合。
最小修复:把每个 profile 的有效 callback chat/config fingerprint 纳入 worker reconciliation,变化时 stop 并重建 consumer;或者每 profile 不做静态 chat filter、收到 callback 后再按 fresh snapshot/bound delivery 严格授权。请补 add/remove 两个方向的回归:chat B 加入后旧 consumer 必须退出、新 callback argv 包含 A+B;chat 移除后也不能继续接受旧绑定。
其他验证:exact head 502f21c76721aa3925a6158dacef431288b19f7a 上,五组 focused Python suites 共 100 passed in 9.66s;affected TypeScript tests 8 passed;control-plane typecheck、所有 changed Python Ruff、git diff --check 均通过。按 capability 的 wait_for_ci=false,本次没有抓取或轮询 GitHub CI。未做真实 Lark 写入;作者声明的 post-install click-through 仍是修复 subscription lifecycle 后的剩余验收。
语义与 CI 对齐
typed proposal/first-wins/apply authority 语义与现有 action owner 对齐,公共协议名也没有扩大 actor authority。当前不对齐的是 runtime configuration semantics:binding 是动态可刷新事实,而 callback chat membership 被当成仅在 profile 出现/消失时才变化的静态事实。应由 typed subscription fingerprint 或 fresh authorization 明确拥有这个状态,而不是依赖 30 分钟 consumer 周期或进程重启偶然修复。
我的整体评价
核心设计、effect authority 和大部分恢复语义是对的,代码量虽大但主题总体凝聚,operation callback 抽取也有实际复用价值;未来向的小范围重构已经应用在正确边界,不需要再扩展框架。
但动态 binding 是这个 runtime 的普通生产路径,不是极端边角。新卡“发送成功、可见、可点击”却没有 callback consumer 覆盖,会直接破坏 PR 的主承诺,因此当前 exact head 不能批准。修复 profile subscription reconciliation 并补上述 add/remove 回归后,我会按完整 PR 重新验证。
English verdict: REQUEST_CHANGES - exact head 502f21c leaves newly added same-profile Goal chats outside the running callback subscription.
502f21c to
fe1cf38
Compare
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>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
fe1cf38 to
b1cb0ee
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
精确审查提交:b1cb0eeae6b429b90c828f7e8ed5a20fe9a84257
动机
这项改动解决的是一个完整且可观察的问题:Lark 管理会话可以生成 canonical team.plan preview,但此前管理会话与目标 Goal 会话没有共享、可恢复、只消费一次的确认入口。该 PR 让两张卡都绑定同一个 proposal id 与 state fingerprint,任何一张卡上的首个合法决定都由现有 ChatActionStore/ChatActionService.apply 结算,另一张卡只观察同一结果。它不引入第二套 Todo 写入权限,也不扩大 Goal 或操作者范围。
我将该 slice 判断为 goal_achieved:双 audience 投递、回调鉴权、首次决定原子性、进程重启恢复以及 source ACK fencing 都已进入同一条生产路径。非目标是证明某个真实飞书租户的安装验收;这一点仍是部署后的 owner acceptance,而不是本地证据。
改动思路
入口仍是 ChatHTTPServer -> LarkGoalTopicRuntimeService -> goal_topic_runtime。管理 turn 返回 canonical proposal ids 后,manager delivery 先持久化 ids,再解析最新 manager/Goal binding,冻结两个 audience,分别经现有 GoalChannelMessageDeliverySession 发送并做 native readback。只有双卡回执完整后才允许 ACK 源事件。
回调路径先核对 schema、proposal/state/audience、message/chat/app/profile、卡片可见内容、租户成员身份和授权 principal,再由 ChatActionStore.decide_review_card 原子消费首个决定;confirm 继续调用既有 ChatActionService.apply,reject 只改变同一 proposal。共享的 Card 2.0 readback/update/membership 逻辑从 operation card 中抽到 card_callback.py,没有复制 transport authority。
两处自修复也沿用现有 owner:worker fingerprint 现在包含 active callback chat set,binding 增删会替换旧 consumer;双卡部分写入失败时,重试读取 ChatActionStore 的已验证 per-audience checkpoint,只补缺失卡片,不增加新的 retry ledger。
具体改动
action_review_plan.ts定义 provider-neutral review frame;presentation/team_plan.py只负责 Lark Card 2.0 文案与按钮投影。chat_action_store.py冻结 audience/principal、记录 exact delivery、消费首个决定,并持久化 result readback;ChatActionService.apply仍是唯一效果 owner。team_plan_confirmation.py负责双 audience 解析、投递、回调校验、dispatch 和两端结果收敛;manager_reply_delivery.py负责 proposal delivery receipt 与重启恢复。card_callback.py承接 Goal operation 与 team plan 共用的 verified callback transport;goal_channel_operation.py保留 operation-specific lifecycle。goal_topic_runtime_service.py承接 profile worker 生命周期;goal_topic_runtime.py保留编排兼容入口,chat_lark_api.py在 binding commit 后触发 refresh。- 6 个测试文件覆盖 frame、store、双卡正反路径、replay、动态 chat 增删、callback disconnect、部分投递恢复和 manager source ACK。
关键代码讲解
compileReviewCardFrame只接受已 admission 的非 applying steward plan,以 proposal/state 作为 identity,并把 confirmation、pending、result 做成 typed union;不合格输入返回undefined。ChatActionStore.prepare_review_card_delivery在第一次外部写之前冻结完整 audience 与 principal;后续 replay 若 audience、fingerprint 或 principal 漂移会 fail closed。deliver_team_plan_review_cards每次从 active registry 解析两端 binding,发送并 readback;已完成 audience 必须与当前 binding/card 精确一致,随后作为 durable checkpoint 复用。handle_team_plan_review_callback在 apply 前完成全链身份校验,并在 proposal dispatch lock 下保证首个决定只触发一次 canonical effect;clicked card 用 callback token 更新,另一张卡用 message patch 收敛。LarkGoalTopicRuntimeService.refresh用(cli_bin, bot_app_id, sorted callback chats)作为 worker identity,配置变化时 stop 旧 worker、start 新 worker,并避免旧线程退出覆盖 replacement health。
对主干的风险
最高风险是外部 effect 与本地 receipt 跨进程不同步:新绑定的卡片可能可见但 callback consumer 仍使用旧 filter;或者 manager 卡成功、Goal 卡失败后重试重复第一张 actionable card。这两条都已通过故障注入修复:动态 binding 用例验证旧 worker 被停止且新 filter 完整;部分投递用例验证调用序列为 manager、Goal 失败、仅 Goal 重试,最终两个 audience receipt 完整。
另一个安全边界是 callback authority。实现要求 exact action schema、proposal/state/audience、stored message/chat/app/profile、card content、tenant membership 与授权 principal 全部一致;audience 未全部 delivered 时不能 decide,重复或相反的第二次点击只能读到首个 durable decision。
本地证据均绑定该 head:
- 受影响 Python 矩阵:
102 passed; - 既有 Goal operation + Goal Topic 同命令基线/当前对照:base
78 passed,head83 passed; - 全量 TypeScript control-plane:
2197 passed, 18 skipped, 0 failed; - TypeScript typecheck、strict mypy(22 files)、全部变更 Python 的 Ruff、
git diff --check均通过; - 18 个变更文件的 public/private boundary scan 干净;
- premerge canary
10/10,0 failure、0 warning、0 manual hold; - exact-scope quality receipt:
cqr_e7587ac6500f7eb87f66。
按 wait_for_ci=false 没有获取、轮询或等待远端 CI。仓库级 Python 全量测试在共享主机运行超过 13 分钟仍无终态后停止,因此明确记为未完成;真实授权 Lark workspace 的双卡点击也未执行。它们是 residual risk,但不否定已覆盖的 exact readback、生产入口、权限拒绝、重启和部分失败恢复。
我的整体评价
未发现 blocking finding。完整 diff 虽然有 +3885/-734,但其中包含两处较大的 owner extraction 和 6 个测试文件;实际机制没有增加第二个 action store、CLI、scheduler 或权限层。更小的 manager-only 卡、纯文本链接或复用 operation.execute 都无法同时满足双 audience、proposal identity、exact callback authority 和 restart recovery,因此我认为规模与原问题相称。
语义上这是对现有 action-review vocabulary 的有界扩展:typed TypeScript 仍拥有 review projection,Python Lark extension 适配 provider,Chat action store/service 仍拥有 state/effect。future-facing refactor 已应用在共享 callback transport 与 worker lifecycle;未发现需要在本 PR 中再扩大的相邻重构。
结论:exact head 可批准,等待独立维护者合并。仓库规则明确禁止作者自行合并此类 loopx/** runtime、持久化 receipt 与 callback authority 变更;canary 的本地 self_merge_allowed 不能覆盖该规则。
English verdict: APPROVE - exact head b1cb0ee has no blocking finding; 102 focused tests, 2197/2215 control-plane tests, static checks, boundary scan, exact-scope quality receipt, and 10/10 premerge canaries passed. Independent maintainer merge is required by repository policy.
Merge-readiness recheck — exact head
|
Outcome
Make one canonical
team.planproposal reviewable from both the manager Lark conversation and the target Goal conversation without creating a second action authority.ChatActionService.apply;The existing Chat proposal UI and action owner are unchanged. Lark is an additional presentation and input surface over the same proposal, so no dashboard companion change is required.
Structure
GoalChannelMessageDeliverySessionand the canonicalChatActionStoreproposal record.goal_topic_runtime.pybelow its inherited maintainability ceiling instead of raising the budget.Self-repair
The first exact-head review found that adding or removing a Goal chat under an already-running Lark profile changed delivery bindings without changing the callback consumer's startup filter. That could produce a visible, clickable card whose callback was never consumed.
The repaired worker identity now includes its static CLI/app configuration and the sorted set of active callback chats. A committed binding change therefore stops the old profile worker and starts a consumer from the fresh snapshot. Add/remove regressions prove that the old worker is stopped, the replacement subscription contains the complete current chat set, and a removed chat is no longer retained. The previous fresh-per-event authorization path remains unchanged.
The final exact-head review also fault-injected a partial dual-audience write. Previously, a successful manager card followed by a failed Goal card caused the retry to revisit the manager transport before the second card, risking a duplicate actionable message and a receipt conflict. Delivery now validates and reuses each already-verified audience checkpoint from
ChatActionStore, then writes only the missing audience. The regression proves the retry sends only the missing Goal card and completes the two-audience receipt.Validation
102 passed: focused Lark Goal Topic lifecycle, team-plan confirmation, manager delivery, Chat action, and steward preview tests on exact headb1cb0eeae6b429b90c828f7e8ed5a20fe9a84257.2197 passed, 18 skipped, 0 failed.npm run typecheck:control-planepassed.git diff --check origin/main...HEADpassed.loopx canary premerge --from-git-diff --goal-id loopx-meta: 10/10 selected canaries passed, with no failures, warnings, skips, or manual holds.cqr_e7587ac6500f7eb87f66matches exact scope fingerprinte7587ac6500f7eb87f66fa560bd0a4d6afc4e48db45fa6f682d83c358531d777.Per the Goal's
wait_for_ci=falsepolicy, remote CI was not fetched, polled, or awaited. No private state, credentials, local paths, or generated evidence are included.Remaining acceptance
The provider transport is covered with exact readback, retry, and dynamic-subscription regressions, but a real manager/Goal dual-card click-through requires this branch to be installed against an authorized Lark workspace. Keep that as post-install owner acceptance; it is not simulated as live evidence here.
Merge decision
This changes runtime, persisted proposal receipts, and callback authority. It requires an independent maintainer merge under repository policy even after exact-head self-review and local validation; local canary success does not authorize author self-merge.