-
Notifications
You must be signed in to change notification settings - Fork 781
Refactor CLI: dev umbrella, default main.py, smart server defaults, clearer init #469
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
Changes from 5 commits
f553b98
2139f84
1a37972
749dbaf
e5623ef
d983c65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,10 +16,13 @@ | |||||||||||||
| attach_stdio_servers, | ||||||||||||||
| attach_url_servers, | ||||||||||||||
| load_user_app, | ||||||||||||||
| detect_default_script, | ||||||||||||||
| select_servers_from_config, | ||||||||||||||
| ) | ||||||||||||||
| from mcp_agent.cli.utils.url_parser import generate_server_configs, parse_server_urls | ||||||||||||||
| from mcp_agent.workflows.factory import create_llm | ||||||||||||||
| from mcp_agent.agents.agent import Agent | ||||||||||||||
| from mcp_agent.config import get_settings | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| app = typer.Typer(help="Ephemeral REPL for quick iteration") | ||||||||||||||
|
|
@@ -99,14 +102,17 @@ def chat( | |||||||||||||
| npx: Optional[str] = typer.Option(None, "--npx"), | ||||||||||||||
| uvx: Optional[str] = typer.Option(None, "--uvx"), | ||||||||||||||
| stdio: Optional[str] = typer.Option(None, "--stdio"), | ||||||||||||||
| script: Optional[Path] = typer.Option(Path("agent.py"), "--script"), | ||||||||||||||
| script: Optional[Path] = typer.Option(None, "--script"), | ||||||||||||||
| list_servers: bool = typer.Option(False, "--list-servers"), | ||||||||||||||
| list_tools: bool = typer.Option(False, "--list-tools"), | ||||||||||||||
| list_resources: bool = typer.Option(False, "--list-resources"), | ||||||||||||||
| server: Optional[str] = typer.Option( | ||||||||||||||
| None, "--server", help="Filter to a single server" | ||||||||||||||
| ), | ||||||||||||||
| ) -> None: | ||||||||||||||
| # Resolve script with auto-detection | ||||||||||||||
| script = detect_default_script(script) | ||||||||||||||
|
|
||||||||||||||
| server_list = servers_csv.split(",") if servers_csv else None | ||||||||||||||
|
|
||||||||||||||
| url_servers = None | ||||||||||||||
|
|
@@ -140,12 +146,21 @@ def chat( | |||||||||||||
| else: | ||||||||||||||
| server_list.extend(list(stdio_servers.keys())) | ||||||||||||||
|
|
||||||||||||||
| # Smart defaults for servers | ||||||||||||||
| resolved_server_list = select_servers_from_config( | ||||||||||||||
| servers_csv, url_servers, stdio_servers | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| # Listing mode (no generation) | ||||||||||||||
| if list_servers or list_tools or list_resources: | ||||||||||||||
| try: | ||||||||||||||
|
|
||||||||||||||
| async def _list(): | ||||||||||||||
| app_obj = load_user_app(script or Path("agent.py")) | ||||||||||||||
| # Disable progress display for cleaner listing output | ||||||||||||||
| settings = get_settings() | ||||||||||||||
| if settings.logger: | ||||||||||||||
| settings.logger.progress_display = False | ||||||||||||||
| app_obj = load_user_app(script, settings_override=settings) | ||||||||||||||
| await app_obj.initialize() | ||||||||||||||
| attach_url_servers(app_obj, url_servers) | ||||||||||||||
| attach_stdio_servers(app_obj, stdio_servers) | ||||||||||||||
|
|
@@ -163,7 +178,7 @@ async def _list(): | |||||||||||||
| agent = Agent( | ||||||||||||||
| name="chat-lister", | ||||||||||||||
| instruction="You list tools and resources", | ||||||||||||||
| server_names=target_servers, | ||||||||||||||
| server_names=resolved_server_list or target_servers, | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
| ) | ||||||||||||||
|
Comment on lines
+181
to
183
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Honor --server filter in Agent.server_names. If resolved_server_list is non-empty, it currently overrides a provided --server. Prefer the explicit filter. Apply: - server_names=resolved_server_list or target_servers,
+ server_names=([server] if server else (resolved_server_list or target_servers)),📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| async with agent: | ||||||||||||||
|
|
@@ -203,7 +218,11 @@ async def _list(): | |||||||||||||
| ): | ||||||||||||||
|
|
||||||||||||||
| async def _parallel_repl(): | ||||||||||||||
| app_obj = load_user_app(script or Path("agent.py")) | ||||||||||||||
| # Disable progress display for cleaner multi-model REPL | ||||||||||||||
| settings = get_settings() | ||||||||||||||
| if settings.logger: | ||||||||||||||
| settings.logger.progress_display = False | ||||||||||||||
| app_obj = load_user_app(script, settings_override=settings) | ||||||||||||||
| await app_obj.initialize() | ||||||||||||||
| attach_url_servers(app_obj, url_servers) | ||||||||||||||
| attach_stdio_servers(app_obj, stdio_servers) | ||||||||||||||
|
|
@@ -227,7 +246,7 @@ async def _parallel_repl(): | |||||||||||||
| provider = prov_guess | ||||||||||||||
| llm = create_llm( | ||||||||||||||
| agent_name=m, | ||||||||||||||
| server_names=server_list or [], | ||||||||||||||
| server_names=resolved_server_list or [], | ||||||||||||||
| provider=(provider or "openai"), | ||||||||||||||
| model=m, | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
|
|
@@ -276,7 +295,9 @@ async def _parallel_repl(): | |||||||||||||
| ag = _Agent( | ||||||||||||||
| name="chat-lister", | ||||||||||||||
| instruction="list tools", | ||||||||||||||
| server_names=[srv] if srv else (server_list or []), | ||||||||||||||
| server_names=[srv] | ||||||||||||||
| if srv | ||||||||||||||
| else (resolved_server_list or []), | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
| ) | ||||||||||||||
| async with ag: | ||||||||||||||
|
|
@@ -294,7 +315,9 @@ async def _parallel_repl(): | |||||||||||||
| ag = _Agent( | ||||||||||||||
| name="chat-lister", | ||||||||||||||
| instruction="list resources", | ||||||||||||||
| server_names=[srv] if srv else (server_list or []), | ||||||||||||||
| server_names=[srv] | ||||||||||||||
| if srv | ||||||||||||||
| else (resolved_server_list or []), | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
| ) | ||||||||||||||
| async with ag: | ||||||||||||||
|
|
@@ -367,8 +390,8 @@ async def _gen(llm_instance): | |||||||||||||
| try: | ||||||||||||||
| out = asyncio.run( | ||||||||||||||
| _run_single_model( | ||||||||||||||
| script=script or Path("agent.py"), | ||||||||||||||
| servers=server_list, | ||||||||||||||
| script=script, | ||||||||||||||
| servers=resolved_server_list, | ||||||||||||||
| url_servers=url_servers, | ||||||||||||||
| stdio_servers=stdio_servers, | ||||||||||||||
| model=m, | ||||||||||||||
|
|
@@ -392,9 +415,12 @@ async def _gen(llm_instance): | |||||||||||||
| and not models | ||||||||||||||
| and not (list_servers or list_tools or list_resources) | ||||||||||||||
| ): | ||||||||||||||
| # Interactive loop | ||||||||||||||
| # Interactive loop - disable progress display for cleaner REPL experience | ||||||||||||||
| async def _repl(): | ||||||||||||||
| app_obj = load_user_app(script or Path("agent.py")) | ||||||||||||||
| settings = get_settings() | ||||||||||||||
| if settings.logger: | ||||||||||||||
| settings.logger.progress_display = False | ||||||||||||||
| app_obj = load_user_app(script, settings_override=settings) | ||||||||||||||
| await app_obj.initialize() | ||||||||||||||
| attach_url_servers(app_obj, url_servers) | ||||||||||||||
| attach_stdio_servers(app_obj, stdio_servers) | ||||||||||||||
|
|
@@ -416,7 +442,7 @@ async def _repl(): | |||||||||||||
| provider = model_id.split(":", 1)[0] | ||||||||||||||
| llm = create_llm( | ||||||||||||||
| agent_name=(name or "chat"), | ||||||||||||||
| server_names=server_list or [], | ||||||||||||||
| server_names=resolved_server_list or [], | ||||||||||||||
| provider=(provider or "openai"), | ||||||||||||||
| model=model_id, | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
|
|
@@ -476,7 +502,7 @@ async def _repl(): | |||||||||||||
| # Recreate LLM with new model | ||||||||||||||
| llm_local = create_llm( | ||||||||||||||
| agent_name=(name or "chat"), | ||||||||||||||
| server_names=server_list or [], | ||||||||||||||
| server_names=resolved_server_list or [], | ||||||||||||||
| provider=(prov or "openai"), | ||||||||||||||
| model=model_id, | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
|
|
@@ -502,7 +528,9 @@ async def _repl(): | |||||||||||||
| ag = Agent( | ||||||||||||||
| name="chat-lister", | ||||||||||||||
| instruction="list tools", | ||||||||||||||
| server_names=[srv] if srv else (server_list or []), | ||||||||||||||
| server_names=[srv] | ||||||||||||||
| if srv | ||||||||||||||
| else (resolved_server_list or []), | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
| ) | ||||||||||||||
| async with ag: | ||||||||||||||
|
|
@@ -521,7 +549,9 @@ async def _repl(): | |||||||||||||
| ag = Agent( | ||||||||||||||
| name="chat-lister", | ||||||||||||||
| instruction="list resources", | ||||||||||||||
| server_names=[srv] if srv else (server_list or []), | ||||||||||||||
| server_names=[srv] | ||||||||||||||
| if srv | ||||||||||||||
| else (resolved_server_list or []), | ||||||||||||||
| context=app_obj.context, | ||||||||||||||
| ) | ||||||||||||||
| async with ag: | ||||||||||||||
|
|
@@ -698,8 +728,8 @@ async def _repl(): | |||||||||||||
| else: | ||||||||||||||
| out = asyncio.run( | ||||||||||||||
| _run_single_model( | ||||||||||||||
| script=script or Path("agent.py"), | ||||||||||||||
| servers=server_list, | ||||||||||||||
| script=script, | ||||||||||||||
| servers=resolved_server_list, | ||||||||||||||
| url_servers=url_servers, | ||||||||||||||
| stdio_servers=stdio_servers, | ||||||||||||||
| model=model, | ||||||||||||||
|
|
||||||||||||||
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.
Smart server defaults wired — good. One functional inconsistency remains.
/prompt still uses server_list instead of resolved_server_list, so dynamic/configured servers are ignored there.
📝 Committable suggestion
🤖 Prompt for AI Agents
💡 Verification agent
🧩 Analysis chain
Quick sweep for any other legacy
server_listusages.Run this to ensure no other call paths still rely on server_list for server_names.
🏁 Script executed:
Length of output: 1248
Replace remaining legacy
server_listusages withresolved_server_list.rg found three call sites still passing
server_list; update them to pass the resolved list (or callselect_servers_from_configin that scope) and preserveor []defaults where appropriate.🤖 Prompt for AI Agents