feat(comments): notify collaborators when adding a comment

[scottlovegrove]

Closes #509

The problem

Comments posted through this server notify nobody. add-comments never sent uidsToNotify, so a teammate named in an agent's comment found out only if they happened to open the task.

That also breaks the next comment, which is the part users notice and can't reproduce. Todoist's clients pick the recipients themselves and send them with each comment; the API notifies exactly who it is handed and derives nobody on its own. On a first comment the clients notify the assignee, the assigner and the creator; on a reply they notify the previous comment's participants, so threads keep flowing without everyone being re-tagged. A comment posted with an empty list therefore silences the comment that follows it — including one a human writes in the app. That's the behaviour @MelisUnal described on the issue (Zendesk 940230), where a scheduled agent replying in-task was "cut off right at the delivery point".

What this does

add-comments gains a per-comment notifyUsers, accepting user IDs, emails, full names or "me" for each person — the same reference shapes responsibleUser already takes — resolved through the existing userResolver. A new resolveUserRefs wraps the singular resolver for lists, deduping and reporting every unresolvable reference in one error rather than failing on the first.

Omitting notifyUsers mirrors the clients: the assignee, assigner and creator on a task's first comment, or the previous comment's participants on a reply. Passing ["none"] stays silent, following the repo's existing "remove" / "unassign" convention.

Recipients are worked out once per distinct target, so several comments on one task in a single batch read the thread once and notify the same people. Comments now also report notifiedUserIds, which lets a caller see a thread's participants before replying, and flows through find-comments and update-comments for free.

@mentions in the comment text are deliberately not parsed — the model passes notifyUsers explicitly, which avoids guessing at multi-word names, email addresses and code snippets. The server instructions tell it to do so.

Notification on edit is out of scope: UpdateCommentArgs is { content } only, and only the sync note_update command carries recipients.

The SDK fix this needed

Live testing turned up a bug in @doist/todoist-sdk: addComment joined uidsToNotify into a comma-separated string, which the API rejects outright with 400 INVALID_ARGUMENT_VALUE / uids_to_notify / "Input should be a valid list". No caller could notify anyone, and nothing caught it because no consumer set the field. Fixed in Doist/todoist-sdk-typescript#664 and released as 14.0.1, which this PR bumps to.

Verification

npx tsc --noEmit, all 1205 tests, npm run format:check and npm run build pass.

End-to-end against the live API on the published 14.0.1, using a throwaway task in a shared project (since removed):

case	result
first comment, unassigned self-created task	no recipients
notifyUsers: ["Ada Lovelace"]	notifiedUserIds: ["9876543"], "Notified 1 person"
reply, defaults	previous comment's participants, author excluded
notifyUsers: ["none"]	silent, no recipient field sent
notifyUsers: ["ada@example.com", "9876543"]	both refs resolve to one user, deduped to 1
two unresolvable names	one error naming both

The multi-user paths — a reply picking up someone else's participants, and a first comment on a task assigned to another person — are covered by unit tests rather than live calls, since verifying them for real means sending test notifications to an actual colleague.

🤖 Generated with Claude Code

[doistbot]

This PR adds per-comment notifyUsers to add-comments, resolving user refs through the existing userResolver and defaulting to the Todoist clients' recipient logic (assignee, assigner, creator on first comment; previous participants on replies) when the field is omitted.

Few things worth tightening:

• Cap and dedupe notifyUsers before resolving. The array has no .max(...) bound and resolveUserRefs fans out every ref via Promise.all before deduping, so duplicates or unresolvable names each trigger getUser plus full collaborator pagination against the operator's token — a single untrusted argument can amplify into unbounded parallel SDK calls.
• Load shared collaborator data once per list, not per ref. Because each non-ID ref independently misses the user/collaborator caches before any concurrent call populates them, a comment naming several people re-runs getProjects and every shared project's collaborator lookup once per name. Resolve the current user and collaborators up front, then match each ref against that pre-loaded set.
• Reject or normalize notifyUsers: []. An empty array is truthy in resolveRecipientsPerComment, so it bypasses the default-recipient path and posts silently — contradicting the documented "none" opt-out and risking an accidental notification-chain break. Add .min(1) or explicitly treat [] as "use defaults."
• Stream the latest-comment lookup instead of materializing the full thread. fetchAllPages loads every comment in the thread just for the reduce to pick one item, which can be a large allocation on long-lived tasks. Iterate the cursors and retain only the latest comment seen to keep the scan constant-memory.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (3)

• src/mcp-server.ts:47: This bullet duplicates the notifyUsers parameter guidance that is already fully described in add-comments' own input schema. The instructions block (and AGENTS.md) says to reserve this section for cross-tool routing guidance — the next bullet about add-vs-edit is that — and to keep per-tool parameter detail in the tool description. Consider keeping only the routing-relevant sentence and dropping the notifyUsers/["none"] instructions from here to avoid drift and duplicated token cost.
• src/tools/add-comments.ts:152: if (!target || !currentUser) return [] silently degrades to "notify nobody" — the exact failure this PR is meant to eliminate. Both conditions are already guaranteed by the caller: needsDefaultRecipients fetches currentUser whenever any comment omits notifyUsers, and targets is built from the same comments array, so target is always present in the default branch. Prefer throwing an invariant error here so a future refactor fails loudly instead of quietly dropping notifications.
• src/tools/add-comments.test.ts:526: The first half of "should surface who was notified" posts with notifyUsers: ['none'] (line 529) while the addComment mock returns uidsToNotify: ['111', '222'], so the notifiedUserIds: ['111', '222'] assertion passes only because mapComment copies the SDK field back. It can't fail if the tool misroutes recipients, and the ['none'] input vs. two-user output reads as a bug. Use a coherent scenario — e.g. notifyUsers: ['111', '222'] with resolveUserRefs mocked to return those two IDs — so the assertion reflects the tool's own recipient computation. The "none" → omitted-field behavior is already covered separately.

_{Share Feedback • Review Logs}

[doist-release-bot]

🎉 This PR is included in version 12.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