Refactor server architecture to separate concerns and enable stdio transport #172
Conversation
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
… stdio - Added privacy utilities for telemetry to redact identifiable information for people who opt-in to additional telemetry.
…ddleware package to a subpackage within mcp package from root-level. Separated CodeWeaver management services from fastmcp initialization and instance creation. Not done with this refactor -- goal is separating business logic from the mcp transports, which will allow use of stdio transport (which needs to spawn multiple instances).
- Renamed AppState to CodeWeaverState throughout the codebase for clarity and consistency. - Updated ToolRegistrationDict to enforce required fields and allow optional fields. - Removed app_bindings.py as it was no longer needed; integrated functionality directly into management.py. - Adjusted health endpoint and lifespan management to utilize the new CodeWeaverState. - Updated tests to reflect changes in state management and tool registration. - Enhanced documentation to clarify the purpose and constraints of CodeWeaverState. - Increased cache duration for favicon to 72 hours.
Co-authored-by: bashandbone <[email protected]>
…roken references Co-authored-by: bashandbone <[email protected]>
Co-authored-by: bashandbone <[email protected]>
Fix MCP server refactor: add ServerSetup, app_bindings, fix broken imports
…er.py into multiple modules and move health related modules to their own package in 'server.health'. Completed 'mcp' package and http server creation logic.
…issues from the refactors. 1. Created Two Separate Lifespan Functions (src/codeweaver/server/lifespan.py): - background_services_lifespan() - For daemon mode (no MCP server dependency) - http_lifespan() - For HTTP MCP server mode (composes background lifespan) - combined_lifespan - Backward compatibility alias 2. Fixed Circular Import (src/codeweaver/server/management.py): - Moved CodeWeaverState import to TYPE_CHECKING block - Used quoted type annotation in __init__ method 3. Fixed Dataclass PrivateAttr Syntax (src/codeweaver/server/server.py): - Updated telemetry, _mcp_http_server, and _tasks fields to use Annotated properly
… clarity and testability. Clarified docs.
…nagers; fixed incorrect signal handling in main.py
…s; clean up imports and logging configuration
Co-authored-by: bashandbone <[email protected]>
Co-authored-by: bashandbone <[email protected]>
…rage Fix IndexingStats computed_field usage and CodeWeaverState fixture initialization
…ersion scenarios; fix several flaky tests
…dling in CodeWeaverSettings - Updated AWSProviderSettings docstring to clarify the need for region-specific credentials for Bedrock models. - Removed redundant phrases in AWS access key, secret access key, and session token descriptions. - Added a new class method 'python_json_schema' to CodeWeaverSettings for better JSON validation schema retrieval. - Modified 'json_schema' method to utilize the new 'python_json_schema' method and adjusted the schema path handling in 'save_schema'.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @bashandbone, your pull request is larger than the review limit of 150000 diff characters
Contributor
|
👋 Hey @bashandbone, Thanks for your contribution to codeweaver! 🧵You need to agree to the CLA first... 🖊️Before we can accept your contribution, you need to agree to our Contributor License Agreement (CLA). To agree to the CLA, please comment:
Those exact words are important1, so please don't change them. 😉 You can read the full CLA here: Contributor License Agreement ✅ @bashandbone has signed the CLA. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. Footnotes
|
Fixed bug where `_extracted_from__prepare_vectors_34()` (a refactoring artifact) was called instead of the correct method `_prepare_sparse_vector_data()`. This was causing sparse-only vector searches to fail with 0 results because the method didn't exist, leading to errors during vector preparation. Fixes #115 Co-authored-by: Adam Poulemanos <[email protected]>
- Change Qdrant health check endpoint from /health (404) to /healthz - Update docker-compose.yml healthcheck to use correct /healthz endpoint - Replace timeout-based waits with retry loops with max attempts - Add better error messages and logging for debugging - Keep continue-on-error for CodeWeaver (may fail without valid API keys) Co-authored-by: bashandbone <[email protected]>
…env vars - Use raw bash TCP/HTTP check in docker-compose since curl not in Qdrant image - Extract health check URLs to environment variables for maintainability - Check /healthz endpoint for proper HTTP 200 status Co-authored-by: bashandbone <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Adam Poulemanos <[email protected]>
Co-authored-by: bashandbone <[email protected]>
Co-authored-by: bashandbone <[email protected]>
…ed failures with this action's step 2
…ed failures with this action's step 2
…once' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Adam Poulemanos <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 115 out of 128 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Adam Poulemanos <[email protected]>
Signed-off-by: Adam Poulemanos <[email protected]>
Contributor
Author
|
All tests pass. CLA check is faulty (expecting bot signatures). Everything looks solid. Time to merge! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Comprehensive Server Refactor
This branch refactors services within the server architecture to improve separation and maintability/testability, and enable use of mcp stdio transport protocol.
Background
CodeWeaver's FastMCP server was previously tightly coupled to core CodeWeaver services -- like indexing/chunking/embedding generation, and file change watching. This made it very difficult to run any core services without running all services. Additionally, CodeWeaver exposed several endpoints through the FastMCP server (using its FastMCP.custom_route() method), which further required the mcp server to always run when taking any action.
The bigger problem was this practically prevented use of the mcp stdio transport, because the stdio transport has several characteristics that make it unique: 1) clients create stdio servers and control their lifetimes, 2) All stdio sessions are fully isolated -- there's no shared state. The tight coupling of codeweaver, combined with the lack of shared state, meant potentially destructive and minimally resource intensive overlapping chunking/indexing and embedding operations per stdio session.
At the same time, today's MCP clients largely assume local clients will communicate using stdio, creating a barrier to adoption.
Key Changes
codeweaver.mcp. It was further broken up into multiple modules there for clarity.codeweaver.middlewarepackage was moved into the newmcppackage as a subpackage. Corresponding with this change, the FastMCP Middleware type/class was aliased across the codebase asMcpMiddleware. This helps clarify a continual source of confusion where Mcp middleware, which only applies to Mcp transports, is conflated with HTTP (i.e. Starlette) middleware.mcppackage and reduced role of the FastMCP server enabled a significant reduction in initialization complexity.management.pyto define a new administrative Starlette app that directly exposes the previously proxied-through-fastmcp endpoints. This administrative server will be on port 9329 by default, whereas mcp defaults to the previous 9328.server.background_services.py, a new modules, and logging setup was moved toserver.logging.py.asyncio.gather()health_prefixed modules incodeweaver.serverwe moved to a newhealthsubpackage inserverand two lost theirhealthprefixes -- becomingmodels.py,endpoint.py, and the other remaininghealth_service.py. I also refactored several overly large functions in this package.codeweaver.mcp.server.pystill requires an mcp http server. We wrap stdio servers in a tcp bridge before returning them to the requesting client.startandstopCLI command was added to support the new daemon functionality, and CLI commands were updated to match the new entrypointsBackwards Compatibility