-
Notifications
You must be signed in to change notification settings - Fork 781
Add git tag/commit breadcrumbs to deploy flow #472
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
Changes from 1 commit
1ff5460
94d6630
63fe86a
16888fd
0f85d38
9073383
178975b
a34a451
538e211
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,6 +309,7 @@ async def get_app_id_by_name(self, name: str) -> Optional[str]: | |
| async def deploy_app( | ||
| self, | ||
| app_id: str, | ||
| deployment_metadata: Optional[Dict[str, Any]] = None, | ||
| ) -> MCPApp: | ||
| """Deploy an MCP App via the API. | ||
|
|
||
|
|
@@ -326,9 +327,10 @@ async def deploy_app( | |
| if not app_id or not is_valid_app_id_format(app_id): | ||
| raise ValueError(f"Invalid app ID format: {app_id}") | ||
|
|
||
| payload = { | ||
| "appId": app_id, | ||
| } | ||
| payload: Dict[str, Any] = {"appId": app_id} | ||
| if deployment_metadata: | ||
| # Tentative field; include only when requested | ||
| payload["deploymentMetadata"] = deployment_metadata | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll need to wait for www side to land and get deployed before landing this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noting that the server-side changes have been deployed so this is good to go |
||
|
|
||
| # Use a longer timeout for deployments | ||
| deploy_timeout = 300.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,167 @@ | ||||||||||||||
| """Lightweight git helpers for deployment metadata and tagging. | ||||||||||||||
| These helpers avoid third-party dependencies and use subprocess to query git. | ||||||||||||||
| All functions are safe to call outside a git repo (they return None/fallbacks). | ||||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
| from __future__ import annotations | ||||||||||||||
|
|
||||||||||||||
| import hashlib | ||||||||||||||
| import os | ||||||||||||||
| import subprocess | ||||||||||||||
| from dataclasses import dataclass | ||||||||||||||
| from datetime import datetime, timezone | ||||||||||||||
| from pathlib import Path | ||||||||||||||
| from typing import Optional | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| @dataclass | ||||||||||||||
| class GitMetadata: | ||||||||||||||
| """Key git details about the working copy to embed with deployments.""" | ||||||||||||||
|
|
||||||||||||||
| commit_sha: str | ||||||||||||||
| short_sha: str | ||||||||||||||
| branch: Optional[str] | ||||||||||||||
| dirty: bool | ||||||||||||||
| tag: Optional[str] | ||||||||||||||
| commit_message: Optional[str] | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| 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 get_git_metadata(project_dir: Path) -> Optional[GitMetadata]: | ||||||||||||||
| """Return GitMetadata for the repo containing project_dir, if any. | ||||||||||||||
| Returns None if git is unavailable or project_dir is not inside a repo. | ||||||||||||||
| """ | ||||||||||||||
| # Fast probe: are we inside a work-tree? | ||||||||||||||
| inside = _run_git(["rev-parse", "--is-inside-work-tree"], project_dir) | ||||||||||||||
| if inside is None or inside != "true": | ||||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| commit_sha = _run_git(["rev-parse", "HEAD"], project_dir) | ||||||||||||||
| if not commit_sha: | ||||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| short_sha = ( | ||||||||||||||
| _run_git(["rev-parse", "--short", "HEAD"], project_dir) or commit_sha[:7] | ||||||||||||||
| ) | ||||||||||||||
| branch = _run_git(["rev-parse", "--abbrev-ref", "HEAD"], project_dir) | ||||||||||||||
| status = _run_git(["status", "--porcelain"], project_dir) | ||||||||||||||
| dirty = bool(status) | ||||||||||||||
| tag = _run_git(["describe", "--tags", "--exact-match"], project_dir) | ||||||||||||||
| commit_message = _run_git(["log", "-1", "--pretty=%s"], project_dir) | ||||||||||||||
|
|
||||||||||||||
| return GitMetadata( | ||||||||||||||
| commit_sha=commit_sha, | ||||||||||||||
| short_sha=short_sha, | ||||||||||||||
| branch=branch, | ||||||||||||||
| dirty=dirty, | ||||||||||||||
| tag=tag, | ||||||||||||||
| commit_message=commit_message, | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def utc_iso_now() -> str: | ||||||||||||||
| return datetime.now(timezone.utc).isoformat() | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def compute_directory_hash(root: Path, *, ignore_names: set[str] | None = None) -> str: | ||||||||||||||
| """Compute SHA256 over file names and contents under root. | ||||||||||||||
| NOTE: This reads file contents and can be expensive for very large trees. | ||||||||||||||
| Prefer `compute_directory_fingerprint` below for fast fingerprints. | ||||||||||||||
| """ | ||||||||||||||
| if ignore_names is None: | ||||||||||||||
| ignore_names = set() | ||||||||||||||
|
|
||||||||||||||
| h = hashlib.sha256() | ||||||||||||||
| for dirpath, dirnames, filenames in os.walk(root): | ||||||||||||||
| # Filter dirnames in-place to prune traversal | ||||||||||||||
| dirnames[:] = [ | ||||||||||||||
| d for d in dirnames if d not in ignore_names and not d.startswith(".") | ||||||||||||||
| ] | ||||||||||||||
| for fname in sorted(filenames): | ||||||||||||||
|
Comment on lines
+102
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Make directory traversal deterministic to stabilize hashes across platforms os.walk() doesn’t guarantee directory order; since you hash in traversal order, results can vary. Sort dirnames to ensure stable digests. Apply this diff in both walkers: - 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(".")
+ )Also applies to: 126-130 🤖 Prompt for AI Agents |
||||||||||||||
| if fname in ignore_names or fname.startswith("."): | ||||||||||||||
| # Allow .env explicitly | ||||||||||||||
| if fname == ".env": | ||||||||||||||
| pass | ||||||||||||||
| else: | ||||||||||||||
| continue | ||||||||||||||
| fpath = Path(dirpath) / fname | ||||||||||||||
| if fpath.is_symlink(): | ||||||||||||||
| continue | ||||||||||||||
| 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") | ||||||||||||||
| return h.hexdigest() | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def compute_directory_fingerprint( | ||||||||||||||
| root: Path, *, ignore_names: set[str] | None = None | ||||||||||||||
| ) -> str: | ||||||||||||||
| """Compute a cheap, stable SHA256 over file metadata under root. | ||||||||||||||
| This avoids reading file contents. The hash includes the relative path, | ||||||||||||||
| file size and modification time for each included file. Hidden files/dirs | ||||||||||||||
| and any names in `ignore_names` are skipped, as are symlinks. | ||||||||||||||
| """ | ||||||||||||||
| if ignore_names is None: | ||||||||||||||
| ignore_names = set() | ||||||||||||||
|
|
||||||||||||||
| h = hashlib.sha256() | ||||||||||||||
| for dirpath, dirnames, filenames in os.walk(root): | ||||||||||||||
| dirnames[:] = [ | ||||||||||||||
| d for d in dirnames if d not in ignore_names and not d.startswith(".") | ||||||||||||||
| ] | ||||||||||||||
| for fname in sorted(filenames): | ||||||||||||||
| if fname in ignore_names or (fname.startswith(".") and fname != ".env"): | ||||||||||||||
| continue | ||||||||||||||
|
Comment on lines
+148
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current condition has a logic issue with if fname == ".env":
pass # Always include .env files
elif fname in ignore_names or fname.startswith("."):
continueThis ensures
Suggested change
Spotted by Diamond |
||||||||||||||
| fpath = Path(dirpath) / fname | ||||||||||||||
| if fpath.is_symlink(): | ||||||||||||||
| continue | ||||||||||||||
| rel = fpath.relative_to(root).as_posix() | ||||||||||||||
| try: | ||||||||||||||
| st = fpath.stat() | ||||||||||||||
| size = st.st_size | ||||||||||||||
| mtime = int(st.st_mtime) | ||||||||||||||
| except Exception: | ||||||||||||||
| size = -1 | ||||||||||||||
| mtime = 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(b"\n") | ||||||||||||||
| return h.hexdigest() | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def create_git_tag(project_dir: Path, tag_name: str, message: str) -> bool: | ||||||||||||||
| """Create an annotated git tag at HEAD. Returns True on success. | ||||||||||||||
| Does nothing and returns False if not a repo or git fails. | ||||||||||||||
| """ | ||||||||||||||
| inside = _run_git(["rev-parse", "--is-inside-work-tree"], project_dir) | ||||||||||||||
| if inside is None or inside != "true": | ||||||||||||||
| return False | ||||||||||||||
| try: | ||||||||||||||
| subprocess.check_call( | ||||||||||||||
| ["git", "tag", "-a", tag_name, "-m", message], cwd=str(project_dir) | ||||||||||||||
| ) | ||||||||||||||
| return True | ||||||||||||||
| except Exception: | ||||||||||||||
| return False | ||||||||||||||
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.
Insufficient input sanitization: The app name sanitization only allows alphanumeric characters, hyphens, and underscores, but replaces all other characters with hyphens. This could create collisions (e.g., 'app@name' and 'app#name' both become 'app-name') and doesn't prevent potential issues with git tag naming rules. Git tags have specific restrictions (can't start with '.', can't contain certain sequences like '..', etc.). The sanitization should follow git tag naming conventions more strictly.
Spotted by Diamond

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