fix: auto-follow new thread when parent channel is followed

[yujiawei]

What

Web users creating a thread under an already-followed channel didn't get the new thread auto-followed — it only showed up in the Recent tab and needed a manual right-click → follow. Bot-created threads already land in Follow because the backend auto_follow_threads fanout handles them, but that fanout is bot-only and eventually consistent, so the web manual-create paths can't rely on it.

How

New best-effort helper Utils/followNewThread.ts → maybeFollowNewThread(groupNo, thread):

• After threadCreate succeeds, reads the sidebar follow snapshot (SidebarService.sync({ tab: "follow" })).
• Parent-channel follow state comes from that snapshot (target_type === 2 && target_id === groupNo), not from channelInfo.orgData.is_followed (commented as unreliable).
• Only when the parent is already followed: FollowService.followThread({ thread_channel_id: thread.channel_id }), then emit("sidebar-reload") so the new thread shows in the Follow tab immediately.
• Wrapped in try/catch — failures never block the create feedback.

Wired symmetrically into all three manual create entry points:

• Components/ThreadCreate/index.tsx
• Components/ThreadCreateModal/index.tsx
• Components/ThreadPanel/index.tsx (inline wkConfirm create)

No schema/API changes. Backend fanout untouched.

Tests

• [api] 6 new Vitest cases (2 per entry point), symmetric:
  • parent followed → followThread called exactly once with { thread_channel_id: "g1____t1" }
  • parent not followed → followThread not called, create feedback still fires
• Existing ThreadPanel.archive.test.tsx (16) still green.

A [browser] smoke check (create a thread under a followed channel → it appears in the Follow tab ≤2s, persists after refresh) is left for post-merge regression.

Fixes #292

[Jerry-Xin]

The PR is relevant to octo-web, but it misses an existing manual web thread creation path, so the fix is incomplete.

🔴 Blocking

• 🔴 Critical: packages/dmworkbase/src/module.tsx:932 registers the “create thread from message” context menu, and packages/dmworkbase/src/module.tsx:993-1025 creates the thread directly via WKApp.apiClient.post(...). This path does not call maybeFollowNewThread, so users creating a thread from a message under an already-followed channel still will not get the new thread auto-followed. Please wire the helper into this path too, and add a regression test for it.

💬 Non-blocking

• 🟡 Warning: maybeFollowNewThread is described as best-effort, but the create flows now await it before showing success or calling onSuccess/onClose (ThreadCreate/index.tsx:66, ThreadCreateModal/index.tsx:81, ThreadPanel/index.tsx:983). Failures are swallowed, but slow /sidebar/sync or /follow/thread requests can still delay create feedback. Consider fire-and-forget behavior if immediate feedback is the priority.

✅ Highlights

• The helper correctly uses the follow sidebar snapshot instead of channelInfo.orgData.is_followed.
• The new tests cover followed and unfollowed parent states for the three edited entry points.
• The helper keeps auto-follow failures isolated from the primary thread creation result.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

octo-web PR#391 Review Report

Reviewer: Octo-Q (automated review)
Head SHA: 40d0b5daa961d81204ffb74a0dd210a53087b187
PR: #391 — fix: auto-follow new thread when parent channel is followed

---

1. Verification Summary

Item	Status	Evidence
maybeFollowNewThread data-flow correctness	✅	threadCreate → toThread() always sets channel_id = buildThreadChannelId(groupNo, short_id) (datasource.ts:361-389). Thread.channel_id is required non-empty string. FollowService.followThread({ thread_channel_id }) type-matches FollowThreadReq.
SidebarService.sync return shape	✅	SidebarSyncResp.items: SidebarItem[] with target_type: number and target_id: string. SidebarTargetType.CHANNEL === 2 confirmed.
All 3 datasource.threadCreate call-sites wired	✅	ThreadCreate/index.tsx:66, ThreadCreateModal/index.tsx:81, ThreadPanel/index.tsx:983 — all call maybeFollowNewThread after create.
Error isolation (best-effort)	✅	maybeFollowNewThread wraps entire body in try/catch, logs via console.warn, never throws to caller. Outer component catch blocks remain unreachable from this helper.
Test coverage	✅	6 new Vitest cases (2 per entry point), symmetric: parent-followed → followThread called once with correct arg; parent-not-followed → followThread not called, create feedback still fires.
"sidebar-reload" event listener exists	✅	useFollowSidebar.ts:126 — single listener calls load({ silent: true }).

2. Findings

F1 — P2: 4th thread-create path in module.tsx not wired (completeness gap)

File: packages/dmworkbase/src/module.tsx:1003-1028

A 4th manual thread-creation path exists in the message context menu handler. It calls WKApp.apiClient.post(groups/${channelID}/threads, ...) directly (bypassing datasource.threadCreate), emits wk:thread-created, and navigates to the new thread — but does not call maybeFollowNewThread.

