fix(discord): guard botID/appID/session under p.mu in event callbacks

[hi-neason]

Summary

platform/discord/discord.go had the same p.mu field used for two different protection domains: p.session was written under p.mu.Lock on connect and in Stop, but p.botID / p.appID were written in the Ready callback without any lock while the MessageCreate, GuildCreate, and RegisterCommands paths read them from separate goroutines. discordgo dispatches each gateway event in its own goroutine, so the string-header reads in MessageCreate raced with the Ready write under -race. The field declarations in the struct also put botID/appID above the mu that guards session, which made the intended protection domain non-obvious.

Change

• Take p.mu.Lock in the Ready callback when assigning botID/appID.
• Snapshot botID and the connected session under RLock at the top of the MessageCreate handler and use the locals throughout (including resolveThreadReplyContext).
• Snapshot session/appID under RLock in RegisterCommands, and return an error if called before the session is connected instead of panicking on a nil pointer.
• Pass botID as a parameter through cacheBotRoleIDForGuild → resolveBotRoleIDForGuild so both helpers read it once under RLock rather than touching the field from inside the request path.
• No behavior change on the happy path; ordering, dedup, and mention-detection logic are untouched.

Type of change

• Bug fix (non-breaking change that fixes an issue)

Testing

Automated tests added in this PR

• platform/discord/identity_race_test.go
  • TestPlatform_IdentityFields_Race — drives concurrent Ready-style identity writes against cacheBotRoleIDForGuild (which calls into resolveBotRoleIDForGuild and reads p.botID). Uses a stub http.RoundTripper that fails instantly so the test never makes a real network call. Passes only when both writer and reader hold p.mu.

For bug fixes only — regression test

• Regression test name: TestPlatform_IdentityFields_Race.
• Manual verification this test catches the regression:
  • Reverted the production fix locally; go test -race -run TestPlatform_IdentityFields_Race ./platform/discord/ failed with race detected during execution of test pointing at the unlocked p.botID = r.User.ID write and the unlocked p.botID read in cacheBotRoleIDForGuild.

Critical User Journeys (CUJ) impact

• H — multi-platform / multi-project isolation (Discord identity/session state is per-Platform and shared across guilds and channels; the race was on shared identity state).
• go test ./core/ -run TestCUJ passes locally.
• No user-visible flow changes; this is a concurrency-correctness fix only.

Manual / user-visible behavior change

None under normal operation. RegisterCommands now returns a clear "discord: session not connected" error if invoked before the gateway Ready event, instead of panicking on a nil p.session — this only matters for callers that ignore the existing readyCh synchronization.

Checklist (reviewer will verify)

• go build ./... passes
• go test -race ./platform/discord/ passes
• AGENTS.md Pre-Commit Checklist items are satisfied
• No new hardcoded platform/agent names in core/
• i18n strings have all-language translations (no new user-facing strings)
• No secrets / credentials in source

Related

• Stop already took p.mu.Lock around p.session; the connect loop at discord.go:714 already took p.mu.Lock around p.session = session. This change extends that existing locking contract to botID/appID and the remaining reads rather than introducing a new lock.

[chenhg5]

结论: Approve

总体判断: 一个干净的 concurrency 修复——把 Discord 平台 struct 里 p.botID/p.appID/p.session 的访问都统一到 p.mu 锁下，与既有 Stop / connect loop 的锁约定对齐。修复面精准，race test 在 fix 前会红、fix 后绿。建议合入。

Review 范围:

• 看了 platform/discord/discord.go 中 Ready / MessageCreate / RegisterCommands / cacheBotRoleIDForGuild / resolveBotRoleIDForGuild 五个点的锁添加。
• 看了新增测试 TestPlatform_IdentityFields_Race 验证 race fix（用 stub http.RoundTripper 避免真实网络）。
• CI: run 31349761110 全绿（lint / unit-test / smoke / regression / performance）。

✅ 做得好的地方:

• 修复范围对齐既有约定：作者明确指出 Stop 已经用 p.mu.Lock 包 p.session，connect loop 也是——只是 Ready 回调里 botID/appID 漏了锁。这条 fix 不是「引入新锁」，而是「扩展既有 lock 协议到剩余字段」。最小侵入、最符合 reviewer 心智模型。
• Lock 选择合理：写用 Lock、读用 RLock，符合 sync.RWMutex 语义。MessageCreate 是 hot path，用 RLock 让 Ready 写不会阻塞其他 MessageCreate 读。
• Snapshot 模式干净：MessageCreate 顶部一次性 RLock + 读三个字段 + RUnlock，后续整段 handler 用本地 snapshot。避免锁内调用 cacheBotRoleIDForGuild 这种可能 reentrance 的代码——这是教科书做法（避免 lock-in-lock / lock-coupling）。
• botID 通过参数传递而不是直接读 struct：cacheBotRoleIDForGuild → resolveBotRoleIDForGuild 链改成「botID 作为参数传入」，让调用者负责锁，helper 内部无锁。这种「pass-the-token」模式让并发模型可推理。
• Race test 用 stub RoundTripper 避免真实网络：http.RoundTripper 直接 fail 让测试不依赖 Discord 真连接——这是 hermetic 测试的标准做法，避免 flaky CI。
• RegisterCommands 改 panic 为 error：原代码 p.session 为 nil 会 panic——这种「Ready 之前手动调 RegisterCommands」是 misuse，但返回 error 比 panic 更友好，让调用方可以 fail-gracefully。
• PR body 描述堪称模范：明确「same field used for two different protection domains」、「string-header reads」这种 race 机制解释，加 diff 行号引用，加 -race 复现命令——reviewer 不必自己爬代码。

🚨/🔴 必须处理:

• 未发现。

🟠 建议改进（不阻塞）:

• TestPlatform_IdentityFields_Race 复现 race 的复现性：作者说「revert fix locally → go test -race 失败 with race detected during execution of test」。建议在测试里加一行注释明确 race 的最小复现条件（多少 goroutine / 多少次写读），便于 future reader 理解为什么这个测试在 fix 前会红。
• p.appID 在 RegisterCommands 改 error 路径：return nil, fmt.Errorf("discord: session not connected")——但上层 caller 是否真的处理这个 error？建议在 cmd/cc-connect 启动流程里 grep 一下 RegisterCommands 调用点，确认上层确实把 error 上报 / fail。
• cacheBotRoleIDForGuild 的新参数是 botID string 还是 *Platform？ 看描述「pass botID as a parameter」应该是 string，但 helper 内部如果还需要 p.session 还得传 session。建议作者确认 helper 链上的最小信息传递——避免「string snapshot + Platform 还是同时存在」的混淆。

🔵 可选优化:

• 测试名 identity_race_test.go 暗示这是「identity 字段的 race」测试，可以补充一个 TestPlatform_RegisterCommands_PreReady_ReturnsError 把「RegisterCommands 改 error 路径」也钉死。低优先。

Testing / Risk:

• 已看到的验证: -race 模式下测试通过；CI 全绿；fix revert 后测试确实失败（作者 self-verify）。
• 未覆盖风险: 真生产环境下多个 guild 同时 register commands 时锁竞争——但 RegisterCommands 是启动期一次性调用，hot path 不会有性能问题。
• Blast radius: 仅 platform/discord/，不影响其他 platform。

Next step:

• 建议 owner 直接 merge。Concurrency fix 范围精准，race test 设计好，PR body 描述清晰。可以现在合。