diff --git a/backend/__tests__/unit/services/attentionItemService.test.js b/backend/__tests__/unit/services/attentionItemService.test.js index 50dd51fc80..778a647af5 100644 --- a/backend/__tests__/unit/services/attentionItemService.test.js +++ b/backend/__tests__/unit/services/attentionItemService.test.js @@ -49,7 +49,7 @@ describe('attentionItemService', () => { }); it('materializes a mention only for mentioned human members other than the author', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }, { userId: 'bot' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam', 'bot'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -64,7 +64,7 @@ describe('attentionItemService', () => { }); it('names the author from the User row when the message carries only user_id (PG rows)', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -86,7 +86,7 @@ describe('attentionItemService', () => { }); it('does not re-materialize a legacy-acknowledged mention during backfill', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -101,7 +101,7 @@ describe('attentionItemService', () => { }); it('does not resurrect a resolved attention row when its source write is retried', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -134,7 +134,7 @@ describe('attentionItemService', () => { // writers (author username, runtime agentName, nothing on decisions), so the // Your Team ring keys on the principal's id instead. Each writer is pinned. it('a mention carries its author id as actorUserId', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -151,7 +151,7 @@ describe('attentionItemService', () => { }); it('a decision carries the agent user id as actorUserId (it has no actorName at all)', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: [] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: ['sam'] })); mockUserFind.mockReturnValue(chain([{ _id: 'sam', username: 'Sam', isBot: false }])); mockUpdateOne.mockResolvedValue({ matchedCount: 0, upsertedCount: 1 }); @@ -163,7 +163,7 @@ describe('attentionItemService', () => { }); it('an approval carries the requester id as actorUserId when the source has one', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: [] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: ['sam'] })); mockUserFind.mockReturnValue(chain([{ _id: 'sam', username: 'Sam', isBot: false }])); mockUpdateOne.mockResolvedValue({ matchedCount: 0, upsertedCount: 1 }); @@ -178,7 +178,7 @@ describe('attentionItemService', () => { { _id: 'attention-1', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-1', kind: 'decision', source: { type: 'decision_request', id: 'd-1' }, title: 'Which?', actorUserId: { toString: () => 'kai-user' }, createdAt: new Date() }, { _id: 'attention-2', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-1', kind: 'mention', source: { type: 'message', id: '41' }, title: 'Mention', createdAt: new Date() }, ] }) }); - mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: [] }])); + mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: ['507f191e810c19729de860ea'] }])); const queue = await AttentionItemService.getOpenQueue('507f191e810c19729de860ea'); expect(queue.items.find((item) => item.id === 'd-1').actorUserId).toBe('kai-user'); @@ -191,7 +191,7 @@ describe('attentionItemService', () => { { _id: 'attention-9', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-1', kind: 'approval', source: { type: 'approval_action', id: 'appr-1' }, title: 'Scout requests approval', createdAt: new Date() }, { _id: 'attention-8', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-1', kind: 'approval', source: { type: 'approval', id: 'act-1' }, title: 'Access', createdAt: new Date() }, ] }) }); - mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: [] }])); + mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: ['507f191e810c19729de860ea'] }])); const queue = await AttentionItemService.getOpenQueue('507f191e810c19729de860ea'); expect(queue.items.find((item) => item.id === 'appr-1')).toMatchObject({ kind: 'approval', sourceType: 'approval_action' }); @@ -204,7 +204,7 @@ describe('attentionItemService', () => { { _id: 'attention-2', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-2', kind: 'approval', source: { type: 'approval', id: 'a-1' }, title: 'Old access', createdAt: new Date() }, ] }) }); mockPodFind.mockReturnValue(chain([ - { _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: [] }, + { _id: 'pod-1', name: 'Current', createdBy: '507f191e810c19729de860ea', members: ['507f191e810c19729de860ea'] }, { _id: 'pod-2', name: 'Removed', createdBy: 'someone-else', members: [] }, ])); @@ -217,6 +217,19 @@ describe('attentionItemService', () => { ); }); + it('serves a creator who has left the pod none of its rows', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [ + { _id: 'attention-1', recipientUserId: '507f191e810c19729de860ea', podId: 'pod-1', kind: 'mention', source: { type: 'message', id: '41' }, title: 'A private ask', createdAt: new Date() }, + ] }) }); + mockPodFind.mockReturnValue(chain([ + { _id: 'pod-1', name: 'Left behind', createdBy: '507f191e810c19729de860ea', members: ['someone-else'] }, + ])); + + const queue = await AttentionItemService.getOpenQueue('507f191e810c19729de860ea'); + + expect(queue.items).toEqual([]); + }); + it('hides a mention when the same message already has a decision row — in any state, not only open', async () => { const recipient = '507f191e810c19729de860ea'; const mention = { _id: 'attention-m', recipientUserId: recipient, podId: 'pod-1', kind: 'mention', source: { type: 'message', id: 'msg-1' }, messageId: 'msg-1', title: 'Ada mentioned you', createdAt: new Date('2026-09-01T00:00:00Z') }; @@ -226,7 +239,7 @@ describe('attentionItemService', () => { mockFind.mockImplementation((query) => (query.kind === 'decision' ? chain([{ _id: 'attention-d', podId: 'pod-1', messageId: 'msg-1', status: decisionState }]) : { sort: () => ({ lean: async () => open }) })); - mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: recipient, members: [] }])); + mockPodFind.mockReturnValue(chain([{ _id: 'pod-1', name: 'Current', createdBy: recipient, members: [recipient] }])); const both = await AttentionItemService.getOpenQueue(recipient); expect(both.items).toEqual([expect.objectContaining({ attentionItemId: 'attention-d', kind: 'decision' })]); @@ -264,8 +277,8 @@ describe('attentionItemService', () => { ]; mockFind.mockReturnValue({ sort: () => ({ lean: async () => rows }) }); mockPodFind.mockReturnValue(chain([ - { _id: 'pod-1', name: 'Decisions', createdBy: recipient, members: [] }, - { _id: 'pod-2', name: 'Mentions', createdBy: recipient, members: [] }, + { _id: 'pod-1', name: 'Decisions', createdBy: recipient, members: [recipient] }, + { _id: 'pod-2', name: 'Mentions', createdBy: recipient, members: [recipient] }, ])); const queue = await AttentionItemService.getOpenQueue(recipient, { limit: 12 }); @@ -374,7 +387,7 @@ describe('attentionItemService', () => { }); it('materializes an agent action proposal as an approval ask for its owner only, keyed as approval_action (#1650)', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -399,7 +412,7 @@ describe('attentionItemService', () => { }); it('an action proposal whose owner has left the pod projects nothing — no dead row for nobody (#1652)', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'sam', members: ['sam'] })); mockUserFind.mockReturnValue(chain([{ _id: 'sam', username: 'Sam', isBot: false }])); await AttentionItemService.recordActionApproval({ _id: 'appr-2', podId: 'pod-1', ownerUserId: 'gone', agentName: 'scout', summary: 'May I?' }, 'Scout'); @@ -408,7 +421,7 @@ describe('attentionItemService', () => { }); it('materializes a blocked board row once for each current human recipient', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['owner', 'sam'] })); mockUserFind.mockReturnValue(chain([ { _id: 'owner', username: 'owner', isBot: false }, { _id: 'sam', username: 'Sam', isBot: false }, @@ -427,6 +440,29 @@ describe('attentionItemService', () => { ); }); + // createdBy keeps a departed creator's id after leavePod. Membership is + // Pod.members alone (isListedPodMember), so the creator who left gets no + // row from the pod they can no longer open. + it('materializes no row for a creator who has left the pod', async () => { + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: ['sam'] })); + mockUserFind.mockReturnValue(chain([ + { _id: 'owner', username: 'owner', isBot: false }, + { _id: 'sam', username: 'Sam', isBot: false }, + ])); + + await AttentionItemService.recordTaskAttention({ + _id: 'task-1', podId: 'pod-1', taskId: 'TASK-1', status: 'blocked', + title: 'Choose a deploy shape', updates: [{ _id: 'update-1', text: 'Blocked on an upstream choice.' }], + }, { includeBlocked: true }); + + expect(mockUpdateOne).toHaveBeenCalledTimes(1); + expect(mockUpdateOne).toHaveBeenCalledWith( + expect.objectContaining({ recipientUserId: 'sam' }), + expect.any(Object), + { upsert: true }, + ); + }); + it('acknowledges only recipient-owned mentions and handoffs, never decisions or approvals', async () => { // The recipient is a real ObjectId, not a placeholder: `acknowledgeAttention` // refuses a caller id it cannot cast BEFORE it reaches the update, and the @@ -516,7 +552,7 @@ describe('an agent answering the person who asked', () => { beforeEach(() => { jest.clearAllMocks(); mockUpdateOne.mockResolvedValue({ modifiedCount: 1 }); - mockPodFindById.mockReturnValue(chain({ _id: POD, name: 'My Workspace', createdBy: 'lee', members: members.map((m) => ({ userId: m._id })) })); + mockPodFindById.mockReturnValue(chain({ _id: POD, name: 'My Workspace', createdBy: 'lee', members: members.map((m) => m._id) })); mockUserFind.mockReturnValue(chain(members)); mockUserFindById.mockReturnValue(chain(scoutBot)); mockLiveClaims.mockResolvedValue(['900']); diff --git a/backend/services/attentionItemService.ts b/backend/services/attentionItemService.ts index b0efcdbcee..8f1c9f4e21 100644 --- a/backend/services/attentionItemService.ts +++ b/backend/services/attentionItemService.ts @@ -16,6 +16,8 @@ const Task = require('../models/Task'); const { ESCALATION_MARKERS } = require('./connectorRelayPolicy'); // eslint-disable-next-line global-require const { humansAnsweredBy } = require('./agentAnswerService'); +// eslint-disable-next-line global-require +const { isListedPodMember } = require('../utils/isPodMember'); type SourceType = 'message' | 'approval' | 'approval_action' | 'decision_request' | 'task'; type Kind = 'mention' | 'approval' | 'decision' | 'handoff'; @@ -26,22 +28,16 @@ type TaskAttentionOptions = { includeBlocked?: boolean }; const compact = (value: unknown, max = 220): string => String(value || '').replace(/\s+/g, ' ').trim().slice(0, max); const sourceKey = (type: SourceType, id: unknown): string => String(id || '').trim(); -const isCurrentMember = (pod: any, userId: unknown): boolean => ( - String(pod?.createdBy || '') === String(userId) - || (pod?.members || []).some((member: any) => String(member?.userId || member?._id || member) === String(userId)) -); - +// Membership is Pod.members and nothing else (isListedPodMember, TASK-161/170): +// createdBy keeps a departed creator's id after leavePod, so counting it gave +// that person "waiting on you" rows from a pod they can no longer open. const currentHumanMembers = async (podId: unknown): Promise> => { - const pod = await Pod.findById(podId).select('_id name createdBy members').lean(); + const pod = await Pod.findById(podId).select('_id name members').lean(); if (!pod) return []; - const ids = new Set([String(pod.createdBy || '')]); - for (const member of (pod.members || [])) { - const id = (member as any)?.userId || (member as any)?._id || member; - if (id) ids.add(String(id)); - } - const users = await User.find({ _id: { $in: [...ids].filter(Boolean) }, isBot: { $ne: true } }) + const ids = (pod.members || []).map((member: any) => String(member?._id || member)).filter(Boolean); + const users = await User.find({ _id: { $in: ids }, isBot: { $ne: true } }) .select('_id username isBot').lean(); - return users.filter((user: any) => isCurrentMember(pod, user._id)); + return users.filter((user: any) => isListedPodMember(pod, user._id)); }; const recordForRecipients = async ( @@ -581,8 +577,8 @@ export const getOpenQueue = async (recipientUserId: unknown, options: OpenQueueO && row.messageId && superseded.has(`${String(row.podId)}:${String(row.messageId)}`))) : rows; const podIds = [...new Set(visible.map((row: any) => String(row.podId)))]; - const pods = await Pod.find({ _id: { $in: podIds } }).select('_id name createdBy members').lean(); - const allowed = new Map(pods.filter((pod: any) => isCurrentMember(pod, recipientUserId)).map((pod: any) => [String(pod._id), pod])); + const pods = await Pod.find({ _id: { $in: podIds } }).select('_id name members').lean(); + const allowed = new Map(pods.filter((pod: any) => isListedPodMember(pod, recipientUserId)).map((pod: any) => [String(pod._id), pod])); const priority: Record = { approval: 0, decision: 1, handoff: 1, mention: 2 }; const renderKind = (row: any): Kind => ( row.kind === 'decision' && row.source?.type === 'task' ? 'handoff' : row.kind