Skip to content

Commit c0fc9f6

Browse files
authored
do not allow unsupported server URLs as arguments (#423)
### TL;DR Improved app identifier handling by removing URL support and enforcing proper ID formats. ### What changed? - Modified `parse_app_identifier()` to only accept proper app IDs (`app_...`) or app configuration IDs (`apcnf_...`), removing URL support - Updated the function to return a tuple of just `(app_id, config_id)` instead of including server URL - Added proper error handling with descriptive error messages when invalid identifiers are provided - Updated help text in CLI commands to reflect the new identifier requirements - Removed server URL parameters from internal functions that no longer need them - Updated example commands to use proper app IDs instead of URLs ### How to test? 1. Try using the logger tail command with proper app IDs: ``` mcp-agent cloud logger tail app_abc123 mcp-agent cloud logger tail apcnf_xyz789 ``` 2. Verify that using URLs now fails with a clear error message: ``` mcp-agent cloud logger tail https://app.mcpac.dev/abc123 ``` 3. Test server commands with proper IDs: ``` mcp-agent cloud servers describe app_abc123 mcp-agent cloud servers delete apcnf_xyz789 ``` ### Why make this change? This change standardizes how we identify apps and configurations throughout the CLI, making the interface more consistent and reducing confusion. By enforcing proper ID formats and providing clear error messages, users will better understand what identifiers are expected. Removing URL support simplifies the codebase and creates a more predictable user experience.
1 parent 0827cb1 commit c0fc9f6

File tree

10 files changed

+81
-130
lines changed

10 files changed

+81
-130
lines changed

src/mcp_agent/cli/cloud/commands/app/status/main.py

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
)
2828
from mcp_agent.cli.utils.ux import (
2929
console,
30+
print_error,
3031
)
3132

3233

@@ -210,13 +211,7 @@ async def print_server_tools(session: MCPClientSession) -> None:
210211
console.print(Panel(Group(*panels), title="Server Tools", border_style="blue"))
211212

212213
except Exception as e:
213-
console.print(
214-
Panel(
215-
f"[red]Error fetching tools: {str(e)}[/red]",
216-
title="Server Tools",
217-
border_style="red",
218-
)
219-
)
214+
print_error(f"Error fetching tools: {str(e)}")
220215

221216

222217
async def print_server_prompts(session: MCPClientSession) -> None:
@@ -263,13 +258,7 @@ async def print_server_prompts(session: MCPClientSession) -> None:
263258
Panel(Group(*panels), title="Server Prompts", border_style="blue")
264259
)
265260
except Exception as e:
266-
console.print(
267-
Panel(
268-
f"[red]Error fetching prompts: {str(e)}[/red]",
269-
title="Server Prompts",
270-
border_style="red",
271-
)
272-
)
261+
print_error(f"Error fetching prompts: {str(e)}")
273262

274263

275264
async def print_server_resources(session: MCPClientSession) -> None:
@@ -304,13 +293,7 @@ async def print_server_resources(session: MCPClientSession) -> None:
304293
)
305294
console.print(Panel(table, title="Server Resources", border_style="blue"))
306295
except Exception as e:
307-
console.print(
308-
Panel(
309-
f"[red]Error fetching resources: {str(e)}[/red]",
310-
title="Server Resources",
311-
border_style="red",
312-
)
313-
)
296+
print_error(f"Error fetching resources: {str(e)}")
314297

315298

316299
async def print_server_workflows(session: MCPClientSession) -> None:
@@ -347,10 +330,4 @@ async def print_server_workflows(session: MCPClientSession) -> None:
347330
Panel(Group(*panels), title="Server Workflows", border_style="blue")
348331
)
349332
except Exception as e:
350-
console.print(
351-
Panel(
352-
f"[red]Error fetching workflows: {str(e)}[/red]",
353-
title="Server Workflows",
354-
border_style="red",
355-
)
356-
)
333+
print_error(f"Error fetching workflows: {str(e)}")

src/mcp_agent/cli/cloud/commands/app/workflows/main.py

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
)
3131
from mcp_agent.cli.utils.ux import (
3232
console,
33+
print_error,
3334
)
3435

3536

@@ -212,13 +213,7 @@ async def print_workflows_list(session: MCPClientSession) -> None:
212213
console.print(Panel(Group(*panels), title="Workflows", border_style="blue"))
213214

