-
Notifications
You must be signed in to change notification settings - Fork 750
feat: bundle comments and file ignore #511
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 all commits
cb4f64b
c4f120c
b698c44
6c94486
ce313b3
4888557
4a57ba3
d2842e6
51daafb
620d492
93eadef
660ac95
f35880b
ae754fa
94be33f
f21a84e
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 |
---|---|---|
@@ -0,0 +1,80 @@ | ||
"""Ignore-file helpers for the deploy bundler. | ||
|
||
This module focuses on two things: | ||
- Parse an ignore file (gitignore-compatible syntax) into a `PathSpec` matcher. | ||
- Provide an adapter that works with `shutil.copytree(ignore=...)` to decide | ||
which directory entries to skip during a copy. | ||
|
||
There is no implicit reading of `.gitignore` here. Callers must explicitly | ||
pass the ignore file path they want to use (e.g., `.mcpacignore`). | ||
""" | ||
|
||
from pathlib import Path | ||
from typing import Optional, Set | ||
import pathspec | ||
|
||
|
||
def create_pathspec_from_gitignore(ignore_file_path: Path) -> Optional[pathspec.PathSpec]: | ||
"""Create and return a `PathSpec` from an ignore file. | ||
|
||
The file is parsed using the `gitwildmatch` (gitignore) syntax. If the file | ||
does not exist, `None` is returned so callers can fall back to default | ||
behavior. | ||
|
||
Args: | ||
ignore_file_path: Path to the ignore file (e.g., `.mcpacignore`). | ||
|
||
Returns: | ||
A `PathSpec` that can match file/directory paths, or `None`. | ||
""" | ||
if not ignore_file_path.exists(): | ||
return None | ||
|
||
with open(ignore_file_path, "r", encoding="utf-8") as f: | ||
spec = pathspec.PathSpec.from_lines("gitwildmatch", f) | ||
|
||
return spec | ||
|
||
|
||
|
||
def should_ignore_by_gitignore( | ||
path_str: str, names: list, project_dir: Path, spec: Optional[pathspec.PathSpec] | ||
) -> Set[str]: | ||
"""Return the subset of `names` to ignore for `shutil.copytree`. | ||
|
||
This function is designed to be passed as the `ignore` callback to | ||
`shutil.copytree`. For each entry in the current directory (`path_str`), it | ||
computes the path relative to the `project_dir` root and checks it against | ||
the provided `spec` (a `PathSpec` created from an ignore file). | ||
|
||
Notes: | ||
- If `spec` is `None`, this returns an empty set (no additional ignores). | ||
- For directories, we also check the relative path with a trailing slash | ||
(a common gitignore convention). | ||
""" | ||
if spec is None: | ||
return set() | ||
|
||
ignored: Set[str] = set() | ||
current_path = Path(path_str) | ||
|
||
for name in names: | ||
full_path = current_path / name | ||
try: | ||
rel_path = full_path.relative_to(project_dir) | ||
except ValueError: | ||
# If `full_path` is not under `project_dir`, ignore matching is skipped. | ||
continue | ||
|
||
# Normalize to POSIX separators so patterns work cross-platform (Windows too) | ||
rel_path_str = rel_path.as_posix() | ||
|
||
# Match files exactly; for directories also try with a trailing slash | ||
# to respect patterns like `build/`. | ||
if spec.match_file(rel_path_str): | ||
ignored.add(name) | ||
elif full_path.is_dir() and spec.match_file(rel_path_str + "/"): | ||
ignored.add(name) | ||
|
||
|
||
return ignored |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,10 @@ | |
utc_iso_now, | ||
) | ||
|
||
from .bundle_utils import ( | ||
create_pathspec_from_gitignore, | ||
should_ignore_by_gitignore, | ||
) | ||
from .constants import ( | ||
CLOUDFLARE_ACCOUNT_ID, | ||
CLOUDFLARE_EMAIL, | ||
|
@@ -107,7 +111,9 @@ def _handle_wrangler_error(e: subprocess.CalledProcessError) -> None: | |
print_error(clean_output) | ||
|
||
|
||
def wrangler_deploy(app_id: str, api_key: str, project_dir: Path) -> None: | ||
def wrangler_deploy( | ||
app_id: str, api_key: str, project_dir: Path, ignore_file: Path | None = None | ||
) -> None: | ||
"""Bundle the MCP Agent using Wrangler. | ||
|
||
A thin wrapper around the Wrangler CLI to bundle the MCP Agent application code | ||
alienzach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -128,6 +134,7 @@ def wrangler_deploy(app_id: str, api_key: str, project_dir: Path) -> None: | |
app_id (str): The application ID. | ||
api_key (str): User MCP Agent Cloud API key. | ||
project_dir (Path): The directory of the project to deploy. | ||
ignore_file (Path | None): Optional path to a gitignore-style file for excluding files from the bundle. | ||
""" | ||
|
||
# Copy existing env to avoid overwriting | ||
|
@@ -165,18 +172,41 @@ def wrangler_deploy(app_id: str, api_key: str, project_dir: Path) -> None: | |
with tempfile.TemporaryDirectory(prefix="mcp-deploy-") as temp_dir_str: | ||
temp_project_dir = Path(temp_dir_str) / "project" | ||
|
||
# Copy the entire project to temp directory, excluding unwanted directories and secrets file | ||
def ignore_patterns(_path, names): | ||
# Load ignore rules (gitignore syntax) only if an explicit ignore file is provided | ||
ignore_spec = ( | ||
create_pathspec_from_gitignore(ignore_file) if ignore_file else None | ||
) | ||
if ignore_file: | ||
if ignore_spec is None: | ||
print_warning( | ||
f"Ignore file '{ignore_file}' not found; applying default excludes only" | ||
) | ||
else: | ||
print_info(f"Using ignore patterns from {ignore_file}") | ||
else: | ||
print_info("No ignore file provided; applying default excludes only") | ||
|
||
# Copy the entire project to temp directory, excluding unwanted directories and the live secrets file | ||
def ignore_patterns(path_str, names): | ||
ignored = set() | ||
|
||
# Keep existing hardcoded exclusions (highest priority) | ||
for name in names: | ||
if (name.startswith(".") and name not in {".env"}) or name in { | ||
"logs", | ||
alienzach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"__pycache__", | ||
"node_modules", | ||
"venv", | ||
MCP_SECRETS_FILENAME, | ||
MCP_SECRETS_FILENAME, # Exclude mcp_agent.secrets.yaml only | ||
}: | ||
ignored.add(name) | ||
|
||
# Apply explicit ignore file patterns (if provided) | ||
spec_ignored = should_ignore_by_gitignore( | ||
path_str, names, project_dir, ignore_spec | ||
) | ||
ignored.update(spec_ignored) | ||
|
||
return ignored | ||
Comment on lines
+190
to
210
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. 🧩 Analysis chainVerify .env exclusion for secret safety. Line 185 whitelists Based on past review comments, this was flagged before. Should 🏁 Script executed: #!/bin/bash
# Check if .env is mentioned in documentation or tests as intentionally bundled
rg -n "\.env" docs/ README.md --type md -C2 Length of output: 760 🏁 Script executed: #!/bin/bash
# Check if .env is in .gitignore (standard practice)
if [ -f .gitignore ]; then
echo "=== .gitignore contents related to .env ==="
rg -n "\.env" .gitignore
else
echo ".gitignore not found"
fi Length of output: 52 🏁 Script executed: #!/bin/bash
# Find and examine the should_ignore_by_gitignore function implementation
echo "=== Searching for should_ignore_by_gitignore function ==="
ast-grep --pattern 'def should_ignore_by_gitignore($$$) {
$$$
}' Length of output: 57 🏁 Script executed: #!/bin/bash
# Look for any tests or documentation about deployment exclusions
echo "=== Searching for deployment-related tests ==="
rg -n "ignore_patterns|\.env" --type py -g '*test*' -C3 Length of output: 14340 🏁 Script executed: #!/bin/bash
# Check for any other references to .env handling in deployment code
echo "=== Searching for .env in deployment-related code ==="
rg -n "\.env" src/mcp_agent/cli/cloud/commands/deploy/ --type py -C2 Length of output: 1517 🏁 Script executed: #!/bin/bash
# Get the full test context around line 412 to see what's actually being tested
echo "=== Full test context for .env handling ==="
rg -n "def test.*" tests/cli/commands/test_wrangler_wrapper.py -A 50 | rg -A 50 "Check that hidden files" Length of output: 2543 🏁 Script executed: #!/bin/bash
# Look for any assertion about .env in the test file
echo "=== Searching for .env assertions in wrangler tests ==="
rg -n "\.env" tests/cli/commands/test_wrangler_wrapper.py -B2 -A2 Length of output: 257 🏁 Script executed: #!/bin/bash
# Check if the test fixture creates a .env file
echo "=== Searching for .env in test fixtures and conftest ==="
rg -n "\.env" tests/cli/commands/test_wrangler_wrapper.py tests/cli/commands/conftest.py -B3 -A3 Length of output: 763 🏁 Script executed: #!/bin/bash
# Look for the complex_project_structure fixture definition
echo "=== Finding complex_project_structure fixture ==="
ast-grep --pattern 'def complex_project_structure($$$):
$$$' Length of output: 4461 CRITICAL: Remove Line 185 explicitly whitelists
If a developer has a local Required fix: if name.startswith(".") or name in {
"logs",
"__pycache__",
"node_modules",
"venv",
MCP_SECRETS_FILENAME,
}:
ignored.add(name) Remove 🤖 Prompt for AI Agents
|
||
|
||
shutil.copytree(project_dir, temp_project_dir, ignore=ignore_patterns) | ||
|
@@ -209,6 +239,26 @@ def ignore_patterns(_path, names): | |
# Rename in place | ||
file_path.rename(py_path) | ||
|
||
# Compute and log which original files are being bundled (skip internal helpers) | ||
bundled_original_files: list[str] = [] | ||
internal_bundle_files = {"wrangler.toml", "mcp_deploy_breadcrumb.py"} | ||
for root, _dirs, files in os.walk(temp_project_dir): | ||
for filename in files: | ||
rel = Path(root).relative_to(temp_project_dir) / filename | ||
if filename in internal_bundle_files: | ||
continue | ||
if filename.endswith(".mcpac.py"): | ||
orig_rel = str(rel)[: -len(".mcpac.py")] | ||
bundled_original_files.append(orig_rel) | ||
else: | ||
bundled_original_files.append(str(rel)) | ||
|
||
bundled_original_files.sort() | ||
if bundled_original_files: | ||
print_info(f"Bundling {len(bundled_original_files)} project file(s):") | ||
for p in bundled_original_files: | ||
console.print(f" - {p}") | ||
|
||
# Collect deployment metadata (git if available, else workspace hash) | ||
git_meta = get_git_metadata(project_dir) | ||
deploy_source = "git" if git_meta else "workspace" | ||
|
Uh oh!
There was an error while loading. Please reload this page.