// module.tsx:1003-1028 (simplified)
const resp = await WKApp.apiClient.post(
  `groups/${message.channel.channelID}/threads`, { name, source_message_id, source_message_payload }
);
Toast.success(...);
WKApp.mittBus.emit("wk:thread-created", { ... });
// ← no maybeFollowNewThread call

Diff-scope 3 questions (R2):

1. Pre-existing — this path was not auto-following before the PR either.
2. PR diff does not touch module.tsx. The behavioral divergence between the 3 wired paths and this unwired path is new (before this PR, no path auto-followed; now 3 do and 1 doesn't).
3. Not a regression — the module.tsx path behavior is unchanged. But it creates a user-visible inconsistency: creating a thread from the message context menu under a followed channel won't auto-follow, while creating from the other 3 entry points will.

Severity rationale (R1/R4): Does not make a previously-working path unavailable or produce incorrect data. The path was never auto-followed. This is a completeness gap, not a regression → P2.

Suggestion: Add maybeFollowNewThread(message.channel.channelID, resp) after the Toast.success in module.tsx:1012. Note: resp here is the raw API response, not a Thread object — either construct a Thread from it or refactor to use datasource.threadCreate (which also handles the wk:thread-created emit and toThread mapping).

---

F2 — P2: await maybeFollowNewThread blocks success feedback on 2 network round-trips

Files: ThreadCreate/index.tsx:66, ThreadCreateModal/index.tsx:81, ThreadPanel/index.tsx:983

All three call-sites await maybeFollowNewThread(groupNo, created) before showing Toast.success / calling onSuccess / onClose / loadThreads. The helper makes up to 2 serial network calls (POST /sidebar/sync + POST /follow/thread) before resolving.

Impact: The user perceives thread creation as slower than it actually is — the thread is already created on the server, but the success toast and UI transitions are gated on the follow logic completing. Under normal latency (~100-200ms per call) this adds ~200-400ms. Under degraded network conditions, it could be more (bounded by whatever HTTP timeout the API client uses).

Diff-scope: New — introduced by this PR (previously no follow logic existed between create and feedback).

Severity rationale: Not P1 — the delay is bounded and the try/catch ensures it always resolves. UX degradation under poor network, not a functional break → P2.

Suggestion: Consider fire-and-forget: maybeFollowNewThread(groupNo, created) without await. The helper's internal try/catch already prevents unhandled rejections. The sidebar-reload emit inside the helper will still refresh the sidebar asynchronously. Trade-off: the sidebar may not immediately show the new thread in the Follow tab if the user navigates there before the follow completes, but the scheduleThreadReload staggered polling (300/1000/2000ms) in useFollowSidebar will catch it.

---

F3 — P2: Redundant sidebar sync calls (up to 5 per thread creation)

Files: followNewThread.ts:22,31, datasource.ts:286 (emits wk:thread-created), useFollowSidebar.ts:126,207

A single thread creation now triggers:

1. SidebarService.sync from maybeFollowNewThread (1 call)
2. FollowService.followThread (1 call, conditional)
3. mittBus.emit("sidebar-reload") → useFollowSidebar.load({ silent: true }) → another SidebarService.sync (1 call)
4. mittBus.emit("wk:thread-created") (from datasource.threadCreate) → scheduleThreadReload → up to 3 staggered SidebarService.sync calls at 300/1000/2000ms

Total: up to 5 /sidebar/sync calls per thread creation. The scheduleThreadReload dedup (requestedThreadReloadsRef) and early-exit on followedKeysRef.has(threadKey) mitigate this, but the immediate sidebar-reload and the first 300ms timer likely both fire redundant reloads.

Diff-scope: Amplified — wk:thread-created reloads were pre-existing; this PR adds 2 more sync calls on top.

Severity: P2 — performance waste, not a functional issue. The calls are silent (no loading spinner) and the server should handle them, but it's unnecessary load.

Suggestion: Since maybeFollowNewThread already emits sidebar-reload after following, consider skipping the explicit SidebarService.sync call inside the helper and instead reading the follow state from a lighter source (e.g., a local cache or the sidebar state already held by useFollowSidebar). Alternatively, suppress the sidebar-reload emit and let scheduleThreadReload handle the refresh.

---

F4 — Nit: "sidebar-reload" as any type-safety gap

File: followNewThread.ts:31

WKApp.mittBus.emit("sidebar-reload" as any) — "sidebar-reload" is not defined in the MittEvents type (App.tsx:3-30). All existing emitters/listeners use the same as any cast.

Diff-scope: Pre-existing pattern, PR adds one more instance.

Suggestion: Add "sidebar-reload": void to the MittEvents interface to get compile-time safety. Low priority.

---

3. Suggestions

1. Wire module.tsx (F1): Either add maybeFollowNewThread to the message-context-menu create path, or refactor it to use datasource.threadCreate so it benefits from the same wiring. This closes the behavioral inconsistency.
2. Consider fire-and-forget (F2): Drop the await on maybeFollowNewThread to avoid gating UX feedback on follow-side latency. The helper's try/catch already prevents unhandled rejections.
3. Reduce redundant syncs (F3): Coordinate with scheduleThreadReload to avoid 5 sync calls per creation.

4. Additional Findings

• Test mock fidelity: Tests directly instantiate class components and hand-feed mock data, bypassing React rendering and real service integration. This is acknowledged in test comments and acceptable for verifying wiring correctness, but won't catch issues in the real SidebarService.sync → FollowService.followThread integration path. The [browser] smoke check mentioned in the PR body is the right complement.
• threadCreate side effect: datasource.threadCreate emits wk:thread-created before returning. If any future listener for that event adds auto-follow logic, it would race with maybeFollowNewThread. Currently safe (the only listener is useFollowSidebar which only schedules reloads).

5. Data-Flow Trace

Consumed Data	Upstream Source	Reaches Consumer?
thread.channel_id	datasource.toThread() → buildThreadChannelId(groupNo, short_id) → "${groupNo}____${shortId}"	✅ Always non-empty string
resp.items (from SidebarService.sync)	POST /sidebar/sync → SidebarSyncResp.items: SidebarItem[]	✅ Array with target_type/target_id fields
SidebarTargetType.CHANNEL	Enum in SidebarService.ts → value 2	✅ Matches server-side target type for channels
WKApp.shared.deviceId	StorageService (localStorage), fallback UUID	✅ Always non-empty string after init
groupNo (param)	Passed from component props/state	✅ Matches target_id in sidebar items for the parent channel
FollowService.followThread param	{ thread_channel_id: thread.channel_id } → FollowThreadReq	✅ Type-compatible

6. Blind-Spot Checklist (R5)

• C1 — Dual-path parity: N/A. No symmetric add/remove or create/delete pairs touched. The 3 wired create paths are consistent with each other. The unwired module.tsx path is a completeness gap (F1), not a parity violation (it uses a different API path, not datasource.threadCreate).
• C2 — Control-flow ordering / nesting reuse: Clear. maybeFollowNewThread is a leaf helper with no nested reuse. The try/catch wrapping is correct and the await ordering (create → follow → feedback) is intentional.
• C3 — Authorization boundary ≠ capability boundary: N/A. No new endpoints, tools, or capabilities exposed. The helper calls existing services (SidebarService.sync, FollowService.followThread) with the same auth context as the calling component.

---

[Octo-Q] verdict: APPROVE

No P0/P1 findings. The core logic is correct: data-flow from threadCreate → maybeFollowNewThread → followThread is sound, error isolation is proper (best-effort, never blocks create feedback), and all 3 datasource.threadCreate call-sites are symmetrically wired. Tests validate the wiring at each entry point. The 4 findings are P2/nit: a missed 4th create path in module.tsx (pre-existing gap, not regression), UX latency from await blocking, redundant sidebar syncs, and a type-safety gap. None meet the P1 bar per R1 (no path becomes unavailable, no incorrect user-visible data, no regression).

[lml2468]

PR #391 审查结论 — request-changes

改动概览:7 文件 +357/-2,关联 #292。新增 best-effort 工具 Utils/followNewThread.ts 的 maybeFollowNewThread,在 threadCreate 成功后读 sidebar follow 快照,父群已关注则自动关注新子区;接入 ThreadCreate / ThreadCreateModal / ThreadPanel 三个手建入口,并新增 6 条测试(每入口 2 条)。

测试验证:testsPass=true,无新增失败=true(基线 57→57),build=true,lint=true。

问题统计:阻断问题 1 条,小问题 / nit 5 条;另有 8 条经核验不成立的误报已丢弃。

---

[major] 第 4 个建子区入口未接入新抽象,自动关注覆盖不全 — packages/dmworkbase/src/module.tsx:1011

本 PR 把自动关注逻辑抽到 Utils/followNewThread.ts 并接入了 ThreadCreate / ThreadCreateModal / ThreadPanel 三个入口。但仓库里还有第 4 个建子区入口:module.tsx 中消息右键『创建子区』(WKApp.apiClient.post(\groups/${...}/threads`)→emit('wk:thread-created')→showConversation),它没有调用 maybeFollowNewThread`。

抽象到公共 util 的目的恰恰是『所有手建入口语义一致』,漏掉一个入口意味着同一个用户操作(手建子区)在不同入口表现不一致——从右键消息建的子区不会自动关注,从面板 / 弹窗建的会。这是抽象边界不完整。

建议:在 module.tsx 该 onOk 成功分支(拿到 resp.channel_id、emit('wk:thread-created') 附近)同样 await maybeFollowNewThread(message.channel.channelID, resp)。resp 是 apiClient 原始返回,已确认含 channel_id 字段(符合 Thread 形状);若字段不全可只取 channel_id 构造最小入参。这样四个入口共用同一条自动关注链路,抽象才真正闭合。

---

其余 5 条小问题 / nit 已贴为行内评论(ThreadCreateModal:81、followNewThread.ts:15/21/25/33),核心是:自动关注被串在创建反馈的 await 关键路径上(应 fire-and-forget),以及 best-effort 契约(失败不阻塞)、resp.items 为空、target_type 判别等边界缺测试覆盖。

结论:request-changes(存在 1 条阻断问题:第 4 个建子区入口未接入,自动关注覆盖不全)。

[lml2468]

[minor] 自动关注串行 await 阻塞创建反馈,sidebar 拉取/关注慢时创建成功 toast 与弹窗关闭被延迟

三个入口都把 await maybeFollowNewThread(...) 串在 threadCreate 成功之后、创建反馈(Toast.success / onSuccess / onClose / loadThreads)之前。maybeFollowNewThread 内部要先 SidebarService.sync(一次网络全量同步)再 followThread(再一次网络),这两跳全部 await 完成后用户才看到『创建成功』和弹窗关闭。

这不是功能性 bug(maybeFollowNewThread 内部已 try/catch 吞掉异常,不会破坏成功路径),但属于一致性 / 体验层面的正确性问题:子区其实已经创建成功,反馈却要等一个 best-effort 的附带动作完成才出现;sidebar/follow 接口慢或抖动时,创建会显得『卡住』。函数注释也写明了设计意图是『失败不阻塞创建反馈』,但当前实现只做到了『失败不抛出』,没做到『不阻塞』。

建议:把自动关注做成 fire-and-forget,先给创建反馈再异步补关注,例如在调用处不 await(void maybeFollowNewThread(groupNo, created)),或在该函数内部不 await(自身 catch 已就绪)。ThreadCreate/index.tsx:64、ThreadPanel/index.tsx:983 同此问题。注意:测试当前依赖 await 顺序断言 followThread 被调,改成不 await 后需相应调整测试(可在函数内部仍同步发起、仅不阻塞外层)。

[lml2468]

[minor] auto-follow 链路把重量级 /sidebar/sync 放在创建反馈的 await 关键路径上,阻塞 toast/onSuccess

三个入口(ThreadCreate / ThreadCreateModal / ThreadPanel)都是 const created = await threadCreate(...); await maybeFollowNewThread(...) 之后才 Toast.success/onSuccess。而 maybeFollowNewThread 内部要先 await SidebarService.sync({tab:'follow'})(POST /sidebar/sync,见 SidebarService.ts:55),父群已关注时还要再串行 await FollowService.followThread。也就是说子区已经创建成功后,用户仍要多等 1 次(已关注时 2 次)网络往返才能看到『创建成功』反馈并关闭弹窗。函数注释自称『失败不抛出、不阻塞创建反馈』,但当前实现实际把它放进了反馈的 await 链,语义不符。

建议:不要 await 它。在 threadCreate 成功后立即 Toast.success/onSuccess/关闭弹窗,自动关注以 fire-and-forget 方式后台进行,例如调用处改为 void maybeFollowNewThread(groupNo, created)(函数内部已 try/catch 吞错,天然适合 best-effort 后台执行),让创建反馈不再被关注往返阻塞。

[lml2468]

[nit] 核心契约『自动关注失败不阻塞创建反馈』完全没测

followNewThread.ts 的整个设计意图(JSDoc 明写『失败不抛出、不阻塞创建反馈』)就是 best-effort——sync 或 followThread 失败时静默吞掉(line 33-34 的 catch + console.warn),且组件里 await maybeFollowNewThread(...) 是排在 Toast.success/onSuccess 之前的。一旦该函数意外抛出,成功反馈就会被跳过。但新增的 6 个测试全部只走 sync resolve + followThread resolve 的成功分支,catch 块零覆盖,关键保证(follow 失败仍要触发创建成功反馈 / onSuccess)没有任何断言守护。这是本 PR 最重要却缺失的路径。

建议:补两条用例——(a) hoisted.sidebarSync.mockRejectedValue(new Error('x')) 时,断言 threadCreate 已成功且 Toast.success/onSuccess 仍被调用、函数 resolve 不 reject;(b) 父群已关注但 hoisted.followThread.mockRejectedValue(...) 时,同样断言创建反馈照常触发。这两条直接覆盖 catch 分支与『不阻塞』契约。

[lml2468]

[nit] 新增工具 followNewThread.ts 没有独立单测,逻辑只被三个组件间接覆盖

本 PR 全部新逻辑都集中在 maybeFollowNewThread 这一个纯函数里,但没有 Utils/__tests__/followNewThread.test.ts,它只在 ThreadCreate / ThreadCreateModal / ThreadPanel 三个测试里被间接触发,且三处用的都是同一套 happy-path mock。这导致每个组件测试都重复了一遍 sidebar mock 样板,而真正该被穷尽测试的边界(见 line 25 / line 33 评论)反而都漏了。

建议:为该工具补一个独立 test 文件,直接 import maybeFollowNewThread 并集中覆盖:已关注 / 未关注 / sync 失败 / follow 失败 / items 为空 等分支;组件测试则退化为只验证『调用了 maybeFollowNewThread 且接线参数正确』,减少重复 mock。

[lml2468]

[minor] 未覆盖边界:resp.items 为 undefined 与 target_type 类型判别

line 25 的 (resp.items ?? []) 空值兜底,以及 line 26 的 it.target_type === SidebarTargetType.CHANNEL 类型判别,都没被测到。所有『父群未关注』用例的 mock item 都是 {target_type: 2, target_id: 'other'}——只在 target_id 上失配,从不在 target_type 上失配。因此一个回归(比如把 CHANNEL 误判成 THREAD,或漏判 target_type)不会被现有测试捕获;sync 返回 {}(items 缺失)也没人验证不会崩。

建议:补用例——(a) mockResolvedValue({}) 断言不抛、不调 followThread;(b) 在 items 里放一个 {target_type: THREAD(5), target_id: 'g1'}(id 相同但类型是子区)断言 parentFollowed 判 false、followThread 不被调,以锁住 target_type 判别。

[lml2468]

✅ APPROVE（校准 workflow request-changes → approve）。本 PR 范围正确：helper 用 sidebar follow 快照判父群已关注、失败 try/catch 吞掉；改的 3 入口都正确接入+6 测试、CI 全绿。workflow 标的 🔴 第 4 入口 module.tsx 经核实不在本 PR diff、pre-existing，按不拿 out-of-scope 卡新 PR 降 follow-up。0 真 blocker → APPROVE。🟡 follow-up：第 4 入口另开 PR；helper await 改 fire-and-forget；补 catch/边界测试。

[yujiawei]

Code Review — PR #391 (octo-web)

Head reviewed: 40d0b5daa961d81204ffb74a0dd210a53087b187
Scope: fix: auto-follow new thread when parent channel is followed (Fixes #292)

Verdict: Request changes

The implementation is clean and the approach is sound — a best-effort maybeFollowNewThread helper that reads the authoritative tab=follow sidebar snapshot and only follows when the parent channel is already followed. The parent-followed detection (target_type === CHANNEL && target_id === groupNo) is consistent with the existing convention in Hooks/useFollowSidebar.ts and Components/ChatConversationList/index.tsx, error-swallowing is correct, and the 6 new unit tests pass.

However, the fix is incomplete, which blocks merge given the PR explicitly claims Fixes #292 and covers "all three thread-create entry points."

P1 — A fourth thread-create entry point is not covered, leaving the #292 symptom live

packages/dmworkbase/src/module.tsx:933 registers the contextmenus.createThread message context menu ("create thread from a group message"). It is a live, user-facing path gated only by remoteConfig.threadOn + ChannelTypeGroup + non-system-message:

// module.tsx:1003
const resp = await WKApp.apiClient.post(
  `groups/${message.channel.channelID}/threads`,
  { name: threadName.trim(), source_message_id: parseInt(message.messageID), source_message_payload: sourcePayload }
);
Toast.success(t("base.module.createThread.success"));
if (resp && resp.channel_id) {
  WKApp.mittBus.emit("wk:thread-created", { ... });   // reload only — never calls followThread
  ...
}

This path creates the thread via a direct apiClient.post('groups/.../threads'), not via channelDataSource.threadCreate, so a grep for threadCreate does not surface it — there are 4 create paths, not 3. It does not call maybeFollowNewThread. A user right-clicking a message under a followed channel and choosing "create thread" still does not get the new thread auto-followed — exactly the bug #292 sets out to fix.

The wk:thread-created emit here only schedules a sidebar reload (via useFollowSidebar's backoff), never a follow write, so it does not compensate — it merely surfaces threads the backend already auto-followed, and the backend fanout is bot-only + eventually-consistent (as the helper's own docstring notes).

Required: either invoke maybeFollowNewThread(message.channel.channelID, <thread from resp>) on this path too (the response exposes channel_id / short_id), or narrow the PR description / open a tracked follow-up. Cross-file sibling-shape blind spot — a threadCreate grep cannot see a raw apiClient.post('/threads').

Non-blocking notes (P2 / nit — fix at author's discretion)

• P2 — Success feedback is serialized behind the follow round-trips. In all three wired call sites, await maybeFollowNewThread(...) runs before Toast.success / onSuccess / onClose / loadThreads (ThreadCreate/index.tsx:66, ThreadCreateModal/index.tsx:81, ThreadPanel/index.tsx:983). The helper does an unconditional await SidebarService.sync(...) and a conditional await FollowService.followThread(...) — up to two network round-trips that now gate the create acknowledgement on every thread create, even when the parent is not followed. Since the helper is best-effort and self-contains its try/catch, fire the success feedback first and run it as a non-awaited side effect (or move the await after the success calls). The codebase's own manual follow path (ThreadPanel.handleFollow) does optimistic UI first to avoid exactly this stall.

• P2 — The single emit("sidebar-reload") is the weaker of two reload mechanisms. It does one non-retrying load(); if the next /sidebar/sync hasn't yet reflected the new follow, the thread only appears thanks to the separate wk:thread-created → scheduleThreadReload [300,1000,2000]ms backoff that threadCreate already fires. Consider dropping the redundant emit or reusing the retry/backoff so the helper is robust on its own.

• nit — Test coverage of the helper's two defining behaviors is missing. No test exercises the swallow-on-error contract (no mockRejectedValue asserting onSuccess/Toast.success still fire on a rejected sync/followThread), and the emit("sidebar-reload") is never asserted. The thread_channel_id argument is well covered. Adding a rejected-sync test and an emit assertion would lock in the two behaviors a "best-effort" helper most needs guarded.

• nit — followThread is called unconditionally when the parent is followed, without checking whether the new thread is already in the just-synced snapshot. Harmless today (the thread was just created; backend auto-follow is bot-only) and relies on backend idempotency; a cheap guard would make it self-consistent.

Summary

Approach and code quality are good, but the stated fix does not cover the message-context-menu create path, so #292 is only partially closed. Please address the P1 (cover the 4th entry point or rescope) before merge; the P2/nit items are optional polish.

[lml2468]

QA Engineer Review — PR #391

Verdict: PASS-WITH-RISK (review:done:qa:comment)
Head SHA: 40d0b5daa961d81204ffb74a0dd210a53087b187
Verified against: PR diff + CI rollup (build SUCCESS, secret-scan SUCCESS, osv-scan SUCCESS, dependency-review SUCCESS).

Coverage (what the PR wires)

6 new vitest cases under __tests__/*.followOnCreate.test.tsx, symmetric across the 3 wired entry points:

• ThreadCreate — parent-followed → followThread({ thread_channel_id: "g1____t1" }) called exactly once; parent-unfollowed → not called, onSuccess/Toast.success still fire.
• ThreadCreateModal — same shape, plus asserts onSuccess("t1") carries the short_id.
• ThreadPanel (inline wkConfirm) — same shape, plus asserts loadThreads still fires in both branches.

Wiring is verified at all 3 modified call-sites. Existing ThreadPanel.archive.test.tsx (16 cases) untouched and CI green.

Risks (why not full PASS)

R1 — Bug #292 not fully resolved: 4th manual create path uncovered.
packages/dmworkbase/src/module.tsx:1003-1028 registers the message right-click "create thread" context menu. It posts to WKApp.apiClient.post(groups/${channelID}/threads, ...) directly — bypassing datasource.threadCreate and bypassing maybeFollowNewThread. A user creating a thread from a message under an already-followed channel via right-click still will not get the new thread auto-followed. The PR body claims "all three manual create entry points" — there are 4. From a QA standpoint the user-facing symptom in #292 reproduces from this path post-merge.

R2 — Best-effort contract is undertested.
Helper's docstring promises "失败不抛出、不阻塞创建反馈", but no test mocks a rejected SidebarService.sync / FollowService.followThread to assert onSuccess / Toast.success still fire. The contract is the helper's main value; it should be locked in.

R3 — emit("sidebar-reload") never asserted.
The side effect that surfaces the new thread in the Follow tab is unverified.

R4 — await blocks success feedback.
All 3 call-sites await maybeFollowNewThread(...) before Toast.success / onSuccess / onClose / loadThreads. Under normal latency this adds ~200-400ms perceived delay to every thread create (even unfollowed-parent cases pay 1 round-trip). Not a functional break — UX regression risk only.

Recommendation

Either wire module.tsx:1003-1028 (so the 4 paths are symmetric and #292 actually closes), or narrow PR scope/title and file a tracked follow-up issue before merge. R2/R3/R4 are non-blocking polish for the test suite.

[lml2468]

qa-engineer persona — PASS-WITH-RISK. See verdict comment (review:done:qa:comment): bug #292 not fully resolved because the 4th manual create path (module.tsx:1003 message right-click) bypasses the helper. Wired-path tests are solid but best-effort contract (rejected sync / rejected followThread) and sidebar-reload emit are untested.

[lml2468]

Security Engineer Review — PR #391

Verdict: CLEARED (review:done:security:approve)
Head SHA: 40d0b5daa961d81204ffb74a0dd210a53087b187
CI evidence: Secret Scan (gitleaks) SUCCESS, OSV-Scanner SUCCESS, Dependency Review SUCCESS, Build SUCCESS.

STRIDE / OWASP scan

Vector	Assessment
Spoofing / AuthN	No new auth surface. FollowService.followThread and SidebarService.sync are pre-existing authenticated endpoints; helper calls them with the caller's existing session — no privilege change.
Tampering	Server is source of truth; thread.channel_id originates from datasource.threadCreate's server response. No client-side trust elevation.
Repudiation	N/A — no audit-relevant action.
Information disclosure	console.warn("[maybeFollowNewThread] auto-follow new thread failed", err) logs server-error contents to dev console. These are response bodies from already-authorized endpoints; no token / session / PII exposure beyond what the user could already observe in DevTools network tab. Acceptable.
DoS	Adds up to 2 serial round-trips per thread create. Bounded, user-rate-limited, no amplification.
Elevation of privilege	None — only invokes endpoints the user already has access to (parent channel was already followed in the only branch where followThread fires).
Injection (XSS / SQLi / cmd)	thread.channel_id is a server-issued opaque string passed as a JSON body field; never rendered as HTML / executed / interpolated into a shell or SQL boundary.
SSRF / open redirect	No URLs constructed from user input.
CSRF	Existing service-layer auth headers; no new state-changing surface beyond what already exists.
Crypto / secret handling	No crypto, no secrets touched. WKApp.shared.deviceId is read-only access to an existing client identifier.
SBOM / supply chain	No new runtime deps; CI dependency-review and OSV scans clean.

authZ boundary check

The if (parentFollowed) gate uses sidebar snapshot (tab: "follow") as the authoritative source — this is data the server already exposes to the caller and the parent channel was already in their follow list, so this is not an authZ bypass: a non-member cannot induce a follow they couldn't already perform.

Conclusion

No findings. Helper introduces no new attack surface. Cleared for merge from a security standpoint.

[lml2468]

security-engineer persona — CLEARED. See verdict comment (review:done:security:approve): no new auth surface; existing endpoints; STRIDE/OWASP scan clean; CI secret-scan + OSV + dependency-review all SUCCESS.

[lml2468]

Code Reviewer Persona — PR #391

Verdict: REQUEST_CHANGES (review:done:code:changes)
Head SHA: 40d0b5daa961d81204ffb74a0dd210a53087b187
Files reviewed: 7 (+357/-2). Build SUCCESS on head SHA.

Blocking — B1: Incomplete coverage of the bug-fix scope

packages/dmworkbase/src/module.tsx:932-1028 registers the contextmenus.createThread message right-click handler — a 4th manual thread-create path. It posts to WKApp.apiClient.post(groups/${...}/threads, ...) directly (bypassing channelDataSource.threadCreate), emits wk:thread-created, and never invokes maybeFollowNewThread.

The helper was extracted precisely to give all manual create entry points symmetric auto-follow semantics. Shipping it without the 4th wiring:

• Leaves bug Web 端创建子区后未自动关注（父 channel 已关注时） #292 reproducible from the message right-click path post-merge.
• Creates a new user-visible inconsistency (3 of 4 paths auto-follow; 1 doesn't), which is harder to debug than the pre-PR uniform behavior.
• The PR body explicitly claims "Wired symmetrically into all three manual create entry points" — there are 4, not 3. The factual claim doesn't hold.

Fix options (pick one):

1. Add await maybeFollowNewThread(message.channel.channelID, resp) after the Toast.success in module.tsx:1012. Construct a minimal { channel_id: resp.channel_id } if resp lacks the full Thread shape.
2. Refactor module.tsx to use channelDataSource.threadCreate (which would also inherit the existing wk:thread-created emit centralization).
3. If neither is in scope: narrow the PR title/body to "three entry points", open a tracked follow-up issue, and explicitly note in the PR that Web 端创建子区后未自动关注（父 channel 已关注时） #292 closes only partially.

Non-blocking — N1: await on best-effort helper blocks success feedback

All 3 wired call-sites (ThreadCreate/index.tsx:66, ThreadCreateModal/index.tsx:81, ThreadPanel/index.tsx:983) await maybeFollowNewThread(...) before showing Toast.success / onSuccess / onClose / loadThreads. The helper's try/catch already guarantees no unhandled rejection, so fire-and-forget is safe and avoids ~200-400ms perceived latency per create. The existing manual-follow path in ThreadPanel.handleFollow uses optimistic-UI-first for exactly this reason — be consistent.

Non-blocking — N2: "sidebar-reload" as any type cast

followNewThread.ts:31 — WKApp.mittBus.emit("sidebar-reload" as any). The event isn't in the MittEvents type (App.tsx:3-30). Pre-existing pattern across the codebase; cheap to fix by adding "sidebar-reload": void to MittEvents. Low priority.

Non-blocking — N3: Redundant sidebar sync calls

A single thread creation now fires up to 5 /sidebar/sync requests: helper's explicit sync, helper's emit("sidebar-reload") → useFollowSidebar.load, plus wk:thread-created → scheduleThreadReload's 300/1000/2000ms staggered triple. requestedThreadReloadsRef dedup mitigates but doesn't eliminate the duplication. Consider dropping the explicit sidebar-reload emit and letting scheduleThreadReload handle the refresh, or reading the follow state from a cheaper local source.

Positive notes

• Helper extraction is clean — single responsibility, narrow signature, isolated error handling.
• Parent-followed detection via sidebar snapshot is the right primitive (the PR's docstring correctly notes channelInfo.orgData.is_followed is unreliable).
• 6 vitest cases cover the wiring at each call-site symmetrically.
• No schema / API changes; backend fanout untouched.

Summary

Approach and code quality are good. Blocking on B1 (scope completeness vs. the stated fix). N1/N2/N3 are polish.

[lml2468]

code-reviewer persona — REQUEST_CHANGES (review:done:code:changes). See verdict comment: B1 blocking — 4th create path module.tsx:1003 (right-click message menu) bypasses the helper, leaving #292 reproducible from that path post-merge. PR body claim of 'all three manual create entry points' is factually off (there are 4). Fix options listed. N1/N2/N3 non-blocking polish.

[lml2468]

Aggregate Verdict: CHANGES_REQUESTED

Head SHA: 40d0b5daa961d81204ffb74a0dd210a53087b187
Triggering verdict: review:done:code:changes (REQUEST_CHANGES). With any changes in the trio, aggregate is CHANGES_REQUESTED regardless of the other two.

Trio recap

Role	Verdict	Label
qa-engineer	PASS-WITH-RISK	review:done:qa:comment
security-engineer	CLEARED	review:done:security:approve
code-reviewer	REQUEST_CHANGES	review:done:code:changes

Blocking ask (shared by qa-R1 and code-B1)

packages/dmworkbase/src/module.tsx:932-1028 registers a 4th manual thread-create path (message right-click → contextmenus.createThread) that posts to WKApp.apiClient.post(\groups/${...}/threads`, ...)directly and never callsmaybeFollowNewThread`. The PR body claims "all three manual create entry points" but the bug-fix surface is 4. As-is, #292 remains reproducible from the right-click path post-merge, and the codebase ends up with 3-of-4 asymmetric auto-follow behavior.

Pick one to unblock:

1. Wire await maybeFollowNewThread(message.channel.channelID, resp) after Toast.success in module.tsx:1012 (construct minimal { channel_id: resp.channel_id } if needed).
2. Refactor module.tsx to route through channelDataSource.threadCreate.
3. Narrow PR title/body to "three of four entry points" + open a tracked follow-up + note partial close of Web 端创建子区后未自动关注（父 channel 已关注时） #292.

Non-blocking polish (not gating merge — file as follow-ups if deferred)

• N1 / qa-R4: All 3 wired call-sites await the best-effort helper before Toast.success / onSuccess. The helper's try/catch already guarantees no unhandled rejection — fire-and-forget removes ~200-400ms perceived latency per create and matches the existing optimistic-UI pattern in ThreadPanel.handleFollow.
• qa-R2: Helper's "best-effort, never blocks" contract has no test that mocks SidebarService.sync / FollowService.followThread rejection asserting onSuccess still fires.
• qa-R3: emit("sidebar-reload") side effect is never asserted.
• N2: WKApp.mittBus.emit("sidebar-reload" as any) — add "sidebar-reload": void to MittEvents (App.tsx:3-30).
• N3: A single create now fires up to 5 /sidebar/sync round-trips (helper sync + sidebar-reload → useFollowSidebar.load + wk:thread-created → scheduleThreadReload triple). Consider dropping the explicit emit and relying on scheduleThreadReload, or reading follow state from a cheaper local source.

Security

No findings. Helper adds no new attack surface — pre-existing authenticated endpoints, no privilege change, no new state-changing surface. Cleared.

Next step

Engineer to address B1 (the 4th path) — push a follow-up commit and the loop will re-tick. No human-review escalation needed: there is no risk / with-risk blocker without an accompanying changes, so this is a normal changes-requested cycle.

Labels: applying review:complete. No gh pr merge — merges are human-only.