-
Notifications
You must be signed in to change notification settings - Fork 780
Add cloud auth #410
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
Add cloud auth #410
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit 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. Warning Rate limit exceeded@jtcorbett has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughReplaces the standalone login command with a new cloud auth package exposing login, logout, and whoami. Adds JSON-backed UserCredentials, credential persistence/clear/load APIs, updates CLI wiring to register cloud/auth subgroup while preserving root aliases, and adjusts exported symbols accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as mcp-agent (Typer)
participant Auth as cloud.auth handlers
participant API as APIClient
participant Store as Credential Store
rect rgba(230,240,255,0.5)
note over User,CLI: Login flow (api key or browser)
User->>CLI: mcp-agent cloud auth login [--api-key|--no-open]
CLI->>Auth: login(options)
alt api_key provided
Auth->>Auth: validate prefix
Auth->>API: POST "user/get_profile" (with key)
API-->>Auth: profile or error
Auth->>Store: save_credentials(UserCredentials)
Auth-->>CLI: success / return key
else browser/manual
Auth->>User: print/open auth URL
User-->>Auth: provide API key
Auth->>Auth: validate prefix
Auth->>API: POST "user/get_profile"
API-->>Auth: profile
Auth->>Store: save_credentials(UserCredentials)
Auth-->>CLI: success
end
end
rect rgba(235,255,235,0.5)
note over User,CLI: Whoami
User->>CLI: mcp-agent cloud auth whoami
CLI->>Auth: whoami()
Auth->>Store: load_credentials()
alt missing or expired
Auth-->>CLI: CLIError (exit_code=4)
else present
Auth-->>CLI: render user info panel
end
end
rect rgba(255,235,235,0.5)
note over User,CLI: Logout
User->>CLI: mcp-agent cloud auth logout
CLI->>Auth: logout()
Auth->>Store: load_credentials()
alt none found
Auth-->>CLI: "Not currently logged in."
else found
Auth->>User: confirm logout
alt confirmed
Auth->>Store: clear_credentials()
Auth-->>CLI: "Successfully logged out."
else cancelled
Auth-->>CLI: "Logout cancelled."
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
47-53: Make token expiry timezone-safe (optional).If
token_expires_atmay be naive or non-UTC, label “UTC” only after converting:expiry = token_expires_at.astimezone(timezone.utc); then format. Addfrom datetime import timezone.src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
84-86: Prefer “No” as default when already logged in (safer UX).Reduces accidental re-login.
- if not Confirm.ask("You are already logged in. Do you want to login again?"): + if not Confirm.ask( + "You are already logged in. Do you want to login again?", + default=False, + ):src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
10-36: Optional: support non-interactive logout with --yes/-y.
For CI/scripts, add a flag to skip the prompt. This also justifies keeping thetyperimport.- def logout() -> None: + def logout( + yes: bool = typer.Option(False, "--yes", "-y", help="Do not prompt for confirmation"), + ) -> None: @@ - if not Confirm.ask(f"Are you sure you want to logout {user_info}?"): + if not yes and not Confirm.ask(f"Are you sure you want to logout {user_info}?", default=False): print_info("Logout cancelled.") returnAnd re-introduce the import at the top if you take this route:
+import typersrc/mcp_agent/cli/cloud/main.py (1)
137-175: CLI grouping and aliases are well-structured.
Nestedcloud authwith root-level aliases is ergonomic and backwards-compatible.Consider de-duplicating the multi-line login help string (lines 153-155 and 169-171) into a constant to avoid drift, and capitalize “API keys”.
-Direct to the api keys page for obtaining credentials, routing through login. +Direct to the API keys page for obtaining credentials, routing through login.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
src/mcp_agent/cli/cloud/commands/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/login/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/logout/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/whoami/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/login/main.py(0 hunks)src/mcp_agent/cli/cloud/main.py(2 hunks)
💤 Files with no reviewable changes (1)
- src/mcp_agent/cli/cloud/commands/login/main.py
🧰 Additional context used
🧬 Code graph analysis (8)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
src/mcp_agent/cli/utils/ux.py (2)
print_info(32-49)print_success(52-63)
src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
logout(10-37)
src/mcp_agent/cli/cloud/commands/auth/__init__.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
login(57-103)src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
logout(10-37)src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
whoami(15-57)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (2)
src/mcp_agent/cli/exceptions.py (1)
CLIError(4-9)src/mcp_agent/cli/utils/ux.py (2)
print_error(80-91)print_info(32-49)
src/mcp_agent/cli/cloud/commands/__init__.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
login(57-103)src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
logout(10-37)src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
whoami(15-57)
src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
whoami(15-57)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (3)
src/mcp_agent/cli/core/api_client.py (2)
APIClient(53-130)post(75-87)src/mcp_agent/cli/exceptions.py (1)
CLIError(4-9)src/mcp_agent/cli/utils/ux.py (3)
print_info(32-49)print_success(52-63)print_warning(66-77)
src/mcp_agent/cli/cloud/main.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
login(57-103)src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
logout(10-37)src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
whoami(15-57)
🪛 GitHub Actions: Pull Request Checks
src/mcp_agent/cli/cloud/commands/auth/logout/main.py
[error] 3-3: Ruff lint error: F401 'typer' imported but unused. Command: ruff check.
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py
[error] 3-3: Ruff lint error: F401 'typing.Optional' imported but unused. Command: ruff check.
[error] 5-5: Ruff lint error: F401 'typer' imported but unused. Command: ruff check.
[error] 12-12: Ruff lint error: F401 'print_error' imported but unused. Command: ruff check.
[error] 12-12: Ruff lint error: F401 'print_info' imported but unused. Command: ruff check.
src/mcp_agent/cli/cloud/commands/auth/login/main.py
[error] 7-7: ModuleNotFoundError: No module named 'mcp_agent_cloud' during import in login/main.py (line 7) while running tests (command: uv run coverage run -m pytest tests -m not integration).
🔇 Additional comments (8)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
3-13: Fix incorrect imports and remove unused ones. RemoveOptional,typer,print_error,print_info. ImportCLIErrorfrommcp_agent.cli.exceptions. Confirm and correct the path toload_credentials—no definition was found undermcp_agent.cli.cloud.commands.auth—before updating its import.src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
7-19: Keep imports pointing tomcp_agent_cloud
These imports reference the external core library (mcp_agent_cloud) that this CLI wraps—local modules undermcp_agent.cli.*are not substitutes.Likely an incorrect or invalid review comment.
src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1)
1-5: LGTM.Re-export is clean and conventional.
src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1)
3-5: Simple, clean re-export. LGTM.
Public surface is minimal and clear.src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
6-7: Do not change UX/auth imports
Cloud auth commands (login, whoami, logout) consistently importclear_credentials,load_credentials, and UX helpers from the externalmcp_agent_cloudpackage; switching tomcp_agent.cli.utils.uxwould break that pattern.Likely an incorrect or invalid review comment.
src/mcp_agent/cli/cloud/commands/__init__.py (1)
9-11: Re-export shape looks good.
Imports from.authand updated__all__correctly exposelogin/logout/whoami.src/mcp_agent/cli/cloud/commands/auth/__init__.py (1)
3-7: Auth package surface is tidy.
Single-point re-exports forlogin/logout/whoamisimplify CLI wiring.src/mcp_agent/cli/cloud/main.py (1)
16-16: Import update matches the new command layout.
Bringinglogoutandwhoamialongsideloginfromcommandsis consistent.
00baf6f to
4de6535
Compare
c36b45a to
71daae2
Compare
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.
Actionable comments posted: 7
♻️ Duplicate comments (3)
src/mcp_agent/cli/auth/main.py (1)
21-25: Fix double-open race and permission leak; write atomically with 0600.Current code creates the file with default perms, then reopens it, leaving a window where credentials may be world-readable and risking incorrect final mode. Replace with atomic write to a 0600 temp file + os.replace.
Apply:
- # Create file with restricted permissions (0600) to prevent leakage - with open(credentials_path, "w", encoding="utf-8") as f: - fd = os.open(credentials_path, os.O_WRONLY | os.O_CREAT, 0o600) - with os.fdopen(fd, "w") as f: - f.write(credentials.to_json()) + # Atomic write with restricted permissions (0600) + cred_dir = os.path.dirname(credentials_path) + fd, tmp_path = tempfile.mkstemp(prefix="credentials.", dir=cred_dir, text=True) + try: + try: + os.fchmod(fd, 0o600) # posix + except AttributeError: + os.chmod(tmp_path, 0o600) # windows fallback + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(credentials.to_json()) + f.flush() + try: + os.fsync(f.fileno()) + except (AttributeError, OSError): + pass + if os.path.islink(credentials_path): + raise RuntimeError("Refusing to overwrite symlink at credentials path") + os.replace(tmp_path, credentials_path) + finally: + try: + os.unlink(tmp_path) + except FileNotFoundError: + passsrc/mcp_agent/cli/auth/__init__.py (1)
6-12: Restore legacy export to keep backward compatibility (and prevent import errors).
save_api_key_credentialswas removed from exports, but the PR claims BC. Provide a thin wrapper and re-export it.Apply:
from .main import ( clear_credentials, load_api_key_credentials, load_credentials, save_credentials, ) from .models import UserCredentials +def save_api_key_credentials(api_key: str) -> None: + """Backward-compat wrapper; prefer save_credentials(UserCredentials(...)).""" + save_credentials(UserCredentials(api_key=api_key)) __all__ = [ "clear_credentials", "load_api_key_credentials", "load_credentials", "save_credentials", + "save_api_key_credentials", "UserCredentials", ]Run to confirm any legacy imports are now satisfied:
#!/bin/bash rg -nP '\bsave_api_key_credentials\b' -C2Also applies to: 14-20
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
132-147: Mask API key input and avoid returning secrets from manual prompt.Hide input and normalize whitespace; do not return the key.
Apply:
-def _handle_manual_key_input() -> str: +def _handle_manual_key_input() -> None: """Handle manual API key input. - Returns: - API key string + Returns: + None """ - input_api_key = Prompt.ask("Please enter your API key :key:") + input_api_key = Prompt.ask("Please enter your API key :key:", password=True) + if input_api_key is not None: + input_api_key = input_api_key.strip() if not input_api_key: print_warning("No API key provided.") raise CLIError("Failed to set valid API key") if not _is_valid_api_key(input_api_key): print_warning("Invalid API key provided.") raise CLIError("Failed to set valid API key") credentials = _load_user_credentials(input_api_key) save_credentials(credentials) print_success("API key set.") if credentials.username: print_info(f"Logged in as: {credentials.username}") - return input_api_key + return NoneAlso applies to: 150-156
🧹 Nitpick comments (4)
src/mcp_agent/cli/auth/main.py (2)
1-6: Imports required for secure atomic writes and path resolution.Add tempfile and updated constants.
Apply:
-import json +import json import os from typing import Optional @@ -from .constants import DEFAULT_CREDENTIALS_PATH +import tempfile +from .constants import DEFAULT_CREDENTIALS_PATH, DEFAULT_CREDENTIALS_ENV, LEGACY_CREDENTIALS_PATHS
51-55: Make delete robust and race-safe; optionally remove legacy files too.Handle TOCTOU and permission errors gracefully; consider cleaning legacy files.
Apply:
- credentials_path = os.path.expanduser(DEFAULT_CREDENTIALS_PATH) - if os.path.exists(credentials_path): - os.remove(credentials_path) - return True - return False + deleted = False + for path in [os.path.expanduser(DEFAULT_CREDENTIALS_PATH), *[os.path.expanduser(p) for p in LEGACY_CREDENTIALS_PATHS]]: + try: + os.remove(path) + deleted = True + except FileNotFoundError: + pass + except OSError: + pass + return deletedsrc/mcp_agent/cli/cloud/commands/auth/login/main.py (2)
101-117: Optional: warn on non-HTTPS API base URL.Reduce MITM risk by validating scheme.
base_url = settings.API_BASE_URL +if not base_url.startswith("https://"): + print_warning("Non-HTTPS API base URL detected; proceed with caution.")
93-99: Ensure credentials file is written atomically with 0600 perms.Current save implementation (in auth/main.py) appears to open twice and may not enforce permissions atomically. Consider a single secure, atomic write.
Proposed implementation for save_credentials (in src/mcp_agent/cli/auth/main.py):
import os, tempfile def save_credentials(credentials: UserCredentials) -> None: path = os.path.expanduser(DEFAULT_CREDENTIALS_PATH) os.makedirs(os.path.dirname(path), exist_ok=True) dir_ = os.path.dirname(path) or "." with tempfile.NamedTemporaryFile("w", dir=dir_, delete=False, encoding="utf-8") as tf: os.chmod(tf.name, 0o600) tf.write(credentials.to_json()) tmp_name = tf.name os.replace(tmp_name, path)Also applies to: 148-154
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
src/mcp_agent/cli/auth/__init__.py(1 hunks)src/mcp_agent/cli/auth/constants.py(1 hunks)src/mcp_agent/cli/auth/main.py(1 hunks)src/mcp_agent/cli/auth/models.py(1 hunks)src/mcp_agent/cli/cloud/commands/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/login/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/logout/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py(1 hunks)src/mcp_agent/cli/cloud/commands/auth/whoami/main.py(1 hunks)src/mcp_agent/cli/cloud/commands/login/main.py(0 hunks)src/mcp_agent/cli/cloud/main.py(2 hunks)
💤 Files with no reviewable changes (1)
- src/mcp_agent/cli/cloud/commands/login/main.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/mcp_agent/cli/cloud/commands/auth/init.py
- src/mcp_agent/cli/cloud/commands/init.py
- src/mcp_agent/cli/cloud/commands/auth/whoami/main.py
- src/mcp_agent/cli/cloud/commands/auth/whoami/init.py
- src/mcp_agent/cli/cloud/commands/auth/logout/init.py
- src/mcp_agent/cli/cloud/main.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/utils/config.py : Configuration values such as quality_threshold, max_refinement_attempts, consolidation_interval, and evaluator_model_provider must be loaded from mcp_agent.config.yaml.
Applied to files:
src/mcp_agent/cli/auth/constants.py
🧬 Code graph analysis (4)
src/mcp_agent/cli/auth/__init__.py (2)
src/mcp_agent/cli/auth/main.py (4)
clear_credentials(45-55)load_api_key_credentials(58-65)load_credentials(28-42)save_credentials(9-25)src/mcp_agent/cli/auth/models.py (1)
UserCredentials(10-64)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (2)
src/mcp_agent/cli/auth/main.py (2)
clear_credentials(45-55)load_credentials(28-42)src/mcp_agent/cli/utils/ux.py (2)
print_info(32-49)print_success(52-63)
src/mcp_agent/cli/auth/main.py (1)
src/mcp_agent/cli/auth/models.py (3)
UserCredentials(10-64)to_json(56-58)from_json(61-64)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (5)
src/mcp_agent/cli/auth/models.py (2)
UserCredentials(10-64)is_token_expired(22-26)src/mcp_agent/cli/auth/main.py (2)
load_credentials(28-42)save_credentials(9-25)src/mcp_agent/cli/core/api_client.py (2)
APIClient(53-130)post(75-87)src/mcp_agent/cli/exceptions.py (1)
CLIError(4-9)src/mcp_agent/cli/utils/ux.py (3)
print_info(32-49)print_success(52-63)print_warning(66-77)
🔇 Additional comments (3)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
9-36: LGTM – safe-by-default confirmation and clear UX.Interactive confirmation defaults to No and messages avoid exposing secrets. Looks good.
src/mcp_agent/cli/auth/main.py (1)
58-66: Backward-compat wrapper looks good.Simple and correct delegation to the new model.
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
57-104: Remove login() return value safely
No internal references to login()’s return value were found; changing the return type to None poses no risk.
| @classmethod | ||
| def from_dict(cls, data: dict) -> "UserCredentials": | ||
| """Create from dictionary loaded from JSON.""" | ||
|
|
||
| token_expires_at = None | ||
| if "token_expires_at" in data: | ||
| token_expires_at = datetime.fromisoformat(data["token_expires_at"]) | ||
|
|
||
| return cls( | ||
| api_key=data["api_key"], | ||
| token_expires_at=token_expires_at, | ||
| username=data.get("username"), | ||
| email=data.get("email"), | ||
| ) |
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.
🛠️ Refactor suggestion
Harden ISO8601 parsing and normalize timezone.
datetime.fromisoformat does not accept a trailing 'Z'; also normalize naive timestamps to UTC.
Apply:
@classmethod
def from_dict(cls, data: dict) -> "UserCredentials":
"""Create from dictionary loaded from JSON."""
- token_expires_at = None
- if "token_expires_at" in data:
- token_expires_at = datetime.fromisoformat(data["token_expires_at"])
+ token_expires_at = None
+ raw = data.get("token_expires_at")
+ if raw:
+ ts = raw
+ if isinstance(ts, str) and ts.endswith("Z"):
+ ts = ts[:-1] + "+00:00"
+ try:
+ token_expires_at = datetime.fromisoformat(ts)
+ if token_expires_at.tzinfo is None:
+ token_expires_at = token_expires_at.replace(tzinfo=timezone.utc)
+ except ValueError:
+ token_expires_at = NoneCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/mcp_agent/cli/auth/models.py around lines 41 to 54, the ISO8601 parsing
uses datetime.fromisoformat which fails on a trailing 'Z' and leaves naive
datetimes un-normalized; change the logic to: if token_expires_at exists and is
a string, normalize a trailing 'Z' to '+00:00' before parsing with
datetime.fromisoformat, then if the parsed datetime has no tzinfo set
tzinfo=datetime.timezone.utc, otherwise convert it to UTC via
.astimezone(datetime.timezone.utc); ensure datetime.timezone is imported and
handle non-string/invalid values safely (e.g., treat parse failures as None or
re-raise as appropriate for current behavior).
| def _handle_browser_auth(base_url: str, no_open: bool) -> str: | ||
| """Handle browser-based authentication flow. | ||
| Args: | ||
| base_url: API base URL | ||
| no_open: Whether to skip automatic browser opening | ||
| Returns: | ||
| API key string | ||
| """ | ||
| auth_url = f"{base_url}/{DEFAULT_API_AUTH_PATH}" |
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.
🛠️ Refactor suggestion
Align browser-auth helper with non-returning flow.
Make it return None and not pass secrets upward.
-def _handle_browser_auth(base_url: str, no_open: bool) -> str:
+def _handle_browser_auth(base_url: str, no_open: bool) -> None:
"""Handle browser-based authentication flow.
Args:
base_url: API base URL
no_open: Whether to skip automatic browser opening
- Returns:
- API key string
+ Returns:
+ None
"""
auth_url = f"{base_url}/{DEFAULT_API_AUTH_PATH}"
@@
- return _handle_manual_key_input()
+ _handle_manual_key_input()
+ return NoneAlso applies to: 129-129
🤖 Prompt for AI Agents
In src/mcp_agent/cli/cloud/commands/auth/login/main.py around lines 106-116 (and
also at line 129), the browser auth helper currently returns the API key which
leaks secrets upward; change the function to return None instead of the API key,
update the docstring and type hints to indicate no return value, remove any
return statements that pass the secret back, and ensure callers treat the
browser flow as non-returning (handle None appropriately or branch to a separate
non-secret path). Also adjust any unit tests or call-sites that expect a string
to accept None or use the alternative non-returning flow.

Summary
Enhanced authentication system for MCP Agent Cloud CLI with improved credential management, new auth commands, and a more structured credentials storage format.
Changelist
Restructured authentication module to support richer user credentials
Added new UserCredentials model to store API keys, user info, and token expiry
Changed credentials storage path to ~/.mcp-agent/credentials.json
Added new auth commands:
whoami: Display current user identity information
logout: Clear stored credentials
Reorganized command structure with new cloud auth command group
Maintained backward compatibility with existing API key functions
Added unit tests for new authentication functionality
Test Plan
Summary by CodeRabbit