Skip to content
This repository was archived by the owner on Apr 12, 2026. It is now read-only.

fix: skip Slack context parameters in tool call execution #124

Open
domnikl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: skip Slack context parameters in tool call execution #124

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions internal/handlers/llm_mcp_bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,12 @@ func (b *LLMMCPBridge) getClientForTool(toolName string) mcp.MCPClientInterface
// executeToolCall executes a detected tool call (using the new ToolCall struct)
func (b *LLMMCPBridge) executeToolCall(ctx context.Context, toolCall *ToolCall, extraArgs map[string]interface{}) (string, error) {
for k, v := range extraArgs {
// Skip Slack context parameters as they're for internal tracking, not tool parameters
if k == "channel_id" || k == "thread_ts" {
b.logger.DebugKV("Skipping Slack context parameter", "key", k, "tool", toolCall.Tool)
continue
}
Comment on lines +445 to +449
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Sep 9, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Good guard, but also scrub these keys from toolCall.Args to fully meet the PR objective

Right now we only skip adding Slack context from extraArgs. If the LLM (or upstream) already put channel_id or thread_ts into toolCall.Args, they’ll still be sent to MCP. Scrub them from toolCall.Args as well and centralize the denylist to avoid drift.

Apply:

 func (b *LLMMCPBridge) executeToolCall(ctx context.Context, toolCall *ToolCall, extraArgs map[string]interface{}) (string, error) {
-	for k, v := range extraArgs {
-		// Skip Slack context parameters as they're for internal tracking, not tool parameters
-		if k == "channel_id" || k == "thread_ts" {
-			b.logger.DebugKV("Skipping Slack context parameter", "key", k, "tool", toolCall.Tool)
-			continue
-		}
+	// Denylist of Slack context keys that must never be forwarded to MCP tools
+	disallowedKeys := map[string]struct{}{
+		"channel_id": {},
+		"thread_ts":  {},
+	}
+
+	// Also scrub these keys if they were present in the LLM-supplied args
+	if toolCall.Args != nil {
+		for k := range disallowedKeys {
+			if _, ok := toolCall.Args[k]; ok {
+				b.logger.DebugKV("Removing disallowed argument from tool args", "key", k, "tool", toolCall.Tool)
+				delete(toolCall.Args, k)
+			}
+		}
+	}
+
+	for k, v := range extraArgs {
+		// Skip Slack context parameters as they're for internal tracking, not tool parameters
+		if _, disallowed := disallowedKeys[k]; disallowed {
+			b.logger.DebugKV("Skipping Slack context parameter", "key", k, "tool", toolCall.Tool)
+			continue
+		}
 
 		// Add any extra arguments to the tool call args
 		if toolCall.Args == nil {
 			toolCall.Args = make(map[string]interface{})
 		}
 		toolCall.Args[k] = v
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Skip Slack context parameters as they're for internal tracking, not tool parameters
if k == "channel_id" || k == "thread_ts" {
b.logger.DebugKV("Skipping Slack context parameter", "key", k, "tool", toolCall.Tool)
continue
}
func (b *LLMMCPBridge) executeToolCall(ctx context.Context, toolCall *ToolCall, extraArgs map[string]interface{}) (string, error) {
// Denylist of Slack context keys that must never be forwarded to MCP tools
disallowedKeys := map[string]struct{}{
"channel_id": {},
"thread_ts": {},
}
// Also scrub these keys if they were present in the LLM-supplied args
if toolCall.Args != nil {
for k := range disallowedKeys {
if _, ok := toolCall.Args[k]; ok {
b.logger.DebugKV("Removing disallowed argument from tool args", "key", k, "tool", toolCall.Tool)
delete(toolCall.Args, k)
}
}
}
for k, v := range extraArgs {
// Skip Slack context parameters as they're for internal tracking, not tool parameters
if _, disallowed := disallowedKeys[k]; disallowed {
b.logger.DebugKV("Skipping Slack context parameter", "key", k, "tool", toolCall.Tool)
continue
}
// Add any extra arguments to the tool call args
if toolCall.Args == nil {
toolCall.Args = make(map[string]interface{})
}
toolCall.Args[k] = v
}
// ... rest of executeToolCall ...
}
🤖 Prompt for AI Agents 
In internal/handlers/llm_mcp_bridge.go around lines 445 to 449, the current
guard skips adding Slack context keys from extraArgs but does not remove them if
they already exist in toolCall.Args; update the code to centralize a denylist
(e.g., a package-level slice or map containing "channel_id" and "thread_ts") and
before sending to MCP iterate that denylist to delete those keys from
toolCall.Args (and extraArgs if still needed), keeping the existing DebugKV log
when a key is found and removed; ensure the denylist is reused wherever similar
filtering is required to avoid drift.


// Add any extra arguments to the tool call args
if toolCall.Args == nil {
toolCall.Args = make(map[string]interface{})
Expand Down