214215
except Exception as e:
215-
console.print(
216-
Panel(
217-
f"[red]Error fetching workflows: {str(e)}[/red]",
218-
title="Workflows",
219-
border_style="red",
220-
)
221-
)
216+
print_error(f"Error fetching workflows: {str(e)}")
222217

223218

224219
async def print_runs_list(session: MCPClientSession) -> None:
@@ -293,10 +288,4 @@ def get_start_time(run: WorkflowRun):
293288
console.print(table)
294289

295290
except Exception as e:
296-
console.print(
297-
Panel(
298-
f"[red]Error fetching workflow runs: {str(e)}[/red]",
299-
title="Workflow Runs",
300-
border_style="red",
301-
)
302-
)
291+
print_error(f"Error fetching workflow runs: {str(e)}")

src/mcp_agent/cli/cloud/commands/logger/configure/main.py

Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from rich.panel import Panel
1111

1212
from mcp_agent.cli.exceptions import CLIError
13+
from mcp_agent.cli.utils.ux import print_error
1314

1415
console = Console()
1516

@@ -32,31 +33,29 @@ def configure_logger(
3233
),
3334
) -> None:
3435
"""Configure OTEL endpoint and headers for log collection.
35-
36+
3637
This command allows you to configure the OpenTelemetry endpoint and headers
3738
that will be used for collecting logs from your deployed MCP apps.
38-
39+
3940
Examples:
4041
mcp-agent cloud logger configure https://otel.example.com:4318/v1/logs
4142
mcp-agent cloud logger configure https://otel.example.com --headers "Authorization=Bearer token,X-Custom=value"
4243
mcp-agent cloud logger configure --test # Test current configuration
4344
"""
4445
if not endpoint and not test:
45-
console.print("[red]Error: Must specify endpoint or use --test[/red]")
46+
print_error("Must specify endpoint or use --test")
4647
raise typer.Exit(1)
47-
48+
4849
config_path = _find_config_file()
49-
50+
5051
if test:
5152
if config_path and config_path.exists():
5253
config = _load_config(config_path)
5354
otel_config = config.get("otel", {})
5455
endpoint = otel_config.get("endpoint")
5556
headers_dict = otel_config.get("headers", {})
5657
else:
57-
console.print(
58-
"[yellow]No configuration file found. Use --endpoint to set up OTEL configuration.[/yellow]"
59-
)
58+
console.print("[yellow]No configuration file found. Use --endpoint to set up OTEL configuration.[/yellow]")
6059
raise typer.Exit(1)
6160
else:
6261
headers_dict = {}
@@ -66,71 +65,56 @@ def configure_logger(
6665
key, value = header_pair.strip().split("=", 1)
6766
headers_dict[key.strip()] = value.strip()
6867
except ValueError:
69-
console.print(
70-
"[red]Error: Headers must be in format 'key=value,key2=value2'[/red]"
71-
)
68+
print_error("Headers must be in format 'key=value,key2=value2'")
7269
raise typer.Exit(1)
73-
70+
7471
if endpoint:
7572
console.print(f"[blue]Testing connection to {endpoint}...[/blue]")
76-
73+
7774
try:
7875
with httpx.Client(timeout=10.0) as client:
7976
response = client.get(
80-
endpoint.replace("/v1/logs", "/health")
81-
if "/v1/logs" in endpoint
82-
else f"{endpoint}/health",
83-
headers=headers_dict,
77+
endpoint.replace("/v1/logs", "/health") if "/v1/logs" in endpoint else f"{endpoint}/health",
78+
headers=headers_dict
8479
)
85-
86-
if response.status_code in [
87-
200,
88-
404,
89-
]: # 404 is fine, means endpoint exists
80+
81+
if response.status_code in [200, 404]: # 404 is fine, means endpoint exists
9082
console.print("[green]✓ Connection successful[/green]")
9183
else:
92-
console.print(
93-
f"[yellow]⚠ Got status {response.status_code}, but endpoint is reachable[/yellow]"
94-
)
95-
84+
console.print(f"[yellow]⚠ Got status {response.status_code}, but endpoint is reachable[/yellow]")
85+
9686
except httpx.RequestError as e:
97-
console.print(f"[red]✗ Connection failed: {e}[/red]")
87+
print_error(f"✗ Connection failed: {e}")
9888
if not test:
99-
console.print(
100-
"[yellow]Configuration will be saved anyway. Check your endpoint URL and network connection.[/yellow]"
101-
)
102-
89+
console.print("[yellow]Configuration will be saved anyway. Check your endpoint URL and network connection.[/yellow]")
90+
10391
if not test:
10492
if not config_path:
10593
config_path = Path.cwd() / "mcp_agent.config.yaml"
106-
94+
10795
config = _load_config(config_path) if config_path.exists() else {}
108-
96+
10997
if "otel" not in config:
11098
config["otel"] = {}
111-
99+
112100
config["otel"]["endpoint"] = endpoint
113101
config["otel"]["headers"] = headers_dict
114-
102+
115103
try:
116104
config_path.parent.mkdir(parents=True, exist_ok=True)
117105
with open(config_path, "w") as f:
118106
yaml.dump(config, f, default_flow_style=False, sort_keys=False)
119-
120-
console.print(
121-
Panel(
122-
f"[green]✓ OTEL configuration saved to {config_path}[/green]\n\n"
123-
f"Endpoint: {endpoint}\n"
124-
f"Headers: {len(headers_dict)} configured"
125-
+ (f" ({', '.join(headers_dict.keys())})" if headers_dict else ""),
126-
title="Configuration Saved",
127-
border_style="green",
128-
)
129-
)
130-
107+
108+
console.print(Panel(
109+
f"[green]✓ OTEL configuration saved to {config_path}[/green]\n\n"
110+
f"Endpoint: {endpoint}\n"
111+
f"Headers: {len(headers_dict)} configured" + (f" ({', '.join(headers_dict.keys())})" if headers_dict else ""),
112+
title="Configuration Saved",
113+
border_style="green"
114+
))
115+
131116
except Exception as e:
132-
console.print(f"[red]Error saving configuration: {e}[/red]")
133-
raise typer.Exit(1)
117+
raise CLIError(f"Error saving configuration: {e}")
134118

135119

136120
def _find_config_file() -> Optional[Path]:
@@ -150,4 +134,4 @@ def _load_config(config_path: Path) -> dict:
150134
with open(config_path, "r") as f:
151135
return yaml.safe_load(f) or {}
152136
except Exception as e:
153-
raise CLIError(f"Failed to load config from {config_path}: {e}")
137+
raise CLIError(f"Failed to load config from {config_path}: {e}")

src/mcp_agent/cli/cloud/commands/logger/tail/main.py

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from mcp_agent.cli.cloud.commands.servers.utils import setup_authenticated_client
2121
from mcp_agent.cli.core.api_client import UnauthenticatedError
2222
from mcp_agent.cli.core.utils import parse_app_identifier, resolve_server_url
23+
from mcp_agent.cli.utils.ux import print_error
2324

2425
console = Console()
2526

@@ -28,7 +29,7 @@
2829

2930
def tail_logs(
3031
app_identifier: str = typer.Argument(
31-
help="Server ID, URL, or app configuration ID to retrieve logs for"
32+
help="App ID or app configuration ID to retrieve logs for"
3233
),
3334
since: Optional[str] = typer.Option(
3435
None,
@@ -83,7 +84,7 @@ def tail_logs(
8384
mcp-agent cloud logger tail app_abc123 --limit 50
8485
8586
# Stream logs continuously
86-
mcp-agent cloud logger tail https://app.mcpac.dev/abc123 --follow
87+
mcp-agent cloud logger tail app_abc123 --follow
8788
8889
# Show logs from the last hour with error filtering
8990
mcp-agent cloud logger tail app_abc123 --since 1h --grep "ERROR|WARN"
@@ -94,49 +95,48 @@ def tail_logs(
9495

9596
credentials = load_credentials()
9697
if not credentials:
97-
console.print("[red]Error: Not authenticated. Run 'mcp-agent login' first.[/red]")
98+
print_error("Not authenticated. Run 'mcp-agent login' first.")
9899
raise typer.Exit(4)
99100

100101
# Validate conflicting options
101102
if follow and since:
102-
console.print("[red]Error: --since cannot be used with --follow (streaming mode)[/red]")
103+
print_error("--since cannot be used with --follow (streaming mode)")
103104
raise typer.Exit(6)
104105

105106
if follow and limit != DEFAULT_LOG_LIMIT:
106-
console.print("[red]Error: --limit cannot be used with --follow (streaming mode)[/red]")
107+
print_error("--limit cannot be used with --follow (streaming mode)")
107108
raise typer.Exit(6)
108109

109110
if follow and order_by:
110-
console.print("[red]Error: --order-by cannot be used with --follow (streaming mode)[/red]")
111+
print_error("--order-by cannot be used with --follow (streaming mode)")
111112
raise typer.Exit(6)
112113

113114
if follow and (asc or desc):
114-
console.print("[red]Error: --asc/--desc cannot be used with --follow (streaming mode)[/red]")
115+
print_error("--asc/--desc cannot be used with --follow (streaming mode)")
115116
raise typer.Exit(6)
116117

117118
# Validate order_by values
118119
if order_by and order_by not in ["timestamp", "severity"]:
119-
console.print("[red]Error: --order-by must be 'timestamp' or 'severity'[/red]")
120+
print_error("--order-by must be 'timestamp' or 'severity'")
120121
raise typer.Exit(6)
121122

122123
# Validate that both --asc and --desc are not used together
123124
if asc and desc:
124-
console.print("[red]Error: Cannot use both --asc and --desc together[/red]")
125+
print_error("Cannot use both --asc and --desc together")
125126
raise typer.Exit(6)
126127

127128
# Validate format values
128129
if format and format not in ["text", "json", "yaml"]:
129-
console.print("[red]Error: --format must be 'text', 'json', or 'yaml'[/red]")
130+
print_error("--format must be 'text', 'json', or 'yaml'")
130131
raise typer.Exit(6)
131132

132-
app_id, config_id, server_url = parse_app_identifier(app_identifier)
133+
app_id, config_id = parse_app_identifier(app_identifier)
133134

134135
try:
135136
if follow:
136137
asyncio.run(_stream_logs(
137138
app_id=app_id,
138139
config_id=config_id,
139-
server_url=server_url,
140140
credentials=credentials,
141141
grep_pattern=grep,
142142
app_identifier=app_identifier,
@@ -146,7 +146,6 @@ def tail_logs(
146146
asyncio.run(_fetch_logs(
147147
app_id=app_id,
148148
config_id=config_id,
149-
server_url=server_url,
150149
credentials=credentials,
151150
since=since,
152151
grep_pattern=grep,
@@ -161,14 +160,12 @@ def tail_logs(
161160
console.print("\n[yellow]Interrupted by user[/yellow]")
162161
sys.exit(0)
163162
except Exception as e:
164-
console.print(f"[red]Error: {e}[/red]")
165-
raise typer.Exit(5)
163+
raise CLIError(str(e))
166164

167165

168166
async def _fetch_logs(
169167
app_id: Optional[str],
170168
config_id: Optional[str],
171-
server_url: Optional[str],
172169
credentials: UserCredentials,
173170
since: Optional[str],
174171
grep_pattern: Optional[str],
@@ -241,17 +238,15 @@ async def _fetch_logs(
241238

242239
async def _stream_logs(
243240
app_id: Optional[str],
244-
config_id: Optional[str],
245-
server_url: Optional[str],
241+
config_id: Optional[str],
246242
credentials: UserCredentials,
247243
grep_pattern: Optional[str],
248244
app_identifier: str,
249245
format: str,
250246
) -> None:
251247
"""Stream logs continuously via SSE."""
252248

253-
if not server_url:
254-
server_url = await resolve_server_url(app_id, config_id, credentials)
249+
server_url = await resolve_server_url(app_id, config_id, credentials)
255250

256251
parsed = urlparse(server_url)
257252
stream_url = f"{parsed.scheme}://{parsed.netloc}/logs"

src/mcp_agent/cli/cloud/commands/servers/delete/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
@handle_server_api_errors
1919
def delete_server(
20-
id_or_url: str = typer.Argument(..., help="Server ID or URL to delete"),
20+
id_or_url: str = typer.Argument(..., help="Server ID or app configuration ID to delete"),
2121
force: bool = typer.Option(False, "--force", "-f", help="Force deletion without confirmation prompt"),
2222
) -> None:
2323
"""Delete a specific MCP Server."""

src/mcp_agent/cli/cloud/commands/servers/describe/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
@handle_server_api_errors
2121
def describe_server(
22-
id_or_url: str = typer.Argument(..., help="Server ID or URL to describe"),
22+
id_or_url: str = typer.Argument(..., help="Server ID or app configuration ID to describe"),
2323
format: Optional[str] = typer.Option("text", "--format", help="Output format (text|json|yaml)"),
2424
) -> None:
2525
"""Describe a specific MCP Server."""

0 commit comments

Comments
 (0)