-
Notifications
You must be signed in to change notification settings - Fork 967
feat: add mcp switch #1039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add mcp switch #1039
Conversation
Please make sure all the checkboxes are checked:
|
WalkthroughThe entrypoint script now sets a default transport mode ( Changes
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cognee-mcp/entrypoint.sh (2)
35-35: Minor: quote the variable in log messages
Although unlikely, quoting prevents word-splitting if the value ever contains spaces.-echo "Starting Cognee MCP Server with transport mode: $TRANSPORT_MODE" +echo "Starting Cognee MCP Server with transport mode: \"$TRANSPORT_MODE\""
40-61: Collapse the nested transport/debug logic to eliminate duplication
Four near-identical branches make the script harder to maintain and easy to drift. Build the common argument array once and delegate to eitherdebugpyorcognee.-# Modified startup with transport mode selection and error handling -if [ "$ENVIRONMENT" = "dev" ] || [ "$ENVIRONMENT" = "local" ]; then - if [ "$DEBUG" = "true" ]; then - echo "Waiting for the debugger to attach..." - if [ "$TRANSPORT_MODE" = "sse" ]; then - exec python -m debugpy --wait-for-client --listen 0.0.0.0:5678 -m cognee --transport sse - else - exec python -m debugpy --wait-for-client --listen 0.0.0.0:5678 -m cognee --transport stdio - fi - else - if [ "$TRANSPORT_MODE" = "sse" ]; then - exec cognee --transport sse - else - exec cognee --transport stdio - fi - fi -else - if [ "$TRANSPORT_MODE" = "sse" ]; then - exec cognee --transport sse - else - exec cognee --transport stdio - fi -fi +# Build common args once +CMD_ARGS=(--transport "$TRANSPORT_MODE") + +# Choose launcher +if [[ "$ENVIRONMENT" == "dev" || "$ENVIRONMENT" == "local" ]] && [[ "$DEBUG" == "true" ]]; then + echo "Waiting for the debugger to attach..." + exec python -m debugpy --wait-for-client --listen 0.0.0.0:5678 -m cognee "${CMD_ARGS[@]}" +else + exec cognee "${CMD_ARGS[@]}" +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cognee-mcp/entrypoint.sh(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: End-to-End Tests / Test using different async databases in parallel in Cognee
- GitHub Check: End-to-End Tests / Server Start Test
- GitHub Check: End-to-End Tests / Deletion Test
- GitHub Check: End-to-End Tests / S3 Bucket Test
- GitHub Check: End-to-End Tests / Run Telemetry Pipeline Test
- GitHub Check: End-to-End Tests / Deduplication Test
- GitHub Check: Basic Tests / Run Basic Graph Tests
- GitHub Check: End-to-End Tests / Run Telemetry Test
- GitHub Check: Basic Tests / Run Simple Examples
- GitHub Check: Basic Tests / Run Integration Tests
- GitHub Check: Basic Tests / Run Unit Tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
| # Set default transport mode if not specified | ||
| TRANSPORT_MODE=${TRANSPORT_MODE:-"stdio"} | ||
| echo "Transport mode: $TRANSPORT_MODE" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add upfront validation for unsupported transport modes
Right now any value other than sse silently falls back to stdio. Explicitly rejecting unknown values avoids hard-to-trace misconfigurations.
TRANSPORT_MODE=${TRANSPORT_MODE:-"stdio"}
+case "$TRANSPORT_MODE" in
+ stdio|sse) ;; # accepted values
+ *)
+ echo "Unsupported TRANSPORT_MODE='$TRANSPORT_MODE'. Allowed: stdio | sse."
+ exit 1
+esac
echo "Transport mode: $TRANSPORT_MODE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Set default transport mode if not specified | |
| TRANSPORT_MODE=${TRANSPORT_MODE:-"stdio"} | |
| echo "Transport mode: $TRANSPORT_MODE" | |
| # Set default transport mode if not specified | |
| TRANSPORT_MODE=${TRANSPORT_MODE:-"stdio"} | |
| case "$TRANSPORT_MODE" in | |
| stdio|sse) ;; # accepted values | |
| *) | |
| echo "Unsupported TRANSPORT_MODE='$TRANSPORT_MODE'. Allowed: stdio | sse." | |
| exit 1 | |
| esac | |
| echo "Transport mode: $TRANSPORT_MODE" |
🤖 Prompt for AI Agents
In cognee-mcp/entrypoint.sh around lines 7 to 10, the script sets TRANSPORT_MODE
with a default of "stdio" but does not validate if the provided value is
supported. Add a validation step after setting TRANSPORT_MODE to explicitly
check if its value is either "stdio" or "sse". If it is not one of these, print
an error message and exit the script with a non-zero status to prevent silent
fallback and misconfiguration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think the upfront validation is beneficial and can simplify the code below to simply
--transport $TRANSPORT_MODE, avoiding the if checks, since we already know the value is valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.
Description
DCO Affirmation
I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin.