Remove server hostname function and simplify server naming

[jtcorbett]

TL;DR

Simplified server naming and added a restart notification for MCP client installations.

What changed?

• Removed the _server_hostname() function that generated server names from URLs
• Replaced dynamic server naming with a simple default value of "mcp_agent"
• Added a notification message informing users they may need to restart their MCP client after installation
• Removed redundant app info fetching in the ChatGPT client flow

Summary by CodeRabbit

• New Features

  • Added a reminder after successful installation prompting users to restart the MCP client for changes to take effect.

• Refactor

  • Simplified server name selection during installation; when no name is provided, it now defaults to the application name or "mcp_agent", reducing hostname derivation complexity.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Removed URL-based hostname parsing and the _server_hostname helper from the install command; app info fetches are no longer conditionally re-fetched in the ChatGPT flow and mcp_client.get_app is used synchronously; added an early restart reminder output after successful installation.

Changes

Cohort / File(s)	Change Summary
Install command src/mcp_agent/cli/commands/install.py	Removed urlparse import and _server_hostname helper; eliminated URL-derived server name logic; switched from run_async(mcp_client.get_app(...)) to synchronous mcp_client.get_app(...); removed fallback app-info fetch in ChatGPT flow; added a restart reminder message after successful install.
Tests for install tests/cli/commands/test_install.py	Deleted tests for _server_hostname and URL-based hostname generation; retained other install-related tests (_build_server_config, _merge_mcp_json, install flows).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as Install CLI
  participant Client as mcp_client
  participant User as User Output

  rect rgb(230, 245, 255)
    CLI->>Client: get_app(app_id)  %% synchronous call (was async)
    Client-->>CLI: app_info
  end

  rect rgb(245, 255, 230)
    CLI->>CLI: resolve server_name (app_name or "mcp_agent")
    Note right of CLI: removed _server_hostname(url)
  end

  CLI->>User: print install success
  CLI->>User: print "restart reminder"

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Inspect src/mcp_agent/cli/commands/install.py for any remaining async/sync mismatch or blocking implications.
• Verify call sites and tests that previously relied on _server_hostname behavior to ensure no hidden assumptions remain.
• Confirm user-facing messages and restart reminder placement are correct and covered by tests.

Possibly related PRs

• add install command to CLI #565 — Adds the _server_hostname helper and tests that this PR removes; high overlap/conflict on hostname logic.

Suggested reviewers

• rholinshead
• saqadri

Poem

🐰 I hopped through code and found a name,
URLs pruned back from hostname fame.
"mcp_agent" hums a simple tune,
Sync calls snug beneath the moon.
Remember to restart — and dance in June! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Remove server hostname function and simplify server naming" directly and accurately reflects the primary changes in the changeset. The title specifically captures the removal of the _server_hostname() function and the simplification of server naming logic (replacing dynamic URL-based naming with a fixed "mcp_agent" default). While the changeset also includes secondary modifications like adding a restart notification and removing the run_async() wrapper, the title focuses appropriately on the main architectural refactoring. The title is concise, specific, and clear enough that a developer scanning the repository history would understand the core change without ambiguity.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch Remove_server_hostname_function_and_simplify_server_naming

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jtcorbett]

• Remove server hostname function and simplify server naming #594 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/mcp_agent/cli/commands/install.py (1)

324-329: Critical: Initialize app_info in exception handler to prevent NameError.

If the get_app() call on line 324 raises an exception, app_info is never assigned. The ChatGPT flow at line 334 then attempts to access app_info.unauthenticatedAccess, which will raise a NameError. While the outer try-catch (lines 333-366) will catch it, the error message will be confusing.

Apply this diff to fix:

 except Exception as e:
     print_info(f"Warning: Could not fetch app info: {e}")
+    app_info = None
     app_name = None

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7418086 and 94f6cb6.

📒 Files selected for processing (2)

• src/mcp_agent/cli/commands/install.py (3 hunks)
• tests/cli/commands/test_install.py (0 hunks)

💤 Files with no reviewable changes (1)

• tests/cli/commands/test_install.py

🔇 Additional comments (1)

src/mcp_agent/cli/commands/install.py (1)

519-522: LGTM! Helpful user notification.

The restart reminder is a good UX improvement that helps users understand what action to take after installation to ensure the changes take effect.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/mcp_agent/cli/commands/install.py (1)

386-386: Consider the UX impact of potential duplicate server names.

The fallback to a fixed "mcp_agent" default can lead to duplicate name errors when:

• The app_info fetch fails (line 329 sets app_name = None), or
• Multiple instances of the same app are installed from different URLs (same app_name)

Users will then be forced to use --name or --force to proceed. While the duplicate check at lines 447-449 prevents silent overwrites, this is a UX regression from the previous URL-based unique name generation. If this trade-off is intentional for simplification, consider documenting it or providing a more helpful error message that suggests using --name with a sample value.

🧹 Nitpick comments (1)

src/mcp_agent/cli/commands/install.py (1)

520-523: Good addition, though slightly redundant with existing restart messages.

The generic restart reminder is helpful, but note that the success Panel above (lines 486-518) already includes client-specific restart instructions for Claude Desktop (line 489), VSCode (line 494), and Cursor (line 499). The additional reminder reinforces the message, which is fine for UX, but you could consider whether the generic message is needed given the specific instructions already shown.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94f6cb6 and 7f605be.

📒 Files selected for processing (1)

• src/mcp_agent/cli/commands/install.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/mcp_agent/cli/commands/install.py (1)

src/mcp_agent/core/context.py (1)

• name (193-196)

🔇 Additional comments (1)

src/mcp_agent/cli/commands/install.py (1)

324-330: No action needed—the async call handling is correct.

The verification confirms that run_async() properly wraps the async get_app() call. Both implementations of get_app() in api_client.py and mock_client.py are defined as async def, and run_async() in src/mcp_agent/cli/core/utils.py correctly executes the coroutine using asyncio.run(). The extensive test suite in test_install.py confirms the installation flow works end-to-end without issues, including app info fetching.