Repository navigation
Remove MCP: Migrate to REST-based tools-api architecture with project restructure - #15
Merged
Merged
Conversation
Replace Model Context Protocol (MCP) with direct REST API for tool execution. Changes: - Add tools-api service with FastAPI endpoints for 12 filesystem tools, 3 GitHub/dev tools, and 1 web search tool - Implement path translation system (PathMapper) for transparent host<->container path mapping - Add docker-compose configuration with volume mount support and healthchecks - Remove api/web_search.py (555 lines) - web search will be migrated to tools-api separately - Update environment examples with Tools API configuration - Add path security validation with read-write/read-only access control - Update Makefile to include tools_api in linting - Add comprehensive test suite (101 passing tests) - Create docker-compose.override.yml.example for user-specific volume mounts Architecture improvements: - Structured JSON responses instead of text-based MCP protocol - Simplified Docker networking (no bridge/protocol layers) - Better testability with standard HTTP mocking - Path translation allows users to reference natural filesystem paths Migration status documented in docs/path_translation_migration_status.md Implementation plan in docs/implementation_plans/2025-11-23-remove-mcp.md
Restructure the multi-service repository to follow Python best practices with clear service ownership and proper package organization: - Move api/ → reasoning_api/reasoning_api/ (flat layout) - Move tests/ → reasoning_api/tests/ with unit/integration subdirectories - Move prompts/ → reasoning_api/prompts/ - Move alembic/ → reasoning_api/alembic/ with alembic.ini - Move tools_api/ sources → tools_api/tools_api/ package - Organize tools_api/tests/ into unit/integration subdirectories - Remove temporary workspace/ directory (tests use tmp_path) Update configuration for new structure: - Makefile: Service-specific test targets, Alembic commands, updated demo_api - docker-compose.yml: Update volume mount for prompts - Test fixtures: Fix conftest paths, update alembic migration command - Update all import statements from api.* to reasoning_api.* - Update test imports from reasoning_api.tests.* to tests.* Final structure provides clear separation between services while maintaining backward-compatible Makefile targets. All tests pass (493 reasoning unit, 212 reasoning integration, 15 tools tests).
…TP client and enabling dual execution mode (tools-api preferred, MCP legacy fallback). Changes: - Add ToolsAPIClient HTTP client with methods for tool discovery, execution, and prompt rendering - Integrate client into ServiceContainer lifecycle with health check and graceful degradation - Update Tool class to support dual execution modes: tools-api client (preferred) or direct callable (legacy) - Centralize OTEL tracing in Tool.__call__() to eliminate duplicate spans across reasoning agent - Simplify reasoning agent tool execution by removing _execute_single_tool_with_tracing in favor of _safe_execute_tool - Add comprehensive unit and integration tests following existing patterns (11 new tests, all passing) - Fix docker-compose.yml tools-api volumes syntax Testing: - Unit tests verify ServiceContainer initialization, tool client integration, and execution mode preference - Integration tests verify full ReasoningAgent → Tool → ToolsAPIClient flow with mocked responses - All 377 tests passing (374 unit + 3 integration)
Remove all MCP (Model Context Protocol) code and configuration, completing the migration to the tools-api REST architecture. This change removes the MCP bridge, client code, configuration files, and all related tests. The system now uses a clean REST API architecture with the tools-api service. Key changes: - Delete mcp_bridge/ directory and all MCP server code - Remove MCP client from reasoning-api dependencies - Delete MCP configuration files (mcp_servers.json, mcp_bridge_config.json, mcp_overrides.yaml) - Remove MCP-related test classes (600+ lines) - Update documentation to reflect tools-api architecture Architecture changes: - Before: reasoning-api → mcp_bridge (host) → stdio MCP servers - After: reasoning-api (Docker) → tools-api (Docker) → direct implementations Benefits: - Structured JSON responses with metadata - Simpler deployment (single docker compose up) - Better testability (mock HTTP endpoints) - Direct implementations without protocol translation - Improved observability with standard HTTP logs Test results: - All 200 integration tests passing - All 106 unit tests passing - MCP-related tests removed/replaced with tools-api equivalents
shane-kercheval
marked this pull request as draft
November 24, 2025 17:02
…atting - Add path mapper support to GetLocalGitChangesInfoTool for proper host-to-container path translation - Move GetDirectoryTreeTool from github_dev_tools.py to filesystem.py (better categorization) - Add comprehensive error logging to tools router with execution time tracking - Improve error messages to include stderr output for better debugging Client improvements: - Create shared formatToolResult utility to handle JSON formatting with proper newline unescaping - Fix tool result display to render \n as actual newlines instead of escaped characters - Apply formatting consistently in both ToolExecutionDialog and send-to-chat functionality Configuration: - Remove redundant client/.env.example (consolidated into root .env files) - Add toolsApiUrl configuration to Electron preload for tools-API access - Make docker-compose service URLs configurable via environment variables - Update .env example files with consistent structure for Docker internal vs client external URLs
Add WebScraperTool for fetching and parsing web pages into structured, agent-friendly output with numbered link references (Lynx-style). Features: - Extract readable text with [n] link markers - Resolve relative URLs and detect external links - Parse metadata (title, description, language) - Handle errors (timeouts, rate limits, non-HTML content) - Pydantic models for type-safe responses (WebScraperResult, LinkReference) Also includes: - Add beautifulsoup4 and lxml dependencies - Remove lynx from Dockerfile (replaced by pure Python scraper) - Rename BRAVE_SEARCH_API to BRAVE_API_KEY for consistency - Fix test mocking for BraveSearchTool (mock settings before client) - Add skip markers for tests requiring external CLI tools (tree, gh) - Add comprehensive test coverage for filesystem tools: - EditFileTool edge cases (multiple occurrences, special chars, empty) - SearchFilesTool edge cases (no matches, invalid globs, empty pattern) - Concurrent write behavior documentation - Add path traversal security tests - Update VSCode pytest path to tools_api/tests
Major README cleanup: - Remove outdated Option 2 (local development) - Docker-only setup now - Remove Deployment section, Custom System Prompts, web interface references - Add architecture diagram showing all Docker services and data flow - Add clear volume mount instructions for tools-api (read_write/read_only) - Reference Makefile targets instead of raw commands for single source of truth - Simplify sections: Tools API, Desktop Client, Authentication, Troubleshooting - Update observability section to show Phoenix receives OTLP from both reasoning-api and litellm Makefile updates: - Add help target with all available commands - Remove stale references: mcp_bridge, mcp_servers, demo_mcp_server - Update linting to only check existing directories - Fix docker_up output to show correct service names and ports - Rename cleanup to kill_servers Delete obsolete documentation: - README_ARCHITECTURE.MD (content consolidated into main README) - README_DOCKER.md (setup instructions now in main README)
Implement a file-based prompt system that allows defining prompts as Markdown files with YAML frontmatter, loaded from a configurable directory. Key changes: - Add category field to tools and prompts for better organization - Rename PromptDefinition to PromptInfo, change PromptResult.messages to content - Create PromptTemplate class with Jinja2 rendering (ChainableUndefined for silent empty strings on undefined variables) - Add parser for Markdown files with YAML frontmatter and loader for recursive directory scanning - Add PROMPTS_DIRECTORY config option with fail-fast on missing directory or duplicate prompt names - Update docker-compose.yml with prompts volume mount Breaking changes: - PromptResult now returns content: str instead of messages: list[dict] - BasePrompt.render() returns str instead of message list New dependencies: jinja2, python-frontmatter
Client changes:
- Rename MCP* types to simpler names (MCPPrompt -> Prompt, MCPTool -> Tool, MCPToolResult -> ToolExecutionResult, MCPPromptArgument -> PromptArgument)
- Rename API methods (listMCPPrompts -> listPrompts, executeMCPTool -> executeTool, etc.)
- Rename hooks (useMCPPrompts -> usePrompts, useMCPTools -> useTools)
- Fix prompt execution to use REST API response format ({content} not {messages})
- Add toolsApiUrl to ElectronAPI type definition
Tools API changes:
- Remove GreetingPrompt hardcoded registration from startup
- Add greeting.md fixture for tests that depend on greeting prompt
- Document prompt file format and PROMPTS_HOST_PATH in README
Documentation:
- Add prompts directory setup instructions to Quick Start (step 7)
- Document PROMPTS_HOST_PATH in .env.dev.example
Other:
- Fix reasoning_api Dockerfile to copy prompts directory
- Update prompt_manager default path for Docker context
- Clarify next_action behavior in reasoning system prompts
Refactor web search tool to use provider-agnostic naming: - Rename BraveSearchTool class to WebSearchTool - Update tool name from 'brave_search' to 'web_search' - Remove Brave-specific references from description and tags - Update all imports and test references - Add integration tests for web search functionality - Configure test environment to load .env for API credentials - Fix notebook imports to use correct tools_api package path
Implement typed output schemas for tools API so clients know what to expect from tool results: - Add abstract result_model property to BaseTool that returns a Pydantic model class - Add derived output_schema property that auto-generates JSON Schema via model_json_schema() - Add output_schema field to ToolDefinition model and /tools/ endpoint response - Create Pydantic result models for all tools: - File system tools (15+ models): ReadTextFileResult, WriteFileResult, EditFileResult, etc. - GitHub tools: GetGitHubPullRequestInfoResult, GetLocalGitChangesInfoResult - Web tools: WebSearchResult, WebScraperResult with nested models - Example tool: EchoResult - Update all _execute() methods to return typed Pydantic models instead of dicts - Update tests to use dict access on result.result since results are now serialized via model_dump()
Remove model_dump() call from BaseTool.__call__() so ToolResult.result contains the actual Pydantic model instance rather than a serialized dict. This enables proper type-safe attribute access (result.result.field) instead of dict subscripting (result.result["field"]). Changes: - services/base.py: Remove model_dump() - store Pydantic model directly - mcp_server.py: Add handling to serialize Pydantic models via model_dump() when returning JSON to MCP clients - Update all unit tests to use attribute access on result.result - Fix path translation in GetDirectoryTreeTool and GetLocalGitChangesInfoTool error messages
- Install gh CLI and jq in tools-api Dockerfile to enable GitHub PR info tool functionality - Fix UTF-8 decoding errors in github_dev_tools.py by using errors="replace" when decoding subprocess output
Switch PromptTemplate to StrictUndefined mode for fail-fast behavior on undefined variables instead of silently rendering empty strings.
Changes:
- Validate unknown arguments passed to render() to catch typos early
- Auto-populate optional arguments with None so {% if arg %} conditionals
work correctly with StrictUndefined
- Add comprehensive tests for unknown argument validation and edge cases
- Format tool execution logging with JSON pretty-printing
This improves developer experience by surfacing errors immediately
rather than producing silently incorrect output.
In stateless MCP mode, when a session terminates after responding, the message_router task logs ClosedResourceError when trying to read from the closed stream. This is expected behavior but appears as ERROR in logs. - Add MCPClosedResourceFilter class to suppress these benign errors - Apply filter to mcp.server.streamable_http logger
- Prefix tool descriptions with `[category]` when a category is set, making it easier for clients to identify tool groupings - Add type hints (`Any`) to `**kwargs` parameters in `BaseTool` and `BasePrompt` abstract methods - Use `ClassVar` type annotation for registry class variables to fix type checker warnings - Move `MAX_FILE_SIZE` constant to module level in `file_system.py` - Add missing docstrings to `GetDirectoryTreeTool` property methods - Fix various line length violations with `noqa` comments
shane-kercheval
changed the title
shane-kercheval
marked this pull request as ready for review
November 29, 2025 20:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace Model Context Protocol (MCP) architecture with a direct REST API (
tools-api) for tool execution, along with project restructuring and new features.Changes
tools-apiFastAPI service providing 16+ tools (filesystem, GitHub, web search/scraper)reasoning_api/,tools_api/) with proper Python package layoutPathMappersystem for transparent host↔container path mapping with read-write/read-only access controlWebScraperTool(BeautifulSoup-based) with numbered link references, typed output schemas via Pydantic models for all toolsStrictUndefined)MCPTool→Tool,MCPPrompt→Prompt, etc.)Testing
Impact
Breaking changes:
PromptResultreturnscontent: strinstead ofmessages: list[dict]api.*toreasoning_api.*New dependencies:
jinja2,python-frontmatter,beautifulsoup4,lxmlDeployment: Docker-only setup; requires
PROMPTS_HOST_PATHconfiguration and volume mounts for tools-api read/write access