-
Notifications
You must be signed in to change notification settings - Fork 781
Fix/template updates #587
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
Fix/template updates #587
Conversation
WalkthroughThis PR reorganizes template scaffolding in the init command, renames server.py to main.py, adds requirements.txt creation for templates, introduces a new server README template, and refactors the basic agent server example to remove argparse, switch to OpenAIAugmentedLLM, simplify logger handling, and use SSE instead of stdio. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple concerns—template reorganization, new documentation, and backend server refactoring—with moderate logic density in the basic agent server modifications involving LLM switching, logger restructuring, and server method changes. The template reorganization is relatively straightforward but requires careful verification of renamed references. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| "server": "MCP server with workflow and parallel agents", | ||
| "token": "Token counting example with monitoring", | ||
| "factory": "Agent factory with router-based selection", | ||
| # "token": "Token counting example with monitoring", |
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.
Just removing these for now; generally not sure about their usefulness as templates (vs just examples in the repo) but also they were copying unrelated README so would at least need an updated README on top of validating that they work (e.g. we know token counting doesn't work for cloud workflow runs atm)
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: 2
🧹 Nitpick comments (1)
src/mcp_agent/data/templates/README_server.md (1)
131-133: Consider formatting URLs as markdown links for better accessibility.While bare URLs are functional, markdown links improve readability and accessibility for screen readers.
Apply this diff:
-- GitHub: https://github.com/lastmile-ai/mcp-agent -- Docs: https://docs.mcp-agent.com/ -- Discord: https://lmai.link/discord/mcp-agent +- [GitHub](https://github.com/lastmile-ai/mcp-agent) +- [Docs](https://docs.mcp-agent.com/) +- [Discord](https://lmai.link/discord/mcp-agent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mcp_agent/cli/commands/init.py(6 hunks)src/mcp_agent/data/templates/README_server.md(1 hunks)src/mcp_agent/data/templates/basic_agent_server.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.
Applied to files:
src/mcp_agent/data/templates/basic_agent_server.py
🧬 Code graph analysis (2)
src/mcp_agent/data/templates/basic_agent_server.py (4)
src/mcp_agent/workflows/llm/augmented_llm_openai.py (1)
OpenAIAugmentedLLM(83-926)src/mcp_agent/data/templates/basic_agent.py (1)
finder_agent(42-71)src/mcp_agent/agents/agent.py (1)
attach_llm(157-193)src/mcp_agent/server/app_server.py (2)
app(213-215)create_mcp_server_for_app(480-2018)
src/mcp_agent/cli/commands/init.py (1)
src/mcp_agent/cli/commands/config.py (1)
_load_template(36-49)
🪛 LanguageTool
src/mcp_agent/data/templates/README_server.md
[uncategorized] ~77-~77: The official name of this software platform is spelled with a capital “H”.
Context: ...p-agent cloud account through Google or Github. Set up your mcp-agent cloud API Key a...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
src/mcp_agent/data/templates/README_server.md
131-131: Bare URL used
(MD034, no-bare-urls)
132-132: Bare URL used
(MD034, no-bare-urls)
133-133: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks / test
🔇 Additional comments (8)
src/mcp_agent/data/templates/basic_agent_server.py (5)
22-24: LGTM!The comment helpfully guides users on swapping LLM providers, making the template more approachable.
30-30: Verify that removing the description is intentional.The FastMCP constructor no longer includes a
descriptionparameter. Based on thecreate_mcp_server_for_appimplementation, when an external FastMCP instance is provided (as in this template), it's attached as-is without modification. This means the server won't advertise a description to MCP clients, which could affect discoverability.If the description is not needed, this is fine. Otherwise, consider restoring it:
-mcp = FastMCP(name="basic_agent_server") +mcp = FastMCP(name="basic_agent_server", description="Basic agent server example")
61-61: LGTM!The consistent refactoring from
app.loggertocontext.loggerimproves clarity and aligns with the context-based logging pattern throughout the codebase. The fallback handling at lines 126-127 is properly implemented.Also applies to: 126-127, 190-197
85-85: LGTM!The switch to
OpenAIAugmentedLLMis intentional, well-documented in the comments, and follows the Agent abstraction pattern correctly by usingattach_llm. The template maintains flexibility for users to swap providers as needed.
196-200: LGTM!The switch from
run_stdio_async()torun_sse_async()modernizes the template to use HTTP-based SSE transport, which is better suited for remote/networked deployments. This change is well-documented in the README (lines 46, 60) and aligns with the deployment guidance.src/mcp_agent/cli/commands/init.py (3)
377-380: LGTM!The consistent renaming from
server.pytomain.pyaligns with the updated template structure and improves clarity. Console output correctly reflects the new filename in usage instructions.Also applies to: 458-460
370-374: LGTM!Adding
requirements.txtgeneration improves the developer experience by providing dependency management out of the box. The implementation uses consistent patterns and handles existing files gracefully with fallback behavior.Also applies to: 394-399
363-363: All four README template files referenced in the code are present and correctly located insrc/mcp_agent/data/templates/. No action is needed.
| # "token": "Token counting example with monitoring", | ||
| # "factory": "Agent factory with router-based selection", |
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.
🧩 Analysis chain
Inconsistency: Token and factory templates are commented out but code paths remain.
Lines 156-157 comment out the token and factory templates, but:
- Their template handling code (lines 401-439) is still present and functional
- Interactive mode (lines 496-497) still offers them as choices
- Using
--template tokenor--template factorywill fail at line 266 with "Unknown template" error - Selecting them in interactive mode will also fail
If these templates are intentionally excluded (per PR objectives), consider:
- Either remove the code paths entirely (lines 401-439 and interactive choices at 496-497)
- Or keep them but add a check to display a helpful message like "This template is temporarily disabled pending verification"
Run this script to verify template references:
🏁 Script executed:
#!/bin/bash
# Check for any other references to token/factory templates that might break
rg -n "token.*template|factory.*template" --type py -C 2Length of output: 1282
I need to verify the template validation logic to confirm the exact error path:That search won't help. Let me check the actual validation code in the file:
🏁 Script executed:
#!/bin/bash
# Find the template validation logic and line 266
sed -n '250,280p' src/mcp_agent/cli/commands/init.pyLength of output: 1097
🏁 Script executed:
#!/bin/bash
# Find where scaffolding_templates dict is used for validation
rg -n "scaffolding_templates\[|in scaffolding_templates|get.*template" src/mcp_agent/cli/commands/init.py -A2 -B2Length of output: 514
🏁 Script executed:
#!/bin/bash
# Check the complete init function signature and early validation
sed -n '240,280p' src/mcp_agent/cli/commands/init.pyLength of output: 1521
I need to find where the templates variable is assigned:
🏁 Script executed:
#!/bin/bash
# Find where templates variable is defined in the init function
sed -n '130,180p' src/mcp_agent/cli/commands/init.pyLength of output: 77
🏁 Script executed:
#!/bin/bash
# Find where templates variable is assigned in init function
rg -n "templates\s*=" src/mcp_agent/cli/commands/init.py -B2 -A2 | head -30Length of output: 992
🏁 Script executed:
#!/bin/bash
# Also check the function signature to understand the parameters
rg -n "def init\(" src/mcp_agent/cli/commands/init.py -A20Length of output: 985
🏁 Script executed:
#!/bin/bash
# Check the complete interactive mode templates dict and what happens after selection
rg -n "def init\(" src/mcp_agent/cli/commands/init.py -A 370 | grep -A50 "Template selection"Length of output: 74
🏁 Script executed:
#!/bin/bash
# Get lines 490-520 to see interactive template selection and handling
rg -n "Interactive mode|if interactive|Template selection" src/mcp_agent/cli/commands/init.py -A15Length of output: 689
🏁 Script executed:
#!/bin/bash
# Also search for where the interactive choice is processed
rg -n 'choice\s*=' src/mcp_agent/cli/commands/init.py -A3 -B3Length of output: 292
🏁 Script executed:
#!/bin/bash
# Check code after line 507 to see how init() is called
rg -n "template_name" src/mcp_agent/cli/commands/init.py | grep -v "^\s*#"Length of output: 254
Interactive mode offers token/factory templates but execution will fail.
The original analysis was correct. Here's the confirmed issue:
- Lines 496-497: Interactive mode offers "token" and "factory" as choices
- Line 506: User's selection is extracted as
template_name - Line 542: This
template_nameis passed toinit(template=template_name) - Line 265 in
init(): Validation checksif template not in templates: - The main
templatesdict (line 180) only includes "basic", "server", "workflow", "researcher", "gen_coding", and "chat" - Token and factory are commented out from
scaffolding_templates(lines 156-157) - Result: Users selecting token/factory interactively will hit the "Unknown template" error at line 265
- Lines 401-439: The elif blocks handling these templates are now dead code
Fix required: Either remove token/factory from interactive choices (lines 496-497) AND remove their code paths (lines 401-439), OR uncomment them in scaffolding_templates (lines 156-157) if they should be available.
🤖 Prompt for AI Agents
In src/mcp_agent/cli/commands/init.py around lines 156-157 and 401-439
(interactive choices at ~496-497 and template validation at ~265), the
interactive menu presents "token" and "factory" but those keys are commented out
in scaffolding_templates, causing an "Unknown template" error and leaving the
template-specific code paths dead; fix by doing one of two: A) Re-enable the
templates by uncommenting the "token" and "factory" entries at lines 156-157 so
they exist in the templates dict used by init(), keeping the existing elif
blocks, or B) Remove "token" and "factory" from the interactive choices (~lines
496-497) and delete the corresponding elif handling code blocks (lines ~401-439)
so the UI and implementation stay consistent. Ensure whichever option you choose
also updates any tests/docs referencing those templates.
| uv run mcp-agent login | ||
| ``` | ||
|
|
||
| to authenticate to mcp-agent cloud. You will be redirected to the login page, create an mcp-agent cloud account through Google or Github. |
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.
Fix capitalization: "Github" → "GitHub".
The official brand name uses a capital "H".
Apply this diff:
-to authenticate to mcp-agent cloud. You will be redirected to the login page, create an mcp-agent cloud account through Google or Github.
+to authenticate to mcp-agent cloud. You will be redirected to the login page, create an mcp-agent cloud account through Google or GitHub.📝 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.
| to authenticate to mcp-agent cloud. You will be redirected to the login page, create an mcp-agent cloud account through Google or Github. | |
| to authenticate to mcp-agent cloud. You will be redirected to the login page, create an mcp-agent cloud account through Google or GitHub. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: The official name of this software platform is spelled with a capital “H”.
Context: ...p-agent cloud account through Google or Github. Set up your mcp-agent cloud API Key a...
(GITHUB)
🤖 Prompt for AI Agents
In src/mcp_agent/data/templates/README_server.md around line 77, fix the brand
capitalization by changing "Github" to "GitHub" in the sentence "to authenticate
to mcp-agent cloud. You will be redirected to the login page, create an
mcp-agent cloud account through Google or Github." so it reads "...through
Google or GitHub.".
* Update templates * Remove factory and token templates for now
Summary
Fix the server template for init and exclude factory and token for now until we can make sure they work (and have correct README contents) --> or just remove them in a later PR.
Testing
Summary by CodeRabbit
New Features
Documentation
Changes