fix(manager): preserve rich replies and visible list markers - #4246
Conversation
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
|
No blocking finding remains after review. The packaged asset references and the old plain-text capability documentation identified during independent review are fixed in the final branch. Product/architecture: this resolves two user-visible causes of unreadable manager answers: text-only Lark transport and reset list markers in Chat. Immediate answers and delegated conclusions reuse the existing Lark extension, authority checks, retry/readback and idempotency owner; the frontend reuses its existing Markdown renderer. No new permission, scheduler or state owner is added. A small shared outbound-format helper was the bounded refactor; broader transport redesign is unnecessary here. Main compatibility risks are message size, mentions and uncertain sends. Focused tests cover format verification, tampered previews/readback, pre-send size fallback without truncation, text/mention parity, routing and return recovery. Raw provider post JSON and the currently supported CLI rendered-content shape are covered; unsupported future CLI shapes fail readback conservatively rather than claiming delivery. Format diagnostics remain transport receipts rather than a new public lifecycle state. Validation: 265 focused Python tests; Ruff; diff/public-boundary checks; packaged browser smoke including automatic return, exact list/emphasis elements, computed list markers, and desktop/mobile screenshots; a real authorized bot post with provider preview and exact readback. Native Feishu client rendering has not been independently screenshot-verified. This is the risk-based premerge validation set; unrelated full repository suites were not rerun. CI status is recorded separately. Merge decision: eligible for self-merge under the owner's standing authorization for manager/Lark fixes after final-head checks. This PR contains no sub-agent configuration changes. |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
审阅对象:PR #4246,exact head 28c22155db316f8403b5ad4c5d265c0419f8d3b3,合并提交 f3237a34e699b95a2d7138420d10e61a5173e04e(base 50d0e29d62bb99843325bed7eefb23b298b098ba)。本记录是该 exact head 的合并后审计,只覆盖这一 head 的完整增量(15 files, +407/-30)。
动机
这个 PR 修的是两个面向 operator 的可见呈现缺陷。第一,manager 的 Lark 回复此前走纯文本投递(loopx/chat_manager.py 的经理目标文本当时明确要求"Lark 只用纯文本段落和项目符号,不要 Markdown"),于是作者写好的段落、列表和强调在到达 operator 时被压平。第二,打包后的个人工作区把 Markdown 列表标记重置掉了:样式表依赖浏览器默认 marker,而应用的全局 reset 在构建产物里把 <ul>/<ol> 的 marker 去掉,列表渲染成没有圆点、没有序号的连续文本。
这两点都不是美观问题。manager 交付的内容本质是"结论—证据—下一步"的结构化报告,压平之后 operator 拿到的是已经交付但难以扫读的证据;列表标记消失后同一份内容在聊天面板里读起来像整段文字。目标很明确:让作者写的 Markdown 在两个 surface 上原样到达 operator,同时不改变会话范围、回复位置、授权与 ACK 语义。
改动思路
改动沿用既有 owner,没有新建第二个 transport 或第二个渲染器:
- 传输判定留在 Lark 扩展自己的投递函数里(
loopx/extensions/lark/inbox_reply.py)。新增 typed 的content_format(默认text,只允许text/markdown),由 manager 的两条真实调用点显式请求markdown;普通 inbox 回复、通知与结构化 mention 完全不变。 - 编码与校验放在同层的
loopx/extensions/lark/outbound.py,与既有 text 版常量和匹配器并列:一个 30 KB 的 post 请求上限常量、一个单节点 md post 编码器、一个严格的单节点解析器,供预览与回读共用。 - 把"富文本投递"变成可观测要求:预览必须显示
msg_type=post且 md 文本一致,回读必须是 post、不带 mention、md 文本完全相等。这样纯文本的"看起来一样"不会被当成富文本成功。 - 超限回退是本地且在发送之前:序列化后的请求体超过 30 KB 时,用同一次调用以
content_format="text"重入,保留旧通道的 150 KB 容量,并回传format_fallback="post_size_limit",不截断答案。 - 前端继续使用既有的
.personal-md渲染器,只在样式表补回显式的list-style-type规则,并重新构建打包产物(loopx/web/chat/index.html指向新的 hash 资产)。
正/反向路径都在代码里显式:manager 路径(或显式请求)走 post;结构化 mention 即使请求了 markdown 也降级回身份校验的文本通道;超限回退到文本;非法 content_format 在接触 provider 之前就报错。
具体改动
运行时代码:loopx/extensions/lark/inbox_reply.py(+43/-10)新增 content_format 参数、mention 降级、preserve_format 归一化、post 参数构造、post 大小检查与发送前回退;loopx/extensions/lark/outbound.py(+66/-2)新增 LARK_POST_REQUEST_MAX_BYTES、lark_markdown_post_content、_single_markdown_post、lark_markdown_preview_matches、lark_markdown_readback_matches,并给 normalize_lark_outbound_text 增加 preserve_format;loopx/extensions/lark/goal_topic_runtime.py(+1)按 route["conversation_kind"] == "manager" 选择 markdown;loopx/extensions/lark/manager_returns.py(+1)让委派结论走 markdown;loopx/chat_manager.py(+1/-1)把经理的 Lark 格式指令从"纯文本"改为"可读的 Markdown 段落与列表"。
呈现与产物:apps/presentation/dashboard/src/features/personal-workspace/personal-workspace.css(+2)显式声明 ul.personal-md-list -> disc、ol.personal-md-list -> decimal;loopx/web/chat/assets/index-B88OPx8f.js(+129)、index-DYYo3B8I.css(+1)与 loopx/web/chat/index.html(+2/-2)是该样式改动的重新构建产物(新 hash 文件 + HTML 引用更新),不是手写逻辑。
文档:loopx/capabilities/manager_context/README.md 说明经理 Lark 回复现在保留段落、列表与强调;loopx/extensions/lark/docs/lark-event-inbox.md 新增一段契约说明,明确富 post 的形状、30 KB 回退发生在任何发送之前、普通回复/通知/结构化 mention 保持文本通道、以及会话范围/回复位置/授权/ACK 语义不变。
测试:新增 tests/extensions/test_lark_markdown_reply.py(+108)断言精确的 post JSON、纯文本回读被拒、mention 走文本通道、format_fallback == "post_size_limit";tests/extensions/test_lark_goal_topic_runtime.py(+13/-7)与 tests/extensions/test_lark_manager_returns.py(+6/-5)改为识别 post 载荷并保留原有"正文不被截断"断言(含 40000/50000 字的长答案);examples/personal-workspace-browser-smoke.mjs(+16/-1)新增列表标记与强调断言。
关键代码讲解
LARK_POST_REQUEST_MAX_BYTES(loopx/extensions/lark/outbound.py):把富 post 的 provider 请求上限(30 KB)与 text 的 150 KB 上限并列放在同一 owner 里。比较对象是 provider 预览返回的整个序列化请求体,不是文本长度,因此 multi-byte 内容不会被低估。lark_markdown_post_content:把作者 Markdown 编码为 CLI 期望的精确形状——一个zh_cnlocale、一行、一个{"tag": "md", "text": ...}节点、空 title,且不做图片抓取或改写。_single_markdown_post:严格反向解析。任何多余键、非空 title、多行/多节点、缺失text、非mdtag 都返回None。预览与回读共用它,所以"纯文本伪装成富文本"不会被计入成功。lark_markdown_preview_matches/lark_markdown_readback_matches:把富文本投递变成可观测要求——预览必须msg_type=post且 md 文本一致;回读必须是 post、无 mention、md 文本完全相等。改动前只有文本匹配器,且回读会做行归一化,结构并不是被验证的对象。_deliver_lark_inbox_outbound(content_format=...):投递决策的唯一 owner。默认text保持所有既有调用者行为;非法值在接触 provider 前抛错;mention 存在时即使请求 markdown 也走身份校验的文本通道;超过 30 KB 时在任何发送之前以 text 重入一次并回传format_fallback。
浏览器 smoke 的断言同样值得点名:它不只截图,而是读取 .personal-md ul/ol/li 的 computed list-style-type(disc/decimal)与 li display: list-item,以及 .personal-md strong 的文本,正是被全局 reset 破坏的那一层。
对主干的风险
最大的风险面是这个投递函数是共享的:普通 inbox 回复、通知与结构化 mention 都经过它,因此必须证明它们没有被富文本化。逐条证据:
- 默认不变:
content_format默认text,只有 manager 的两条真实调用点请求 markdown;loopx lark-inbox reply等调用者不传该参数。 - mention 隔离:请求 markdown 但正文含结构化 mention 时会降级回身份校验的文本通道,测试断言了这一支。
- 不截断:序列化请求体超过 30 KB 时在发送前回退到 text 通道并回传
post_size_limit,测试覆盖了 40000/50000 字的长答案并断言正文逐字保留。 - 不误判:回读必须是无 mention 的单节点 md post,纯文本回读被视为失败,而不是"看起来一样就通过"。
- 幂等键:格式被绑定进富回复的幂等键,因此同一来源的文本尝试与富文本尝试不会互相覆盖。
次生风险与已验证的边界:30 KB 是 provider 侧限制,若飞书调整需同步该常量;打包产物是生成物,样式改动必须重新构建(本例已构建,CSS 中可见 personal-md ul.personal-md-list{list-style-type:disc},JS 产物引用同一 class,index.html 指向两个新 hash)。关于 CI:这个 head 的 hosted checks 是 24 项中 21 绿、3 红(checks、pytest、merge-gate),但 pytest/merge-gate 只是转发任务(日志为 test "$CHECKS_RESULT" = success,CHECKS_RESULT: failure),唯一实质失败是浏览器 smoke 的验收项 Reload did not restore the active Goal conversation and reconnect its active Turn within 10 seconds.;该失败在父提交 50d0e29 的同一个 job(103294304890)上逐字复现,因此不是本 PR 引入,同日由 #4250、#4251 修复,最近 main 上承载 checks 的 Python Tests 工作流已恢复绿色。
尚未验证的部分如实标注:真实飞书租户对 md post 的渲染没有在本轮审计中实测(依赖 provider dry-run/回读契约、记录的 CLI 载荷与本地测试);本地浏览器 smoke 未复跑;清单中的"未运行"项不构成通过证明。
本地验证(在该合并 head 的独立 worktree 上执行):python -m pytest -q tests/extensions/test_lark_markdown_reply.py tests/extensions/test_lark_goal_topic_runtime.py tests/extensions/test_lark_manager_returns.py -> 55 passed in 3.15s。当前 origin/main 仍保留这些符号与规则,未被回退。
我的整体评价
APPROVE。这个改动修的是真实且可复现的呈现缺陷,落在正确的 owner 上:复用既有 CLI 通道、既有归一化入口、既有预览/回读校验模式与既有文本通道作为超限回退,新增的只是一个编码器、一个严格解析器和一次格式判定,没有第二套 transport、没有新增持久化状态、没有改动会话范围/回复位置/授权/ACK。默认值保持 text 使 feature-off 路径与改动前逐字一致,mention 降级与发送前回退把两个最容易出错的边界固定成了可测行为。
无阻断发现。残余风险与最强缺失验证:真实租户的 md post 渲染与浏览器 smoke 未在本地复跑(前者依赖 provider 契约,后者由 CI 承载);30 KB 常量属于 provider 契约,需要在其变化时复核;打包产物必须随样式重新构建。合并后审计记录仅作为该 exact head 的结论,不授予任何合并、回滚或再次修改的权限。
English verdict: APPROVE - #4246 at exact head 28c2215 (merged as f3237a3) fixes two operator-visible presentation defects in the existing Lark outbound owner (manager replies now travel as a single-node Markdown post with strict preview/readback verification and a pre-send text fallback above 30 KB; the packaged workspace regains explicit list markers), with a default-preserving format argument, an unchanged text path for ordinary replies/notifications/mentions, and no change to conversation scope, placement, authorization or ACK semantics. Verified locally at the merged head: 55 focused tests passed (tests/extensions/test_lark_markdown_reply.py, test_lark_goal_topic_runtime.py, test_lark_manager_returns.py); the packaged CSS/JS and index.html carry the presentation rule; the hosted checks failure at this head reproduces identically on the parent commit 50d0e29 (pre-existing fixture defect, repaired by #4250/#4251), so it is not attributed to this head. No blocking finding; residual risks are the unrerun live-tenant rendering and local browser smoke, the provider-owned 30 KB limit, and the generated bundle.
Manager answers and delegated conclusions lost readable lists for two reasons: Lark sent them as plain text and collapsed indentation; the frontend global reset removed list markers even though the Markdown renderer produced list elements. Both entry points now preserve readable lists and emphasis, including the packaged frontend.
Lark reuses the existing inbox transport with a single Markdown post, verified preview and content readback, and format-aware idempotency. Structured mentions retain the existing text path. Posts over the rich-message request limit fall back before sending to the existing text path without truncation; receipts identify that fallback. No image fetching, new permissions, configuration owner or delivery queue is introduced.
Validation: 265 focused Python tests passed across reply transport, routing, manager context and return recovery. Packaged browser smoke passed with automatic conclusion delivery, list/emphasis DOM assertions, computed marker styles, and desktop/mobile screenshots. A scoped live bot reply passed real provider preview, post delivery and full content readback. Native Feishu client appearance is not asserted from API readback alone. Private routing, messages and evidence are excluded from the PR.