diff --git a/backend/__tests__/service/mention-nonmember-notice.test.js b/backend/__tests__/service/mention-nonmember-notice.test.js new file mode 100644 index 0000000000..509bd3e26b --- /dev/null +++ b/backend/__tests__/service/mention-nonmember-notice.test.js @@ -0,0 +1,51 @@ +const mongoose = require('mongoose'); +const { setupMongoDb, closeMongoDb } = require('../utils/testUtils'); +const Pod = require('../../models/Pod'); +const { AgentInstallation } = require('../../models/AgentRegistry'); +const { findNonMemberMentionNotices } = require('../../services/mentionNonMemberNoticeService'); + +describe('non-member mention destinations with real Mongo', () => { + let sender; + let current; + let shared; + let hidden; + + beforeAll(setupMongoDb); + afterAll(closeMongoDb); + beforeEach(async () => { + await Promise.all([Pod.deleteMany({}), AgentInstallation.deleteMany({})]); + [sender, current, shared, hidden] = Array.from({ length: 4 }, () => new mongoose.Types.ObjectId()); + await Pod.collection.insertMany([ + { _id: current, name: 'Current', members: [sender] }, + { _id: shared, name: 'Review room', members: [sender] }, + { _id: hidden, name: 'Private strategy', members: [new mongoose.Types.ObjectId()] }, + ]); + await AgentInstallation.collection.insertMany([shared, hidden].map((podId) => ({ + podId, agentName: 'rhea', instanceId: 'default', displayName: 'Rhea', status: 'active', + }))); + }); + + test('returns only shared destinations and hides private names and ids', async () => { + const notices = await findNonMemberMentionNotices(String(current), '@rhea review this', String(sender)); + expect(notices).toEqual([{ + code: 'MENTION_NOT_IN_POD', handle: 'rhea', + message: "rhea isn't in this pod. Ask in: Review room.", + pods: [{ id: String(shared), name: 'Review room' }], + }]); + expect(JSON.stringify(notices)).not.toContain(String(hidden)); + expect(JSON.stringify(notices)).not.toContain('Private strategy'); + }); + + test('keeps a generic notice for a sender without a shared destination', async () => { + const notices = await findNonMemberMentionNotices(String(current), '@rhea', String(new mongoose.Types.ObjectId())); + expect(notices[0]).not.toHaveProperty('pods'); + expect(notices[0].message).toContain("rhea isn't in this pod"); + }); + + test('does not warn when the agent is installed in the current pod', async () => { + await AgentInstallation.collection.insertOne({ + podId: current, agentName: 'rhea', instanceId: 'default', displayName: 'Rhea', status: 'active', + }); + expect(await findNonMemberMentionNotices(String(current), '@rhea', String(sender))).toEqual([]); + }); +}); diff --git a/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js b/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js index 281d9b9a8c..3d322909ff 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js @@ -127,13 +127,14 @@ describe('agentsRuntime post message installation config forwarding', () => { const installation = { podId: 'pod-1', agentName: 'openclaw', instanceId: 'tarik', config: {} }; const req = { params: { podId: 'pod-1' }, body: { content: 'Please review @rhea' }, + agentUser: { _id: 'bot-1' }, agentInstallation: installation, agentInstallations: [installation], }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await handler(req, res); - expect(findNonMemberMentionNotices).toHaveBeenCalledWith('pod-1', 'Please review @rhea'); + expect(findNonMemberMentionNotices).toHaveBeenCalledWith('pod-1', 'Please review @rhea', 'bot-1'); expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ mentionNotices: [{ code: 'MENTION_NOT_IN_POD', handle: 'rhea', message: "rhea isn't in this pod. Invite them, or ask in a pod they're in." }], notice: "rhea isn't in this pod. Invite them, or ask in a pod they're in.", diff --git a/backend/__tests__/unit/services/mentionNonMemberNoticeService.test.js b/backend/__tests__/unit/services/mentionNonMemberNoticeService.test.js index ee8e95faaf..170840f939 100644 --- a/backend/__tests__/unit/services/mentionNonMemberNoticeService.test.js +++ b/backend/__tests__/unit/services/mentionNonMemberNoticeService.test.js @@ -1,4 +1,4 @@ -jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); +jest.mock('../../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); jest.mock('../../../models/User', () => ({ find: jest.fn() })); jest.mock('../../../models/AgentProfile', () => ({ find: jest.fn() })); jest.mock('../../../models/AgentRegistry', () => ({ @@ -17,6 +17,7 @@ describe('non-member mention feedback', () => { beforeEach(() => { jest.clearAllMocks(); Pod.findById.mockReturnValue(rows({ members: ['member-user'] })); + Pod.find.mockReturnValue({ select: () => ({ sort: () => ({ lean: async () => [] }) }) }); User.find.mockReturnValue(rows([ { _id: 'member-user', username: 'sam' }, { _id: 'other-user', username: 'Lily' }, @@ -51,6 +52,39 @@ describe('non-member mention feedback', () => { expect(Pod.findById).not.toHaveBeenCalled(); }); + it('suggests only pods shared with the sender and never includes private pod metadata', async () => { + AgentInstallation.find.mockReturnValue(rows([ + { podId: 'shared-pod', agentName: 'rhea', instanceId: 'rhea' }, + { podId: 'private-pod', agentName: 'rhea', instanceId: 'rhea' }, + ])); + Pod.find.mockReturnValue({ select: () => ({ sort: () => ({ lean: async () => [ + { _id: 'shared-pod', name: 'Reviews' }, + ] }) }) }); + const notices = await findNonMemberMentionNotices('pod-1', '@rhea please review', 'member-user'); + expect(Pod.find).toHaveBeenCalledWith({ + _id: { $in: ['shared-pod', 'private-pod'] }, members: 'member-user', + }); + expect(notices).toEqual([{ + code: 'MENTION_NOT_IN_POD', handle: 'rhea', + message: "rhea isn't in this pod. Ask in: Reviews.", + pods: [{ id: 'shared-pod', name: 'Reviews' }], + }]); + expect(JSON.stringify(notices)).not.toContain('private-pod'); + }); + + it('does not reveal pod locations without an authenticated sender', async () => { + const notices = await findNonMemberMentionNotices('pod-1', '@rhea'); + expect(notices[0]).not.toHaveProperty('pods'); + expect(Pod.find).not.toHaveBeenCalled(); + }); + + it('warns for a bot left in members without an active installation in this pod', async () => { + Pod.findById.mockReturnValue(rows({ members: ['rhea-user'] })); + User.find.mockReturnValue(rows([{ _id: 'rhea-user', username: 'rhea', isBot: true }])); + const notices = await findNonMemberMentionNotices('pod-1', '@rhea'); + expect(notices[0]).toMatchObject({ code: 'MENTION_NOT_IN_POD', handle: 'rhea' }); + }); + it('recognizes an in-pod legacy alias and profile display slug as delivered', async () => { AgentInstallation.find.mockReturnValue(rows([ { podId: 'pod-1', agentName: 'scout', instanceId: 'scout', displayName: '' }, diff --git a/backend/__tests__/unit/services/messageAgentDeliveryService.test.js b/backend/__tests__/unit/services/messageAgentDeliveryService.test.js index 53453256e5..639b4beeeb 100644 --- a/backend/__tests__/unit/services/messageAgentDeliveryService.test.js +++ b/backend/__tests__/unit/services/messageAgentDeliveryService.test.js @@ -80,6 +80,7 @@ describe('messageAgentDeliveryService', () => { }); expect(response.mentionNotices).toEqual([notice]); + expect(findNonMemberMentionNotices).toHaveBeenCalledWith('p1', '@rhea please review', 'u1'); expect(message).not.toHaveProperty('mentionNotices'); }); diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index 830b29fbc1..61fb4b1e55 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -2237,6 +2237,7 @@ router.post('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (r try { mentionNotices = await findNonMemberMentionNotices( podId, String((result.message as { content?: string }).content || ''), + req.agentUser?._id ? String(req.agentUser._id) : undefined, ); } catch (noticeError) { // The post already landed. A failed advisory lookup cannot make the diff --git a/backend/services/mentionNonMemberNoticeService.ts b/backend/services/mentionNonMemberNoticeService.ts index a3e6eac15d..692561e277 100644 --- a/backend/services/mentionNonMemberNoticeService.ts +++ b/backend/services/mentionNonMemberNoticeService.ts @@ -1,5 +1,6 @@ // A sender can address only pod members. When a real handle names someone -// outside this pod, return sender-only feedback without exposing where they are. +// outside this pod, return sender-only feedback. Suggested pods must also have +// the sender as a member; another agent's private memberships stay private. // eslint-disable-next-line global-require const Pod = require('../models/Pod'); // eslint-disable-next-line global-require @@ -15,6 +16,7 @@ export interface MentionNotice { code: 'MENTION_NOT_IN_POD'; handle: string; message: string; + pods?: Array<{ id: string; name: string }>; } const mentionedHandles = (content: string): string[] => { @@ -32,6 +34,7 @@ const exactHandle = (handle: string): RegExp => new RegExp(`^${handle}$`, 'i'); export const findNonMemberMentionNotices = async ( podId: string, content: string, + senderUserId?: string, ): Promise => { const handles = mentionedHandles(content); if (handles.length === 0) return []; @@ -42,7 +45,7 @@ export const findNonMemberMentionNotices = async ( // lowercases them, so use the same anchored case-insensitive match as // human mention routing. User.find({ $or: handles.map((handle) => ({ username: exactHandle(handle) })) }) - .select('_id username').lean(), + .select('_id username isBot').lean(), // Display-name slugs and instance handles are routable in the pod, so // inspect the same active installation records rather than only names. AgentInstallation.find({ status: 'active' }) @@ -54,10 +57,13 @@ export const findNonMemberMentionNotices = async ( const members = new Set((pod.members || []).map((id: unknown) => String(id))); const known = new Set(); const inPod = new Set(); + const podsByHandle = new Map>(); for (const user of users || []) { const handle = String(user.username || '').toLowerCase(); known.add(handle); - if (members.has(String(user._id))) inPod.add(handle); + // A leftover bot membership cannot receive a wake without an active + // installation. Human membership alone is sufficient for addressing. + if (!user.isBot && members.has(String(user._id))) inPod.add(handle); } const installationsByPod = new Map>>(); for (const installation of installations || []) { @@ -73,14 +79,31 @@ export const findNonMemberMentionNotices = async ( const { map } = buildMentionMap(podInstallations, podProfiles); for (const handle of map.keys()) { known.add(handle); + if (!podsByHandle.has(handle)) podsByHandle.set(handle, new Set()); + podsByHandle.get(handle)!.add(installationPodId); if (installationPodId === String(podId)) inPod.add(handle); } } - return handles.filter((handle) => known.has(handle) && !inPod.has(handle)) - .map((handle) => ({ + const missing = handles.filter((handle) => known.has(handle) && !inPod.has(handle)); + const candidatePodIds = [...new Set(missing.flatMap((handle) => [...(podsByHandle.get(handle) || [])]))]; + const visiblePods = senderUserId && candidatePodIds.length + ? await Pod.find({ _id: { $in: candidatePodIds }, members: senderUserId }) + .select('_id name').sort({ name: 1, _id: 1 }).lean() + : []; + return missing.map((handle) => { + const podIds = podsByHandle.get(handle); + const pods = visiblePods.filter((item: { _id: unknown }) => podIds?.has(String(item._id))) + .slice(0, 5).map((item: { _id: unknown; name?: string }) => ({ + id: String(item._id), name: String(item.name || 'Pod'), + })); + return { code: 'MENTION_NOT_IN_POD' as const, handle, - message: `${handle} isn't in this pod. Invite them, or ask in a pod they're in.`, - })); + message: pods.length + ? `${handle} isn't in this pod. Ask in: ${pods.map((item: { name: string }) => item.name).join(', ')}.` + : `${handle} isn't in this pod. Invite them, or ask in a pod they're in.`, + ...(pods.length ? { pods } : {}), + }; + }); }; diff --git a/backend/services/messageAgentDeliveryService.ts b/backend/services/messageAgentDeliveryService.ts index 687e4b19eb..dcab8ccde8 100644 --- a/backend/services/messageAgentDeliveryService.ts +++ b/backend/services/messageAgentDeliveryService.ts @@ -39,9 +39,9 @@ interface AgentDelivery { type DeliveredMessage = T | (T & { mentionNotices?: unknown[]; agentDelivery?: AgentDelivery }); -const senderNotices = async (podId: string, message: { content?: string }): Promise => { +const senderNotices = async (podId: string, message: { content?: string }, userId?: string): Promise => { try { - return await findNonMemberMentionNotices(podId, message.content || ''); + return await findNonMemberMentionNotices(podId, message.content || '', userId); } catch (error) { // The message is already stored. Advisory feedback cannot turn a send // into an apparent failure and tempt the caller to post it again. @@ -80,7 +80,7 @@ export const deliverMessageToAgents = async ( onlyExplicitMentions = false, }: DeliveryOptions): Promise> => { const username = authorUsername(message, requestUser); - const mentionNotices = await senderNotices(podId, message); + const mentionNotices = await senderNotices(podId, message, userId ? String(userId) : undefined); if (AgentMentionService.isAutoRoutedDmPod(podType)) { if (suppressImplicitReply) return mentionNotices.length ? { ...message, mentionNotices } : message; diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 799d695987..552be1f348 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -493,6 +493,7 @@ "replyingTo": "Replying to {{author}}:", "deliveryHint": "No agent was notified — @mention one to get a reply. Try @{{handle}}.", "mentionNotInPod": "{{handle}} isn't in this pod. Invite them, or ask in a pod they're in.", + "mentionNotInPodWithPods": "{{handle}} isn't in this pod. Ask in:", "noAgentsInPod": { "kicker": "no agent in this pod", "body": "Nobody here answers yet — people will see this when they open the pod.", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 49aeeea657..a390d68913 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -493,6 +493,7 @@ "replyingTo": "回复 {{author}}:", "deliveryHint": "尚未通知任何智能体。@提及一个智能体即可让它回复,例如 @{{handle}}。", "mentionNotInPod": "{{handle}} 不在这个 Pod 中。请邀请对方加入,或到对方已加入的 Pod 中提问。", + "mentionNotInPodWithPods": "{{handle}} 不在这个 Pod 中。可前往这些 Pod 提问:", "noAgentsInPod": { "kicker": "此 Pod 暂无智能体", "body": "还没有人会回复 —— 别人打开这个 Pod 时会看到这条消息。", diff --git a/frontend/src/v2/__tests__/V2MessageRow.test.tsx b/frontend/src/v2/__tests__/V2MessageRow.test.tsx index 9e456b64cb..b8e4f6afbf 100644 --- a/frontend/src/v2/__tests__/V2MessageRow.test.tsx +++ b/frontend/src/v2/__tests__/V2MessageRow.test.tsx @@ -47,6 +47,17 @@ describe('V2MessageRow', () => { expect(screen.getByRole('status')).not.toHaveTextContent('English MCP text'); }); + it('links the sender to a visible pod where the mentioned agent is installed', () => { + render(); + expect(screen.getByRole('status')).toHaveTextContent("rhea isn't in this pod. Ask in:"); + expect(screen.getByRole('link', { name: 'Reviews' })).toHaveAttribute('href', '/v2/pods/chat/reviews-pod'); + }); + it('keeps the runtime beside the agent name before its timestamp', () => { render( diff --git a/frontend/src/v2/components/V2MessageRow.tsx b/frontend/src/v2/components/V2MessageRow.tsx index aa7381632f..c6caaba2f4 100644 --- a/frontend/src/v2/components/V2MessageRow.tsx +++ b/frontend/src/v2/components/V2MessageRow.tsx @@ -1,6 +1,6 @@ import React, { useEffect, useRef, useState } from 'react'; import V2Lightbox, { type V2LightboxImage } from './V2Lightbox'; -import { useNavigate } from 'react-router-dom'; +import { Link, useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import ReactMarkdown from 'react-markdown'; import V2Avatar from './V2Avatar'; @@ -746,7 +746,12 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d )} {message.mentionNotices?.map((notice) => (
- {t('podChat.mentionNotInPod', { handle: notice.handle })} + {notice.pods?.length ? <> + {t('podChat.mentionNotInPodWithPods', { handle: notice.handle })}{' '} + {notice.pods.map((pod, index) => + {index > 0 ? ', ' : ''}{pod.name} + )} + : t('podChat.mentionNotInPod', { handle: notice.handle })}
))} {galleryImages.length > 0 && ( diff --git a/frontend/src/v2/hooks/useV2PodDetail.ts b/frontend/src/v2/hooks/useV2PodDetail.ts index 30aaa67d1b..3342066812 100644 --- a/frontend/src/v2/hooks/useV2PodDetail.ts +++ b/frontend/src/v2/hooks/useV2PodDetail.ts @@ -63,7 +63,10 @@ export interface V2Message { }; // Sender-only feedback from the POST response. Socket broadcasts and // history omit it, so other pod members never see the notice. - mentionNotices?: Array<{ code: 'MENTION_NOT_IN_POD'; handle: string; message: string }>; + mentionNotices?: Array<{ + code: 'MENTION_NOT_IN_POD'; handle: string; message: string; + pods?: Array<{ id: string; name: string }>; + }>; // Sprint B5: per-message reactions, aggregated server-side. // Each entry is `{emoji, count, mine, users?}` — `users` is the // Google-Chat-style attribution list decorated by diff --git a/frontend/src/v2/v2.css b/frontend/src/v2/v2.css index a656e93d76..0864a7228c 100644 --- a/frontend/src/v2/v2.css +++ b/frontend/src/v2/v2.css @@ -2236,6 +2236,11 @@ body.modern-ui.v2-canvas { margin-top: 6px; } +.v2-root .v2-msg__sender-notice a { + color: var(--v2-accent); + text-decoration: underline; +} + /* Agent typing indicator — sits between message list and composer. Collapses to nothing when typingAgents is empty. Subtle so it doesn't compete with the messages above; the animated dots carry the "alive" cue. */