Skip to content

feat: expose Extra as an MCP server via stdio - #126

Closed
Karn2898 wants to merge 0 commit into
extra-org:mainfrom
Karn2898:main
Closed

feat: expose Extra as an MCP server via stdio#126
Karn2898 wants to merge 0 commit into
extra-org:mainfrom
Karn2898:main

Conversation

@Karn2898

Copy link
Copy Markdown
Contributor

This adds a new Agentctl mcp serve command that starts an MCP server over stdio, exposing a single extra_chat tool backed by the existing ConversationService and engine.

The MCP layer is intentionally thin — it builds the engine and DB connection once at startup, reuses them for every request, and cleans them up on shutdown (including SIGINT/SIGTERM and stdio close). Session handling mirrors the existing �gentctl run behavior: omit session_id to generate a fresh conversation, or pass one to continue an existing one. Input validation happens before any engine work, and errors are returned as proper MCP tool errors.

Tests added:

  • 17 unit tests for the MCP adapter (input validation, response mapping, tool listing, error handling)
  • 4 integration tests that spin up the server in-process with an anyio memory stream connected to the MCP client SDK, verifying the full stdio round trip, session reuse, session isolation, and error paths
  • 2 CLI tests confirming mcp serve wires up correctly and rejects invalid config

An example under examples/mcp_server/ shows how to start the server and connect to it with a Python MCP client.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for the contribution — I really like the direction of exposing Extra as an MCP server over stdio, and keeping the MCP layer thin around the existing ConversationService is the right approach.

There are a few blockers I think we should address before merging:

  1. The MCP adapter is using the old ConversationService API.

The current service expects a Principal:

caller = Principal.external(effective_user_id)

await service.create(caller, session_id=effective_session_id)
result = await service.send(effective_session_id, message, caller)

The MCP implementation currently passes user_id= directly instead. Besides being incompatible with the current API, this bypasses the identity boundary that the rest of the application now relies on.

Please reuse the same identity flow already used by agentctl run.

  1. The MCP runtime wiring should match the existing CLI runtime.

agentctl run passes the shared repositories into the engine:

LangGraphEngine(
    base_dir,
    session_approval_repository=repositories.session_approvals,
    tool_usage_repository=repositories.tool_usage,
    run_repository=repositories.runs,
)

and also passes run_repository=repositories.runs to ConversationService.

The MCP server currently only wires the session approval repository, which means run/tool-usage state can silently fall back to process-local in-memory implementations.

I would strongly prefer extracting/reusing the existing runtime composition rather than maintaining a second, slightly different setup path for MCP.

  1. Persistent DB migrations are missing.

agentctl run calls upgrade_database() before starting, while mcp serve currently does not.

application_repositories() does not perform migrations itself, so a fresh SQLite/Postgres deployment can fail on the first request because the tables do not exist.

  1. The example agents.yml does not appear to be runnable.

echo_agent is referenced in graph, but it is not declared under agents or orchestrators.

Since this example is the entry point for the new feature, it should pass the normal validation and be directly runnable.

  1. We need an explicit decision around HITL approvals.

ConversationService.send() can return a pending approval with an empty answer, but extra_chat currently only returns:

{
  "session_id": "...",
  "answer": "...",
  "visited": [],
  "used_tools": []
}

So a run that pauses for approval may look like an empty successful response to the MCP client.

Either MCP mode should expose approval state and a way to resume it, or the initial version should explicitly support only auto-execution and enforce/document that limitation.

Finally, the current CI is failing in the quality gate, starting with formatting, so that needs to be green before approval.

The feature itself is valuable and the overall architecture is promising — I’d just like the MCP entry point to reuse the same identity, persistence, and runtime composition guarantees as the existing CLI rather than becoming a parallel runtime path.

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.

2 participants