feat(comments): notify collaborators when adding a comment

package.json

@@ -65,7 +65,7 @@
         "prepare": "husky"
     },
     "dependencies": {
-        "@doist/todoist-sdk": "14.0.0",
+        "@doist/todoist-sdk": "14.0.1",
         "@modelcontextprotocol/ext-apps": "1.2.2",
         "date-fns": "4.1.0",
         "dompurify": "3.3.3",

src/mcp-server.ts

@@ -42,6 +42,11 @@ What each tool does and how to fill its parameters is in the tool's own descript
 - To find out whether a task hides subtasks, use **fetch-object** with includeChildren rather than a speculative **find-tasks** call.
 - Filter tasks by label **name**. Label IDs are only for **delete-object** and **update-labels**. Shared labels can be renamed but not recoloured, reordered or favourited.
 
+**Comments**
+
+- Comments notify only the people **add-comments** is handed. When a comment mentions someone, name them in \`notifyUsers\` — writing "@Ana" in the text notifies nobody. Omit \`notifyUsers\` to notify whoever the Todoist apps would, or pass \`["none"]\` to stay silent.
+- Notification cannot be sent when editing a comment, only when adding one.
+
 **Deleting and archiving**
 
 - **delete-object** removes every object type; there is no per-type delete tool. Reminders use type "reminder", location reminders "location_reminder".

src/tool-helpers.ts

@@ -377,6 +377,7 @@ function mapComment(comment: Comment) {
         content: comment.content,
         postedAt: comment.postedAt.toISOString(),
         postedUid: comment.postedUid ?? undefined,
+        notifiedUserIds: comment.uidsToNotify?.length ? comment.uidsToNotify : undefined,
         fileAttachment: comment.fileAttachment
             ? {
                   resourceType: comment.fileAttachment.resourceType,

src/tools/add-comments.test.ts

@@ -1,13 +1,23 @@
-import type { Comment, TodoistApi } from '@doist/todoist-sdk'
+import type { Comment, Task, TodoistApi } from '@doist/todoist-sdk'
 import { type Mocked, vi } from 'vitest'
-import { createMockUser } from '../utils/test-helpers.js'
+import { z } from 'zod'
+import { ApiLimits } from '../utils/constants.js'
+import { createMockTask, createMockUser } from '../utils/test-helpers.js'
 import { ToolNames } from '../utils/tool-names.js'
+import { resolveUserRefs } from '../utils/user-resolver.js'
 import { addComments } from './add-comments.js'
 
+vi.mock('../utils/user-resolver.js', async (importOriginal) => {
+    const actual = await importOriginal<typeof import('../utils/user-resolver.js')>()
+    return { ...actual, resolveUserRefs: vi.fn() }
+})
+
 // Mock the Todoist API
 const mockTodoistApi = {
     addComment: vi.fn(),
     getUser: vi.fn(),
+    getComments: vi.fn(),
+    getTask: vi.fn(),
 } as unknown as Mocked<TodoistApi>
 
 const { ADD_COMMENTS } = ToolNames
@@ -28,10 +38,26 @@ function createMockComment(overrides: Partial<Comment> = {}): Comment {
     }
 }
 
+const CURRENT_USER_ID = 'current-user'
+
+function createMockTaskWithUids(overrides: Partial<Task> = {}): Task {
+    return {
+        ...createMockTask(),
+        addedByUid: CURRENT_USER_ID,
+        assignedByUid: null,
+        responsibleUid: null,
+        ...overrides,
+    }
+}
+
 describe(`${ADD_COMMENTS} tool`, () => {
     beforeEach(() => {
         vi.clearAllMocks()
-        mockTodoistApi.getUser.mockResolvedValue(createMockUser())
+        mockTodoistApi.getUser.mockResolvedValue(createMockUser({ id: CURRENT_USER_ID }))
+        // Default: an empty thread on an unassigned, self-created task, so
+        // nobody is notified unless a test says otherwise.
+        mockTodoistApi.getComments.mockResolvedValue({ results: [], nextCursor: null })
+        mockTodoistApi.getTask.mockResolvedValue(createMockTaskWithUids())
     })
 
     describe('adding comments to tasks', () => {
@@ -295,6 +321,253 @@ describe(`${ADD_COMMENTS} tool`, () => {
         })
     })
 
+    describe('notifying users', () => {
+        const mockResolveUserRefs = vi.mocked(resolveUserRefs)
+
+        function mockAddedComment(uidsToNotify: string[] | null = null) {
+            mockTodoistApi.addComment.mockResolvedValue(
+                createMockComment({ taskId: 'task456', uidsToNotify }),
+            )
+        }
+
+        it('should notify explicitly named users, resolving IDs, emails and names alike', async () => {
+            mockResolveUserRefs.mockResolvedValue([
+                { userId: '111', displayName: 'Ana', email: 'ana@example.com' },
+                { userId: '222', displayName: 'Bo', email: 'bo@example.com' },
+                { userId: '333', displayName: 'Cleo', email: 'cleo@example.com' },
+            ])
+            mockAddedComment(['111', '222', '333'])
+
+            const result = await addComments.execute(
+                {
+                    comments: [
+                        {
+                            taskId: 'task456',
+                            content: 'Please review',
+                            notifyUsers: ['111', 'bo@example.com', 'Cleo'],
+                        },
+                    ],
+                },
+                mockTodoistApi,
+            )
+
+            expect(mockResolveUserRefs).toHaveBeenCalledWith(mockTodoistApi, [
+                '111',
+                'bo@example.com',
+                'Cleo',
+            ])
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith({
+                content: 'Please review',
+                taskId: 'task456',
+                uidsToNotify: ['111', '222', '333'],
+            })
+            // Explicit recipients mean the thread never needs reading.
+            expect(mockTodoistApi.getComments).not.toHaveBeenCalled()
+            expect(result.textContent).toContain('Notified 3 people')
+        })
+
+        it('should surface an unresolvable user as an error', async () => {
+            mockResolveUserRefs.mockRejectedValue(
+                new Error('Could not find user(s): "Nobody", "Nobody Else".'),
+            )
+
+            await expect(
+                addComments.execute(
+                    {
+                        comments: [
+                            {
+                                taskId: 'task456',
+                                content: 'Please review',
+                                notifyUsers: ['Nobody', 'Nobody Else'],
+                            },
+                        ],
+                    },
+                    mockTodoistApi,
+                ),
+            ).rejects.toThrow('Could not find user(s): "Nobody", "Nobody Else".')
+        })
+
+        it('should omit uidsToNotify entirely when told to notify nobody', async () => {
+            mockAddedComment()
+
+            await addComments.execute(
+                {
+                    comments: [{ taskId: 'task456', content: 'Quiet note', notifyUsers: ['none'] }],
+                },
+                mockTodoistApi,
+            )
+
+            // Nobody to notify means no recipient field at all, rather than
+            // an empty one.
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith({
+                content: 'Quiet note',
+                taskId: 'task456',
+            })
+            expect(mockResolveUserRefs).not.toHaveBeenCalled()
+            expect(mockTodoistApi.getComments).not.toHaveBeenCalled()
+        })
+
+        it('should default a first task comment to the assignee, assigner and creator', async () => {
+            mockTodoistApi.getTask.mockResolvedValue(
+                createMockTaskWithUids({
+                    responsibleUid: 'assignee',
+                    assignedByUid: 'assigner',
+                    addedByUid: 'creator',
+                }),
+            )
+            mockAddedComment(['assignee', 'assigner', 'creator'])
+
+            await addComments.execute(
+                { comments: [{ taskId: 'task456', content: 'First comment' }] },
+                mockTodoistApi,
+            )
+
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith({
+                content: 'First comment',
+                taskId: 'task456',
+                uidsToNotify: ['assignee', 'assigner', 'creator'],
+            })
+        })
+
+        it('should exclude the comment author and drop unset uids from the defaults', async () => {
+            mockTodoistApi.getTask.mockResolvedValue(
+                createMockTaskWithUids({
+                    responsibleUid: CURRENT_USER_ID,
+                    assignedByUid: null,
+                    addedByUid: 'creator',
+                }),
+            )
+            mockAddedComment(['creator'])
+
+            await addComments.execute(
+                { comments: [{ taskId: 'task456', content: 'First comment' }] },
+                mockTodoistApi,
+            )
+
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith(
+                expect.objectContaining({ uidsToNotify: ['creator'] }),
+            )
+        })
+
+        it("should default a reply to the previous comment's participants", async () => {
+            mockTodoistApi.getComments.mockResolvedValue({
+                results: [
+                    createMockComment({
+                        id: 'older',
+                        postedAt: new Date('2024-01-01T09:00:00Z'),
+                        postedUid: 'stale-author',
+                        uidsToNotify: ['stale-recipient'],
+                    }),
+                    createMockComment({
+                        id: 'newest',
+                        postedAt: new Date('2024-01-01T15:00:00Z'),
+                        postedUid: 'previous-author',
+                        uidsToNotify: ['participant', CURRENT_USER_ID],
+                    }),
+                ],
+                nextCursor: null,
+            })
+            mockAddedComment(['participant', 'previous-author'])
+
+            await addComments.execute(
+                { comments: [{ taskId: 'task456', content: 'Reply' }] },
+                mockTodoistApi,
+            )
+
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith({
+                content: 'Reply',
+                taskId: 'task456',
+                uidsToNotify: ['participant', 'previous-author'],
+            })
+            // A thread that already has comments settles the recipients, so the
+            // task's own assignee/creator are never consulted.
+            expect(mockTodoistApi.getTask).not.toHaveBeenCalled()
+        })
+
+        it('should notify nobody on a first project comment', async () => {
+            mockTodoistApi.addComment.mockResolvedValue(
+                createMockComment({ taskId: undefined, projectId: 'project789' }),
+            )
+
+            await addComments.execute(
+                { comments: [{ projectId: 'project789', content: 'Project note' }] },
+                mockTodoistApi,
+            )
+
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith({
+                content: 'Project note',
+                projectId: 'project789',
+            })
+            expect(mockTodoistApi.getTask).not.toHaveBeenCalled()
+        })
+
+        it('should read a shared thread once for several comments on the same task', async () => {
+            mockTodoistApi.getTask.mockResolvedValue(
+                createMockTaskWithUids({ responsibleUid: 'assignee' }),
+            )
+            mockAddedComment(['assignee'])
+
+            await addComments.execute(
+                {
+                    comments: [
+                        { taskId: 'task456', content: 'First' },
+                        { taskId: 'task456', content: 'Second' },
+                    ],
+                },
+                mockTodoistApi,
+            )
+
+            expect(mockTodoistApi.getComments).toHaveBeenCalledTimes(1)
+            expect(mockTodoistApi.addComment).toHaveBeenCalledTimes(2)
+            expect(mockTodoistApi.addComment).toHaveBeenCalledWith(
+                expect.objectContaining({ content: 'Second', uidsToNotify: ['assignee'] }),
+            )
+        })
+
+        it('should surface who was notified, and omit the field when nobody was', async () => {
+            mockAddedComment(['111', '222'])
+            const notified = await addComments.execute(
+                {
+                    comments: [{ taskId: 'task456', content: 'Heads up', notifyUsers: ['none'] }],
+                },
+                mockTodoistApi,
+            )
+            expect(notified.structuredContent?.comments[0]).toEqual(
+                expect.objectContaining({ notifiedUserIds: ['111', '222'] }),
+            )
+
+            mockAddedComment()
+            const silent = await addComments.execute(
+                {
+                    comments: [{ taskId: 'task456', content: 'Quiet note', notifyUsers: ['none'] }],
+                },
+                mockTodoistApi,
+            )
+            expect(silent.structuredContent?.comments[0]?.notifiedUserIds).toBeUndefined()
+        })
+    })
+
+    describe('notifyUsers schema bounds', () => {
+        const parse = (notifyUsers: unknown) =>
+            z.object(addComments.parameters).safeParse({
+                comments: [{ taskId: 'task456', content: 'x', notifyUsers }],
+            })
+
+        it('rejects an empty list, so ["none"] stays the only way to stay silent', () => {
+            expect(parse([]).success).toBe(false)
+        })
+
+        it('rejects more recipients than a single comment may notify', () => {
+            const tooMany = Array.from(
+                { length: ApiLimits.NOTIFY_USERS_MAX + 1 },
+                (_, i) => `u${i}`,
+            )
+
+            expect(parse(tooMany).success).toBe(false)
+            expect(parse(tooMany.slice(0, ApiLimits.NOTIFY_USERS_MAX)).success).toBe(true)
+        })
+    })
+
     describe('validation', () => {
         it('should throw error when neither taskId nor projectId provided', async () => {
             await expect(