src/mcp_agent/cli/utils/git_utils.py (3)

55-55: Handle detached HEAD: branch shows as "HEAD"

When detached, rev-parse returns "HEAD". Consider normalizing to None to avoid misleading breadcrumbs.

-    branch = _run_git(["rev-parse", "--abbrev-ref", "HEAD"], project_dir)
+    branch_raw = _run_git(["rev-parse", "--abbrev-ref", "HEAD"], project_dir)
+    branch = None if branch_raw in (None, "HEAD") else branch_raw

---

71-73: Prefer second-level precision for ISO timestamps

Microseconds add churn in env/vars; seconds are sufficient and cleaner for display.

-def utc_iso_now() -> str:
-    return datetime.now(timezone.utc).isoformat()
+def utc_iso_now() -> str:
+    return datetime.now(timezone.utc).replace(microsecond=0).isoformat()

---

30-35: Swallowing all git errors hides actionable diagnostics

Catching Exception and returning None makes debugging hard (e.g., missing git binary vs not a repo). Consider distinguishing FileNotFoundError and optionally surfacing stderr in verbose mode.

-def _run_git(args: list[str], cwd: Path) -> Optional[str]:
-    try:
-        out = subprocess.check_output(["git", *args], cwd=str(cwd))
-        return out.decode("utf-8", errors="replace").strip()
-    except Exception:
-        return None
+def _run_git(args: list[str], cwd: Path) -> Optional[str]:
+    try:
+        out = subprocess.check_output(["git", *args], cwd=str(cwd), stderr=subprocess.STDOUT)
+        return out.decode("utf-8", errors="replace").strip()
+    except FileNotFoundError:
+        # git not installed
+        return None
+    except subprocess.CalledProcessError:
+        # command failed (e.g., not a repo, no tag) — treat as absence
+        return None

src/mcp_agent/cli/mcp_app/api_client.py (1)

309-314: Signature change looks good; add brief param doc for deployment_metadata

Add a one-liner in Args to document the optional metadata payload.

     """Deploy an MCP App via the API.
 
     Args:
         app_id: The UUID of the app to deploy
+        deployment_metadata: Optional metadata dict (e.g., git breadcrumb) sent to the API

src/mcp_agent/cli/cloud/commands/deploy/wrangler_wrapper.py (3)

211-219: Expose git tag in [vars] for parity with breadcrumb

You already compute git_meta.tag and include it in the Python breadcrumb. Adding MCP_DEPLOY_GIT_TAG to wrangler vars makes it observable at runtime without importing the module.

         if git_meta:
             meta_vars.update(
                 {
                     "MCP_DEPLOY_GIT_COMMIT": git_meta.commit_sha,
                     "MCP_DEPLOY_GIT_SHORT": git_meta.short_sha,
                     "MCP_DEPLOY_GIT_BRANCH": git_meta.branch or "",
                     "MCP_DEPLOY_GIT_DIRTY": "true" if git_meta.dirty else "false",
+                    "MCP_DEPLOY_GIT_TAG": git_meta.tag or "",
                 }
             )

---

282-285: TOML embedding: prefer single-line string for JSON if it fits

Triple-quoted TOML strings are fine, but single-quoted basic strings avoid surprises with escapes. Not blocking.

-        vars_lines.append(f'MCP_DEPLOY_META = """{meta_json}"""')
+        vars_lines.append(f"MCP_DEPLOY_META = '{meta_json}'")

---

241-279: Breadcrumb file name may collide with user modules

Writing mcp_deploy_breadcrumb.py at project root could shadow user code. Consider a more unique name (e.g., _mcp_deploy_breadcrumb.py) or place it under a private package/subdir.

-        (temp_project_dir / "mcp_deploy_breadcrumb.py").write_text(breadcrumb_py)
+        (temp_project_dir / "_mcp_deploy_breadcrumb.py").write_text(breadcrumb_py)

src/mcp_agent/cli/cloud/commands/deploy/main.py (2)

254-276: Tag format: guard against pathological app names

Sanitization replaces non-alnum with “-”, but names like “---” or very long names can create unwieldy tags or collisions. Consider trimming duplicate dashes and enforcing a max length (e.g., 63 chars for the middle segment).

-                safe_name = "".join(
-                    c if c.isalnum() or c in "-_" else "-" for c in app_name
-                )
+                cleaned = "".join(c if c.isalnum() or c in "-_" else "-" for c in app_name)
+                safe_name = re.sub(r"-{2,}", "-", cleaned).strip("-")[:63]

(Import re at top.)

---

292-304: Metadata is git-only by design — align with wrangler’s workspace fingerprint?

If you want parity with the bundle when outside git, consider including the workspace fingerprint here too (same ignore set), guarded by a flag. Not blocking for this PR.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/mcp_agent/cli/core/utils.py (1)

12-27: run_async fallback will still raise under a running loop.

Calling loop.run_until_complete while the loop is already running (e.g., pytest-asyncio, Jupyter) raises "This event loop is already running". Either:

• document that run_async must not be used inside a running loop; or
• change the fallback to detect loop.is_running() and return a Task (requiring async callers to await), or use a proven pattern (e.g., anyio) to bridge contexts.

src/mcp_agent/cli/secrets/processor.py (1)

356-386: Minor UX: avoid embedding leading newlines in error/info messages.

Messages like f"\nError processing secret..." add blank lines; keep formatting in caller. Not blocking.

tests/cli/commands/test_deploy_command.py (1)

72-77: Optional: assert new flags are shown in help.

Include --git-tag and --send-metadata to keep the help test aligned with the CLI surface.

src/mcp_agent/cli/cloud/commands/deploy/main.py (7)

167-170: Fix grammar in user prompt.

Minor copy nit in the confirmation message.

-                        f"Do you want deploy an update to the existing app ID: {app_id}?"
+                        f"Do you want to deploy an update to the existing app ID: {app_id}?"

---

173-176: Cancellation should exit cleanly, not return a misleading app_id; also avoid print_error for non-exceptional flow.

Returning the existing app_id implies success to callers. Prefer a clean exit with code 0 and a neutral message. Using print_error logs exc_info=True, which adds a bogus stack trace.

-                        print_error(
-                            "Cancelling deployment. Please choose a different app name."
-                        )
-                        return app_id
+                        print_info(
+                            "Deployment canceled by user. Choose a different app name to proceed."
+                        )
+                        raise typer.Exit(code=0)

---

191-216: Non-interactive mode can silently use stale deployed secrets; auto-reprocess when input is newer.

If both secrets files exist and --non-interactive is set, you always reuse the deployed file, even if MCP_SECRETS_FILENAME was updated. This can ship stale secret handles.

                 if non_interactive:
-                    print_info(
-                        "--non-interactive specified, using existing deployed secrets file without changes."
-                    )
+                    # If the input secrets is newer than the deployed one, re-process without prompting.
+                    if secrets_file and deployed_secrets_file and secrets_file.stat().st_mtime > deployed_secrets_file.stat().st_mtime:
+                        print_info(
+                            "--non-interactive: input secrets newer than deployed; re-processing automatically."
+                        )
+                        deployed_secrets_file = None
+                    else:
+                        print_info(
+                            "--non-interactive specified, using existing deployed secrets file without changes."
+                        )

---

224-236: Prefer Path composition over string formatting for filesystem paths.

More robust and cross-platform safe.

-            secrets_transformed_path = Path(
-                f"{config_dir}/{MCP_DEPLOYED_SECRETS_FILENAME}"
-            )
+            secrets_transformed_path = config_dir / MCP_DEPLOYED_SECRETS_FILENAME

---

254-276: Git tag breadcrumbing looks solid; consider clearer failure messaging.

Flow is safe and guarded. If tag creation fails (e.g., already exists), current message conflates “not a repo” with other failures. Optional: differentiate duplicate-tag vs. other errors in git_utils.create_git_tag and surface a specific message.

---

291-305: Trim potentially sensitive metadata; avoid sending commit message by default.

Commit messages can contain PII or internal details. Since the goal is provenance, the SHA/branch/tag/dirty state should suffice.

                         metadata = {
                             "source": "git",
                             "commit": gm.commit_sha,
                             "short": gm.short_sha,
                             "branch": gm.branch,
                             "dirty": gm.dirty,
                             "tag": gm.tag,
-                            "message": gm.commit_message,
                         }

Optionally add a flag like --include-commit-message if needed later.

---

71-76: Track reintroduction of --dry-run behind a follow-up issue.

Nice to have for CI sanity checks and UX. Recommend opening an issue so we don’t lose it.

Do you want me to draft an issue with a proposed scope and acceptance criteria?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

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.

src/mcp_agent/cli/utils/git_utils.py (2)

90-92: Stabilize traversal order to make hashes deterministic across platforms.

Sort dirnames in-place in both walkers; os.walk doesn’t guarantee order. This avoids hash drift.

-        dirnames[:] = [
-            d for d in dirnames if d not in ignore_names and not d.startswith(".")
-        ]
+        dirnames[:] = sorted(
+            d for d in dirnames if d not in ignore_names and not d.startswith(".")
+        )

Apply the same change to the second walker.

Also applies to: 130-132

---

94-99: Clarify the .env exception by prioritizing it in the condition.

Current logic works but is harder to read; prefer explicit early allow for .env.

-            if fname in ignore_names or fname.startswith("."):
-                # Allow .env explicitly
-                if fname == ".env":
-                    pass
-                else:
-                    continue
+            if fname == ".env":
+                pass  # Always include .env
+            elif fname in ignore_names or fname.startswith("."):
+                continue

src/mcp_agent/cli/utils/git_utils.py (3)

103-113: Stream file reads and encode read failures to avoid memory spikes and silent collisions.

Reading entire files can be expensive; and treating unreadable files as empty risks collisions. Stream in chunks and include a sentinel on failure.

-            rel = fpath.relative_to(root).as_posix()
-            try:
-                with open(fpath, "rb") as f:
-                    data = f.read()
-            except Exception:
-                data = b""
-            h.update(rel.encode("utf-8"))
-            h.update(b"\0")
-            h.update(data)
-            h.update(b"\n")
+            rel = fpath.relative_to(root).as_posix()
+            try:
+                h.update(rel.encode("utf-8"))
+                h.update(b"\0")
+                with open(fpath, "rb") as f:
+                    for chunk in iter(lambda: f.read(1024 * 1024), b""):
+                        h.update(chunk)
+                h.update(b"\n")
+            except Exception as e:
+                h.update(rel.encode("utf-8"))
+                h.update(b"\0")
+                h.update(b"<ERROR:READ_FAILED>")
+                h.update(str(type(e).__name__).encode("utf-8"))
+                h.update(b"\n")

---

141-152: Use nanosecond mtime for a more collision‑resistant fingerprint.

Seconds granularity can miss multiple changes within the same second.

-                st = fpath.stat()
-                size = st.st_size
-                mtime = int(st.st_mtime)
+                st = fpath.stat()
+                size = st.st_size
+                mtime_ns = getattr(st, "st_mtime_ns", int(st.st_mtime * 1_000_000_000))
             except Exception:
                 size = -1
-                mtime = 0
+                mtime_ns = 0
             h.update(rel.encode("utf-8"))
             h.update(b"\0")
             h.update(str(size).encode("utf-8"))
             h.update(b"\0")
-            h.update(str(mtime).encode("utf-8"))
+            h.update(str(mtime_ns).encode("utf-8"))

---

56-57: Normalize detached HEAD branch to None.

When in detached HEAD, this returns "HEAD". Consider mapping to None for cleaner metadata.

-        branch = _run_git(["rev-parse", "--abbrev-ref", "HEAD"], project_dir)
+        branch = _run_git(["rev-parse", "--abbrev-ref", "HEAD"], project_dir)
+        if branch == "HEAD":
+            branch = None

src/mcp_agent/cli/cloud/commands/deploy/main.py (1)

268-270: Avoid tag collisions within the same second.

Two rapid deploys can generate the same tag. Include microseconds to reduce collisions.

-                ts = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S")
+                ts = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S.%f")

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/mcp_agent/cli/cloud/commands/workflows/list/main.py (2)

63-75: DRY: compute workflows_data once.

Removes duplication and keeps JSON/YAML branches symmetrical.

Apply this diff:

-                if format == "json":
-                    workflows_data = [workflow.model_dump() for workflow in workflows]
-                    print(
-                        json.dumps({"workflows": workflows_data}, indent=2, default=str)
-                    )
-                elif format == "yaml":
-                    workflows_data = [workflow.model_dump() for workflow in workflows]
-                    print(
-                        yaml.dump(
-                            {"workflows": workflows_data}, default_flow_style=False
-                        )
-                    )
+                workflows_data = [workflow.model_dump() for workflow in workflows]
+                if format == "json":
+                    print(json.dumps({"workflows": workflows_data}, indent=2, default=str))
+                elif format == "yaml":
+                    print(yaml.dump({"workflows": workflows_data}, default_flow_style=False))
                 else:  # text format
                     print_workflows(workflows)

---

23-24: Avoid shadowing built-in name ‘format’.

Rename param to output_format to prevent confusion, keep CLI flag as --format.

Apply this diff:

-async def _list_workflows_async(server_id_or_url: str, format: str = "text") -> None:
+async def _list_workflows_async(server_id_or_url: str, output_format: str = "text") -> None:
@@
-                if format == "json":
+                if output_format == "json":
@@
-                elif format == "yaml":
+                elif output_format == "yaml":
@@
-    format: Optional[str] = typer.Option(
+    output_format: Optional[str] = typer.Option(
         "text", "--format", help="Output format (text|json|yaml)"
     ),
@@
-    validate_output_format(format)
-    run_async(_list_workflows_async(server_id_or_url, format))
+    validate_output_format(output_format)
+    run_async(_list_workflows_async(server_id_or_url, output_format))

Also applies to: 108-109

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro