Skip to content

do not allow unsupported server URLs as arguments #423

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 5, 2025
Merged

do not allow unsupported server URLs as arguments #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
918c0d0
do not allow unsupported server URLs as arguments
jtcorbett Sep 5, 2025
1dd21da
fix formatting
jtcorbett Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 5 additions & 28 deletions src/mcp_agent/cli/cloud/commands/app/status/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
)
from mcp_agent.cli.utils.ux import (
console,
print_error,
)


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

except Exception as e:
console.print(
Panel(
f"[red]Error fetching tools: {str(e)}[/red]",
title="Server Tools",
border_style="red",
)
)
print_error(f"Error fetching tools: {str(e)}")


async def print_server_prompts(session: MCPClientSession) -> None:
Expand Down Expand Up @@ -263,13 +258,7 @@ async def print_server_prompts(session: MCPClientSession) -> None:
Panel(Group(*panels), title="Server Prompts", border_style="blue")
)
except Exception as e:
console.print(
Panel(
f"[red]Error fetching prompts: {str(e)}[/red]",
title="Server Prompts",
border_style="red",
)
)
print_error(f"Error fetching prompts: {str(e)}")


async def print_server_resources(session: MCPClientSession) -> None:
Expand Down Expand Up @@ -304,13 +293,7 @@ async def print_server_resources(session: MCPClientSession) -> None:
)
console.print(Panel(table, title="Server Resources", border_style="blue"))
except Exception as e:
console.print(
Panel(
f"[red]Error fetching resources: {str(e)}[/red]",
title="Server Resources",
border_style="red",
)
)
print_error(f"Error fetching resources: {str(e)}")


async def print_server_workflows(session: MCPClientSession) -> None:
Expand Down Expand Up @@ -347,10 +330,4 @@ async def print_server_workflows(session: MCPClientSession) -> None:
Panel(Group(*panels), title="Server Workflows", border_style="blue")
)
except Exception as e:
console.print(
Panel(
f"[red]Error fetching workflows: {str(e)}[/red]",
title="Server Workflows",
border_style="red",
)
)
print_error(f"Error fetching workflows: {str(e)}")
17 changes: 3 additions & 14 deletions src/mcp_agent/cli/cloud/commands/app/workflows/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
)
from mcp_agent.cli.utils.ux import (
console,
print_error,
)


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

except Exception as e:
console.print(
Panel(
f"[red]Error fetching workflows: {str(e)}[/red]",
title="Workflows",
border_style="red",
)
)
print_error(f"Error fetching workflows: {str(e)}")


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

except Exception as e:
console.print(
Panel(
f"[red]Error fetching workflow runs: {str(e)}[/red]",
title="Workflow Runs",
border_style="red",
)
)
print_error(f"Error fetching workflow runs: {str(e)}")
84 changes: 34 additions & 50 deletions src/mcp_agent/cli/cloud/commands/logger/configure/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from rich.panel import Panel

from mcp_agent.cli.exceptions import CLIError
from mcp_agent.cli.utils.ux import print_error

console = Console()

Expand All @@ -32,31 +33,29 @@ def configure_logger(
),
) -> None:
"""Configure OTEL endpoint and headers for log collection.

This command allows you to configure the OpenTelemetry endpoint and headers
that will be used for collecting logs from your deployed MCP apps.

Examples:
mcp-agent cloud logger configure https://otel.example.com:4318/v1/logs
mcp-agent cloud logger configure https://otel.example.com --headers "Authorization=Bearer token,X-Custom=value"
mcp-agent cloud logger configure --test # Test current configuration
"""
if not endpoint and not test:
console.print("[red]Error: Must specify endpoint or use --test[/red]")
print_error("Must specify endpoint or use --test")
raise typer.Exit(1)

config_path = _find_config_file()

