diff --git a/apps/api/src/api/config_profiles.py b/apps/api/src/api/config_profiles.py index 943180d..81e58b1 100644 --- a/apps/api/src/api/config_profiles.py +++ b/apps/api/src/api/config_profiles.py @@ -2,9 +2,10 @@ import logging import uuid +from typing import Any from fastapi import APIRouter, Depends, HTTPException, Query, status -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, field_validator, model_validator from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -56,18 +57,9 @@ def _calculate_profile_size(data: dict) -> int: return total -class GitMountItem(BaseModel): - remote_url: str = Field(description="Git remote URL (HTTPS or SSH)") - source_path: str = Field(default=".", description="Path within repository (supports glob patterns)") +class GitMountMapping(BaseModel): + source_path: str = Field(description="Path within repository (supports glob patterns)") target_path: str = Field(description="Absolute path inside container") - branch: str | None = Field(default=None, description="Optional branch or tag name") - - @field_validator("remote_url") - @classmethod - def validate_remote_url(cls, v: str) -> str: - if not v.startswith(("http://", "https://", "git@", "ssh://")): - raise ValueError("remote_url must be a valid git URL (https://, git@, or ssh://)") - return v @field_validator("source_path") @classmethod @@ -86,6 +78,51 @@ class GitMountItem(BaseModel): return v +class GitMountItem(BaseModel): + remote_url: str = Field(description="Git remote URL (HTTPS or SSH)") + source_path: str | None = Field(default=None, description="Path within repository (legacy single mapping)") + target_path: str | None = Field(default=None, description="Absolute path inside container (legacy single mapping)") + branch: str | None = Field(default=None, description="Optional branch or tag name") + mappings: list[GitMountMapping] | None = Field(default=None, description="Multiple source/target mappings from the same repo") + + @field_validator("remote_url") + @classmethod + def validate_remote_url(cls, v: str) -> str: + if not v.startswith(("http://", "https://", "git@", "ssh://")): + raise ValueError("remote_url must be a valid git URL (https://, git@, or ssh://)") + return v + + @field_validator("source_path") + @classmethod + def validate_source_path(cls, v: str | None) -> str | None: + if v is None: + return v + if v.startswith("/"): + raise ValueError("source_path must be relative (no leading /)") + if ".." in v: + raise ValueError("source_path cannot contain path traversal (..)") + return v + + @field_validator("target_path") + @classmethod + def validate_target_path(cls, v: str | None) -> str | None: + if v is None: + return v + if ".." in v: + raise ValueError("target_path cannot contain path traversal (..)") + return v + + @model_validator(mode="after") + def check_mappings_or_legacy(self): + has_legacy = self.source_path is not None and self.target_path is not None + has_mappings = self.mappings is not None and len(self.mappings) > 0 + if not has_legacy and not has_mappings: + raise ValueError( + "Git mount must have either 'mappings' (non-empty array) or both 'source_path' and 'target_path'" + ) + return self + + class MountItem(BaseModel): target: str = Field(description="Absolute mount target path") mode: str = Field(default="rw", description="Mount mode: ro or rw") @@ -263,7 +300,7 @@ async def _check_access( async def _validate_git_mounts( session: AsyncSession, user_id: uuid.UUID, - git_mounts: list[dict], + git_mounts: list[Any], project_id: uuid.UUID | None = None, ) -> None: """Validate git mount URLs. diff --git a/apps/api/src/api/tool_instances.py b/apps/api/src/api/tool_instances.py index 38cf1d5..c0b699e 100644 --- a/apps/api/src/api/tool_instances.py +++ b/apps/api/src/api/tool_instances.py @@ -114,6 +114,148 @@ async def _resolve_git_mounts( return volume_mounts +def _normalize_git_mount(entry: dict) -> dict: + """Normalize a git mount entry to the unified mappings format. + + Converts legacy source_path + target_path into a single-entry mappings array. + """ + entry = dict(entry) + if "mappings" not in entry or not entry.get("mappings"): + source = entry.get("source_path", ".") + target = entry.get("target_path") + if target is not None: + entry["mappings"] = [{"source_path": source, "target_path": target}] + entry.pop("source_path", None) + entry.pop("target_path", None) + return entry + + +def _clone_git_repo( + remote_url: str, + branch: str | None, + clone_parent: str, +) -> str: + """Clone or pull a git repository. + + Returns the path to the cloned repo (repo-clone directory). + """ + import hashlib + + url_hash = hashlib.md5(remote_url.encode()).hexdigest()[:12] + repo_name = remote_url.split("/")[-1].replace(".git", "") or "repo" + clone_dir = os.path.join(clone_parent, "git-mounts", f"{repo_name}-{url_hash}") + repo_path = os.path.join(clone_dir, "repo-clone") + + if not os.path.exists(repo_path): + try: + os.makedirs(clone_dir, exist_ok=True) + repo_path = clone_repository( + remote_url, + None, # No SSH key for now - can be added later + clone_dir, + branch or "main", + ) + logger.debug("Cloned git mount repository %s to %s", remote_url, repo_path) + except Exception as exc: + logger.warning("Clone failed for git mount %s: %s", remote_url, exc) + raise + else: + # Repo exists - pull latest updates + try: + _pull_repository_updates(repo_path, remote_url) + logger.debug("Pulled updates for git mount %s", remote_url) + except Exception as exc: + logger.warning("Failed to pull updates for %s: %s", remote_url, exc) + + # Handle branch checkout if specified + if branch and repo_path: + success = _checkout_branch(repo_path, branch) + if success: + logger.debug("Checked out branch %s for %s", branch, remote_url) + else: + logger.warning( + "Branch %s not found in %s, using current branch", branch, remote_url + ) + + return repo_path + + +def _resolve_git_mount_mappings( + repo_path: str, + mappings: list[dict], + working_directory: str | None, +) -> list[dict]: + """Resolve mappings from an already-cloned repo to volume mount entries. + + Returns a flat list of volume mount dicts. + """ + volume_mounts = [] + + for mapping in mappings: + source_path = mapping.get("source_path", ".") + target_path = mapping.get("target_path") + + if not target_path: + logger.warning("Invalid mapping skipped: missing target_path") + continue + + # Resolve relative target paths against working directory + final_target = target_path + if not target_path.startswith("/"): + if not working_directory: + logger.warning( + "Git mount skipped: target_path '%s' is relative but no working_directory is configured. " + "Set working_directory in the tool config or use an absolute path.", + target_path, + ) + continue + final_target = os.path.join(working_directory, target_path) + + # Build source path and expand globs + if source_path and source_path != ".": + source_full = os.path.join(repo_path, source_path) + else: + source_full = repo_path + + # Expand glob patterns + matched_paths = _expand_glob_source(source_full, repo_path) + + if not matched_paths: + logger.warning( + "Git mount skipped: no files matched source path %s in repo", + source_path, + ) + continue + + for matched_path in matched_paths: + if not os.path.exists(matched_path): + continue + + # Determine target path for this match + if len(matched_paths) == 1: + # Single match: mount directly to target_path + mount_target = final_target + else: + # Multiple matches: append relative path to target + rel_path = os.path.relpath(matched_path, repo_path) + mount_target = os.path.join(final_target, rel_path) + + volume_mounts.append( + { + "source": matched_path, + "target": mount_target, + "type": "bind", + } + ) + logger.debug( + "Added git mount: %s -> %s", + matched_path, + mount_target, + ) + + return volume_mounts + + async def _resolve_single_git_mount( session: AsyncSession, git_mount: dict, @@ -125,119 +267,33 @@ async def _resolve_single_git_mount( Clones directly from remote_url, no database lookup needed. Returns a list of volume mounts (one for each matched file/directory). """ + git_mount = _normalize_git_mount(git_mount) remote_url = git_mount.get("remote_url") - source_path = git_mount.get("source_path", ".") - target_path = git_mount.get("target_path") branch = git_mount.get("branch") + mappings = git_mount.get("mappings", []) - if not remote_url or not target_path: - logger.warning("Invalid git mount skipped: missing remote_url or target_path") + if not remote_url: + logger.warning("Invalid git mount skipped: missing remote_url") return [] - # Resolve relative target paths against working directory - if target_path and not target_path.startswith("/"): - if not working_directory: - logger.warning( - "Git mount skipped: target_path '%s' is relative but no working_directory is configured. " - "Set working_directory in the tool config or use an absolute path.", - target_path, - ) - return [] - target_path = os.path.join(working_directory, target_path) - logger.debug("Resolved relative target path to %s", target_path) + if not mappings: + logger.warning("Invalid git mount skipped: no mappings") + return [] if not instance_dir: logger.warning("Git mount skipped: no instance_dir provided for cloning") return [] - # Generate a unique directory name from the URL - import hashlib - - url_hash = hashlib.md5(remote_url.encode()).hexdigest()[:12] - repo_name = remote_url.split("/")[-1].replace(".git", "") or "repo" - clone_parent = os.path.join(instance_dir, "git-mounts", f"{repo_name}-{url_hash}") - # clone_repository always creates 'repo-clone' inside the given directory - repo_path = os.path.join(clone_parent, "repo-clone") - # Clone or pull the repository - if not os.path.exists(repo_path): - try: - os.makedirs(clone_parent, exist_ok=True) - repo_path = await asyncio.to_thread( - clone_repository, - remote_url, - None, # No SSH key for now - can be added later - clone_parent, - branch or "main", - ) - logger.debug("Cloned git mount repository %s to %s", remote_url, repo_path) - except Exception as exc: - logger.warning("Clone failed for git mount %s: %s", remote_url, exc) - return [] - else: - # Repo exists - pull latest updates - try: - await asyncio.to_thread(_pull_repository_updates, repo_path, remote_url) - logger.debug("Pulled updates for git mount %s", remote_url) - except Exception as exc: - logger.warning("Failed to pull updates for %s: %s", remote_url, exc) - - # Handle branch checkout if specified - if branch and repo_path: - success = await asyncio.to_thread(_checkout_branch, repo_path, branch) - if success: - logger.debug("Checked out branch %s for %s", branch, remote_url) - else: - logger.warning( - "Branch %s not found in %s, using current branch", branch, remote_url - ) - - # Build source path and expand globs - if source_path and source_path != ".": - source_full = os.path.join(repo_path, source_path) - else: - source_full = repo_path - - # Expand glob patterns - matched_paths = _expand_glob_source(source_full, repo_path) - - if not matched_paths: - logger.warning( - "Git mount skipped: no files matched source path %s in %s", - source_path, - remote_url, + try: + repo_path = await asyncio.to_thread( + _clone_git_repo, remote_url, branch, instance_dir ) + except Exception: return [] - volume_mounts = [] - for matched_path in matched_paths: - if not os.path.exists(matched_path): - continue - - # Determine target path for this match - if len(matched_paths) == 1: - # Single match: mount directly to target_path - final_target = target_path - else: - # Multiple matches: append relative path to target - rel_path = os.path.relpath(matched_path, repo_path) - final_target = os.path.join(target_path, rel_path) - - volume_mounts.append( - { - "source": matched_path, - "target": final_target, - "type": "bind", - } - ) - logger.debug( - "Added git mount: %s -> %s (url: %s)", - matched_path, - final_target, - remote_url, - ) - - return volume_mounts + # Resolve all mappings from the cloned repo + return _resolve_git_mount_mappings(repo_path, mappings, working_directory) def _checkout_branch(repo_path: str, branch: str) -> bool: diff --git a/apps/api/src/services/config_profile_resolver.py b/apps/api/src/services/config_profile_resolver.py index 0c77224..d6ad2b0 100644 --- a/apps/api/src/services/config_profile_resolver.py +++ b/apps/api/src/services/config_profile_resolver.py @@ -176,21 +176,59 @@ def _merge_git_mounts( ) -> list[dict[str, Any]]: """Merge git mounts from included profiles. - Later mounts override earlier ones with the same remote_url + target_path combo. + Entries with the same remote_url + branch have their mappings concatenated. + Different repos are kept as separate entries. + All entries are normalized to the mappings format. """ result = list(base) - # Build lookup by (remote_url, target_path) - seen = {(m["remote_url"], m["target_path"]): i for i, m in enumerate(result)} + # Normalize existing entries to mappings format + for i, m in enumerate(result): + result[i] = _normalize_git_mount_entry(dict(m)) + + # Build lookup by (remote_url, branch) + seen = {} + for i, m in enumerate(result): + key = (m["remote_url"], m.get("branch")) + seen[key] = i + for mount in overlay: - key = (mount["remote_url"], mount["target_path"]) + mount = _normalize_git_mount_entry(dict(mount)) + key = (mount["remote_url"], mount.get("branch")) if key in seen: - result[seen[key]] = dict(mount) + # Same repo+branch: concatenate mappings, dedup by (source_path, target_path) + existing = result[seen[key]] + existing_sources = { + (m["source_path"], m["target_path"]) + for m in existing.get("mappings", []) + } + for mapping in mount.get("mappings", []): + map_key = (mapping["source_path"], mapping["target_path"]) + if map_key not in existing_sources: + existing["mappings"].append(dict(mapping)) + existing_sources.add(map_key) else: seen[key] = len(result) - result.append(dict(mount)) + result.append(mount) return result +def _normalize_git_mount_entry(entry: dict[str, Any]) -> dict[str, Any]: + """Normalize a git mount entry to the unified mappings format. + + Converts legacy source_path + target_path into a single-entry mappings array. + """ + entry = dict(entry) + if "mappings" not in entry or not entry.get("mappings"): + source = entry.get("source_path", ".") + target = entry.get("target_path") + if target is not None: + entry["mappings"] = [{"source_path": source, "target_path": target}] + # Remove legacy fields once normalized + entry.pop("source_path", None) + entry.pop("target_path", None) + return entry + + async def _resolve_profile_recursive( session: AsyncSession, profile_id: uuid.UUID, diff --git a/apps/api/tests/unit/test_config_profile_resolver.py b/apps/api/tests/unit/test_config_profile_resolver.py index 1208613..c9544d4 100644 --- a/apps/api/tests/unit/test_config_profile_resolver.py +++ b/apps/api/tests/unit/test_config_profile_resolver.py @@ -97,7 +97,7 @@ class TestMergeFunctions: assert overrides == {"/app": "source"} def test_merge_git_mounts_basic(self) -> None: - """Test basic git mount merging.""" + """Test basic git mount merging normalizes to mappings format.""" result = _merge_git_mounts( [], [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app"}], @@ -105,29 +105,54 @@ class TestMergeFunctions: ) assert len(result) == 1 assert result[0]["remote_url"] == "https://github.com/user/repo1.git" - assert result[0]["target_path"] == "/app" + assert "mappings" in result[0] + assert result[0]["mappings"] == [{"source_path": ".", "target_path": "/app"}] - def test_merge_git_mounts_override_same_repo_target(self) -> None: - """Test that git mounts with same repo+target override.""" + def test_merge_git_mounts_concatenate_same_repo_branch(self) -> None: + """Test that git mounts with same repo+branch concatenate mappings.""" result = _merge_git_mounts( [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app", "branch": "main"}], - [{"remote_url": "https://github.com/user/repo1.git", "source_path": "src", "target_path": "/app", "branch": "dev"}], + [{"remote_url": "https://github.com/user/repo1.git", "source_path": "src", "target_path": "/src", "branch": "main"}], "source", ) assert len(result) == 1 - assert result[0]["source_path"] == "src" - assert result[0]["branch"] == "dev" + assert result[0]["branch"] == "main" + mappings: list[dict[str, str]] = result[0]["mappings"] + assert len(mappings) == 2 + assert {"source_path": ".", "target_path": "/app"} in mappings + assert {"source_path": "src", "target_path": "/src"} in mappings - def test_merge_git_mounts_different_targets(self) -> None: - """Test that git mounts with different targets are preserved.""" + def test_merge_git_mounts_dedup_same_mapping(self) -> None: + """Test that duplicate mappings are deduplicated.""" + result = _merge_git_mounts( + [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app", "branch": "main"}], + [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app", "branch": "main"}], + "source", + ) + assert len(result) == 1 + assert len(result[0]["mappings"]) == 1 + + def test_merge_git_mounts_different_repos(self) -> None: + """Test that git mounts with different repos are preserved.""" result = _merge_git_mounts( [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app"}], [{"remote_url": "https://github.com/user/repo2.git", "source_path": ".", "target_path": "/config"}], "source", ) assert len(result) == 2 - targets = {m["target_path"] for m in result} - assert targets == {"/app", "/config"} + urls = {m["remote_url"] for m in result} + assert urls == {"https://github.com/user/repo1.git", "https://github.com/user/repo2.git"} + + def test_merge_git_mounts_different_branches(self) -> None: + """Test that same repo with different branches are kept separate.""" + result = _merge_git_mounts( + [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app", "branch": "main"}], + [{"remote_url": "https://github.com/user/repo1.git", "source_path": ".", "target_path": "/app", "branch": "dev"}], + "source", + ) + assert len(result) == 2 + branches = {m.get("branch") for m in result} + assert branches == {"main", "dev"} class TestResolveProfile: @@ -284,7 +309,7 @@ class TestResolveProfile: @pytest.mark.asyncio async def test_resolve_profile_with_git_mounts(self, db_session: AsyncSession) -> None: - """Test resolving a profile with git mounts.""" + """Test resolving a profile with git mounts normalizes to mappings.""" user_id = uuid.uuid4() profile = ConfigProfile( @@ -303,7 +328,8 @@ class TestResolveProfile: result = await resolve_profile(db_session, profile.id) assert len(result.git_mounts) == 1 assert result.git_mounts[0]["remote_url"] == "https://github.com/user/repo1.git" - assert result.git_mounts[0]["target_path"] == "/app" + assert "mappings" in result.git_mounts[0] + assert result.git_mounts[0]["mappings"] == [{"source_path": ".", "target_path": "/app"}] @pytest.mark.asyncio async def test_resolve_profile_with_git_mount_includes(self, db_session: AsyncSession) -> None: @@ -349,8 +375,10 @@ class TestResolveProfile: result = await resolve_profile(db_session, child.id) assert len(result.git_mounts) == 2 - targets = {m["target_path"] for m in result.git_mounts} - assert targets == {"/app", "/config"} + urls = {m["remote_url"] for m in result.git_mounts} + assert urls == {"https://github.com/user/repo1.git", "https://github.com/user/repo2.git"} + for m in result.git_mounts: + assert "mappings" in m @pytest.mark.asyncio async def test_resolve_profile_not_found(self, db_session: AsyncSession) -> None: diff --git a/apps/api/tests/unit/test_git_mounts.py b/apps/api/tests/unit/test_git_mounts.py new file mode 100644 index 0000000..f1354dd --- /dev/null +++ b/apps/api/tests/unit/test_git_mounts.py @@ -0,0 +1,213 @@ +"""Unit tests for git mount resolution with multi-mapping support.""" + +import os +import tempfile +from unittest.mock import MagicMock, patch + +import pytest + +from src.api.tool_instances import ( + _clone_git_repo, + _expand_glob_source, + _normalize_git_mount, + _resolve_git_mount_mappings, + _resolve_single_git_mount, +) + + +class TestNormalizeGitMount: + """Tests for _normalize_git_mount.""" + + def test_legacy_to_mappings(self) -> None: + """Legacy source_path + target_path becomes mappings array.""" + entry = { + "remote_url": "https://github.com/user/repo.git", + "source_path": "packages/api", + "target_path": "/app/api", + "branch": "main", + } + result = _normalize_git_mount(entry) + assert "mappings" in result + assert result["mappings"] == [ + {"source_path": "packages/api", "target_path": "/app/api"} + ] + assert "source_path" not in result + assert "target_path" not in result + assert result["remote_url"] == "https://github.com/user/repo.git" + assert result["branch"] == "main" + + def test_already_mappings(self) -> None: + """Entry already with mappings is left unchanged.""" + entry = { + "remote_url": "https://github.com/user/repo.git", + "branch": "main", + "mappings": [ + {"source_path": "a", "target_path": "/a"}, + {"source_path": "b", "target_path": "/b"}, + ], + } + result = _normalize_git_mount(entry) + assert result["mappings"] == [ + {"source_path": "a", "target_path": "/a"}, + {"source_path": "b", "target_path": "/b"}, + ] + assert "source_path" not in result + assert "target_path" not in result + + def test_missing_target_path_no_mappings(self) -> None: + """Entry with source_path but no target_path creates empty mappings.""" + entry = { + "remote_url": "https://github.com/user/repo.git", + "source_path": "src", + } + result = _normalize_git_mount(entry) + assert "mappings" not in result + + +class TestResolveGitMountMappings: + """Tests for _resolve_git_mount_mappings.""" + + def test_single_mapping(self) -> None: + """A single mapping produces one volume mount.""" + with tempfile.TemporaryDirectory() as repo_path: + os.makedirs(os.path.join(repo_path, "packages", "api")) + mappings = [ + {"source_path": "packages/api", "target_path": "/app/api"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, None) + assert len(result) == 1 + assert result[0]["source"] == os.path.join(repo_path, "packages", "api") + assert result[0]["target"] == "/app/api" + assert result[0]["type"] == "bind" + + def test_multiple_mappings(self) -> None: + """Multiple mappings from same repo produce multiple mounts.""" + with tempfile.TemporaryDirectory() as repo_path: + os.makedirs(os.path.join(repo_path, "packages", "api")) + os.makedirs(os.path.join(repo_path, "packages", "web")) + mappings = [ + {"source_path": "packages/api", "target_path": "/app/api"}, + {"source_path": "packages/web", "target_path": "/app/web"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, None) + assert len(result) == 2 + targets = {r["target"] for r in result} + assert targets == {"/app/api", "/app/web"} + + def test_relative_target_path(self) -> None: + """Relative target_path is resolved against working_directory.""" + with tempfile.TemporaryDirectory() as repo_path: + os.makedirs(os.path.join(repo_path, "src")) + mappings = [ + {"source_path": "src", "target_path": "code"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, "/workspace") + assert len(result) == 1 + assert result[0]["target"] == "/workspace/code" + + def test_glob_expansion(self) -> None: + """Glob patterns in source_path are expanded.""" + with tempfile.TemporaryDirectory() as repo_path: + os.makedirs(os.path.join(repo_path, "packages", "api")) + os.makedirs(os.path.join(repo_path, "packages", "web")) + mappings = [ + {"source_path": "packages/*", "target_path": "/app/packages"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, None) + assert len(result) == 2 + targets = {r["target"] for r in result} + assert targets == { + os.path.join("/app/packages", "packages", "api"), + os.path.join("/app/packages", "packages", "web"), + } + + def test_missing_target_path_skipped(self) -> None: + """Mapping without target_path is skipped.""" + with tempfile.TemporaryDirectory() as repo_path: + mappings = [ + {"source_path": "src"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, None) + assert len(result) == 0 + + def test_no_working_directory_for_relative_target(self) -> None: + """Relative target without working_directory is skipped.""" + with tempfile.TemporaryDirectory() as repo_path: + os.makedirs(os.path.join(repo_path, "src")) + mappings = [ + {"source_path": "src", "target_path": "code"}, + ] + result = _resolve_git_mount_mappings(repo_path, mappings, None) + assert len(result) == 0 + + +class TestResolveSingleGitMount: + """Tests for _resolve_single_git_mount.""" + + @pytest.mark.asyncio + async def test_missing_remote_url(self) -> None: + """Git mount without remote_url returns empty list.""" + result = await _resolve_single_git_mount( + MagicMock(), {"mappings": [{"source_path": ".", "target_path": "/app"}]}, "/tmp", None + ) + assert result == [] + + @pytest.mark.asyncio + async def test_missing_instance_dir(self) -> None: + """Git mount without instance_dir returns empty list.""" + result = await _resolve_single_git_mount( + MagicMock(), + {"remote_url": "https://github.com/user/repo.git", "mappings": [{"source_path": ".", "target_path": "/app"}]}, + None, + None, + ) + assert result == [] + + @pytest.mark.asyncio + async def test_legacy_format_normalized(self) -> None: + """Legacy format is normalized and resolved.""" + with tempfile.TemporaryDirectory() as instance_dir: + with patch( + "src.api.tool_instances._clone_git_repo", + return_value=os.path.join(instance_dir, "repo-clone"), + ): + os.makedirs(os.path.join(instance_dir, "repo-clone", "src")) + result = await _resolve_single_git_mount( + MagicMock(), + { + "remote_url": "https://github.com/user/repo.git", + "source_path": "src", + "target_path": "/app/src", + }, + instance_dir, + None, + ) + assert len(result) == 1 + assert result[0]["target"] == "/app/src" + + +class TestExpandGlobSource: + """Tests for _expand_glob_source.""" + + def test_no_glob(self) -> None: + """Non-glob path returns single item if exists.""" + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "file.txt") + open(path, "w").close() + result = _expand_glob_source(path, tmp) + assert result == [path] + + def test_no_glob_missing(self) -> None: + """Non-glob path that doesn't exist returns empty list.""" + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "missing.txt") + result = _expand_glob_source(path, tmp) + assert result == [] + + def test_glob_pattern(self) -> None: + """Glob pattern expands to matched paths.""" + with tempfile.TemporaryDirectory() as tmp: + open(os.path.join(tmp, "a.txt"), "w").close() + open(os.path.join(tmp, "b.txt"), "w").close() + result = _expand_glob_source(os.path.join(tmp, "*.txt"), tmp) + assert len(result) == 2 diff --git a/apps/web/src/api/config_profiles.ts b/apps/web/src/api/config_profiles.ts index ec45350..3f08ad0 100644 --- a/apps/web/src/api/config_profiles.ts +++ b/apps/web/src/api/config_profiles.ts @@ -24,11 +24,18 @@ export interface ConfigProfileMount { files: Record; } -export interface GitMount { - remote_url: string; +export interface GitMountMapping { source_path: string; target_path: string; +} + +export interface GitMount { + remote_url: string; branch?: string; + mappings: GitMountMapping[]; + // Legacy fields (for backward compatibility when reading old data) + source_path?: string; + target_path?: string; } export interface ConfigProfileInclude { diff --git a/apps/web/src/components/git-mount-editor.tsx b/apps/web/src/components/git-mount-editor.tsx index 0a76739..0408b71 100644 --- a/apps/web/src/components/git-mount-editor.tsx +++ b/apps/web/src/components/git-mount-editor.tsx @@ -1,96 +1,113 @@ -import { useState } from "react"; +import { useState, useEffect } from "react"; import { Icon } from "./icon"; -import type { GitMount } from "../api/config_profiles"; +import type { GitMount, GitMountMapping } from "../api/config_profiles"; interface GitMountEditorProps { mounts: GitMount[]; onChange: (mounts: GitMount[]) => void; } +function normalizeMount(mount: GitMount): GitMount { + // Auto-convert legacy source_path + target_path to mappings + if ((!mount.mappings || mount.mappings.length === 0) && mount.source_path !== undefined && mount.target_path !== undefined) { + return { + remote_url: mount.remote_url, + branch: mount.branch, + mappings: [{ source_path: mount.source_path || ".", target_path: mount.target_path }], + }; + } + return mount; +} + +function normalizeMounts(mounts: GitMount[]): GitMount[] { + return mounts.map(normalizeMount); +} + export const GitMountEditor = ({ mounts, onChange }: GitMountEditorProps) => { + const [normalizedMounts, setNormalizedMounts] = useState(() => normalizeMounts(mounts)); const [editingIndex, setEditingIndex] = useState(null); - const [newMount, setNewMount] = useState({ - remote_url: "", - source_path: ".", - target_path: "", - branch: "", - }); + const [isAdding, setIsAdding] = useState(false); + + useEffect(() => { + setNormalizedMounts(normalizeMounts(mounts)); + }, [mounts]); const handleAdd = (mount: GitMount) => { - onChange([...mounts, mount]); - setNewMount({ remote_url: "", source_path: ".", target_path: "", branch: "" }); + const updated = [...normalizedMounts, normalizeMount(mount)]; + setNormalizedMounts(updated); + onChange(updated); + setIsAdding(false); }; const handleUpdate = (index: number, updated: GitMount) => { - const updatedMounts = [...mounts]; - updatedMounts[index] = updated; + const updatedMounts = [...normalizedMounts]; + updatedMounts[index] = normalizeMount(updated); + setNormalizedMounts(updatedMounts); onChange(updatedMounts); setEditingIndex(null); }; const handleRemove = (index: number) => { - onChange(mounts.filter((_, i) => i !== index)); - }; - - const validatePath = (path: string, isTarget: boolean): string | null => { - if (!path) return isTarget ? "Target path is required" : null; - if (path.includes("..")) return "Path cannot contain .."; - if (!isTarget && path.startsWith("/")) return "Source path must be relative"; - return null; - }; - - const validateUrl = (url: string): string | null => { - if (!url) return "Git URL is required"; - if (!url.startsWith("http://") && !url.startsWith("https://") && !url.startsWith("git@") && !url.startsWith("ssh://")) { - return "Must be a valid git URL (https://, git@, or ssh://)"; - } - return null; + const updated = normalizedMounts.filter((_, i) => i !== index); + setNormalizedMounts(updated); + onChange(updated); }; return (
-

Git Mounts

- - {mounts.length > 0 && ( -
- {mounts.map((mount, index) => ( -
+

Git Mounts

+

+ Clone a repository once and mount multiple directories from it. +

+ + {normalizedMounts.length > 0 && ( +
+ {normalizedMounts.map((mount, index) => ( +
{editingIndex === index ? ( handleUpdate(index, updated)} onCancel={() => setEditingIndex(null)} - validatePath={validatePath} - validateUrl={validateUrl} /> ) : ( -
-
- {mount.remote_url} - - {mount.source_path || "."} → {mount.target_path} - - {mount.branch && ( - @{mount.branch} - )} -
-
- - +
+
+
+
+ {mount.remote_url} + {mount.branch && ( + + @{mount.branch} + + )} +
+
+ {mount.mappings?.map((m, mi) => ( +
+ {m.source_path || "."} → {m.target_path} +
+ ))} +
+
+
+ + +
)} @@ -99,17 +116,20 @@ export const GitMountEditor = ({ mounts, onChange }: GitMountEditorProps) => {
)} -
-
Add Git Mount
- setNewMount({ remote_url: "", source_path: ".", target_path: "", branch: "" })} - validatePath={validatePath} - validateUrl={validateUrl} - isNew - /> -
+ {isAdding ? ( +
+ setIsAdding(false)} + /> +
+ ) : ( + + )}
); }; @@ -118,104 +138,169 @@ interface GitMountFormProps { mount: GitMount; onSave: (mount: GitMount) => void; onCancel: () => void; - validatePath: (path: string, isTarget: boolean) => string | null; - validateUrl: (url: string) => string | null; - isNew?: boolean; } -const GitMountForm = ({ mount, onSave, onCancel, validatePath, validateUrl, isNew }: GitMountFormProps) => { - const [form, setForm] = useState({ ...mount }); +const GitMountForm = ({ mount, onSave, onCancel }: GitMountFormProps) => { + const [remoteUrl, setRemoteUrl] = useState(mount.remote_url); + const [branch, setBranch] = useState(mount.branch || ""); + const [mappings, setMappings] = useState( + mount.mappings?.length ? mount.mappings : [{ source_path: ".", target_path: "" }] + ); const [errors, setErrors] = useState>({}); - const handleChange = (field: keyof GitMount, value: string) => { - setForm((prev) => ({ ...prev, [field]: value })); - if (errors[field]) { + const validate = (): boolean => { + const newErrors: Record = {}; + + if (!remoteUrl.trim()) { + newErrors.remote_url = "Git URL is required"; + } else if ( + !remoteUrl.startsWith("http://") && + !remoteUrl.startsWith("https://") && + !remoteUrl.startsWith("git@") && + !remoteUrl.startsWith("ssh://") + ) { + newErrors.remote_url = "Must be a valid git URL (https://, git@, or ssh://)"; + } + + mappings.forEach((m, i) => { + if (!m.target_path.trim()) { + newErrors[`mapping_${i}_target`] = "Target path is required"; + } + if (m.source_path.includes("..")) { + newErrors[`mapping_${i}_source`] = "Source path cannot contain .."; + } + if (m.target_path.includes("..")) { + newErrors[`mapping_${i}_target`] = "Target path cannot contain .."; + } + }); + + setErrors(newErrors); + return Object.keys(newErrors).length === 0; + }; + + const handleSubmit = () => { + if (!validate()) return; + onSave({ + remote_url: remoteUrl.trim(), + branch: branch.trim() || undefined, + mappings: mappings.map((m) => ({ + source_path: m.source_path.trim() || ".", + target_path: m.target_path.trim(), + })), + }); + }; + + const addMapping = () => { + setMappings((prev) => [...prev, { source_path: ".", target_path: "" }]); + }; + + const updateMapping = (index: number, field: keyof GitMountMapping, value: string) => { + setMappings((prev) => { + const next = [...prev]; + next[index] = { ...next[index], [field]: value }; + return next; + }); + if (errors[`mapping_${index}_${field}`]) { setErrors((prev) => { const next = { ...prev }; - delete next[field]; + delete next[`mapping_${index}_${field}`]; return next; }); } }; - const handleSubmit = () => { - const newErrors: Record = {}; - - const urlError = validateUrl(form.remote_url); - if (urlError) newErrors.remote_url = urlError; - - const sourceError = validatePath(form.source_path || ".", false); - if (sourceError) newErrors.source_path = sourceError; - - const targetError = validatePath(form.target_path, true); - if (targetError) newErrors.target_path = targetError; - - if (Object.keys(newErrors).length > 0) { - setErrors(newErrors); - return; - } - - onSave(form); - if (isNew) { - setForm({ remote_url: "", source_path: ".", target_path: "", branch: "" }); - } + const removeMapping = (index: number) => { + setMappings((prev) => prev.filter((_, i) => i !== index)); }; return ( -
-
- - handleChange("remote_url", e.target.value)} - placeholder="https://github.com/user/repo.git" - className={errors.remote_url ? "error" : ""} - /> - Repository URL (HTTPS or SSH) - {errors.remote_url && {errors.remote_url}} +
+
+
+ + { + setRemoteUrl(e.target.value); + if (errors.remote_url) { + setErrors((prev) => { + const next = { ...prev }; + delete next.remote_url; + return next; + }); + } + }} + placeholder="https://github.com/user/repo.git" + className={`form-input ${errors.remote_url ? "error" : ""}`} + /> + {errors.remote_url && {errors.remote_url}} +
+
+ + setBranch(e.target.value)} + placeholder="main" + className="form-input" + /> +
-
- - handleChange("source_path", e.target.value)} - placeholder="e.g., . or configs/*.json" - className={errors.source_path ? "error" : ""} - /> - Relative path in repo (supports glob patterns) - {errors.source_path && {errors.source_path}} +
+ +

+ Source paths within the repo and where to mount them in the container. +

+
+ {mappings.map((mapping, index) => ( +
+ updateMapping(index, "source_path", e.target.value)} + placeholder="packages/api" + className={`form-input ${errors[`mapping_${index}_source`] ? "error" : ""}`} + style={{ flex: 1 }} + /> + → + updateMapping(index, "target_path", e.target.value)} + placeholder="/app/api" + className={`form-input ${errors[`mapping_${index}_target`] ? "error" : ""}`} + style={{ flex: 1 }} + /> + {mappings.length > 1 && ( + + )} + {errors[`mapping_${index}_source`] && ( + {errors[`mapping_${index}_source`]} + )} + {errors[`mapping_${index}_target`] && ( + {errors[`mapping_${index}_target`]} + )} +
+ ))} +
+
-
- - handleChange("target_path", e.target.value)} - placeholder="e.g., /app/config" - className={errors.target_path ? "error" : ""} - /> - Use absolute path (e.g. /app/config). Relative paths need working_directory set in tool config. - {errors.target_path && {errors.target_path}} -
- -
- - handleChange("branch", e.target.value)} - placeholder="e.g., main or v1.0" - /> - Branch or tag to checkout -
- -
+
); -}; \ No newline at end of file +}; diff --git a/openspec/changes/config-profile-multi-repo-mounts/.openspec.yaml b/openspec/changes/config-profile-multi-repo-mounts/.openspec.yaml new file mode 100644 index 0000000..4fedfd6 --- /dev/null +++ b/openspec/changes/config-profile-multi-repo-mounts/.openspec.yaml @@ -0,0 +1,8 @@ +name: config-profile-multi-repo-mounts +status: completed +phase: verify +parent: null +type: feature +description: Enable multiple source/target mappings per git mount entry in Config Profiles, cloning the repository only once per entry. +created_at: 2026-05-28 +updated_at: 2026-05-28 diff --git a/openspec/designs/config-profile-multi-repo-mounts.md b/openspec/designs/config-profile-multi-repo-mounts.md new file mode 100644 index 0000000..b6db33f --- /dev/null +++ b/openspec/designs/config-profile-multi-repo-mounts.md @@ -0,0 +1,172 @@ +# Design: Config Profile Multi-Repo Mounts + +## Architecture + +### Data Model + +No database changes. The `git_mounts` JSONB column already stores arbitrary JSON. + +#### Normalized git mount schema (in memory) + +After validation/normalization, every git mount entry is converted to the unified form: + +```python +{ + "remote_url": str, + "branch": str | None, + "mappings": [ + {"source_path": str, "target_path": str}, + ... + ] +} +``` + +The normalization step converts legacy `source_path` + `target_path` into a single-entry `mappings` array. + +### Backend Changes + +#### 1. `_resolve_single_git_mount` refactor + +Split into two functions: + +**`_clone_git_repo(remote_url, branch, clone_parent) -> repo_path`** +- Clones or pulls the repo +- Returns the path to `repo-clone` +- Same as before, but extracts the clone logic + +**`_resolve_git_mount_mappings(repo_path, mappings, working_directory) -> list[dict]`** +- Takes the already-cloned repo path +- For each mapping: + 1. Build source path: `os.path.join(repo_path, mapping["source_path"])` + 2. Expand globs via `_expand_glob_source` + 3. Resolve target path (absolute or relative to working_directory) + 4. Build volume mount entries +- Returns list of volume mount dicts + +**`_resolve_single_git_mount` new flow:** +1. Validate entry (remote_url, mappings or source_path+target_path) +2. Normalize legacy format to `mappings` array +3. Compute clone directory (same as before: `git-mounts/{repo_name}-{hash}/`) +4. Clone/pull repo +5. Resolve all mappings from the cloned repo +6. Return flat list of volume mounts + +#### 2. `_merge_git_mounts` update + +The merge key changes from `(remote_url, target_path)` to `(remote_url, branch)`. + +When two entries have the same `remote_url` and `branch`, their `mappings` arrays are concatenated. When different, they are kept as separate entries. + +```python +def _merge_git_mounts(base, overlay, source_name): + result = list(base) + seen = {} + for i, m in enumerate(result): + key = (m["remote_url"], m.get("branch")) + seen[key] = i + + for mount in overlay: + key = (mount["remote_url"], mount.get("branch")) + if key in seen: + # Same repo+branch: concatenate mappings + result[seen[key]]["mappings"].extend(mount.get("mappings", [])) + else: + seen[key] = len(result) + result.append(dict(mount)) + return result +``` + +#### 3. Validation on save + +In the Config Profile API (create/update), validate `git_mounts`: +- Each entry must have `remote_url` +- Each entry must have either `mappings` OR (`source_path` AND `target_path`) +- Each mapping must have `source_path` and `target_path` +- `mappings` must be a non-empty array + +### Frontend Changes + +#### GitMountEditor component + +New or updated component for editing a single git mount entry: + +``` +Remote URL: [____________________] +Branch: [main________________] + +Mappings: + Source Path → Target Path + [packages/api ] [/app/api ] [×] + [packages/web ] [/app/web ] [×] + [ ] [ ] [+ Add] +``` + +**State shape:** +```typescript +interface GitMountMapping { + source_path: string; + target_path: string; +} + +interface GitMountEntry { + remote_url: string; + branch?: string; + mappings: GitMountMapping[]; + // Legacy fields (read-only for old data) + source_path?: string; + target_path?: string; +} +``` + +**Migration on load:** If an entry has `source_path` and `target_path` but no `mappings`, auto-convert: +```typescript +if (!entry.mappings && entry.source_path && entry.target_path) { + entry.mappings = [{ source_path: entry.source_path, target_path: entry.target_path }]; +} +``` + +### File Changes + +| File | Change | +|------|--------| +| `apps/api/src/api/tool_instances.py` | Refactor `_resolve_single_git_mount` to support mappings | +| `apps/api/src/services/config_profile_resolver.py` | Update `_merge_git_mounts` merge key | +| `apps/api/src/api/config_profiles.py` | Add validation for git_mounts schema | +| `apps/web/src/components/config-profile-editor.tsx` | Add mappings UI for git mounts | +| `apps/web/src/api/config_profiles.ts` | Update types for GitMountEntry | +| `apps/api/tests/unit/test_git_mounts.py` | New unit tests for multi-mapping resolution | +| `apps/api/tests/unit/test_config_profile_resolver.py` | Update merge tests | + +### Testing Strategy + +#### Backend unit tests + +1. `_resolve_single_git_mount` with 3 mappings → single clone, 3 mounts +2. `_resolve_single_git_mount` legacy format → single clone, 1 mount +3. `_merge_git_mounts` same repo+branch → mappings concatenated +4. `_merge_git_mounts` different repos → separate entries +5. Validation: entry with neither mappings nor source_path → error +6. Validation: mapping missing target_path → error + +#### Integration tests + +1. Create profile with 2 mappings from same repo → start instance → verify single clone directory +2. Create profile with legacy format → start instance → verify backward compatibility + +#### Frontend tests + +1. GitMountEditor renders mappings list +2. Adding a mapping updates state correctly +3. Legacy entry auto-converts on load +4. Save sends correct JSON shape + +### Migration Plan + +No database migration. Existing `git_mounts` JSON continues to work because: +- The code normalizes legacy `source_path` + `target_path` to `mappings` on read +- The frontend auto-converts on load +- New saves use the `mappings` format + +### Rollback Plan + +Since there is no schema change, rollback is just reverting the code. Existing profiles with the new `mappings` format will still parse correctly even with old code if we keep the normalization shim. diff --git a/openspec/explorations/config-profile-multi-repo-mounts.md b/openspec/explorations/config-profile-multi-repo-mounts.md new file mode 100644 index 0000000..b3b0187 --- /dev/null +++ b/openspec/explorations/config-profile-multi-repo-mounts.md @@ -0,0 +1,120 @@ +# Exploration: Config Profile Multi-Repo Mounts + +## Problem + +Config Profiles support `git_mounts` — cloning repositories and mounting them into containers. However, each `git_mount` entry clones **one** source path from **one** repo. If a user wants to mount multiple directories from the same repository (e.g., a monorepo), they must add multiple `git_mount` entries, which results in **cloning the same repository multiple times**. + +### Current git_mount schema (one mapping per entry) + +```json +{ + "remote_url": "https://github.com/user/monorepo.git", + "source_path": "packages/backend", + "target_path": "/app/backend", + "branch": "main" +} +``` + +To mount 3 directories from the same monorepo, the profile needs 3 entries, each triggering a separate clone of the full repository. + +## Pain Points + +1. **Redundant clones**: Cloning the same repo N times wastes time and disk space. +2. **Slow instance startup**: Each clone adds 5-30 seconds depending on repo size. +3. **Inconsistent branch state**: Each entry independently checks out the branch — they could drift if the branch moves between clones. +4. **Poor monorepo support**: Monorepos are common; users expect to mount multiple packages. + +## Scenarios + +### Scenario A: Monorepo with multiple packages + +User has a monorepo `corp/monorepo` with: +- `packages/api` → needs to be at `/app/api` +- `packages/web` → needs to be at `/app/web` +- `packages/shared` → needs to be at `/app/shared` + +They want to mount all three into a single tool instance. + +### Scenario B: Docs + Code sidecar + +User wants to mount both: +- `src/` → `/workspace/src` +- `docs/` → `/workspace/docs` + +from the same repo. + +### Scenario C: Backward compatibility + +Existing profiles with single `source_path`/`target_path` should continue working without migration. + +## Design Directions + +### Direction A: `mappings` array on git_mount entry + +Add a `mappings` array to each git_mount entry. The repo is cloned once, and each mapping creates a separate bind mount. + +```json +{ + "remote_url": "https://github.com/user/monorepo.git", + "branch": "main", + "mappings": [ + {"source_path": "packages/api", "target_path": "/app/api"}, + {"source_path": "packages/web", "target_path": "/app/web"} + ] +} +``` + +**Pros**: +- Clean, explicit grouping +- Single clone per `remote_url + branch` combo +- Easy to understand +- Backward-compatible: legacy `source_path` + `target_path` can be treated as a single-entry `mappings` array + +**Cons**: +- Slightly more verbose JSON +- Frontend form needs a nested list UI + +### Direction B: Auto-dedup by remote_url + branch + +Keep the flat list format, but internally group entries by `remote_url + branch` and clone once. + +```json +[ + {"remote_url": "...", "source_path": "a", "target_path": "/a", "branch": "main"}, + {"remote_url": "...", "source_path": "b", "target_path": "/b", "branch": "main"} +] +``` + +**Pros**: +- No schema change +- Transparent to users + +**Cons**: +- Magic behavior (not obvious why clones are shared) +- Harder to reason about branch conflicts (what if same repo, different branches?) +- Frontend doesn't show the grouping + +### Direction C: Repo references + mount definitions split + +Split into two concepts: +1. `git_repos` — list of repos to clone (with branch) +2. `git_mounts` — reference a repo by name and specify source/target + +**Pros**: +- Very explicit +- Supports advanced scenarios (SSH keys per repo) + +**Cons**: +- Breaking schema change +- Overkill for the current use case +- Heavy migration burden + +## Recommendation + +**Direction A (mappings array)** with backward-compatibility shim: +- Add optional `mappings` field to git_mount entries +- If `mappings` is absent, treat `source_path` + `target_path` as a single mapping +- Clone once per `remote_url + branch`, apply all mappings from the same entry +- No migration needed for existing data + +This balances clarity, functionality, and backward compatibility. diff --git a/openspec/proposals/config-profile-multi-repo-mounts.md b/openspec/proposals/config-profile-multi-repo-mounts.md new file mode 100644 index 0000000..a6773f3 --- /dev/null +++ b/openspec/proposals/config-profile-multi-repo-mounts.md @@ -0,0 +1,50 @@ +# Proposal: Config Profile Multi-Repo Mounts + +## Context + +Config Profiles allow users to mount external Git repositories into tool instances via `git_mounts`. Currently, each `git_mount` entry supports only **one** source_path → target_path mapping. If a user wants to mount multiple directories from the same repository (e.g., a monorepo), they must add multiple entries, each cloning the repository independently. + +## Goal + +Enable a single `git_mount` entry to declare **multiple** source/target mappings from the same repository, while cloning the repository only once per entry. + +## Direction + +**Direction A: `mappings` array with backward compatibility** + +Add an optional `mappings` array to each `git_mount` entry. The repository is cloned once, and each mapping creates a separate bind mount from a subdirectory of the cloned repo. + +If `mappings` is absent, the existing `source_path` + `target_path` fields are treated as a single mapping (backward-compatible). + +## Acceptance Criteria + +1. A `git_mount` entry can specify `mappings: [{"source_path": "...", "target_path": "..."}, ...]` +2. The repository is cloned **exactly once** per `git_mount` entry +3. Each mapping creates a separate Docker bind mount from the cloned repo subdirectory +4. Existing profiles with `source_path`/`target_path` continue working without migration +5. The Config Profile editor UI supports adding/removing mappings per git mount +6. Glob patterns are supported in `source_path` within mappings +7. Relative `target_path` values are resolved against `working_directory` as before + +## Out of Scope + +- Cross-entry repo deduplication (two separate git_mount entries with the same `remote_url` still clone twice) +- SSH key per-repo configuration (can be added later) +- Sparse checkout / partial clone optimization +- Mounting from non-Git sources + +## Risks + +| Risk | Mitigation | +|------|------------| +| Schema migration complexity | No migration needed — backward-compatible | +| Frontend UI complexity | Nested form with add/remove mapping buttons | +| Clone directory sharing race condition | Each entry gets its own clone directory (url_hash based) | +| Large monorepo clone time | Out of scope — full clone is existing behavior | + +## Related Artifacts + +- Exploration: `openspec/explorations/config-profile-multi-repo-mounts.md` +- Spec: `openspec/specs/config-profile-multi-repo-mounts.md` +- Design: `openspec/designs/config-profile-multi-repo-mounts.md` +- Tasks: `openspec/tasks/config-profile-multi-repo-mounts.md` diff --git a/openspec/specs/config-profile-multi-repo-mounts.md b/openspec/specs/config-profile-multi-repo-mounts.md new file mode 100644 index 0000000..74e4925 --- /dev/null +++ b/openspec/specs/config-profile-multi-repo-mounts.md @@ -0,0 +1,119 @@ +# Spec: Config Profile Multi-Repo Mounts + +## Requirements + +### Functional + +1. **FR-1**: A `git_mount` entry MAY include a `mappings` array. +2. **FR-2**: Each item in `mappings` MUST have `source_path` and `target_path`. +3. **FR-3**: If `mappings` is absent, `source_path` and `target_path` at the entry level MUST be treated as a single mapping (backward compatibility). +4. **FR-4**: The repository MUST be cloned exactly once per `git_mount` entry. +5. **FR-5**: Each mapping MUST create a separate Docker bind mount. +6. **FR-6**: Glob patterns in `source_path` MUST be expanded per mapping. +7. **FR-7**: Relative `target_path` values MUST be resolved against `working_directory`. +8. **FR-8**: The merge logic for included profiles MUST deduplicate by `(remote_url, branch)` within a single resolved profile's `git_mounts` list. + +### Non-Functional + +1. **NFR-1**: No database schema migration required. +2. **NFR-2**: Existing API responses must remain backward-compatible. +3. **NFR-3**: Frontend type-check must pass without errors. + +## API Contracts + +### ConfigProfile model (git_mounts field) + +```json +{ + "git_mounts": [ + { + "remote_url": "https://github.com/user/monorepo.git", + "branch": "main", + "mappings": [ + {"source_path": "packages/api", "target_path": "/app/api"}, + {"source_path": "packages/web", "target_path": "/app/web"} + ] + }, + { + "remote_url": "https://github.com/user/docs.git", + "source_path": ".", + "target_path": "/docs", + "branch": "main" + } + ] +} +``` + +### Validation rules + +1. `mappings` must be a non-empty array if present. +2. Each mapping must have `source_path` (string) and `target_path` (string). +3. Either `mappings` OR (`source_path` AND `target_path`) must be present. +4. `remote_url` must be a valid HTTPS or SSH Git URL. + +## Database Schema + +No changes. `git_mounts` is stored as JSONB in `config_profiles.git_mounts`. + +## Scenarios + +### Scenario 1: Monorepo with multiple packages + +**Given** a Config Profile with: +```json +{"git_mounts": [{ + "remote_url": "https://github.com/corp/monorepo.git", + "branch": "main", + "mappings": [ + {"source_path": "packages/api", "target_path": "/app/api"}, + {"source_path": "packages/web", "target_path": "/app/web"}, + {"source_path": "packages/shared", "target_path": "/app/shared"} + ] +}]} +``` + +**When** the profile is applied to an instance + +**Then**: +1. `corp/monorepo` is cloned once to `git-mounts/monorepo-{hash}/repo-clone` +2. Three bind mounts are created: + - `{clone}/packages/api` → `/app/api` + - `{clone}/packages/web` → `/app/web` + - `{clone}/packages/shared` → `/app/shared` + +### Scenario 2: Legacy single mapping (backward compatibility) + +**Given** a Config Profile with: +```json +{"git_mounts": [{ + "remote_url": "https://github.com/user/repo.git", + "source_path": "src", + "target_path": "/workspace/src", + "branch": "main" +}]} +``` + +**When** the profile is applied + +**Then** the behavior is identical to before (single clone, single mount). + +### Scenario 3: Glob expansion within mapping + +**Given** a mapping with: +```json +{"source_path": "packages/*", "target_path": "/app/packages"} +``` + +**When** the repo is cloned and the glob is expanded + +**Then** each matched directory is mounted as a separate bind mount with the relative path appended to the target: +- `{clone}/packages/api` → `/app/packages/api` +- `{clone}/packages/web` → `/app/packages/web` + +## Test Strategy + +1. Unit test `_resolve_single_git_mount` with `mappings` array +2. Unit test `_merge_git_mounts` with mappings deduplication +3. Integration test: profile with 3 mappings from same repo → verify single clone +4. Integration test: legacy profile without `mappings` → verify backward compatibility +5. Frontend unit test: GitMountEditor renders mappings form correctly diff --git a/openspec/tasks/config-profile-multi-repo-mounts.md b/openspec/tasks/config-profile-multi-repo-mounts.md new file mode 100644 index 0000000..d0b1c8e --- /dev/null +++ b/openspec/tasks/config-profile-multi-repo-mounts.md @@ -0,0 +1,132 @@ +# Tasks: Config Profile Multi-Repo Mounts + +## T1: Backend — Refactor git mount resolution for mappings + +### T1.1: Normalize legacy git mount format +**File**: `apps/api/src/api/tool_instances.py` +- Add `_normalize_git_mount(entry: dict) -> dict` helper +- Converts `{"source_path": "...", "target_path": "..."}` to `{"mappings": [{...}]}` +- Call normalization at the start of `_resolve_single_git_mount` + +### T1.2: Extract clone logic +**File**: `apps/api/src/api/tool_instances.py` +- Create `_clone_git_repo(remote_url, branch, clone_parent) -> str` function +- Move clone/pull logic from `_resolve_single_git_mount` into it +- Returns `repo_path` (path to `repo-clone`) + +### T1.3: Resolve mappings from cloned repo +**File**: `apps/api/src/api/tool_instances.py` +- Create `_resolve_git_mount_mappings(repo_path, mappings, working_directory) -> list[dict]` +- Iterates over mappings, expands globs, resolves targets +- Returns flat list of volume mount dicts + +### T1.4: Wire it together +**File**: `apps/api/src/api/tool_instances.py` +- Update `_resolve_single_git_mount` to: + 1. Normalize entry + 2. Clone repo once + 3. Resolve all mappings + 4. Return flat volume mounts + +### T1.5: Update merge logic +**File**: `apps/api/src/services/config_profile_resolver.py` +- Change `_merge_git_mounts` merge key from `(remote_url, target_path)` to `(remote_url, branch)` +- When same repo+branch: concatenate `mappings` arrays +- When different: append as separate entry + +### T1.6: Add validation +**File**: `apps/api/src/api/config_profiles.py` +- Validate `git_mounts` on create/update: + - `remote_url` required + - Either `mappings` (non-empty array) OR (`source_path` + `target_path`) + - Each mapping has `source_path` and `target_path` + +### T1.7: Unit tests +**File**: `apps/api/tests/unit/test_git_mounts.py` (new) +- Test normalization: legacy → mappings +- Test single clone with 3 mappings → 3 volume mounts +- Test glob expansion within mapping +- Test relative target resolution + +**File**: `apps/api/tests/unit/test_config_profile_resolver.py` +- Update merge tests for new dedup key + +--- + +## T2: Frontend — Git mount mappings editor + +### T2.1: Update types +**File**: `apps/web/src/api/config_profiles.ts` +- Add `GitMountMapping` interface +- Update `GitMountEntry` to have `mappings: GitMountMapping[]` +- Keep optional `source_path`/`target_path` for backward compat + +### T2.2: Auto-convert legacy entries on load +**File**: `apps/web/src/components/config-profile-editor.tsx` or new `GitMountEditor.tsx` +- On loading a profile, normalize any git_mount entries that lack `mappings` + +### T2.3: Build mappings UI +**File**: `apps/web/src/components/GitMountEditor.tsx` (new) +- Render table/list of mappings per git mount entry +- "Add mapping" button appends empty row +- "Remove" button deletes a mapping row +- Source path and target path inputs + +### T2.4: Integrate into ConfigProfileEditor +**File**: `apps/web/src/components/config-profile-editor.tsx` +- Replace existing git_mounts flat form with GitMountEditor component +- Ensure save sends correct JSON shape + +### T2.5: Frontend tests +**File**: `apps/web/src/components/GitMountEditor.test.tsx` (new) +- Render with 2 mappings +- Add mapping increases count +- Remove mapping decreases count +- Legacy entry auto-converts + +--- + +## T3: Integration & Verification + +### T3.1: Integration test +**File**: `apps/api/tests/integration/test_config_profiles_git_mounts.py` (new) +- Create profile with 2 mappings from same repo +- Start instance +- Verify single clone directory exists +- Verify 2 bind mounts in compose file + +### T3.2: Manual verification +- Create a Config Profile with a monorepo git mount + 3 mappings +- Create and start a pi-agent instance with the profile +- Verify all 3 directories are mounted correctly +- Verify legacy profile (single mapping) still works + +### T3.3: Typecheck & tests +```bash +cd apps/web && npm run typecheck +cd apps/api && pytest tests/unit/test_git_mounts.py tests/unit/test_config_profile_resolver.py +``` + +--- + +## Estimation + +| Task | Effort | Files | +|------|--------|-------| +| T1.1-T1.4 | 2h | 1 | +| T1.5 | 1h | 1 | +| T1.6 | 1h | 1 | +| T1.7 | 2h | 2 | +| T2.1-T2.4 | 3h | 3 | +| T2.5 | 1h | 1 | +| T3.1-T3.3 | 2h | 2 | +| **Total** | **12h** | **11** | + +## PR Strategy + +**Single PR** (~400 lines estimated, within review budget): +- Backend changes (T1) +- Frontend changes (T2) +- Tests (T1.7, T2.5, T3.1) + +All changes are tightly coupled (backend schema + frontend UI + tests) so a single PR is appropriate.