Skip to content

fix(slack): replace Slack MCP stdio client with direct web API#1066

Merged
saddlepaddle merged 1 commit intomainfrom
satya-patel/replace-slack-mcp
Jan 30, 2026
Merged

fix(slack): replace Slack MCP stdio client with direct web API#1066
saddlepaddle merged 1 commit intomainfrom
satya-patel/replace-slack-mcp

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Jan 30, 2026

Summary

  • The Slack MCP client spawned npx @modelcontextprotocol/server-slack via stdio transport, which fails on Vercel serverless (no writable home dir, npm can't cache packages, no persistent child processes)
  • Replaced with a single slack_get_channel_history tool using @slack/web-api directly
  • Superset MCP client (in-memory transport) is unchanged

Test plan

  • Deploy to preview and trigger a Slack mention — verify the agent responds without MCP connection errors
  • Verify the agent can still use Superset tools (create task, list tasks, etc.)
  • Verify channel history tool returns recent messages when the agent uses it

Summary by CodeRabbit

  • New Features

    • Slack integration now includes direct channel message retrieval, enabling access to recent messages for enhanced agent context awareness.
  • Bug Fixes

    • Streamlined Slack agent parameter configuration and dependency handling.

✏️ Tip: You can customize this high-level summary in your review settings.

The Slack MCP client used StdioClientTransport to spawn `npx
@modelcontextprotocol/server-slack` as a child process. This fails on
Vercel serverless (no writable home dir, no persistent processes).

Replace with a single `slack_get_channel_history` tool implemented
directly via @slack/web-api. The Superset MCP client (in-memory
transport) is unaffected.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

The changes remove the Slack MCP client integration and replace it with a simpler built-in tool for fetching channel history. The slackTeamId parameter is removed from agent invocations across all call sites, and the agent runner now exclusively instantiates the Superset MCP client while inlining Slack channel history functionality as an internal tool.

Changes

Cohort / File(s) Summary
Agent Call Sites
process-assistant-message.ts, process-mention.ts
Removed slackTeamId parameter from runSlackAgent function calls.
MCP Client Utilities
mcp-clients.ts
Removed Slack MCP client creator function entirely; converted Client import to type-only; retained Superset MCP client and tool utilities.
Agent Runner
run-agent.ts
Removed Slack MCP client initialization, lifecycle management, and tool listing. Introduced SLACK_GET_CHANNEL_HISTORY_TOOL and handleGetChannelHistory handler for inline channel message fetching (100 message limit). Unified tool execution logic with direct handling for Slack history tool and delegated Superset tool invocation. Removed slackTeamId from RunSlackAgentParams interface. Updated system prompt to reference new tool behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops through the code with glee,
Slack's MCP gone, now simple and free,
Channel history built right in with care,
No teamId baggage floating there,
Tools flow unified, clean as can be! 🐰✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description is incomplete; it lacks structured sections matching the template (Type of Change, Testing, Additional Notes are missing or unfilled). Complete the PR description by filling in the 'Type of Change' checklist, detailed 'Testing' section, and any 'Additional Notes' to match the repository template.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: replacing a Slack MCP stdio client with direct web API usage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch satya-patel/replace-slack-mcp

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 30, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch
  • ✅ Electric Fly.io app

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`:
- Around line 159-202: Extract the magic numbers into named constants (e.g.,
DEFAULT_CHANNEL_HISTORY_LIMIT = 20, MAX_CHANNEL_HISTORY_LIMIT = 100,
EFFECTIVE_MAX_CHANNEL_HISTORY_LIMIT = 15) and update
SLACK_GET_CHANNEL_HISTORY_TOOL.input_schema to declare numeric constraints
(integer type, minimum 1, maximum equal to MAX_CHANNEL_HISTORY_LIMIT) and a
documented default; in handleGetChannelHistory, validate and sanitize the
LLM-supplied limit by coercing to an integer, treating
non-numeric/NaN/negative/zero values as DEFAULT_CHANNEL_HISTORY_LIMIT, and clamp
the value to the allowed max (use Math.min(parsedLimit,
MAX_CHANNEL_HISTORY_LIMIT, EFFECTIVE_MAX_CHANNEL_HISTORY_LIMIT) or similar)
before passing it to WebClient.conversations.history to ensure only a safe,
bounded integer is sent.

Comment on lines +159 to +202
const SLACK_GET_CHANNEL_HISTORY_TOOL: Anthropic.Tool = {
name: "slack_get_channel_history",
description:
"Get recent messages from the current Slack channel. Use this to understand what the team has been discussing.",
input_schema: {
type: "object" as const,
properties: {
limit: {
type: "number",
description: "Number of messages to retrieve (default 20, max 100)",
},
},
required: [],
},
};

async function handleGetChannelHistory({
token,
channelId,
limit = 20,
}: {
token: string;
channelId: string;
limit?: number;
}): Promise<string> {
const slack = new WebClient(token);
const result = await slack.conversations.history({
channel: channelId,
limit: Math.min(limit, 100),
});

if (!result.messages || result.messages.length === 0) {
return JSON.stringify({ messages: [] });
}

const messages = result.messages.map((msg) => ({
user: msg.user,
text: msg.text,
ts: msg.ts,
thread_ts: msg.thread_ts,
}));

return JSON.stringify({ messages });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Slack Web API conversations.history limit parameter valid range behavior

💡 Result:

For conversations.history, Slack documents the limit parameter as:

  • Type: number
  • What it does: “maximum number of items to return”; Slack may return fewer than requested even if more history exists. [1]
  • Default: 100 (unless you’re in the reduced-limit category below). [1]
  • Maximum (normal behavior): 999 (and Slack notes using a limit under 1000 for pagination). [1][4]

Important: reduced limit for some non‑Marketplace apps

Slack introduced lower limits for apps commercially distributed outside the Slack Marketplace:

  • For new apps / new installations (effective May 29, 2025): conversations.history is 1 request/minute and the maximum + default limit are reduced to 15. [1][3]
  • Slack’s developer docs indicate the same reduced limits will apply to existing installations starting March 3, 2026. [4][6]

So in practice, the “valid range” depends on the app category: up to 999 normally, but effectively capped at 15 for affected non‑Marketplace commercially distributed apps. [1][3][4]

Sources: Slack method reference and changelog/rate-limit docs. [1][3][4][6]


🏁 Script executed:

# Check the codebase for similar Slack API calls and validation patterns
rg "conversations\.history" -A 5 -B 5

Repository: superset-sh/superset

Length of output: 1179


🏁 Script executed:

# Look for other instances where the code handles LLM-supplied parameters or similar validation
rg "Math\.min|Math\.max|Number\.isFinite" apps/api/src/app/api/integrations/slack -B 3 -A 3

Repository: superset-sh/superset

Length of output: 800


🏁 Script executed:

# Check if there are existing constants for Slack API limits elsewhere in the codebase
rg "LIMIT|HISTORY|SLACK.*20|SLACK.*100" apps/api/src/app/api/integrations/slack -i

Repository: superset-sh/superset

Length of output: 2824


Extract magic numbers to constants and add input validation for LLM-supplied limit parameter.
limit defaults and caps (20, 100) are magic numbers that should be extracted per guidelines. More critically, limit is LLM-supplied and lacks validation—negative, NaN, or non-integer values will be passed directly to the Slack API, potentially causing issues. The Slack API conversations.history accepts values up to 999 (normally) but effective limits are dropping to 15 for non-Marketplace apps starting March 3, 2026.

🔧 Proposed fix
+const DEFAULT_CHANNEL_HISTORY_LIMIT = 20;
+const MAX_CHANNEL_HISTORY_LIMIT = 100;
+
 const SLACK_GET_CHANNEL_HISTORY_TOOL: Anthropic.Tool = {
 	name: "slack_get_channel_history",
 	description:
 		"Get recent messages from the current Slack channel. Use this to understand what the team has been discussing.",
 	input_schema: {
 		type: "object" as const,
 		properties: {
 			limit: {
 				type: "number",
-				description: "Number of messages to retrieve (default 20, max 100)",
+				description: `Number of messages to retrieve (default ${DEFAULT_CHANNEL_HISTORY_LIMIT}, max ${MAX_CHANNEL_HISTORY_LIMIT})`,
 			},
 		},
 		required: [],
 	},
 };
 
 async function handleGetChannelHistory({
 	token,
 	channelId,
-	limit = 20,
+	limit = DEFAULT_CHANNEL_HISTORY_LIMIT,
 }: {
 	token: string;
 	channelId: string;
 	limit?: number;
 }): Promise<string> {
 	const slack = new WebClient(token);
+	const safeLimit = Number.isFinite(limit)
+		? Math.max(1, Math.min(Math.trunc(limit), MAX_CHANNEL_HISTORY_LIMIT))
+		: DEFAULT_CHANNEL_HISTORY_LIMIT;
 	const result = await slack.conversations.history({
 		channel: channelId,
-		limit: Math.min(limit, 100),
+		limit: safeLimit,
 	});
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`
around lines 159 - 202, Extract the magic numbers into named constants (e.g.,
DEFAULT_CHANNEL_HISTORY_LIMIT = 20, MAX_CHANNEL_HISTORY_LIMIT = 100,
EFFECTIVE_MAX_CHANNEL_HISTORY_LIMIT = 15) and update
SLACK_GET_CHANNEL_HISTORY_TOOL.input_schema to declare numeric constraints
(integer type, minimum 1, maximum equal to MAX_CHANNEL_HISTORY_LIMIT) and a
documented default; in handleGetChannelHistory, validate and sanitize the
LLM-supplied limit by coercing to an integer, treating
non-numeric/NaN/negative/zero values as DEFAULT_CHANNEL_HISTORY_LIMIT, and clamp
the value to the allowed max (use Math.min(parsedLimit,
MAX_CHANNEL_HISTORY_LIMIT, EFFECTIVE_MAX_CHANNEL_HISTORY_LIMIT) or similar)
before passing it to WebClient.conversations.history to ensure only a safe,
bounded integer is sent.

@saddlepaddle saddlepaddle merged commit 1cfe9d4 into main Jan 30, 2026
13 checks passed
@Kitenite Kitenite deleted the satya-patel/replace-slack-mcp branch February 1, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant