diff --git a/README.md b/README.md index 492eb8c..8cb452d 100644 --- a/README.md +++ b/README.md @@ -17,9 +17,13 @@ npm install @doist/comms-mcp Example with [Vercel's AI SDK](https://ai-sdk.dev/docs/ai-sdk-core/generating-text#streamtext): ```js -import { fetchInbox, reply, markDone } from '@doist/comms-mcp' +import { configureBaseUrl, fetchInbox, reply, markDone } from '@doist/comms-mcp' import { streamText } from 'ai' +// Required if your CommsApi targets staging / a custom deployment. +// `getMcpServer` calls this for you; standalone tool consumers must. +configureBaseUrl(process.env.COMMS_BASE_URL) + const result = streamText({ model: yourModel, system: 'You are a helpful Comms assistant', @@ -79,9 +83,30 @@ Add to `claude_desktop_config.json`: #### Claude Code (CLI) +Don't pass the token via `-e KEY=VAL` — even via shell expansion, the +resolved value lands in `claude mcp add`'s argv and shows up in `ps` +output. Use a wrapper script that loads a `chmod 600` env file at +runtime: + ```bash -claude mcp add comms npx @doist/comms-mcp -export COMMS_API_KEY=your-comms-api-key-here +# 1. Token in a private env file (never in shell history or argv). +install -m 600 /dev/null ~/.config/comms-mcp.env +$EDITOR ~/.config/comms-mcp.env +# Contents: COMMS_API_KEY=... + +# 2. Wrapper script reads it at spawn time. +mkdir -p ~/.local/bin +cat > ~/.local/bin/comms-mcp <<'EOF' +#!/bin/sh +set -a +. ~/.config/comms-mcp.env +set +a +exec npx -y @doist/comms-mcp +EOF +chmod +x ~/.local/bin/comms-mcp + +# 3. Register the wrapper — no -e flags, no secrets in argv. +claude mcp add comms -s user -- ~/.local/bin/comms-mcp ``` #### Visual Studio Code @@ -106,15 +131,34 @@ export COMMS_API_KEY=your-comms-api-key-here ### Targeting a non-production deployment By default the server talks to `https://comms.todoist.com`. To point at -staging or a custom deployment, also set `COMMS_BASE_URL`: +staging or a custom deployment, also set `COMMS_BASE_URL`. The staging +token and the prod token are different — a staging token will 403 +against prod and vice versa. + +For Claude Desktop / Cursor / VS Code: ```json "env": { - "COMMS_API_KEY": "your-comms-api-key-here", + "COMMS_API_KEY": "your-staging-api-key-here", "COMMS_BASE_URL": "https://comms.staging.todoist.com" } ``` +For Claude Code: extend the wrapper script from the Claude Code setup +above to also set `COMMS_BASE_URL`: + +```sh +#!/bin/sh +set -a +. ~/.config/comms-mcp.env # contains COMMS_API_KEY=... +COMMS_BASE_URL=https://comms.staging.todoist.com +set +a +exec npx -y @doist/comms-mcp +``` + +The server logs `Comms MCP targeting ` to stderr on startup so +you can confirm at a glance which environment it's hitting. + ### Getting a Comms API key Generate a personal API token from the Comms app console, then export diff --git a/scripts/run-tool.ts b/scripts/run-tool.ts index fb902a0..726e39e 100644 --- a/scripts/run-tool.ts +++ b/scripts/run-tool.ts @@ -34,6 +34,8 @@ import { reply } from '../src/tools/reply.js' import { searchContent } from '../src/tools/search-content.js' import { updateObject } from '../src/tools/update-object.js' import { userInfo } from '../src/tools/user-info.js' +import { buildServerOptions } from '../src/utils/server-options.js' +import { configureBaseUrl } from '../src/utils/url-helpers.js' // Define a minimal type for tool execution that works with any tool type ExecutableTool = { @@ -137,14 +139,16 @@ async function main() { process.exit(1) } - const apiKey = process.env.COMMS_API_KEY - if (!apiKey) { - console.error('COMMS_API_KEY not found in environment or .env file') + let commsApiKey: string + let baseUrl: string | undefined + try { + ;({ commsApiKey, baseUrl } = buildServerOptions()) + } catch (e) { + console.error(e instanceof Error ? e.message : String(e)) process.exit(1) } - - const baseUrl = process.env.COMMS_BASE_URL - const client = new CommsApi(apiKey, { baseUrl }) + configureBaseUrl(baseUrl) + const client = new CommsApi(commsApiKey, { baseUrl }) console.log(`Running ${toolName} with args:`) console.log(JSON.stringify(parsedArgs, null, 2)) diff --git a/src/index.ts b/src/index.ts index 957818e..494d8ea 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ import { reply } from './tools/reply.js' import { searchContent } from './tools/search-content.js' import { updateObject } from './tools/update-object.js' import { userInfo } from './tools/user-info.js' +import { configureBaseUrl } from './utils/url-helpers.js' const tools = { userInfo, @@ -33,7 +34,7 @@ const tools = { getGroups, } -export { tools, getMcpServer } +export { tools, getMcpServer, configureBaseUrl } export { userInfo, diff --git a/src/main.ts b/src/main.ts index 3ad5426..e62be31 100644 --- a/src/main.ts +++ b/src/main.ts @@ -2,19 +2,27 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import dotenv from 'dotenv' import { getMcpServer } from './mcp-server.js' +import { buildServerOptions } from './utils/server-options.js' function main() { - const commsApiKey = process.env.COMMS_API_KEY - if (!commsApiKey) { - throw new Error('COMMS_API_KEY is not set') - } - - const server = getMcpServer({ commsApiKey }) + const options = buildServerOptions() + // Structured stderr log (stdout is reserved for the MCP protocol). + // Surfacing the target up-front turns "why am I getting 403s" + // debugging into one machine-parsable line — staging vs prod + // tokens aren't cross-compatible. + console.error( + JSON.stringify({ + level: 'info', + event: 'startup', + base_url: options.baseUrl ?? 'https://comms.todoist.com', + base_url_source: options.baseUrl ? 'env' : 'default', + }), + ) + const server = getMcpServer(options) const transport = new StdioServerTransport() server .connect(transport) .then(() => { - // We use console.error because standard I/O is being used for the MCP server communication. console.error('Server started') }) .catch((error) => { diff --git a/src/mcp-server.ts b/src/mcp-server.ts index 35031c5..772861d 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -18,6 +18,8 @@ import { reply } from './tools/reply.js' import { searchContent } from './tools/search-content.js' import { updateObject } from './tools/update-object.js' import { userInfo } from './tools/user-info.js' +import type { ServerOptions } from './utils/server-options.js' +import { configureBaseUrl } from './utils/url-helpers.js' const instructions = ` ## Comms Communication Tools @@ -58,7 +60,12 @@ Always provide clear context and maintain professional communication standards. * @param baseUrl - Optional base URL for the Comms API. * @returns the MCP server. */ -function getMcpServer({ commsApiKey, baseUrl }: { commsApiKey: string; baseUrl?: string }) { +function getMcpServer({ commsApiKey, baseUrl }: ServerOptions) { + // Set up host rewriting for SDK + helper URLs before any tool runs. + // Idempotent and applies to every consumer of getMcpServer (CLI, + // importable-tools, future entry points). + configureBaseUrl(baseUrl) + const server = new McpServer( { name: 'comms-mcp-server', version: '0.1.0' }, { diff --git a/src/tools/__tests__/fetch-inbox.test.ts b/src/tools/__tests__/fetch-inbox.test.ts index 3b95460..5d50438 100644 --- a/src/tools/__tests__/fetch-inbox.test.ts +++ b/src/tools/__tests__/fetch-inbox.test.ts @@ -2,6 +2,7 @@ import type { CommsApi } from '@doist/comms-sdk' import { jest } from '@jest/globals' import { extractTextContent, TEST_IDS } from '../../utils/test-helpers.js' import { ToolNames } from '../../utils/tool-names.js' +import { configureBaseUrl } from '../../utils/url-helpers.js' import { fetchInbox } from '../fetch-inbox.js' // Mock the Comms API @@ -136,7 +137,12 @@ describe(`${FETCH_INBOX} tool`, () => { if (threads?.[0] && threads[1]) { expect(threads[0].id).toBe(TEST_IDS.THREAD_1) expect(threads[0].channelName).toBe('Test Channel') - expect(threads[0].threadUrl).toContain('comms.todoist.com') + // toBe (not toContain): 'comms.staging.todoist.com' contains + // 'comms.todoist.com', so toContain wouldn't catch a rewrite + // regression on a staging-targeted run. + expect(threads[0].threadUrl).toBe( + `https://comms.todoist.com/a/${TEST_IDS.WORKSPACE_1}/ch/${TEST_IDS.CHANNEL_1}/t/${TEST_IDS.THREAD_1}/`, + ) expect(threads[0].isUnread).toBe(true) expect(threads[1].isStarred).toBe(true) } @@ -329,7 +335,9 @@ describe(`${FETCH_INBOX} tool`, () => { expect(conversations[0].id).toBe(TEST_IDS.CONVERSATION_1) expect(conversations[0].participantNames).toEqual(['Alice', 'Bob']) expect(conversations[0].isUnread).toBe(true) - expect(conversations[0].conversationUrl).toContain('comms.todoist.com') + expect(conversations[0].conversationUrl).toBe( + `https://comms.todoist.com/a/${TEST_IDS.WORKSPACE_1}/msg/${TEST_IDS.CONVERSATION_1}/`, + ) expect(conversations[1].title).toBe('Project Discussion') } }) @@ -540,10 +548,9 @@ describe(`${FETCH_INBOX} tool`, () => { const { structuredContent } = result expect(structuredContent?.threads).toHaveLength(1) const threadUrl = structuredContent?.threads?.[0]?.threadUrl - expect(threadUrl).toBeDefined() - expect(typeof threadUrl).toBe('string') - expect(threadUrl).toContain('comms.todoist.com') - expect(threadUrl).toContain(String(TEST_IDS.THREAD_1)) + expect(threadUrl).toBe( + `https://comms.todoist.com/a/${TEST_IDS.WORKSPACE_1}/ch/${TEST_IDS.CHANNEL_1}/t/${TEST_IDS.THREAD_1}/`, + ) }) it('should construct conversationUrl via getFullCommsURL when SDK omits url field', async () => { @@ -607,10 +614,9 @@ describe(`${FETCH_INBOX} tool`, () => { const { structuredContent } = result expect(structuredContent?.conversations).toHaveLength(1) const conversationUrl = structuredContent?.conversations?.[0]?.conversationUrl - expect(conversationUrl).toBeDefined() - expect(typeof conversationUrl).toBe('string') - expect(conversationUrl).toContain('comms.todoist.com') - expect(conversationUrl).toContain(String(TEST_IDS.CONVERSATION_1)) + expect(conversationUrl).toBe( + `https://comms.todoist.com/a/${TEST_IDS.WORKSPACE_1}/msg/${TEST_IDS.CONVERSATION_1}/`, + ) }) }) @@ -627,4 +633,46 @@ describe(`${FETCH_INBOX} tool`, () => { ).rejects.toThrow('API Error: Unauthorized') }) }) + + // The unit tests for url-helpers cover the rewrite mechanism in + // isolation; this case covers it end-to-end at the tool boundary, + // which is the layer that previously regressed and shipped prod + // links from a staging-targeted server. + describe('staging baseUrl rewrites tool output', () => { + afterEach(() => { + configureBaseUrl(undefined) + }) + + it('rewrites threadUrl and unreadThreads[*].url to the configured host', async () => { + configureBaseUrl('https://comms.staging.todoist.com') + + mockCommsApi.inbox.getInbox.mockResolvedValue([makeInboxThread()]) + mockCommsApi.inbox.getCount.mockResolvedValue(1) + mockCommsApi.threads.getUnread.mockResolvedValue({ + data: [ + { + threadId: TEST_IDS.THREAD_1, + channelId: TEST_IDS.CHANNEL_1, + objIndex: 1, + directMention: false, + }, + ], + version: 1, + }) + mockCommsApi.conversations.getUnread.mockResolvedValue({ data: [], version: 1 }) + mockCommsApi.channels.getChannel.mockResolvedValue(makeChannel()) + + const result = await fetchInbox.execute( + { workspaceId: TEST_IDS.WORKSPACE_1, limit: 50, onlyUnread: false }, + mockCommsApi, + ) + + const { structuredContent } = result + const stagingThreadUrl = `https://comms.staging.todoist.com/a/${TEST_IDS.WORKSPACE_1}/ch/${TEST_IDS.CHANNEL_1}/t/${TEST_IDS.THREAD_1}/` + expect(structuredContent?.threads?.[0]?.threadUrl).toBe(stagingThreadUrl) + // unreadThreads is the raw SDK array; its `.url` must also + // be rewritten or the structured payload is inconsistent. + expect(structuredContent?.unreadThreads?.[0]?.url).toBe(stagingThreadUrl) + }) + }) }) diff --git a/src/tools/build-link.ts b/src/tools/build-link.ts index 7be68e2..1cc06f0 100644 --- a/src/tools/build-link.ts +++ b/src/tools/build-link.ts @@ -1,9 +1,14 @@ -import { getCommentURL, getFullCommsURL, getMessageURL } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { BuildLinkOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { + getCommentURL, + getFullCommsURL, + getMessageURL, + toRelativeCommsURL, +} from '../utils/url-helpers.js' const ArgsSchema = { workspaceId: z.number().describe('The workspace ID.'), @@ -26,7 +31,7 @@ const ArgsSchema = { .optional() .default(true) .describe( - 'Whether to return a full URL (with https://comms.todoist.com) or relative path.', + 'Whether to return a full URL (including the configured host) or a relative path.', ), } @@ -71,7 +76,7 @@ const buildLink = { const params = { workspaceId, conversationId } url = fullUrl ? getFullCommsURL(params) - : getFullCommsURL(params).replace('https://comms.todoist.com', '') + : toRelativeCommsURL(getFullCommsURL(params)) } } else if (threadId !== undefined) { if (commentId !== undefined) { @@ -90,7 +95,7 @@ const buildLink = { : { workspaceId, threadId } url = fullUrl ? getFullCommsURL(params) - : getFullCommsURL(params).replace('https://comms.todoist.com', '') + : toRelativeCommsURL(getFullCommsURL(params)) } } else { throw new Error('Must provide either conversationId OR threadId to build a link') diff --git a/src/tools/create-thread.ts b/src/tools/create-thread.ts index 3ecf882..9e6668b 100644 --- a/src/tools/create-thread.ts +++ b/src/tools/create-thread.ts @@ -1,9 +1,9 @@ -import { getFullCommsURL } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { type CreateThreadOutput, CreateThreadOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { channelId: z.string().describe('The ID of the channel to create the thread in.'), @@ -48,13 +48,14 @@ const createThread = { : postedValue : new Date() - const threadUrl = + const threadUrl = rewriteToConfiguredHost( thread.url ?? - getFullCommsURL({ - workspaceId: thread.workspaceId, - channelId: thread.channelId, - threadId: thread.id, - }) + getFullCommsURL({ + workspaceId: thread.workspaceId, + channelId: thread.channelId, + threadId: thread.id, + }), + ) const lines: string[] = [ `# Thread Created`, diff --git a/src/tools/fetch-inbox.ts b/src/tools/fetch-inbox.ts index 12c5bf2..0271e50 100644 --- a/src/tools/fetch-inbox.ts +++ b/src/tools/fetch-inbox.ts @@ -5,13 +5,14 @@ import type { UnreadConversation, WorkspaceUser, } from '@doist/comms-sdk' -import { ARCHIVE_FILTER_VALUES, getFullCommsURL } from '@doist/comms-sdk' +import { ARCHIVE_FILTER_VALUES } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { limitedAll } from '../utils/concurrency.js' import { FetchInboxOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { workspaceId: z.number().describe('The workspace ID to fetch inbox for.'), @@ -282,9 +283,9 @@ const fetchInbox = { isUnread: t.isUnread, isArchived: t.isArchived, isStarred: Boolean(t.isSaved), - threadUrl: - t.url ?? - getFullCommsURL({ workspaceId, channelId: t.channelId, threadId: t.id }), + threadUrl: t.url + ? rewriteToConfiguredHost(t.url) + : getFullCommsURL({ workspaceId, channelId: t.channelId, threadId: t.id }), })), conversations: conversationsWithDetails.map((cd) => { const { conversation, participants } = cd @@ -298,16 +299,22 @@ const fetchInbox = { userIds: conversation.userIds, participantNames, isUnread: cd.isUnread, - conversationUrl: - conversation.url ?? - getFullCommsURL({ - workspaceId: conversation.workspaceId, - conversationId: conversation.id, - }), + conversationUrl: conversation.url + ? rewriteToConfiguredHost(conversation.url) + : getFullCommsURL({ + workspaceId: conversation.workspaceId, + conversationId: conversation.id, + }), } }), unreadCount: unreadThreads.length, - unreadThreads: unreadThreadsOriginal, + // InboxThread.url is hardcoded to the SDK prod host; rewrite + // so a staging-targeted server returns staging links here too. + // UnreadConversation has no url field, so no rewrite needed. + unreadThreads: unreadThreadsOriginal.map((t) => ({ + ...t, + url: rewriteToConfiguredHost(t.url), + })), unreadConversations: unreadConversationsOriginal, totalThreads: threads.length, totalConversations: conversationsWithDetails.length, diff --git a/src/tools/get-mentions.ts b/src/tools/get-mentions.ts index 928ef94..93bd559 100644 --- a/src/tools/get-mentions.ts +++ b/src/tools/get-mentions.ts @@ -1,9 +1,10 @@ -import { type SearchResultType, getFullCommsURL } from '@doist/comms-sdk' +import type { SearchResultType } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { GetMentionsOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL } from '../utils/url-helpers.js' const ArgsSchema = { workspaceId: z.number().describe('The workspace ID to search in.'), diff --git a/src/tools/load-conversation.ts b/src/tools/load-conversation.ts index 22e797c..f52abba 100644 --- a/src/tools/load-conversation.ts +++ b/src/tools/load-conversation.ts @@ -1,9 +1,10 @@ -import { getFullCommsURL, type WorkspaceUser } from '@doist/comms-sdk' +import type { WorkspaceUser } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { LoadConversationOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { conversationId: z.string().describe('The conversation ID to load.'), @@ -130,12 +131,12 @@ const loadConversation = { userIds: includeParticipants ? conversation.userIds : [], archived: conversation.archived, lastActive: conversation.lastActive.toISOString(), - conversationUrl: - conversation.url ?? - getFullCommsURL({ - workspaceId: conversation.workspaceId, - conversationId: conversation.id, - }), + conversationUrl: conversation.url + ? rewriteToConfiguredHost(conversation.url) + : getFullCommsURL({ + workspaceId: conversation.workspaceId, + conversationId: conversation.id, + }), }, messages: messages.map((m) => ({ id: m.id, @@ -144,13 +145,13 @@ const loadConversation = { creatorName: userInfo[m.creator]?.fullName, conversationId: m.conversationId, posted: m.posted.toISOString(), - messageUrl: - m.url ?? - getFullCommsURL({ - workspaceId: m.workspaceId, - conversationId: m.conversationId, - messageId: m.id, - }), + messageUrl: m.url + ? rewriteToConfiguredHost(m.url) + : getFullCommsURL({ + workspaceId: m.workspaceId, + conversationId: m.conversationId, + messageId: m.id, + }), })), totalMessages: conversation.messageCount ?? 0, } diff --git a/src/tools/load-thread.ts b/src/tools/load-thread.ts index af16e3f..ffc1e2c 100644 --- a/src/tools/load-thread.ts +++ b/src/tools/load-thread.ts @@ -1,9 +1,9 @@ -import { getFullCommsURL } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { LoadThreadOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { threadId: z.string().describe('The thread ID to load.'), @@ -172,13 +172,13 @@ const loadThread = { .map((id) => userLookup[id]) .filter((name): name is string => name !== undefined) : undefined, - threadUrl: - thread.url ?? - getFullCommsURL({ - workspaceId: thread.workspaceId, - channelId: thread.channelId, - threadId: thread.id, - }), + threadUrl: thread.url + ? rewriteToConfiguredHost(thread.url) + : getFullCommsURL({ + workspaceId: thread.workspaceId, + channelId: thread.channelId, + threadId: thread.id, + }), }, comments: comments.map((c) => ({ id: c.id, @@ -187,14 +187,14 @@ const loadThread = { creatorName: userLookup[c.creator], threadId: c.threadId, posted: c.posted.toISOString(), - commentUrl: - c.url ?? - getFullCommsURL({ - workspaceId: c.workspaceId, - channelId: c.channelId, - threadId: c.threadId, - commentId: c.id, - }), + commentUrl: c.url + ? rewriteToConfiguredHost(c.url) + : getFullCommsURL({ + workspaceId: c.workspaceId, + channelId: c.channelId, + threadId: c.threadId, + commentId: c.id, + }), })), totalComments: thread.commentCount, } diff --git a/src/tools/react.ts b/src/tools/react.ts index 8c62297..a6a6087 100644 --- a/src/tools/react.ts +++ b/src/tools/react.ts @@ -1,10 +1,10 @@ -import { getFullCommsURL } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { ReactOutputSchema } from '../utils/output-schemas.js' import { type ReactionTargetType, ReactionTargetTypeSchema } from '../utils/target-types.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { targetType: ReactionTargetTypeSchema.describe( @@ -43,33 +43,36 @@ const react = { // Fetch target metadata to get URL if (targetType === 'thread') { const thread = await client.threads.getThread(targetId) - targetUrl = + targetUrl = rewriteToConfiguredHost( thread.url ?? - getFullCommsURL({ - workspaceId: thread.workspaceId, - channelId: thread.channelId, - threadId: thread.id, - }) + getFullCommsURL({ + workspaceId: thread.workspaceId, + channelId: thread.channelId, + threadId: thread.id, + }), + ) } else if (targetType === 'comment') { const comment = await client.comments.getComment(targetId) - targetUrl = + targetUrl = rewriteToConfiguredHost( comment.url ?? - getFullCommsURL({ - workspaceId: comment.workspaceId, - channelId: comment.channelId, - threadId: comment.threadId, - commentId: comment.id, - }) + getFullCommsURL({ + workspaceId: comment.workspaceId, + channelId: comment.channelId, + threadId: comment.threadId, + commentId: comment.id, + }), + ) } else { // message const message = await client.conversationMessages.getMessage(targetId) - targetUrl = + targetUrl = rewriteToConfiguredHost( message.url ?? - getFullCommsURL({ - workspaceId: message.workspaceId, - conversationId: message.conversationId, - messageId: message.id, - }) + getFullCommsURL({ + workspaceId: message.workspaceId, + conversationId: message.conversationId, + messageId: message.id, + }), + ) } // Map targetType to the appropriate API parameter diff --git a/src/tools/reply.ts b/src/tools/reply.ts index f0a25a8..a60a300 100644 --- a/src/tools/reply.ts +++ b/src/tools/reply.ts @@ -1,10 +1,11 @@ -import { getFullCommsURL, NOTIFY_AUDIENCES, type NotifyAudience } from '@doist/comms-sdk' +import { NOTIFY_AUDIENCES, type NotifyAudience } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { type ReplyOutput, ReplyOutputSchema } from '../utils/output-schemas.js' import { ReplyTargetTypeSchema } from '../utils/target-types.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { targetType: ReplyTargetTypeSchema.describe( @@ -70,14 +71,15 @@ const reply = { notifyAudience: appliedAudience, }) replyId = comment.id - replyUrl = + replyUrl = rewriteToConfiguredHost( comment.url ?? - getFullCommsURL({ - workspaceId: comment.workspaceId, - channelId: comment.channelId, - threadId: comment.threadId, - commentId: comment.id, - }) + getFullCommsURL({ + workspaceId: comment.workspaceId, + channelId: comment.channelId, + threadId: comment.threadId, + commentId: comment.id, + }), + ) const postedValue = comment.posted created = postedValue ? typeof postedValue === 'string' @@ -90,13 +92,14 @@ const reply = { content, }) replyId = message.id - replyUrl = + replyUrl = rewriteToConfiguredHost( message.url ?? - getFullCommsURL({ - workspaceId: message.workspaceId, - conversationId: message.conversationId, - messageId: message.id, - }) + getFullCommsURL({ + workspaceId: message.workspaceId, + conversationId: message.conversationId, + messageId: message.id, + }), + ) const postedValue = message.posted created = postedValue ? typeof postedValue === 'string' diff --git a/src/tools/search-content.ts b/src/tools/search-content.ts index 320dbd3..088e907 100644 --- a/src/tools/search-content.ts +++ b/src/tools/search-content.ts @@ -1,9 +1,10 @@ -import { type SearchResultType, getFullCommsURL } from '@doist/comms-sdk' +import type { SearchResultType } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' import { SearchContentOutputSchema } from '../utils/output-schemas.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL } from '../utils/url-helpers.js' const ArgsSchema = { query: z.string().min(1).describe('The search query string.'), diff --git a/src/tools/update-object.ts b/src/tools/update-object.ts index b8c7248..3abf025 100644 --- a/src/tools/update-object.ts +++ b/src/tools/update-object.ts @@ -1,4 +1,4 @@ -import { getFullCommsURL, type CommsApi } from '@doist/comms-sdk' +import type { CommsApi } from '@doist/comms-sdk' import { z } from 'zod' import type { CommsTool } from '../comms-tool.js' import { getToolOutput } from '../mcp-helpers.js' @@ -11,6 +11,7 @@ import { } from '../utils/output-schemas.js' import { UpdateTargetTypeSchema } from '../utils/target-types.js' import { ToolNames } from '../utils/tool-names.js' +import { getFullCommsURL, rewriteToConfiguredHost } from '../utils/url-helpers.js' const ArgsSchema = { targetType: UpdateTargetTypeSchema.describe( @@ -45,13 +46,14 @@ async function updateThreadBranch(args: Args, client: CommsApi): Promise const thread = await client.threads.updateThread({ id: targetId, title, content }) - const threadUrl = + const threadUrl = rewriteToConfiguredHost( thread.url ?? - getFullCommsURL({ - workspaceId: thread.workspaceId, - channelId: thread.channelId, - threadId: thread.id, - }) + getFullCommsURL({ + workspaceId: thread.workspaceId, + channelId: thread.channelId, + threadId: thread.id, + }), + ) const lastEdited = thread.lastEdited ? thread.lastEdited.toISOString() : undefined @@ -92,14 +94,15 @@ async function updateCommentBranch(args: Args, client: CommsApi): Promise { + it('returns commsApiKey with undefined baseUrl when COMMS_BASE_URL is unset', () => { + expect(buildServerOptions({ COMMS_API_KEY: 'k' })).toEqual({ + commsApiKey: 'k', + baseUrl: undefined, + }) + }) + + it('passes through COMMS_BASE_URL when set', () => { + expect( + buildServerOptions({ + COMMS_API_KEY: 'k', + COMMS_BASE_URL: 'https://comms.staging.todoist.com', + }), + ).toEqual({ + commsApiKey: 'k', + baseUrl: 'https://comms.staging.todoist.com', + }) + }) + + it('treats empty COMMS_BASE_URL as undefined so the SDK default wins', () => { + expect(buildServerOptions({ COMMS_API_KEY: 'k', COMMS_BASE_URL: '' })).toEqual({ + commsApiKey: 'k', + baseUrl: undefined, + }) + }) + + it('trims whitespace from COMMS_BASE_URL so " " is not passed to the SDK', () => { + expect(buildServerOptions({ COMMS_API_KEY: 'k', COMMS_BASE_URL: ' ' })).toEqual({ + commsApiKey: 'k', + baseUrl: undefined, + }) + expect( + buildServerOptions({ + COMMS_API_KEY: 'k', + COMMS_BASE_URL: ' https://comms.staging.todoist.com ', + }), + ).toEqual({ + commsApiKey: 'k', + baseUrl: 'https://comms.staging.todoist.com', + }) + }) + + it('throws when COMMS_API_KEY is missing', () => { + expect(() => buildServerOptions({})).toThrow('COMMS_API_KEY is not set') + }) +}) diff --git a/src/utils/__tests__/url-helpers.test.ts b/src/utils/__tests__/url-helpers.test.ts new file mode 100644 index 0000000..dd6cbb4 --- /dev/null +++ b/src/utils/__tests__/url-helpers.test.ts @@ -0,0 +1,135 @@ +import { + getCommentURL as sdkGetCommentURL, + getFullCommsURL as sdkGetFullCommsURL, + getMessageURL as sdkGetMessageURL, +} from '@doist/comms-sdk' +import { + configureBaseUrl, + getCommentURL, + getFullCommsURL, + getMessageURL, + rewriteToConfiguredHost, + toRelativeCommsURL, +} from '../url-helpers.js' + +describe('url-helpers', () => { + afterEach(() => { + configureBaseUrl(undefined) + }) + + describe('getFullCommsURL', () => { + it('matches the SDK output when no baseUrl is configured', () => { + // Wrapper must be a no-op so the prod default behavior is + // exactly the SDK's. Compare against the SDK to avoid pinning + // a specific URL format here. + expect(getFullCommsURL({ workspaceId: 1 })).toBe(sdkGetFullCommsURL({ workspaceId: 1 })) + }) + + it('swaps the prod host for the configured host on a full URL', () => { + configureBaseUrl('https://comms.staging.todoist.com') + const sdkUrl = sdkGetFullCommsURL({ workspaceId: 1 }) + // Sanity: the SDK still hardcodes the prod host — that's the + // load-bearing assumption of the rewrite layer. If this ever + // fails the wrapper needs revisiting. + expect(sdkUrl).toMatch(/^https:\/\/comms\.todoist\.com\//) + expect(getFullCommsURL({ workspaceId: 1 })).toBe( + sdkUrl.replace('https://comms.todoist.com', 'https://comms.staging.todoist.com'), + ) + }) + }) + + describe('relative-URL helpers', () => { + // The SDK's `getMessageURL`/`getCommentURL` return relative paths. + // applyBaseUrl's regex only matches a leading prod host, so the + // wrapped versions must be no-ops on relative paths regardless of + // whether staging is configured. Compare against the SDK output + // directly so this stays correct if the SDK changes its URL + // format — the wrapper is what's under test, not the format. + const messageArgs = { workspaceId: 1, conversationId: 'c', messageId: 'm' } + const commentArgs = { workspaceId: 1, channelId: 'ch', threadId: 't', commentId: 'cm' } + + it('pass relative paths through unchanged with no baseUrl', () => { + expect(getMessageURL(messageArgs)).toBe(sdkGetMessageURL(messageArgs)) + expect(getCommentURL(commentArgs)).toBe(sdkGetCommentURL(commentArgs)) + }) + + it('pass relative paths through unchanged with staging configured', () => { + configureBaseUrl('https://comms.staging.todoist.com') + expect(getMessageURL(messageArgs)).toBe(sdkGetMessageURL(messageArgs)) + expect(getCommentURL(commentArgs)).toBe(sdkGetCommentURL(commentArgs)) + }) + }) + + describe('toRelativeCommsURL', () => { + it('strips the prod host', () => { + expect(toRelativeCommsURL('https://comms.todoist.com/a/1/')).toBe('/a/1/') + }) + + // The previous build-link implementation hardcoded the prod host in + // a `.replace(...)` call, which silently failed on staging. The + // regex must strip any host so a future contributor can't reintroduce + // the hardcoded-host pattern without breaking this test. + it('strips a staging or custom host', () => { + expect(toRelativeCommsURL('https://comms.staging.todoist.com/a/1/')).toBe('/a/1/') + }) + }) + + describe('rewriteToConfiguredHost', () => { + it('rewrites the prod host of an SDK-supplied URL when staging is configured', () => { + configureBaseUrl('https://comms.staging.todoist.com') + expect(rewriteToConfiguredHost('https://comms.todoist.com/a/1/ch/x/t/y/')).toBe( + 'https://comms.staging.todoist.com/a/1/ch/x/t/y/', + ) + }) + + it('is idempotent on an already-staging URL', () => { + configureBaseUrl('https://comms.staging.todoist.com') + const staging = 'https://comms.staging.todoist.com/a/1/' + expect(rewriteToConfiguredHost(staging)).toBe(staging) + }) + + it('passes through unchanged when no baseUrl is configured', () => { + expect(rewriteToConfiguredHost('https://comms.todoist.com/a/1/')).toBe( + 'https://comms.todoist.com/a/1/', + ) + }) + + // The regex anchors with `(?=[/:?#]|$)` so an attacker-controlled + // suffix on the host can't sneak through the rewrite. + it('does not match a host with a malicious suffix', () => { + configureBaseUrl('https://comms.staging.todoist.com') + expect(rewriteToConfiguredHost('https://comms.todoist.com.evil.com/path')).toBe( + 'https://comms.todoist.com.evil.com/path', + ) + expect(rewriteToConfiguredHost('https://comms.todoist.comEXTRA/x')).toBe( + 'https://comms.todoist.comEXTRA/x', + ) + }) + + // Case-insensitive so an SDK upgrade emitting `Https://...` or + // `Comms.Todoist.Com` doesn't silently bypass the rewrite. + it('matches case-insensitively', () => { + configureBaseUrl('https://comms.staging.todoist.com') + expect(rewriteToConfiguredHost('HTTPS://Comms.Todoist.Com/a/1/')).toBe( + 'https://comms.staging.todoist.com/a/1/', + ) + }) + + // `$&` in `configuredBaseUrl` must not be interpreted as a + // backreference — use a function replacement. + it('treats $-tokens in the configured base URL as literal', () => { + configureBaseUrl('https://staging.example.com/$&') + expect(rewriteToConfiguredHost('https://comms.todoist.com/a/1/')).toBe( + 'https://staging.example.com/$&/a/1/', + ) + }) + + // Trailing slash in COMMS_BASE_URL must not double up. + it('strips a trailing slash from the configured base URL', () => { + configureBaseUrl('https://comms.staging.todoist.com/') + expect(rewriteToConfiguredHost('https://comms.todoist.com/a/1/')).toBe( + 'https://comms.staging.todoist.com/a/1/', + ) + }) + }) +}) diff --git a/src/utils/server-options.ts b/src/utils/server-options.ts new file mode 100644 index 0000000..c95ffbc --- /dev/null +++ b/src/utils/server-options.ts @@ -0,0 +1,20 @@ +// Kept separate from main.ts so the env wiring is unit-testable — +// importing main.ts triggers server startup at module load. + +export type ServerOptions = { + commsApiKey: string + // Optional so external callers (`getMcpServer({ commsApiKey })`) + // don't have to spell out `baseUrl: undefined`. + baseUrl?: string +} + +export function buildServerOptions(env: NodeJS.ProcessEnv = process.env): ServerOptions { + const commsApiKey = env.COMMS_API_KEY + if (!commsApiKey) { + throw new Error('COMMS_API_KEY is not set') + } + // Trim before the falsy check so `COMMS_BASE_URL= ` (trailing + // space, common in env files) doesn't pass garbage to the SDK. + const baseUrl = env.COMMS_BASE_URL?.trim() || undefined + return { commsApiKey, baseUrl } +} diff --git a/src/utils/url-helpers.ts b/src/utils/url-helpers.ts index 0f5b497..83e9c3d 100644 --- a/src/utils/url-helpers.ts +++ b/src/utils/url-helpers.ts @@ -1,4 +1,56 @@ -import { getFullCommsURL } from '@doist/comms-sdk' +import { + getCommentURL as sdkGetCommentURL, + getFullCommsURL as sdkGetFullCommsURL, + getMessageURL as sdkGetMessageURL, +} from '@doist/comms-sdk' + +// Module-level state set by `getMcpServer` (and scripts/run-tool.ts) +// at startup. The SDK's URL helpers hardcode the prod host, so without +// a wrapper a staging-targeted MCP would still return prod links in +// its tool output. +// +// **Process-scoped, last-call wins** — not multi-tenant safe. Library +// consumers using importable tools standalone must call this +// themselves; otherwise the helpers default to prod. +let configuredBaseUrl: string | undefined + +export function configureBaseUrl(baseUrl: string | undefined): void { + // Normalize trailing slash so `https://staging/` doesn't produce + // double-slashed URLs like `https://staging//a/1/`. + configuredBaseUrl = baseUrl ? baseUrl.replace(/\/+$/, '') : undefined +} + +// Match the SDK's prod host (case-insensitive, with a trailing +// boundary so `comms.todoist.com.evil.com` does NOT match — prevents +// host-injection if a url field is ever attacker-influenced). +const PROD_HOST_RE = /^https:\/\/comms\.todoist\.com(?=[/:?#]|$)/i + +function applyBaseUrl(url: string): string { + if (!configuredBaseUrl) return url + // Use a function replacement so `$` in `configuredBaseUrl` isn't + // interpreted as a backreference token. + return url.replace(PROD_HOST_RE, () => configuredBaseUrl as string) +} + +// Strip whatever host the SDK produced — robust to staging/custom hosts. +export function toRelativeCommsURL(url: string): string { + return url.replace(/^https?:\/\/[^/]+/, '') +} + +// Rewrite SDK-populated url fields (entity.url, etc.) to the configured +// host. The SDK entity schemas hardcode prod, so a staging-targeted +// server would otherwise return prod links in its structured output. +// Idempotent: re-applying on an already-staging URL is a no-op. +export const rewriteToConfiguredHost = applyBaseUrl + +export const getFullCommsURL: typeof sdkGetFullCommsURL = (...args) => + applyBaseUrl(sdkGetFullCommsURL(...args)) + +export const getCommentURL: typeof sdkGetCommentURL = (...args) => + applyBaseUrl(sdkGetCommentURL(...args)) + +export const getMessageURL: typeof sdkGetMessageURL = (...args) => + applyBaseUrl(sdkGetMessageURL(...args)) export function getWorkspaceUrl(workspaceId: number): string { return getFullCommsURL({ workspaceId })