chore: rename to @doist/comms-mcp on @doist/comms-sdk#1
Merged
Conversation
Renames every identifier, import, env var, and prose mention from Twist/twist-sdk to Comms/comms-sdk: - package.json: @doist/twist-ai → @doist/comms-mcp; depend on @doist/comms-sdk@^0.2.0; reset version to 0.1.0; bin → comms-mcp; mcpName → com.doist/comms-mcp - README, AGENTS, .env.example, CHANGELOG: rewritten for Comms - env: TWIST_API_KEY → COMMS_API_KEY, TWIST_BASE_URL → COMMS_BASE_URL - types: TwistApi → CommsApi, TwistTool → CommsTool, TwistObjectType → CommsObjectType; twist-tool.ts → comms-tool.ts - URL helpers: getFullTwistURL → getFullCommsURL, twist.com → comms.todoist.com - MCP server name: comms-mcp-server; tool title prefix "Comms: …" - release.yml: drop twist-post-action announcement + twist-ai-integrations dispatch (not relevant to this repo) Type-check is intentionally still broken. The Comms SDK surface differs from Twist's in three load-bearing ways and each tool needs targeted adaptation in follow-ups: 1. CommsApi has no `batch()` primitive — every batching tool (load-thread, load-conversation, mark-done, search-content, get-mentions, fetch-inbox, get-groups, get-workspaces, list-channels, get-users) needs to switch to Promise.all over individual client calls (or we add a batch primitive to comms-sdk). 2. Object IDs are base58 UUIDv7 strings, not numbers. Zod schemas, structured output types, test fixtures, and url-helpers all assume numeric IDs. 3. User shape differs: `name`/`bot`/`defaultWorkspace`/`awayMode` are gone; `fullName`/`shortName` replace `name`. user-info needs a field-by-field remap. Once these are addressed per-tool, jest snapshots also need regenerating.
Brings the type-check, jest suite, and build to a clean state on top of
the prior mechanical rename. Per-tool highlights:
- away: stubbed `get` to honestly report `isAway: false` (no SDK
endpoint exposes away state); `set` and `clear` now throw with a
clear unsupported-by-SDK error instead of silently returning a fake
success. Also documented in mcp-server.ts instructions.
- list-channels, get-users, get-workspaces, get-groups, fetch-inbox,
load-thread, load-conversation, search-content, get-mentions,
mark-done: replaced `client.batch(...)` with `Promise.all([...])`;
switched to the new positional/args-object SDK signatures; per-call
error tolerance preserved where the original code had a fallback.
- user-info: maps `User.fullName`/`shortName`/`lang`; drops `bot`/
`defaultWorkspace`/`awayMode` (gone from the Comms user payload).
- build-link / create-thread / update-object / delete-object / react /
reply: switched ID Zod schemas and types from `z.number()` to
`z.string()` for thread/comment/conversation/message/channel/group IDs
(workspaceId and userId remain numeric).
- mark-done: fix data-loss bug — passing `workspaceId + channelId`
previously called `archiveAll({ workspaceId })`, archiving the whole
workspace. Now passes `{ workspaceId, channelIds: [channelId] }` and
similarly scopes `markAllRead`. Channel-only archive (no workspaceId)
is still unsupported by the SDK and is documented as a skip.
- fetch-inbox: dedupe channel IDs before calling `channels.getChannel`,
so an inbox dominated by threads from the same channel hits the
endpoint once per channel rather than once per thread.
Foundations:
- src/utils/output-schemas.ts: ID fields switched to z.string() where
the API uses opaque base58 UUIDv7s; UserInfo / GetWorkspaces / GetUsers
schemas re-aligned to the new User/WorkspaceUser/Workspace shapes.
- src/utils/url-helpers.ts: thin wrappers around getFullCommsURL with
string IDs.
- src/utils/test-helpers.ts: TEST_IDS now uses string fixtures for
channel/thread/comment/conversation/message/group IDs; createMockUser
/ createMockWorkspace match the new schemas.
Validated:
- npm run type-check — clean
- npm test — 194 passed / 19 suites
- npm run build — clean
- smoke-tested user-info, list-channels, and fetch-inbox against
staging (workspace 48121); all return real data with string IDs.
Direction review surfaced two "lie about state" failure modes and an
absent latency hedge. Doistbot rounds 2 and 3 surfaced one data-loss
bug (workspaceId + channelId silently archiving the whole workspace),
one Node-version mismatch (p-limit@7 requires Node 20), and a few test
quality nits worth taking.
away:
- Unregistered from the MCP server (was advertising a broken tool to
the model). Symbol stays exported from src/index.ts so the
importable-tools surface still gets the loud failure.
- All three actions now throw an explicit "unsupported by Comms SDK"
error. Previously `get` returned `isAway: false`, which is a silent
lie when the user is genuinely away — the SDK simply can't tell us.
- mcp-server.ts instructions updated (away note removed; tool gone).
- tool-annotations.test no longer expects away in the registry.
mark-done:
- Reject `archive && channelId && !workspaceId` up front instead of
silently dropping the archive step while reporting success.
- Already-fixed (in a prior commit): pass `{ workspaceId, channelIds:
[channelId] }` when both selectors are present so the call stays
scoped to that channel.
- Added two regression tests: the channel-only rejection, and the
scoped-archive path, including assertions on the user-visible
selectors payload (not just the SDK call shape).
Concurrency:
- Added `limitedAll(items, fn)` helper in src/utils/concurrency.ts
that wraps `Promise.all` with `p-limit(8)`. One implementation,
used by `list-channels` (creator lookup), `fetch-inbox` (channel
hydration), and `mark-done` (individual IDs). Keeps inbox/mark-done
fan-outs inside the undici socket pool / rate limiter on large
workspaces; today the burst is invisible.
- Pinned `p-limit@^6` (Node 18 compatible) instead of `@^7` (Node 20+)
to match the README's "Node 18+" requirement.
- jest transformIgnorePatterns extended for p-limit's ESM-only chain
(p-limit, yocto-queue, yoctocolors).
Decisions intentionally not taken:
- Doistbot suggested auto-disabling archive for channel-only bulk
operations rather than throwing. Rejected — the explicit throw was
the expert's prescription precisely because silent skipping is the
failure mode that broke the previous version.
- Doistbot suggested removing the `away` export entirely from
src/index.ts. Rejected — the importable-tools surface intentionally
keeps the symbol so consumers get the same loud-failure contract
they get from the MCP server (which is now: it's not there at all).
Validated:
- npm run type-check — clean
- npm test — 194 / 19 suites
- npm run build — clean
- Smoke-tested list-channels against staging — still returns string
channel IDs end-to-end.
Member
Author
|
@doistbot /review |
scottlovegrove
approved these changes
May 22, 2026
Scott flagged the away stub as user-hostile (LLM might echo "SDK" jargon at users) and the yoctocolors entry in transformIgnorePatterns as suspicious. This is a hard fork — no point keeping a tool that exists only to throw, or pretending we need to transform a package only semantic-release pulls in. - Delete src/tools/away.ts + its test - Remove away from ToolNames, src/index.ts (both `tools` map and named exports), scripts/run-tool.ts, mcp-server.ts comment, and the tool-annotations test commentary - Drop AWAY_ACTIONS / AwayAction / AwayOutputSchema / AwayOutput from src/utils/output-schemas.ts (also removed from StructuredOutputSchema union) - Drop the "away" bullet from README's tool list - jest.config.js: remove yoctocolors from transformIgnorePatterns (only semantic-release > execa pulls it in, never touched at test time); yocto-queue stays — it's p-limit@6's only runtime dep
Hard fork — the schema should describe what's in the Comms user payload, not what Twist's payload "no longer carries." Kept the one-liner mapping `name` → `User.fullName` since that's the only non-obvious bit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fork-and-rename of the MCP server from Twist to Comms. Drops
@doist/twist-sdk@2.6.0for@doist/comms-sdk@^0.2.0(same surface, new product name) and resets the version to0.1.0.What was changed
@doist/twist-ai→@doist/comms-mcp;bin→comms-mcp;mcpName→com.doist/comms-mcp.TwistApi→CommsApi,TwistTool→CommsTool,getFullTwistURL→getFullCommsURL;twist-tool.ts→comms-tool.ts.TWIST_API_KEY→COMMS_API_KEY,TWIST_BASE_URL→COMMS_BASE_URL.workspaceId/userIdstay numeric.client.batch(...)→Promise.all, withlimitedAll(p-limit@^6) gating fan-outs inlist-channels,fetch-inbox, andmark-done.mark-donedata-loss fix:archive + channelId + !workspaceIdis rejected up front;workspaceId + channelIdnow scopes to that channel instead of archiving the whole workspace.away: unregistered from the MCP server (Comms SDK has no away endpoint);set/clearthrow an explicit unsupported-by-SDK error. Symbol still exported for parity.twist.com→comms.todoist.com..env.examplerewritten for Comms.twist-post-actionannounce +twist-ai-integrationsdispatch (not relevant here).Validated
npm run type-checkcleannpm test— 194 passing / 19 suitesnpm run buildcleanuser-info,list-channels,fetch-inboxagainst stagingOut of scope
away(needs a Comms SDK endpoint first).