Conversation
自动评审初步意见(首轮)先说结论:特性设计我认为是对的,尤其是「切到 评审基线:把 PR rebase 到最新 F1(建议合入前修)CI 三条腿红:desktop 外壳少了对应导航项
这个用例把 对照组是决定性的:同一个用例在干净的 修法要三处一起改(我在本地验证过,改完 28/28 绿):
<a href="#/doc-watches" data-dashboard-route="#/doc-watches" data-route="doc-watches">
<svg viewBox="0 0 16 16" aria-hidden="true"><path d="M3.2 2.2h6.2l3.4 3.4v8.2H3.2z"/><path d="M9.2 2.4v3.4h3.4"/><path d="M5.4 9h4M5.4 11.2h2.6"/></svg>
<span data-i18n="nav.docWatches">文档监听</span>
</a>F2(非阻断,但我觉得值得一并处理)dashboard 重复登记会静默抹掉运行态与 auto-sub 溯源
我起真实 IPC server + 真实 store 实测(对一条已有 42 次投递、
为什么我觉得这条不只是「显示字段掉了」: 另外现有用例 建议:把这些字段按 F3(非阻断,同源)从 dashboard 登记一篇已绑到飞书话题的文档,会把它改绑到虚拟
|
首审补充:F2 的定性我要自己更正一处(附实测)我在上一条评论里把 F2(重复登记抹掉运行态与 auto-sub 溯源)写成了 dashboard 新端点的问题。自查后发现这个定性不准确,实测如下。 我按 所以: 这对结论有两个方向的影响,我都说清楚: 往轻的方向:F2 不是本 PR「引入」的新缺陷,而是本 PR 新增的字段继承了既有写入方的一个既有形状。我原来那句「dashboard 重复登记会静默抹掉」措辞上暗示了这是新端点特有的,不准确。 往重的方向(我认为这一半更重要):正因为两个写入方都这样,这条就不是「边角情况」了 —— 所以我的建议不变、但理由换了:不是「修 dashboard 端点的 bug」,而是「本 PR 新增的这批字段需要一个明确的保留策略」。两个写入方都改(或者更省事:在 F3(thread 绑定被改成虚拟 F1(CI 三腿红)结论不变。 |
双审收敛 + 一处我自己建议的修法有缺陷(实测,已找到正解)复审同意了 F1 / F2 / F3 的定性,并且回答了我留的一个问题:除 下面是复审提出「改 我原建议的缺陷:会复活已经不成立的溯源关键是
在「缺省继承」下: owner 明明已经把它变成自己的监听了,界面上仍然挂着「自动创建 · 触发者 ou_stranger」。这比字段丢失更糟:丢失是「少了一条信息」,而这是显示一条已经不成立的审计结论 —— 恰好与本 PR 要解决的问题相反。 正解:两组字段要相反的策略因为它们回答的是不同问题:
于是:运行态那 5 个适合放进 store 缺省继承;溯源那 3 个必须由调用方显式传(dashboard POST 想保留就写 按这个切分实测,两个要求同时满足: 关于实现形状,复审的提醒我同意并采纳: 一处措辞更正我第一条评论里写「建议:把这些字段按 F1 / F3 结论不变。以上仍是自动评审意见,最终以维护者审阅为准。 |
1ebe7ae to
f9161c8
Compare
c425d71 to
1900b81
Compare
把飞书文档评论监听(/watch-comment)补一个 dashboard 页面 #/doc-watches:列表、 切触发范围、退订、贴链接登记,并暴露「最近一次评论为什么没被回复」的运行态,以及 「这条订阅是不是陌生人 @ 出来的」溯源。基于 PR #1366 之后的文档会话模型重做 (watch 用 doc:<token>:watch 内部 anchor、逐评论线程独立会话、WS 投递持久重试)。 store(doc-subs-store) - 新增 8 个 optional 字段:运行态 lastActivityAt/lastOutcome/lastError/ lastDispatchAt/dispatchCount,溯源 autoCreated/autoCreatedBy/autoCreatedAt; 旧记录读 undefined 是正常态。DocWatchOutcome 枚举 + asDocWatchOutcome 收窄。 - recordDocWatchActivity:读后写、绝不抛,dispatched 累加计数并推进 lastDispatchAt, 成功/正常丢弃清旧 lastError;行已删(退订/auto-sub 回滚)时不复活。 - setDocTitle:标题快照 best-effort,未变不写盘。 - putDocSubscription 新增显式 { inheritRuntime }:重新登记时只延续运行态五项 (描述文档投递历史,换绑定不该清零);默认仍整行覆盖,未决 WS 投递照旧保留。 溯源三字段刻意不继承——owner 主动 /watch-comment 接管后这条不再是 auto-sub, 盲目继承会让界面挂着已不成立的「自动创建」结论(比丢字段更糟)。 接线 - event-dispatcher:auto-sub 占位带溯源;六个丢弃出口(no-comment/trigger-missing/ self-authored/not-mentioned/empty-text/audit-rejected)各记结局;dispatch 仅在 daemon 真接纳(accepted,WS settle 之后)记 dispatched;标题 fire-and-forget 补齐。 - daemon poller:pending 重试接纳、--all 轮询接纳两处记 dispatched;读文档失败记 poll-failed(功能「配着」却静默失效,是最需要可见的故障)。 - command-handler /watch-comment:登记时 best-effort 抓标题,put 走 inheritRuntime。 - dashboard-ipc 四端点 GET/PUT/POST/DELETE /api/doc-watches:复用 daemon 单写者 (dashboard 进程不直接写盘);token 形状闸、workingDir 校验(不静默 mkdir)、 切到 all 先清游标重建基线(mention-only 游标极旧,不重置会重放全部历史)。 - F3:POST 已绑真实飞书话题/群(isDocNativeWatchSubscription 取反)时沿用原 anchor/sessionId/scope/chatId/ownerOpenId,只改可配置部分,避免「改个目录」就把 评论落点从群话题搬到独立文档会话;响应 keptBinding 让界面如实提示。 - dashboard.ts 代理四条路由(不在 PUBLIC_READ_PATHS,写操作另过 canManageHost)。 UI - 新页面 doc-watches-page + 数据层 doc-watches.ts;app.tsx NAV_ITEMS/MANAGE_ROUTES/ NAV_GROUPS、dashboard-routes、中英 i18n、style.css 的 dw- 样式;桌面外壳 renderer/index.html + app.ts 同步导航项(desktop-dashboard-webview 全等比较)。 验证 - tsc --noEmit / bun run build 均 rc=0 - 相关 27 文件 1049 passed / 1 skipped:新增 store 11 例(含运行态/溯源策略相反)、 IPC 20 例(真实 IPC server + 真实 store,含基线重置、F2 保留、F3 不改绑、legacy anchor 迁移、503/400/404)、drop-signal 5 条源码形状断言;desktop webview 29/29 - doc-comment.ts fetchDocTitle 只认 metas[0].title,拿不到返回 undefined 绝不抛 待验证:新页面渲染 / 切模式 / 退订 / 基线重置的实际效果仍需真机确认。 Co-Authored-By: Claude Code <noreply@anthropic.com>
复审两条 minor:
1) PUT 切换触发范围时,代码原本先 setCommentTriggerMode 后清游标,注释却写
「先清游标」,声称的安全性质代码并不提供。两步非原子写,若改 mode 成功、清游标
失败(ENOSPC/EIO),会留下「mode=all + 陈旧游标 + baselineReady=true」,poller
下一轮即从远古游标重放全部历史。改为**先清游标、后改 mode**(失败时 mode 仍是
mention-only 不进轮询,无重放窗口;在 mention-only 上清游标本就无害),注释如实
重写。补一条源码形状断言钉死先后(行为测试只见最终状态,钉不住顺序),实测调成
不安全顺序即红。
2) drop-signal 里 dispatched 时序 / 六出口结局的 toContain 纯文本断言能被一行
**注释**骗过(把同名字串写进注释、删掉真实语句,测试仍绿)。改为先剥掉整行/行尾
注释、只对真实代码行断言,并反向保证不存在「无条件」的 noteOutcome('dispatched')。
复现了复审的注释欺骗攻击:旧断言假绿、新断言正确转红。
Co-Authored-By: Claude Code <noreply@anthropic.com>
58d33b0 to
2806d59
Compare
背景
文档评论监听此前只能在飞书里敲
/watch-comment,且是 owner-only。实测下来有几个短板:docTitle || fileToken,而docTitle字段全仓没有任何写入方(grep可验),所以永远只显示 token 前 12 位。event-dispatcher.ts的processCommentEvent),owner 只在当时收到一条私信,之后没有任何界面能复查,订阅会静默累积。docRepoMap只能手改bots.json:全机 55 个 bot 里 0 个配了它。而
doc-subs-store.ts里setCommentTriggerMode的注释早就写着「改某文档订阅的触发范围(dashboard)」、bot-registry.ts:2064也写着「单条订阅的触发范围之后可在 dashboard 逐文档改」——函数和单测都在,只是从来没接线(该函数此前零生产调用方,只有test/doc-subs-store.test.ts调它)。这个 PR 把那条已声明但没建的轨道补完。改了什么
store(
doc-subs-store.ts,+141)新增两组字段,全部 optional(线上既有订阅记录没有这些字段,读到
undefined是正常态,UI 显示「—」):lastActivityAt/lastOutcome/lastError/lastDispatchAt/dispatchCountautoCreated/autoCreatedBy/autoCreatedAt两个写入函数:
recordDocWatchActivity():读后写、绝不抛。诊断字段写失败一律咽掉——如果「记不下日志」能让一条真实评论投递失败,这个可观测特性就成了新的故障源,比没有更糟。读后写而非接受整条 sub,是为了避免调用方几十毫秒前的旧快照覆盖别处刚改的字段(poller 刚推进的游标、dashboard 刚改的 mode)。setDocTitle():标题未变时不写盘,避免每条评论都重写文件。接线(
event-dispatcher.ts/daemon.ts)processCommentEvent的 7 个出口各记一种结局(6 个丢弃 + 1 个成功),poller 的投递成功与读取失败也记。于是「为什么这条 @ 没回复」能直接在界面上读出来,不必翻 daemon 日志。not-mentioned/self-authored归为正常丢弃而非错误:mention-only 下绝大多数事件都是它们,标红会让健康看板一片红、真故障反而被淹掉。fetchDocTitle()(drive/v1/metas/batch_query),在两处登记路径写入标题。事件热路径里 fire-and-forget——用户正等 bot 回复,不能为一个显示字段插一次同步的飞书往返。API(
dashboard-ipc-server.ts/dashboard.ts)daemon IPC 四端点(列表 / 切触发范围 / 退订 / 新增,含
resolveDocFile)+ dashboard 侧同款代理形状(照message-listeners抄)。UI
新页面
#/doc-watches(侧边栏「员工」组):列表带运行态与「自动创建」溯源标记,可切触发范围、停止监听、贴链接新增。一处容易踩的语义(本 PR 最值得复审的地方)
从
mention-only切到all时必须重置轮询基线。mention-only 从不进轮询(poller 只轮commentTriggerMode === 'all'),所以它记录里的游标可能极其陈旧——不重置就会让 poller 把该文档的全部历史评论当成「游标之后的新评论」一次性重放进会话。修法是置
pollBaselineReady = false,交给 poller 既有的建基线分支处理(if (!current.pollBaselineReady …) { setDocCommentPollCursor(latest…) }),而不是在端点里自己去取 latest 游标:后者是异步且可能失败的,取失败时会退化成「重放全部历史」。复用既有分支则失败也只是晚一轮建基线。这条单独有用例钉着(⭐切到 all 必须重置轮询基线)。授权边界
飞书侧
/watch-comment是 owner-only,dashboard 侧是canManageHost,即平台协管者也能改。这不是放宽,而是与 settings / schedules / groups 的既有口径逐字一致(那三个同样是canManageHost而非飞书 owner,见request-identity.ts对legacyAuthed与canManageHost的区分)。要收紧成「只有 owner 本人」,应在 dashboard 侧改用legacyAuthed,而不是在 IPC 里加判断。两条不变量:
doc-subs-store.ts顶注明写「写者只有 daemon 进程本身,单写者,原子写即可,无需跨进程锁」,而 dashboard 是独立进程——所有写入必须经 IPC 回到 daemon 执行。ownerOpenId记getOwnerOpenId(本 app)而非「当前 dashboard 操作者」:open_id是 app-scoped 的,且该字段会被autoCreateDocSession当作 session owner 用。影响面
动到的共用路径:
doc-subs-storeevent-dispatcher.processCommentEventdashboard-ipc-server/dashboard.ts不涉及 CLI 适配器、
PtyBackend/TmuxBackend、话题会话 / 群会话 / adopt / sandbox 的路由判定,因此跨 CLI、跨后端、跨平台无影响面。验证
npx tsc --noEmitrc=0;bun run buildrc=0test/doc-watches-ipc.test.ts(20 例):起真实 IPC server(port 0)+ 真实 store 读写,故意不 mock store——这条特性的价值就在「dashboard 改的和 daemon 读的是同一份盘上数据」,mock 掉只能证明「路由调了函数」,证不到真正要证的那件事doc-subs-store+11 例、doc-comment-drop-signal+6 例(接线覆盖性,沿用该文件既有的源码形状手法)record改 upsert / 不清lastError/dispatchCount不累加 / 远端退订失败保留本地记录 /workingDir开autoCreate/ownerOpenId清空 / 标题失败即拒绝登记 / 热路径改await/ 两出口记同一结局 / 删一个出口的记录 / 丢 auto-sub 溯源。rebase 后抽验 M1 / M2 / M11 仍全红publicReadOnly开启)与 Workbench H5 身份对新路径全 401,持活跃 token 放行——阳性对照在,证明探针不是恒拒的哑弹--project unit:22017 passed。剩余 7 个红文件已在干净origin/master的独立 worktree 上逐一复现(mojo-launcher-env-quarantine/plugin-registry-sandbox-read/session-store-sqlite-*×2 /plugin-mcp-sandbox等),与本改动无关;两次全量之间挂点还发生漂移(native-subagent-runtime-hook只在第二次红、隔离跑绿),属负载 flake 而非确定性缺陷两个开发中发现并修掉的自身失误(留档,便于复审判断)
recordDocWatchActivity是读后写、行不存在直接返回 false,两种顺序落盘结果逐字相同。已把注释改成如实表述(这条顺序当前不承重,只是防住「将来有人把它改成 upsert」的情形),没有把一条假的安全论证留在代码里。DropdownMenu包在label里,被仓库既有的dashboard-rebase-ui-regressions扫描器抓到——这是真 bug:label 隐式关联第一个可 labelable 后代,而选项是button,点一下字段标题就会静默选中第 1 个选项。已改成div,并反向验证(改回 label 立即转红)。另外
test/command-handler.test.ts改了 1 行:那个vi.mock工厂是穷举式白名单,新 import 的fetchDocTitle会解析成undefined并在调用点抛,症状是三个无关断言报Number of calls: 0,完全看不出是 mock 缺项。已补上并加注释提醒后人。已知未做
docRepoMap的 UI 入口(本次只支持逐订阅设workingDir,还不能在界面上改 bot 级的文档→仓库映射表)。待验证
本改动需真机验证(新页面渲染、切模式、退订、贴链接新增,以及基线重置的实际效果)。尚未部署到 live daemon——部署会让全部 55 个 bot 都跑本 checkout 的 build,等安排窗口。
🤖 Generated with Claude Code