if test:
if config_path and config_path.exists():
config = _load_config(config_path)
otel_config = config.get("otel", {})
endpoint = otel_config.get("endpoint")
headers_dict = otel_config.get("headers", {})
else:
console.print(
"[yellow]No configuration file found. Use --endpoint to set up OTEL configuration.[/yellow]"
)
console.print("[yellow]No configuration file found. Use --endpoint to set up OTEL configuration.[/yellow]")
raise typer.Exit(1)
else:
headers_dict = {}
Expand All @@ -66,71 +65,56 @@ def configure_logger(
key, value = header_pair.strip().split("=", 1)
headers_dict[key.strip()] = value.strip()
except ValueError:
console.print(
"[red]Error: Headers must be in format 'key=value,key2=value2'[/red]"
)
print_error("Headers must be in format 'key=value,key2=value2'")
raise typer.Exit(1)

if endpoint:
console.print(f"[blue]Testing connection to {endpoint}...[/blue]")

try:
with httpx.Client(timeout=10.0) as client:
response = client.get(
endpoint.replace("/v1/logs", "/health")
if "/v1/logs" in endpoint
else f"{endpoint}/health",
headers=headers_dict,
endpoint.replace("/v1/logs", "/health") if "/v1/logs" in endpoint else f"{endpoint}/health",
headers=headers_dict
)

if response.status_code in [
200,
404,
]: # 404 is fine, means endpoint exists

if response.status_code in [200, 404]: # 404 is fine, means endpoint exists
console.print("[green]✓ Connection successful[/green]")
else:
console.print(
f"[yellow]⚠ Got status {response.status_code}, but endpoint is reachable[/yellow]"
)

console.print(f"[yellow]⚠ Got status {response.status_code}, but endpoint is reachable[/yellow]")

except httpx.RequestError as e:
console.print(f"[red]✗ Connection failed: {e}[/red]")
print_error(f"✗ Connection failed: {e}")
if not test:
console.print(
"[yellow]Configuration will be saved anyway. Check your endpoint URL and network connection.[/yellow]"
)

console.print("[yellow]Configuration will be saved anyway. Check your endpoint URL and network connection.[/yellow]")

if not test:
if not config_path:
config_path = Path.cwd() / "mcp_agent.config.yaml"

config = _load_config(config_path) if config_path.exists() else {}

if "otel" not in config:
config["otel"] = {}

config["otel"]["endpoint"] = endpoint
config["otel"]["headers"] = headers_dict

try:
config_path.parent.mkdir(parents=True, exist_ok=True)
with open(config_path, "w") as f:
yaml.dump(config, f, default_flow_style=False, sort_keys=False)

console.print(
Panel(
f"[green]✓ OTEL configuration saved to {config_path}[/green]\n\n"
f"Endpoint: {endpoint}\n"
f"Headers: {len(headers_dict)} configured"
+ (f" ({', '.join(headers_dict.keys())})" if headers_dict else ""),
title="Configuration Saved",
border_style="green",
)
)


console.print(Panel(
f"[green]✓ OTEL configuration saved to {config_path}[/green]\n\n"
f"Endpoint: {endpoint}\n"
f"Headers: {len(headers_dict)} configured" + (f" ({', '.join(headers_dict.keys())})" if headers_dict else ""),
title="Configuration Saved",
border_style="green"
))

except Exception as e:
console.print(f"[red]Error saving configuration: {e}[/red]")
raise typer.Exit(1)
raise CLIError(f"Error saving configuration: {e}")


def _find_config_file() -> Optional[Path]:
Expand All @@ -150,4 +134,4 @@ def _load_config(config_path: Path) -> dict:
with open(config_path, "r") as f:
return yaml.safe_load(f) or {}
except Exception as e:
raise CLIError(f"Failed to load config from {config_path}: {e}")
raise CLIError(f"Failed to load config from {config_path}: {e}")
35 changes: 15 additions & 20 deletions src/mcp_agent/cli/cloud/commands/logger/tail/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from mcp_agent.cli.cloud.commands.servers.utils import setup_authenticated_client
from mcp_agent.cli.core.api_client import UnauthenticatedError
from mcp_agent.cli.core.utils import parse_app_identifier, resolve_server_url
from mcp_agent.cli.utils.ux import print_error

console = Console()

Expand All @@ -28,7 +29,7 @@

def tail_logs(
app_identifier: str = typer.Argument(
help="Server ID, URL, or app configuration ID to retrieve logs for"
help="App ID or app configuration ID to retrieve logs for"
),
since: Optional[str] = typer.Option(
None,
Expand Down Expand Up @@ -83,7 +84,7 @@ def tail_logs(
mcp-agent cloud logger tail app_abc123 --limit 50

# Stream logs continuously
mcp-agent cloud logger tail https://app.mcpac.dev/abc123 --follow
mcp-agent cloud logger tail app_abc123 --follow

# Show logs from the last hour with error filtering
mcp-agent cloud logger tail app_abc123 --since 1h --grep "ERROR|WARN"
Expand All @@ -94,49 +95,48 @@ def tail_logs(

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

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

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

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

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

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

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

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

app_id, config_id, server_url = parse_app_identifier(app_identifier)
app_id, config_id = parse_app_identifier(app_identifier)

try:
if follow:
asyncio.run(_stream_logs(
app_id=app_id,
config_id=config_id,
server_url=server_url,
credentials=credentials,
grep_pattern=grep,
app_identifier=app_identifier,
Expand All @@ -146,7 +146,6 @@ def tail_logs(
asyncio.run(_fetch_logs(
app_id=app_id,
config_id=config_id,
server_url=server_url,
credentials=credentials,
since=since,
grep_pattern=grep,
Expand All @@ -161,14 +160,12 @@ def tail_logs(
console.print("\n[yellow]Interrupted by user[/yellow]")
sys.exit(0)
except Exception as e:
console.print(f"[red]Error: {e}[/red]")
raise typer.Exit(5)
raise CLIError(str(e))


async def _fetch_logs(
app_id: Optional[str],
config_id: Optional[str],
server_url: Optional[str],
credentials: UserCredentials,
since: Optional[str],
grep_pattern: Optional[str],
Expand Down Expand Up @@ -241,17 +238,15 @@ async def _fetch_logs(

async def _stream_logs(
app_id: Optional[str],
config_id: Optional[str],
server_url: Optional[str],
config_id: Optional[str],
credentials: UserCredentials,
grep_pattern: Optional[str],
app_identifier: str,
format: str,
) -> None:
"""Stream logs continuously via SSE."""

if not server_url:
server_url = await resolve_server_url(app_id, config_id, credentials)
server_url = await resolve_server_url(app_id, config_id, credentials)
Copy link
Contributor

@graphite-app graphite-app bot Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the null check for server_url creates a potential issue. The resolve_server_url() function may return None in error cases, which would cause a runtime error when urlparse(server_url) is called on line 256.

Consider either:

  1. Restoring the null check with appropriate error handling:
server_url = await resolve_server_url(app_id, config_id, credentials)
if not server_url:
    console.print("[red]Error: Could not resolve server URL[/red]")
    raise typer.Exit(1)
  1. Ensuring resolve_server_url() never returns None by handling errors internally and raising appropriate exceptions that can be caught by the existing error handlers.

This will prevent unexpected crashes when streaming logs from servers that cannot be resolved.

Suggested change
server_url = await resolve_server_url(app_id, config_id, credentials)
server_url = await resolve_server_url(app_id, config_id, credentials)
if not server_url:
console.print("[red]Error: Could not resolve server URL[/red]")
raise typer.Exit(1)

Spotted by Diamond

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.


parsed = urlparse(server_url)
stream_url = f"{parsed.scheme}://{parsed.netloc}/logs"
Expand Down
2 changes: 1 addition & 1 deletion src/mcp_agent/cli/cloud/commands/servers/delete/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

@handle_server_api_errors
def delete_server(
id_or_url: str = typer.Argument(..., help="Server ID or URL to delete"),
id_or_url: str = typer.Argument(..., help="Server ID or app configuration ID to delete"),
force: bool = typer.Option(False, "--force", "-f", help="Force deletion without confirmation prompt"),
) -> None:
"""Delete a specific MCP Server."""
Expand Down
2 changes: 1 addition & 1 deletion src/mcp_agent/cli/cloud/commands/servers/describe/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

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