feat: add /new and /reset session commands for IM channels#342
feat: add /new and /reset session commands for IM channels#342nap-liu wants to merge 4 commits intodataelement:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… in feishu Review concern dataelement#1 from dataelement#368: in the Feishu text flow, /new and /reset were handled before channel_user_service.resolve_channel_user() ran, so handle_channel_command() received creator_id as user_id. That made the replacement P2P session owned by the agent creator instead of the real sender — a mismatch against normal Feishu P2P sessions, which are attributed to platform_user_id. Fix: detect the command early (so we can still short-circuit before LLM work), but defer the actual archive/create until after resolve_channel_user has produced platform_user_id. Mirror the same user_id selection rule that find_or_create_channel_session uses: creator_id for group chats (placeholder owner), platform_user_id for P2P. DingTalk already resolved the user before the command check, so no change there. WeCom does not implement /new /reset, so no change there. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review concern dataelement#2 from dataelement#368: handle_channel_command() was finding the old session using only (agent_id, external_conv_id). Today the external IDs are prefixed per channel (e.g. feishu_p2p_*, dingtalk_p2p_*) so a collision is unlikely, but if the prefix scheme ever changes a /new issued on one channel could archive a session from a different channel. Add source_channel to the WHERE clause of the archive-lookup SELECT. The source_channel parameter was already on the function signature, so no caller changes are needed. Adds unit tests covering both review concerns: - source_channel is included in the lookup filter - new session is created with source_channel from the caller - replacement session's user_id comes from the caller (locks the contract the Feishu fix relies on) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pushed fixes addressing both concerns from #368. Concern 1 — sender attribution (CRITICAL): In DingTalk ( Concern 2 — source_channel scoping: In Tests:
Could you take another look? |
/new and /reset used to immediately create a replacement ChatSession with title="New Session". Since IM channels don't go through the websocket auto-title path (that only fires for web clients), the placeholder stuck forever and every /new'd session showed up as "New Session" in the sidebar regardless of what the user typed next. Defer creation to the user's next message: archive the old session's external_conv_id, then let find_or_create_channel_session build a fresh one on the next inbound message — which already titles it from the first message content (up to 40 chars). The handler's return value no longer includes a session_id (none has been created yet); the only caller (dingtalk.py) only consumes ["message"]. Tests updated: removed pre-creation assertions, added a regression that asserts db.added == [] and no session_id leaks in the response. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
channel_commands.py: parses /new and /reset, archives old session, creates fresh oneTest plan