Skip to content

Allow logger to also take URLs #451

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 11, 2025
Merged

Allow logger to also take URLs #451

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
96f6d85
allow logger to also take URLs
jtcorbett Sep 11, 2025
4b9d901
format
jtcorbett Sep 11, 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
47 changes: 30 additions & 17 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 @@ -6,7 +6,7 @@
import signal
import sys
from datetime import datetime, timezone
from typing import Optional, Dict, Any, List
from typing import Optional, Dict, Any, List, Union
from urllib.parse import urlparse

import httpx
Expand All @@ -19,8 +19,9 @@
from mcp_agent.cli.auth import load_credentials, UserCredentials
from mcp_agent.cli.cloud.commands.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.core.utils import run_async
from mcp_agent.cli.utils.ux import print_error
from mcp_agent.cli.mcp_app.api_client import MCPApp, MCPAppConfiguration

console = Console()

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

def tail_logs(
app_identifier: str = typer.Argument(
help="App ID or app configuration ID to retrieve logs for"
help="App ID, app configuration ID, or server URL to retrieve logs for"
),
since: Optional[str] = typer.Option(
None,
Expand Down Expand Up @@ -91,6 +92,9 @@ def tail_logs(

# Follow logs and filter for specific patterns
mcp-agent cloud logger tail app_abc123 --follow --grep "authentication.*failed"

# Use server URL instead of app ID
mcp-agent cloud logger tail https://abc123.mcpcloud.ai --follow
"""

credentials = load_credentials()
Expand Down Expand Up @@ -130,14 +134,14 @@ def tail_logs(
print_error("--format must be 'text', 'json', or 'yaml'")
raise typer.Exit(6)

app_id, config_id = parse_app_identifier(app_identifier)

client = setup_authenticated_client()
server = run_async(client.get_app_or_config(app_identifier))

try:
Comment on lines +137 to 140
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Catch resolution errors: wrap get_app_or_config in the existing try.
Right now, failures raise before the try/except and bypass clean CLIError formatting.

-    client = setup_authenticated_client()
-    server = run_async(client.get_app_or_config(app_identifier))
-    
-    try:
+    try:
+        client = setup_authenticated_client()
+        server = run_async(client.get_app_or_config(app_identifier))

Optional (outside this hunk): add specific excepts for httpx.HTTPStatusError (401/404) around this try to mirror _fetch_logs.

📝 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.

Suggested change
client = setup_authenticated_client()
server = run_async(client.get_app_or_config(app_identifier))
try:
try:
client = setup_authenticated_client()
server = run_async(client.get_app_or_config(app_identifier))
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/logger/tail/main.py around lines 137 to 140,
the call to client.get_app_or_config(app_identifier) is executed before entering
the try/except so any resolution errors escape the CLIError handling; move or
wrap the run_async(client.get_app_or_config(app_identifier)) invocation inside
the existing try block so its exceptions are caught and formatted correctly, and
optionally add specific except clauses for httpx.HTTPStatusError (handling
401/404) alongside the existing excepts to mirror _fetch_logs.

if follow:
asyncio.run(
_stream_logs(
app_id=app_id,
config_id=config_id,
server=server,
credentials=credentials,
grep_pattern=grep,
app_identifier=app_identifier,
Expand All @@ -147,16 +151,15 @@ def tail_logs(
else:
asyncio.run(
_fetch_logs(
app_id=app_id,
config_id=config_id,
credentials=credentials,
server=server,
since=since,
grep_pattern=grep,
limit=limit,
order_by=order_by,
asc=asc,
desc=desc,
format=format,
app_identifier=app_identifier,
)
)

Expand All @@ -168,19 +171,26 @@ def tail_logs(


async def _fetch_logs(
app_id: Optional[str],
config_id: Optional[str],
credentials: UserCredentials,
server: Union[MCPApp, MCPAppConfiguration],
since: Optional[str],
grep_pattern: Optional[str],
limit: int,
order_by: Optional[str],
asc: bool,
desc: bool,
format: str,
app_identifier: str,
) -> None:
"""Fetch logs one-time via HTTP API."""

# Extract app_id and config_id from the server object
if hasattr(server, 'appId'): # MCPApp
app_id = server.appId
config_id = None
else: # MCPAppConfiguration
app_id = None
config_id = server.appConfigurationId

client = setup_authenticated_client()

# Map order_by parameter from CLI to API format
Expand Down Expand Up @@ -240,20 +250,23 @@ async def _fetch_logs(
console.print("[yellow]No logs found matching the criteria[/yellow]")
return

_display_logs(filtered_logs, title=f"Logs for {app_id or config_id}", format=format)
_display_logs(filtered_logs, title=f"Logs for {app_identifier}", format=format)


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

server_url = await resolve_server_url(app_id, config_id, credentials)
# Get server URL directly from the server object
if not server.appServerInfo or not server.appServerInfo.serverUrl:
raise CLIError("Server URL not available - server may not be deployed")

server_url = server.appServerInfo.serverUrl

Comment on lines +265 to 270
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden server URL handling; add connect timeout for SSE.

  • Append /logs relative to any existing path (not just host root).
  • Derive routing key robustly; hostname may be None or lack dots.
  • Use a finite connect timeout for AsyncClient; keep read timeout None for streaming.
-    if not server.appServerInfo or not server.appServerInfo.serverUrl:
+    if not getattr(server, "appServerInfo", None) or not server.appServerInfo.serverUrl:
         raise CLIError("Server URL not available - server may not be deployed")
-    
-    server_url = server.appServerInfo.serverUrl
+    server_url = server.appServerInfo.serverUrl.rstrip("/")

Additional changes outside this hunk to complete the fix:

-    parsed = urlparse(server_url)
-    stream_url = f"{parsed.scheme}://{parsed.netloc}/logs"
+    parsed = urlparse(server_url)
+    base_path = (parsed.path or "").rstrip("/")
+    stream_url = f"{parsed.scheme}://{parsed.netloc}{base_path}/logs"
-    hostname = parsed.hostname or ""
-    deployment_id = hostname.split(".")[0] if "." in hostname else hostname
+    hostname = (parsed.hostname or "").strip()
+    deployment_id = hostname.split(".", 1)[0] if hostname else ""

And for the client (outside this hunk):

-        async with httpx.AsyncClient(timeout=None) as client:
+        async with httpx.AsyncClient(timeout=httpx.Timeout(connect=10.0, read=None)) as client:
📝 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.

Suggested change
# Get server URL directly from the server object
if not server.appServerInfo or not server.appServerInfo.serverUrl:
raise CLIError("Server URL not available - server may not be deployed")
server_url = server.appServerInfo.serverUrl
# Get server URL directly from the server object
if not getattr(server, "appServerInfo", None) or not server.appServerInfo.serverUrl:
raise CLIError("Server URL not available - server may not be deployed")
server_url = server.appServerInfo.serverUrl.rstrip("/")

parsed = urlparse(server_url)
stream_url = f"{parsed.scheme}://{parsed.netloc}/logs"
Expand Down
3 changes: 1 addition & 2 deletions 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 @@ -6,7 +6,6 @@
from mcp_agent.cli.mcp_app.api_client import MCPApp
from ...utils import (
setup_authenticated_client,
resolve_server,
handle_server_api_errors,
get_server_name,
get_server_id,
Expand All @@ -25,7 +24,7 @@ def delete_server(
) -> None:
"""Delete a specific MCP Server."""
client = setup_authenticated_client()
server = resolve_server(client, id_or_url)
server = run_async(client.get_app_or_config(id_or_url))

# Determine server type and delete function
if isinstance(server, MCPApp):
Expand Down
4 changes: 2 additions & 2 deletions 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 @@ -10,10 +10,10 @@
from ...utils import (
setup_authenticated_client,
validate_output_format,
resolve_server,
handle_server_api_errors,
clean_server_status,
)
from mcp_agent.cli.core.utils import run_async
from mcp_agent.cli.utils.ux import console


Expand All @@ -29,7 +29,7 @@ def describe_server(
"""Describe a specific MCP Server."""
validate_output_format(format)
client = setup_authenticated_client()
server = resolve_server(client, id_or_url)
server = run_async(client.get_app_or_config(id_or_url))
print_server_description(server, format)


Expand Down
30 changes: 0 additions & 30 deletions src/mcp_agent/cli/cloud/commands/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from mcp_agent.cli.auth import load_api_key_credentials
from mcp_agent.cli.core.api_client import UnauthenticatedError
from mcp_agent.cli.core.constants import DEFAULT_API_BASE_URL
from mcp_agent.cli.core.utils import parse_app_identifier, run_async
from mcp_agent.cli.exceptions import CLIError
from mcp_agent.cli.mcp_app.api_client import (
MCPApp,
Expand Down Expand Up @@ -48,35 +47,6 @@ def validate_output_format(format: str) -> None:
)


def resolve_server(
client: MCPAppClient, id_or_url: str
) -> Union[MCPApp, MCPAppConfiguration]:
"""Resolve server from ID.

Args:
client: Authenticated MCP App client
id_or_url: Server identifier (app ID or app config ID)

Returns:
Server object (MCPApp or MCPAppConfiguration)

Raises:
CLIError: If server resolution fails
"""
try:
app_id, config_id = parse_app_identifier(id_or_url)

if config_id:
return run_async(client.get_app_configuration(app_config_id=config_id))
else:
return run_async(client.get_app(app_id=app_id))

except ValueError as e:
raise CLIError(str(e)) from e
except Exception as e:
raise CLIError(f"Failed to resolve server '{id_or_url}': {str(e)}") from e


def handle_server_api_errors(func):
"""Decorator to handle common API errors for server commands.

Expand Down
3 changes: 1 addition & 2 deletions src/mcp_agent/cli/cloud/commands/workflows/cancel/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from mcp_agent.mcp.gen_client import gen_client
from ...utils import (
setup_authenticated_client,
resolve_server,
handle_server_api_errors,
)

Expand All @@ -25,7 +24,7 @@ async def _cancel_workflow_async(
server_url = server_id_or_url
else:
client = setup_authenticated_client()
server = resolve_server(client, server_id_or_url)
server = run_async(client.get_app_or_config(server_id_or_url))

Comment on lines +27 to 28
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: run_async used inside async function

Await instead.

-        server = run_async(client.get_app_or_config(server_id_or_url))
+        server = await client.get_app_or_config(server_id_or_url)
📝 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.

Suggested change
server = run_async(client.get_app_or_config(server_id_or_url))
server = await client.get_app_or_config(server_id_or_url)
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/workflows/cancel/main.py around lines 27-28,
the code calls run_async(client.get_app_or_config(server_id_or_url)) inside an
async function; replace this with an await expression (await
client.get_app_or_config(server_id_or_url)) and remove run_async. Ensure the
enclosing function is declared async and that any callers properly await it or
handle the coroutine.

if hasattr(server, "appServerInfo") and server.appServerInfo:
server_url = server.appServerInfo.serverUrl
Expand Down
3 changes: 1 addition & 2 deletions src/mcp_agent/cli/cloud/commands/workflows/describe/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from mcp_agent.mcp.gen_client import gen_client
from ...utils import (
setup_authenticated_client,
resolve_server,
handle_server_api_errors,
)

Expand All @@ -27,7 +26,7 @@ async def _describe_workflow_async(
server_url = server_id_or_url
else:
client = setup_authenticated_client()
server = resolve_server(client, server_id_or_url)
server = run_async(client.get_app_or_config(server_id_or_url))

Comment on lines +29 to 30
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: run_async used inside async function

Await instead.

-        server = run_async(client.get_app_or_config(server_id_or_url))
+        server = await client.get_app_or_config(server_id_or_url)
📝 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.

Suggested change
server = run_async(client.get_app_or_config(server_id_or_url))
server = await client.get_app_or_config(server_id_or_url)
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/workflows/describe/main.py around lines
29-30, the code calls run_async(client.get_app_or_config(server_id_or_url))
inside an async function; replace that with using await directly (server = await
client.get_app_or_config(server_id_or_url)) so the coroutine is properly awaited
within the async context and remove the run_async wrapper.

if hasattr(server, "appServerInfo") and server.appServerInfo:
server_url = server.appServerInfo.serverUrl
Expand Down
3 changes: 1 addition & 2 deletions src/mcp_agent/cli/cloud/commands/workflows/list/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from mcp_agent.mcp.gen_client import gen_client
from ...utils import (
setup_authenticated_client,
resolve_server,
handle_server_api_errors,
validate_output_format,
)
Expand All @@ -27,7 +26,7 @@ async def _list_workflows_async(server_id_or_url: str, format: str = "text") ->
server_url = server_id_or_url
else:
client = setup_authenticated_client()
server = resolve_server(client, server_id_or_url)
server = run_async(client.get_app_or_config(server_id_or_url))

Comment on lines +29 to 30
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: run_async used inside async function

This will fail under a running event loop. Await the coroutine instead.

-        server = run_async(client.get_app_or_config(server_id_or_url))
+        server = await client.get_app_or_config(server_id_or_url)
📝 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.

Suggested change
server = run_async(client.get_app_or_config(server_id_or_url))
server = await client.get_app_or_config(server_id_or_url)
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/workflows/list/main.py around lines 29-30,
run_async is being used to execute a coroutine inside an already async function
which will fail when an event loop is running; replace the run_async call by
directly awaiting the coroutine (i.e., assign server = await
client.get_app_or_config(server_id_or_url)), ensure the containing function is
declared async, and remove any now-unused run_async import.

if hasattr(server, "appServerInfo") and server.appServerInfo:
server_url = server.appServerInfo.serverUrl
Expand Down
3 changes: 1 addition & 2 deletions src/mcp_agent/cli/cloud/commands/workflows/resume/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from mcp_agent.mcp.gen_client import gen_client
from ...utils import (
setup_authenticated_client,
resolve_server,
handle_server_api_errors,
)

Expand All @@ -29,7 +28,7 @@ async def _signal_workflow_async(
server_url = server_id_or_url
else:
client = setup_authenticated_client()
server = resolve_server(client, server_id_or_url)
server = run_async(client.get_app_or_config(server_id_or_url))

Comment on lines +31 to 32
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: run_async used inside async function

Await instead.

-        server = run_async(client.get_app_or_config(server_id_or_url))
+        server = await client.get_app_or_config(server_id_or_url)
📝 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.

Suggested change
server = run_async(client.get_app_or_config(server_id_or_url))
server = await client.get_app_or_config(server_id_or_url)
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/workflows/resume/main.py around lines 31-32,
the code calls run_async(...) from inside an async function which is incorrect;
replace the run_async call with an await of the coroutine (server = await
client.get_app_or_config(server_id_or_url)), and remove or stop using the
run_async helper/import if it becomes unused.

if hasattr(server, "appServerInfo") and server.appServerInfo:
server_url = server.appServerInfo.serverUrl
Expand Down
3 changes: 1 addition & 2 deletions src/mcp_agent/cli/cloud/commands/workflows/runs/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from ...utils import (
setup_authenticated_client,
validate_output_format,
resolve_server,
)
from mcp_agent.cli.utils.ux import console, print_info

Expand All @@ -27,7 +26,7 @@ async def _list_workflow_runs_async(
server_url = server_id_or_url
else:
client = setup_authenticated_client()
server = resolve_server(client, server_id_or_url)
server = run_async(client.get_app_or_config(server_id_or_url))

Comment on lines +29 to 30
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: run_async used inside async function

Await instead.

-        server = run_async(client.get_app_or_config(server_id_or_url))
+        server = await client.get_app_or_config(server_id_or_url)
📝 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.

Suggested change
server = run_async(client.get_app_or_config(server_id_or_url))
server = await client.get_app_or_config(server_id_or_url)
🤖 Prompt for AI Agents 
In src/mcp_agent/cli/cloud/commands/workflows/runs/main.py around lines 29-30,
the code calls run_async(client.get_app_or_config(server_id_or_url)) inside an
async function; replace this with awaiting the coroutine directly: change the
call to await client.get_app_or_config(server_id_or_url) and assign that result
to server, removing the run_async wrapper so the coroutine is awaited properly.

if hasattr(server, "appServerInfo") and server.appServerInfo:
server_url = server.appServerInfo.serverUrl
Expand Down
Loading
Loading