feat(comment): notify collaborators when adding a comment

[scottlovegrove]

Brings td comment add to parity with the MCP, which shipped this in Doist/todoist-mcp#580 for Doist/todoist-mcp#509.

Stacked on #477 — based on that branch for SDK 14.0.1, which carries the uidsToNotify serialisation fix. Retarget to main once #477 merges.

The problem

Comments posted with td notify nobody. comment add never sent uidsToNotify, so a teammate named in a comment found out only if they happened to open the task.

That also breaks the next comment, which is the part people 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 later writes in the app.

What this does

comment add gains --notify, accepting names, emails, id:xxx or "me", comma-separated in the same style as --labels. --no-notify posts in silence, matching the existing --no-labels negation.

Omitting --notify mirrors the clients: assignee, assigner and creator on a task's first comment, or the previous comment's participants on a reply, always excluding the author. The rules live in src/lib/comment-recipients.ts, ported from the MCP's equivalent.

Two things worth calling out in review:

• Naming yourself is honoured, not filtered out. Self-exclusion is only right when the recipients were inferred rather than asked for. I had this filtering unconditionally at first and caught it in live testing — --notify me silently did nothing.
• The thread walk keeps only the newest comment rather than using paginate(), which accumulates every result. Comments come back oldest-first with no reverse option, so reaching the last page is unavoidable; holding the whole history is not. Review flagged exactly this on the MCP version.

resolveNotifyIds is pure over an already-fetched collaborator list, so the --notify path fetches collaborators once and reuses them to render the names back — my own tests caught a double fetch here.

@mentions in the comment text are deliberately not parsed; the user names people with --notify. SKILL_CONTENT documents this so agents don't assume the text alone notifies.

Notification on edit is out of scope — UpdateCommentArgs is { content } only.

Surfacing

Who was notified is now visible in three places: the confirmation line after adding, a Notified: line in comment view (resolved only when there are recipients, so the common case costs no extra request), and postedUid / uidsToNotify in plain --json rather than only under --full.

Verification

npm run type-check, npm run check, npm run build, npm run check:skill-sync and all 1815 tests pass (suite run three times to rule out flakiness).

Live against the real API on a throwaway task in a shared project, since deleted:

case	result
--dry-run with --notify	previews the raw string unresolved, no API call
defaults, first comment on a solo task	no recipients
--notify "Ada Lovelace"	Notified: Ada L.
--notify "me,ada@example.com"	deduped to one
--notify Ghost,Phantom	one ASSIGNEE_NOT_FOUND naming both
reply, defaults	previous comment's participants, author excluded
--no-notify	silent, no recipient field sent
comment view / --json	recipients shown

The paths involving a second person — a reply inheriting someone else's participants, and a first comment on a task assigned to someone else — 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 --notify/--no-notify to td comment add so comments can notify collaborators, bringing it to parity with the MCP's notification feature — including sensible defaults that mirror the Todoist clients' recipient inference logic.

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

Optional follow-up notes (6)

• src/commands/comment/add.ts:180: describeRecipients here and describeNotified in src/commands/comment/view.ts (line 81) duplicate the same id → short-name map plus raw-id fallback (new Map(collaborators.map(c => [c.id, formatUserShortName(c.name)])) then userIds.map(id => names.get(id) ?? id).join(', ')). Pull that mapping into one shared helper (e.g. formatUserIds(collaborators, userIds) in src/lib/collaborators.ts) and have both callers use it; each file keeps only its own fetch path.
• src/lib/collaborators.ts:168: This recreates the workspace and shared-project pagination/mapping already implemented by CollaboratorCache.fetchWorkspaceUsers and fetchProjectCollaborators. Extract the endpoint-specific fetches into shared helpers (or expose a cache load/read API) and have both paths use them, so cursor handling and collaborator normalization have one implementation.
• src/commands/comment/add.ts:94: When --project is set, resolveProjectRef already returned the full project (it's used for targetName/targetProjectId above), so this api.getProject(targetProjectId) is a redundant request. Hoist the resolved project out of the branch and reuse it here instead of refetching.
• CODEBASE.md:181: Remove the dedicated comment-recipients.ts entry. This helper currently has one production caller (comment/add.ts), so listing it makes the structural map drift toward a file index rather than its intended catalog of broadly reusable helpers.
• src/lib/output.ts:323: The plain---json contract change (adding postedUid/uidsToNotify to COMMENT_ESSENTIAL_FIELDS) isn't pinned by any test. The existing comment view --json and comment add --json tests only assert id/content/postedAt, so dropping these two fields from the essential list would pass CI. Assert that both keys appear in non---full JSON output.
• src/lib/comment-recipients.test.ts:43: This fixture puts the newest comment first, so the test doesn't actually exercise the 'not the first page' behavior its name claims — it would still pass if getLatestComment returned results[0]. The within-page newest-not-first case is already covered by the integration test defaults a reply... in comment.test.ts (older comment listed first). Either reorder the fixture so the stale comment comes first, or drop this test as redundant.

_{Share Feedback • Review Logs}

[pawelgrimm]

🚀

[doist-release-bot]

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