Skip to content

fix: p2p 无话题消息并行执行 - #135

Merged
lishuceo merged 3 commits into
mainfrom
feat/claude-session-e44348
Mar 10, 2026
Merged

fix: p2p 无话题消息并行执行#135
lishuceo merged 3 commits into
mainfrom
feat/claude-session-e44348

Conversation

@lishuceo

Copy link
Copy Markdown
Owner

Summary

  • Bug: 用户在 p2p 私聊中连续发送多条消息,后续消息需等前一条执行完才开始处理(串行等待 5+ 分钟)
  • Root cause: dev agent (replyMode=thread) 的 p2p 消息在无 threadId 时共享同一队列键 dev:${chatId},导致同一 chat 内所有消息被串行化。实际上每条无 threadId 的消息会由 resolveThreadContext 创建独立话题,不需要串行等待
  • Fix: thread 模式下无 threadId 的消息用 messageId 区分队列键,使每条消息可独立并行执行。direct 模式(Chat Agent)不受影响

Test plan

  • 新增 7 个队列键构建策略测试,覆盖 thread/direct 模式
  • 全量测试通过 (991/992, 唯一失败为 pre-existing memory/quality embedding 测试)
  • 手动测试:连续发送 3 条 p2p 消息,验证各自立即创建话题并并行处理

🤖 Generated with Claude Code

lishuceo and others added 2 commits March 11, 2026 03:25
根因:dev agent (replyMode=thread) 的 p2p 消息在无 threadId 时
共享同一队列键 `dev:${chatId}`,导致后续消息必须等前一条完成。
实际上每条无 threadId 的消息会由 resolveThreadContext 创建独立话题,
不需要串行等待。

修复:thread 模式下无 threadId 的消息用 messageId 区分队列键,
使每条消息可独立并行执行。direct 模式不受影响(仍按 userId 串行)。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
覆盖 thread/direct 模式下队列键的构建逻辑:
- thread 模式无 threadId → per-message 并行
- thread 模式有 threadId → per-thread 串行
- direct 模式无 threadId → per-user 串行

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 24s —— View job


Review of PR #135

  • Check for previous review comments
  • Read changed files and understand context
  • Trace logic across module boundaries
  • Post review

✅ Approved with minor suggestion

Overall assessment: Clean, well-scoped fix. The root cause analysis is correct — thread-mode p2p messages without a threadId were sharing a single dev:${chatId} queue key, causing unnecessary serialization. The fix correctly uses messageId as a per-message discriminator so each independent message gets its own queue key and can execute in parallel.

What looks good:

  • The perMessageParallel flag is clear and well-named
  • The fix correctly limits scope — only thread mode without existing threadId is affected; direct mode and threaded messages are unchanged
  • Comments explain the "why" well
  • Test coverage is thorough with 7 cases covering all mode combinations

One issue found:

Severity Issue Confidence
🟡 Warning Tests duplicate makeQueueKey logic in a local buildQueueKey instead of importing the real function via _testing.makeQueueKey — won't catch regressions if the real function changes 80

See inline comment for details and suggested fix.


Comment thread src/feishu/__tests__/event-handler.test.ts Outdated
address PR review feedback: 导入 _testing.makeQueueKey 替代重复实现,
添加 event-handler.ts 依赖链所需的 mock(pipeline/store, agent/router 等)。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 20s —— View job


Review of PR #135 (re-review after fixes)

  • Check previous review comments and resolve fixed ones
  • Read current diff and changed files
  • Trace logic and verify correctness
  • Post review

✅ Approved — 1 previous issue fixed, no new issues found

Previous issue resolved: Tests now import makeQueueKey from _testing export instead of reimplementing the logic locally. Replied and confirmed on the original thread.

Re-review of current changes:

The fix is clean and well-scoped. The core logic at event-handler.ts:658-661 correctly identifies the perMessageParallel case (thread mode, no existing threadId) and uses messageId as the queue key discriminator, allowing independent p2p messages to execute in parallel. The makeQueueKey function's 4th parameter (userId) is reused for messageId — this works because it's just a string discriminator, though the parameter name is slightly misleading. Not worth changing since it's an internal function with clear call-site comments.

Tests are thorough — 7 cases covering all mode/threadId combinations, now using the production makeQueueKey function via _testing export.

No bugs, security issues, or architectural concerns found.

@lishuceo
lishuceo merged commit 70b12ac into main Mar 10, 2026
8 checks passed
@lishuceo
lishuceo deleted the feat/claude-session-e44348 branch March 10, 2026 19:42
lishuceo added a commit that referenced this pull request Apr 8, 2026
* fix: p2p 无话题消息并行执行,避免同 chat 串行等待

根因:dev agent (replyMode=thread) 的 p2p 消息在无 threadId 时
共享同一队列键 `dev:${chatId}`,导致后续消息必须等前一条完成。
实际上每条无 threadId 的消息会由 resolveThreadContext 创建独立话题,
不需要串行等待。

修复:thread 模式下无 threadId 的消息用 messageId 区分队列键,
使每条消息可独立并行执行。direct 模式不受影响(仍按 userId 串行)。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: 队列键并行执行策略测试

覆盖 thread/direct 模式下队列键的构建逻辑:
- thread 模式无 threadId → per-message 并行
- thread 模式有 threadId → per-thread 串行
- direct 模式无 threadId → per-user 串行

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: 测试改用生产代码 makeQueueKey,添加缺失的 mock

address PR review feedback: 导入 _testing.makeQueueKey 替代重复实现,
添加 event-handler.ts 依赖链所需的 mock(pipeline/store, agent/router 等)。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant