Fix: pass httpHeaders to streamable HTTP transport

[jimmynail]

Summary

• HTTP (streamable) transport now passes httpHeaders from config to the MCP client via transport.WithHTTPHeaders()
• Previously only SSE transport forwarded custom headers; HTTP transport ignored them

Problem

MCP servers requiring Bearer token auth (e.g., Omni) fail with Authentication required when using the http transport because headers aren't forwarded.

Fixes #166

Test plan

• Verify HTTP transport connects to an MCP server with Bearer auth
• Verify SSE transport still works unchanged

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Bot now tracks active threads so follow-up messages in threads are recognized and handled.

• Bug Fixes

  • Custom HTTP headers are now included in outgoing streamable HTTP requests.
  • Slack callback handling ignores intermediate tool-output, strips leading "AI:" from final replies, and only sends non-empty final messages.
  • Tool failures are surfaced as structured tool responses (not runtime errors), improving agent resilience.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds passing of configured HTTP headers into the streamable MCP HTTP transport; adds Slack client thread-tracking for follow-ups and hardens final assistant-text extraction/filtering before sending.

Changes

Cohort / File(s)	Summary
MCP HTTP transport internal/mcp/client.go	Import transport, convert resolvedHeaders into map[string]string, and pass them to the streamable HTTP client so http transport includes configured HTTP headers.
Slack client & callback handling internal/slack/client.go, internal/slack/agentCallbackHandler.go	Add activeThreads map[string]bool and initialize it; track app-mention threads and treat messages in tracked threads as follow-ups. Refactor HandleChainEnd to safely read outputs["text"], skip intermediate tool messages, strip leading AI: prefix, and only send non-empty final text.
MCP Tool error handling internal/mcp/mcpTool.go	On tool call failure, embed the error into the tool result string (prefixed with [Tool Error] ...) and return nil error so the agent loop receives the error as text rather than a Go error.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Pr/jescarri/98 #109 — Adds HTTP header propagation to MCP HTTP/SSE client (directly related to internal/mcp/client.go changes).
• Feature- Added Thread and User Context, Unique tool name #83 — Modifies MCP client transport setup and headers handling (related to HTTP transport changes).
• Chore/v0.0.8 #91 — Changes internal/slack/client.go and Client wiring (potential overlap with activeThreads addition).

Suggested reviewers

• tuannvm

Poem

"I hopped through headers, light and fleet,
Carried tokens where servers meet.
I keep the threads both warm and bright,
Trim the fluff, deliver right.
🥕 — a rabbit, coding through the night."

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR partially addresses #166's core requirement but introduces unrelated changes to Slack handler logic (agentCallbackHandler.go, client.go) and tool error handling (mcpTool.go) beyond the HTTP headers fix.	The PR should focus on the HTTP headers fix for #166. Separate the Slack and tool error handling changes into distinct PRs to maintain clear issue-to-code traceability.
Out of Scope Changes check	⚠️ Warning	Multiple unrelated changes are included: Slack thread tracking (activeThreads map), Slack message filtering logic, and tool error string embedding—none of which relate to the HTTP headers transport fix in #166.	Extract Slack callback/client and mcpTool.go changes into separate feature branches and PRs to keep this PR focused on resolving the #166 issue.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly addresses the main change: passing httpHeaders to HTTP transport, which is the primary fix in internal/mcp/client.go.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@internal/slack/client.go`:
- Line 40: activeThreads currently uses map[string]bool and is never evicted,
causing unbounded growth; change it to store timestamps (e.g.,
map[string]time.Time or map[string]int64) instead of bool, update the code paths
that insert keys (the AppMentionEvent handling where entries are added) to write
time.Now(), and add a pruning strategy — either a simple TTL-based prune helper
that removes entries older than N (called periodically via a background
goroutine or lazily on writes/reads), or replace with a bounded LRU structure;
ensure all access to activeThreads is protected by the existing mutex (or add
one) when reading/writing to avoid races.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Unbounded growth of activeThreads map — entries are added but never evicted.

Every AppMentionEvent inserts a key into activeThreads, but there is no expiration, size cap, or cleanup path. For a long-running bot in an active workspace this map will grow indefinitely, leaking memory.

Consider one of:

• A TTL-based eviction (e.g., remove entries older than N hours using a timestamp value instead of bool).
• A bounded LRU / ring buffer.
• Periodic pruning in a background goroutine.

Example: store timestamps and add a simple pruning helper

-	activeThreads   map[string]bool // channel:threadTS keys where bot has been mentioned
+	activeThreads   map[string]time.Time // channel:threadTS → last-activity time

-		c.activeThreads[threadKey] = true
+		c.activeThreads[threadKey] = time.Now()

-			isActiveThread = c.activeThreads[threadKey]
+			if ts, ok := c.activeThreads[threadKey]; ok && time.Since(ts) < 24*time.Hour {
+				isActiveThread = true
+			}

And periodically (or lazily) prune stale entries.

Also applies to: 269-273

🤖 Prompt for AI Agents

In `@internal/slack/client.go` at line 40, activeThreads currently uses
map[string]bool and is never evicted, causing unbounded growth; change it to
store timestamps (e.g., map[string]time.Time or map[string]int64) instead of
bool, update the code paths that insert keys (the AppMentionEvent handling where
entries are added) to write time.Now(), and add a pruning strategy — either a
simple TTL-based prune helper that removes entries older than N (called
periodically via a background goroutine or lazily on writes/reads), or replace
with a bounded LRU structure; ensure all access to activeThreads is protected by
the existing mutex (or add one) when reading/writing to avoid races.