Skip to content

feat: inject per-user MCP servers into ACP sessions#345

Closed
Reese-max wants to merge 7 commits into
openabdev:mainfrom
Reese-max:feat/mcp-session-bridge
Closed

feat: inject per-user MCP servers into ACP sessions#345
Reese-max wants to merge 7 commits into
openabdev:mainfrom
Reese-max:feat/mcp-session-bridge

Conversation

@Reese-max
Copy link
Copy Markdown

What problem does this solve?

Users can manage MCP servers via Discord slash commands such as /mcp-add, /mcp-remove, and /mcp-list, but those profile entries are never injected into ACP sessions. The profile JSON is written, but the backend session still starts without mcpServers, so /mcp-add is a dead-end UI.

This change closes the loop from Discord MCP profile management to runtime MCP activation.

At a Glance

Discord user
  |
  | /mcp-add mempalace http://...
  v
profile JSON: data/mcp-profiles/{bot}/{user_id}.json
  |
  v
message / slash command
  |
  v
discord.rs -> mcp_servers_for_user(user_id)
  |
  v
pool.get_or_create(thread, mcp_servers)
  |
  v
connection.session_new/session_load(..., mcp_servers)
  |
  v
ACP JSON-RPC session/new | session/load
  { cwd, mcpServers: [{ name, type, url, headers }] }
  |
  v
Claude / Copilot / Gemini / Codex session loads MCP tools

Prior Art

OpenClaw

OpenClaw merges MCP config from bundle defaults plus global config and injects it at session/runtime creation. For CLI backends it relies on backend-specific config/CLI arg injection such as --mcp-config, not ACP session/new.

Relevant difference:

  • Scope is global, not per user.
  • Injection is backend-specific.
  • Good reference for runtime lifecycle and config invalidation, but not for Discord multi-user isolation.

Hermes Agent

Hermes supports mcp_servers on ACP session creation, but tools are registered into a process-wide registry rather than isolated per Discord user/session.

Relevant difference:

  • Uses ACP-style injection similar to this PR.
  • Registry is global, so one user's MCP setup can affect others.
  • Better fit for ACP transport shape, weaker fit for Discord multi-user isolation.

Comparison

Aspect OpenClaw Hermes Agent OpenAB in this PR
MCP config scope Global Global/process-wide Per-user per-bot
Injection point Config merge + CLI args ACP session params ACP session params
User isolation No No Yes
Management surface Chat / CLI CLI Discord slash commands
Activation timing Runtime/config reload Session/runtime Next session / /new-session

Proposed Solution

  1. Add McpServerEntry and read_mcp_profile() in src/config.rs to read {mcp_profiles_dir}/{user_id}.json.
  2. Extend session_new() / session_load() in src/acp/connection.rs to accept mcp_servers and send them through ACP.
  3. Extend SessionPool::get_or_create() in src/acp/pool.rs to thread mcp_servers into new or resumed sessions.
  4. In src/discord.rs, resolve the calling user's MCP profile and inject it into message handling and session-creating slash commands.
  5. Keep diagnostic commands on &[] so health/status flows do not depend on user MCP state.

Profile JSON stays in the existing Discord-managed format:

{
  "discord_user_id": "844236700611379200",
  "mcpServers": {
    "mempalace": {
      "type": "http",
      "url": "http://127.0.0.1:18793/mcp",
      "headers": []
    }
  },
  "enabled": true
}

Converted at runtime to ACP format:

[
  {
    "name": "mempalace",
    "type": "http",
    "url": "http://127.0.0.1:18793/mcp",
    "headers": []
  }
]

Why this approach?

  • ACP-native: all target backends accept mcpServers on session creation.
  • Per-user isolation: MCP config is keyed by Discord user ID, not global process state.
  • Per-bot isolation: each bot keeps its own mcp_profiles_dir.
  • Graceful fallback: missing or invalid profile returns an empty list.
  • No backend-specific config mutation: avoids touching ~/.claude*, Copilot config files, or other local machine state.

Alternatives Considered

  1. Backend-specific config file mutation.
    Rejected because it is fragile, backend-specific, and risks overwriting user-managed config.

  2. Global singleton MCP registry.
    Rejected because OpenAB is a shared Discord bot process and would leak one user's MCP tools into another user's sessions.

  3. In-session hot reload.
    Deferred because the current ACP/session model is simpler and the UX is already covered by “next session” plus /new-session.

Validation

Current branch validation:

  • cargo check
  • cargo build --release
  • cargo test (42 passed, 0 failed)
  • cargo fmt --check
  • Branch diff excludes personal bat / local profile / runtime script files

Previously validated during the implementation session:

  • Claude ACP E2E: profile -> session/new(mcpServers) -> tool discovery -> mempalace_search
  • Backend format verification for Claude / Copilot / Gemini / Codex
  • /mcp-add UX updated to note that changes apply on next session and /new-session can apply immediately

Notes

This PR branch also includes a small follow-up cleanup to remove dead-code warnings in the model picker path so the branch stays warning-free during review.

isxreese and others added 7 commits April 14, 2026 20:58
… sessions

When a user adds MCP servers via /mcp-add, the servers are now automatically
injected into ACP session/new and session/load calls. This completes the
"UI management → runtime activation" loop.

Changes:
- config.rs: add McpServerEntry struct + read_mcp_profile() + mcp_profiles_dir config
- connection.rs: session_new/session_load accept mcp_servers parameter + cfg(unix) guards
- pool.rs: get_or_create passes mcp_servers through + saturating_duration_since fix
- discord.rs: mcp_servers_for_user() reads profile dir, message handler injects
- main.rs: handler gets mcp_profiles_dir

Verified: all 4 backends (Claude, Copilot, Gemini, Codex) accept the unified
[{name, type:"http", url, headers:[]}] format. E2E tested with Claude ACP.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- mcp_servers_for_user() now async with spawn_blocking (no blocking I/O on Tokio)
- Log level downgraded info → debug (avoid per-message log spam)
- Non-object MCP profile entries skipped with warning (validation)
- kill_on_drop(true) added to ACP child spawn (Windows cleanup)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 14, 2026
@Reese-max Reese-max closed this Apr 14, 2026
@Reese-max Reese-max deleted the feat/mcp-session-bridge branch April 14, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants