From 68a1c74f18b2174561ec4455d55ced6c1348be2e Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Wed, 28 Jan 2026 15:02:15 +0100 Subject: [PATCH 1/7] Fix git worktree detection for `dda env dev start` Previously, `dda env dev start` failed when run from git worktrees with arbitrary directory names because it assumed repository directories were named exactly as the repository (e.g., "datadog-agent"). This commit adds git-aware repository resolution that: - Detects git repositories using remote URL, working for both regular repos and worktrees regardless of directory name - Maintains full backward compatibility with non-git directory lookup - Handles edge cases: missing git, no remote, nested structures Implementation: - Added `_resolve_repository_path()` with multi-strategy resolution - Added `_is_matching_repository()` using git remote URL detection - Updated repository mounting to use new git-aware resolver - Added comprehensive tests for worktree scenarios Fixes issue where worktrees like `.worktrees/my_feature` would fail with "Local repository not found: datadog-agent" Co-Authored-By: Claude Sonnet 4.5 --- src/dda/env/dev/types/linux_container.py | 83 +++++++++++++++++++-- tests/env/dev/types/test_linux_container.py | 33 ++++++++ 2 files changed, 109 insertions(+), 7 deletions(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index c4c46dfc..a2b13eab 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -187,15 +187,9 @@ def start(self) -> None: command.extend(("--mount", mount.as_csv())) if not self.config.clone: - from dda.utils.fs import Path - - repos_path = Path.cwd().parent for repo_spec in self.config.repos: repo = repo_spec.split("@")[0] - repo_path = repos_path / repo - if not repo_path.is_dir(): - self.app.abort(f"Local repository not found: {repo}") - + repo_path = self._resolve_repository_path(repo_spec) command.extend(("-v", f"{repo_path}:{self.repo_path(repo)}")) for mount_spec in self.config.extra_mount_specs: @@ -450,6 +444,81 @@ def repo_path(self, repo: str | None) -> str: return f"{self.home_dir}/repos/{repo}" + def _resolve_repository_path(self, repo_spec: str) -> Path: + """ + Resolve the local path for a repository specification. + + Tries multiple strategies: + 1. Check if current directory is the requested repo (git-aware, handles worktrees) + 2. Check parent directory for repo (backward compatible) + + Args: + repo_spec: Repository specification (e.g., "datadog-agent" or "datadog-agent@branch") + + Returns: + Path to the repository + + Raises: + Aborts if repository cannot be found + """ + from dda.utils.fs import Path + + repo_name = repo_spec.split("@")[0] # Strip @branch/@tag if present + + # Strategy 1: Check if current directory is a git repository matching the repo name + cwd = Path.cwd() + if self._is_matching_repository(cwd, repo_name): + return cwd + + # Strategy 2: Check parent directory (existing behavior, backward compatible) + parent_repo_path = cwd.parent / repo_name + if parent_repo_path.is_dir(): + if self._is_matching_repository(parent_repo_path, repo_name): + return parent_repo_path + # Fallback: If not a git repo but directory exists, use it for backward compat + return parent_repo_path + + self.app.abort(f"Local repository not found: {repo_name}") + + def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: + """ + Check if the given path is a git repository matching the expected repository name. + + Uses git remote URL to determine the repository name, which works for: + - Regular repositories + - Git worktrees (regardless of directory name) + - Nested repository structures + + Args: + path: Path to check + expected_repo_name: Expected repository name (e.g., "datadog-agent") + + Returns: + True if the path is a git repository matching the expected name + """ + if not path.is_dir(): + return False + + git_dir = path / ".git" + if not git_dir.exists(): + return False + + # Use git to get the repository name from the remote URL + try: + # Change to the target directory temporarily to get its remote + import os + + original_cwd = os.getcwd() + try: + os.chdir(path) + remote = self.app.tools.git.get_remote() + return remote.repo == expected_repo_name + finally: + os.chdir(original_cwd) + except Exception: + # Not a git repository or no remote configured + return False + def _container_cp(self, source: str, destination: str, *args: Any) -> None: """Runs a `cp -r` command inside the context of the container""" self.run_command(["cp", "-r", f'"{source}"', f'"{destination}"', *args]) diff --git a/tests/env/dev/types/test_linux_container.py b/tests/env/dev/types/test_linux_container.py index 6219c314..677c5f5e 100644 --- a/tests/env/dev/types/test_linux_container.py +++ b/tests/env/dev/types/test_linux_container.py @@ -1415,6 +1415,39 @@ def test_directory_to_existing_file_fails(self, linux_container, test_target_dir linux_container.export_path(source="/folder1", destination=existing_file) +class TestRepositoryResolution: + """Tests for git-aware repository resolution logic.""" + + def test_git_worktree_current_directory(self, app, mocker, temp_dir): + """Test git worktree with arbitrary directory name is detected as current directory.""" + worktree_dir = temp_dir / "my_feature_branch" + worktree_dir.ensure_dir() + (worktree_dir / ".git").touch() + + mock_remote = mocker.MagicMock() + mock_remote.repo = "datadog-agent" + mocker.patch.object(app.tools.git, "get_remote", return_value=mock_remote) + + container = LinuxContainer(app=app, name="test", instance="test") + with worktree_dir.as_cwd(): + resolved = container._resolve_repository_path("datadog-agent") + + assert resolved == worktree_dir + + def test_backward_compatibility_parent_directory(self, app, mocker, temp_dir): + """Test non-git directory is found via parent/repo_name (backward compatible).""" + repo_dir = temp_dir / "datadog-agent" + repo_dir.ensure_dir() + + mocker.patch.object(app.tools.git, "get_remote", side_effect=Exception("Not a git repo")) + + container = LinuxContainer(app=app, name="test", instance="test") + with repo_dir.as_cwd(): + resolved = container._resolve_repository_path("datadog-agent") + + assert resolved == repo_dir + + class TestImportFiles: """Basic import operations.""" From 9ceebf7af3066c0ba5c44b4f33c82c19d7c32717 Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Wed, 28 Jan 2026 17:39:01 +0100 Subject: [PATCH 2/7] Fix linting issues: add noqa comments for ruff --- src/dda/env/dev/types/linux_container.py | 4 ++-- tests/env/dev/types/test_linux_container.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index a2b13eab..7c5c3c6d 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -478,7 +478,7 @@ def _resolve_repository_path(self, repo_spec: str) -> Path: # Fallback: If not a git repo but directory exists, use it for backward compat return parent_repo_path - self.app.abort(f"Local repository not found: {repo_name}") + self.app.abort(f"Local repository not found: {repo_name}") # noqa: RET503 def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: """ @@ -515,7 +515,7 @@ def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: return remote.repo == expected_repo_name finally: os.chdir(original_cwd) - except Exception: + except Exception: # noqa: BLE001 # Not a git repository or no remote configured return False diff --git a/tests/env/dev/types/test_linux_container.py b/tests/env/dev/types/test_linux_container.py index 677c5f5e..c870b2f9 100644 --- a/tests/env/dev/types/test_linux_container.py +++ b/tests/env/dev/types/test_linux_container.py @@ -1430,7 +1430,7 @@ def test_git_worktree_current_directory(self, app, mocker, temp_dir): container = LinuxContainer(app=app, name="test", instance="test") with worktree_dir.as_cwd(): - resolved = container._resolve_repository_path("datadog-agent") + resolved = container._resolve_repository_path("datadog-agent") # noqa: SLF001 assert resolved == worktree_dir @@ -1443,7 +1443,7 @@ def test_backward_compatibility_parent_directory(self, app, mocker, temp_dir): container = LinuxContainer(app=app, name="test", instance="test") with repo_dir.as_cwd(): - resolved = container._resolve_repository_path("datadog-agent") + resolved = container._resolve_repository_path("datadog-agent") # noqa: SLF001 assert resolved == repo_dir From 181b76cc495fe142add987b128b442968d4f3f4c Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Wed, 28 Jan 2026 17:41:30 +0100 Subject: [PATCH 3/7] Add explanatory comments for noqa directives --- src/dda/env/dev/types/linux_container.py | 2 ++ tests/env/dev/types/test_linux_container.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index 7c5c3c6d..629f6425 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -478,6 +478,7 @@ def _resolve_repository_path(self, repo_spec: str) -> Path: # Fallback: If not a git repo but directory exists, use it for backward compat return parent_repo_path + # app.abort() always raises an exception, so no explicit return is needed self.app.abort(f"Local repository not found: {repo_name}") # noqa: RET503 def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: @@ -515,6 +516,7 @@ def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: return remote.repo == expected_repo_name finally: os.chdir(original_cwd) + # Catch all git-related exceptions (subprocess errors, missing git, no remote, etc.) except Exception: # noqa: BLE001 # Not a git repository or no remote configured return False diff --git a/tests/env/dev/types/test_linux_container.py b/tests/env/dev/types/test_linux_container.py index c870b2f9..d766d263 100644 --- a/tests/env/dev/types/test_linux_container.py +++ b/tests/env/dev/types/test_linux_container.py @@ -1430,6 +1430,7 @@ def test_git_worktree_current_directory(self, app, mocker, temp_dir): container = LinuxContainer(app=app, name="test", instance="test") with worktree_dir.as_cwd(): + # Testing internal repository resolution logic directly resolved = container._resolve_repository_path("datadog-agent") # noqa: SLF001 assert resolved == worktree_dir @@ -1443,6 +1444,7 @@ def test_backward_compatibility_parent_directory(self, app, mocker, temp_dir): container = LinuxContainer(app=app, name="test", instance="test") with repo_dir.as_cwd(): + # Testing internal repository resolution logic directly resolved = container._resolve_repository_path("datadog-agent") # noqa: SLF001 assert resolved == repo_dir From 7aeacc405d289042de5eba6ec335640ed1f9ec70 Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Wed, 28 Jan 2026 17:48:47 +0100 Subject: [PATCH 4/7] Fix RET503 linting error by adding unreachable return statement --- src/dda/env/dev/types/linux_container.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index 629f6425..2ae72fa8 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -478,8 +478,8 @@ def _resolve_repository_path(self, repo_spec: str) -> Path: # Fallback: If not a git repo but directory exists, use it for backward compat return parent_repo_path - # app.abort() always raises an exception, so no explicit return is needed - self.app.abort(f"Local repository not found: {repo_name}") # noqa: RET503 + self.app.abort(f"Local repository not found: {repo_name}") + return None # type: ignore[return-value] # abort() never returns def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: """ From 70e7695cd2b2e7fac363f3e8b988fbd8b847b4c2 Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Wed, 28 Jan 2026 17:53:17 +0100 Subject: [PATCH 5/7] Fix mypy unused-ignore error by removing type: ignore comment --- src/dda/env/dev/types/linux_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index 2ae72fa8..8336bf91 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -479,7 +479,7 @@ def _resolve_repository_path(self, repo_spec: str) -> Path: return parent_repo_path self.app.abort(f"Local repository not found: {repo_name}") - return None # type: ignore[return-value] # abort() never returns + return None # abort() never returns, but ruff requires an explicit return def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: """ From bf54664d912a8b34c90339d86622b33ec6ee6b84 Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Thu, 29 Jan 2026 10:56:42 +0100 Subject: [PATCH 6/7] Mount main repository for worktrees instead of worktree directory When running from a git worktree, the worktree's .git file points to the main repository's .git/worktrees/ directory. If we only mount the worktree directory, git commands fail inside the container because the referenced .git directory doesn't exist. This change detects worktrees by checking if .git is a file (not a directory), reads the gitdir path, and resolves to the main repository root. The main repository is then mounted instead, ensuring git commands work correctly inside the container. Updated test to verify worktrees resolve to main repo path. --- src/dda/env/dev/types/linux_container.py | 46 +++++++++++++++------ tests/env/dev/types/test_linux_container.py | 16 +++++-- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index 8336bf91..dc242c46 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -11,7 +11,7 @@ import msgspec from dda.env.dev.interface import DeveloperEnvironmentConfig, DeveloperEnvironmentInterface -from dda.utils.fs import cp_r, temp_directory +from dda.utils.fs import Path, cp_r, temp_directory from dda.utils.git.constants import GitEnvVars if TYPE_CHECKING: @@ -20,7 +20,6 @@ from dda.tools.docker import Docker from dda.utils.container.model import Mount from dda.utils.editors.interface import EditorInterface - from dda.utils.fs import Path class LinuxContainerConfig(DeveloperEnvironmentConfig): @@ -467,21 +466,23 @@ def _resolve_repository_path(self, repo_spec: str) -> Path: # Strategy 1: Check if current directory is a git repository matching the repo name cwd = Path.cwd() - if self._is_matching_repository(cwd, repo_name): - return cwd + is_match, resolved_path = self._is_matching_repository(cwd, repo_name) + if is_match: + return resolved_path # Strategy 2: Check parent directory (existing behavior, backward compatible) parent_repo_path = cwd.parent / repo_name if parent_repo_path.is_dir(): - if self._is_matching_repository(parent_repo_path, repo_name): - return parent_repo_path + is_match, resolved_path = self._is_matching_repository(parent_repo_path, repo_name) + if is_match: + return resolved_path # Fallback: If not a git repo but directory exists, use it for backward compat return parent_repo_path self.app.abort(f"Local repository not found: {repo_name}") return None # abort() never returns, but ruff requires an explicit return - def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: + def _is_matching_repository(self, path: Path, expected_repo_name: str) -> tuple[bool, Path]: """ Check if the given path is a git repository matching the expected repository name. @@ -490,19 +491,24 @@ def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: - Git worktrees (regardless of directory name) - Nested repository structures + For worktrees, returns the main repository root path instead of the worktree path, + since worktrees need access to the main repository's .git directory to function. + Args: path: Path to check expected_repo_name: Expected repository name (e.g., "datadog-agent") Returns: - True if the path is a git repository matching the expected name + Tuple of (is_match, resolved_path) where: + - is_match: True if the path is a git repository matching the expected name + - resolved_path: For worktrees, the main repo root; for regular repos, the path itself """ if not path.is_dir(): - return False + return False, path git_dir = path / ".git" if not git_dir.exists(): - return False + return False, path # Use git to get the repository name from the remote URL try: @@ -513,13 +519,29 @@ def _is_matching_repository(self, path: Path, expected_repo_name: str) -> bool: try: os.chdir(path) remote = self.app.tools.git.get_remote() - return remote.repo == expected_repo_name + + if remote.repo != expected_repo_name: + return False, path + + # If this is a worktree, find the main repository root + if git_dir.is_file(): + # This is a worktree - read the .git file to find the main repo + gitdir_content = git_dir.read_text().strip() + if gitdir_content.startswith("gitdir: "): + # Extract path like: gitdir: /path/to/main/.git/worktrees/branch + worktree_git_path = gitdir_content[8:] # Remove "gitdir: " prefix + # Navigate up to find main repo: /path/to/main/.git/worktrees/branch -> /path/to/main + main_repo_path = Path(worktree_git_path).parent.parent.parent + return True, main_repo_path + + # Regular repository + return True, path finally: os.chdir(original_cwd) # Catch all git-related exceptions (subprocess errors, missing git, no remote, etc.) except Exception: # noqa: BLE001 # Not a git repository or no remote configured - return False + return False, path def _container_cp(self, source: str, destination: str, *args: Any) -> None: """Runs a `cp -r` command inside the context of the container""" diff --git a/tests/env/dev/types/test_linux_container.py b/tests/env/dev/types/test_linux_container.py index d766d263..1204b3a3 100644 --- a/tests/env/dev/types/test_linux_container.py +++ b/tests/env/dev/types/test_linux_container.py @@ -1418,11 +1418,18 @@ def test_directory_to_existing_file_fails(self, linux_container, test_target_dir class TestRepositoryResolution: """Tests for git-aware repository resolution logic.""" - def test_git_worktree_current_directory(self, app, mocker, temp_dir): - """Test git worktree with arbitrary directory name is detected as current directory.""" + def test_git_worktree_resolves_to_main_repo(self, app, mocker, temp_dir): + """Test git worktree resolves to main repository root, not the worktree directory.""" + # Create main repository structure + main_repo = temp_dir / "datadog-agent" + main_repo.ensure_dir() + (main_repo / ".git").ensure_dir() + + # Create worktree with arbitrary directory name worktree_dir = temp_dir / "my_feature_branch" worktree_dir.ensure_dir() - (worktree_dir / ".git").touch() + # Write .git file that points to main repo (like real worktrees do) + (worktree_dir / ".git").write_text(f"gitdir: {main_repo}/.git/worktrees/my_feature_branch") mock_remote = mocker.MagicMock() mock_remote.repo = "datadog-agent" @@ -1433,7 +1440,8 @@ def test_git_worktree_current_directory(self, app, mocker, temp_dir): # Testing internal repository resolution logic directly resolved = container._resolve_repository_path("datadog-agent") # noqa: SLF001 - assert resolved == worktree_dir + # Should resolve to main repo, not worktree, so git commands work in container + assert resolved == main_repo def test_backward_compatibility_parent_directory(self, app, mocker, temp_dir): """Test non-git directory is found via parent/repo_name (backward compatible).""" From 52d8ce1f29424eb193314a3afad951c011890693 Mon Sep 17 00:00:00 2001 From: Maxime Riaud Date: Thu, 29 Jan 2026 11:25:32 +0100 Subject: [PATCH 7/7] Add worktree working directory support - repo_path() now detects if running from a worktree and returns the correct working directory path (e.g., /root/repos/repo/.worktrees/branch) - Mount destination is always the base repo path without worktree subdirectory - Detects external worktrees and shows helpful error message - Working directory correctly points to worktree subdirectory when running commands --- src/dda/env/dev/types/linux_container.py | 55 +++++++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/src/dda/env/dev/types/linux_container.py b/src/dda/env/dev/types/linux_container.py index dc242c46..f1236bd6 100644 --- a/src/dda/env/dev/types/linux_container.py +++ b/src/dda/env/dev/types/linux_container.py @@ -189,7 +189,9 @@ def start(self) -> None: for repo_spec in self.config.repos: repo = repo_spec.split("@")[0] repo_path = self._resolve_repository_path(repo_spec) - command.extend(("-v", f"{repo_path}:{self.repo_path(repo)}")) + # Mount to base repo path (without worktree subdirectory) + mount_dest = f"{self.home_dir}/repos/{repo}" + command.extend(("-v", f"{repo_path}:{mount_dest}")) for mount_spec in self.config.extra_mount_specs: command.extend(("--mount", mount_spec)) @@ -441,7 +443,56 @@ def repo_path(self, repo: str | None) -> str: if repo is None: repo = self.default_repo - return f"{self.home_dir}/repos/{repo}" + base_path = f"{self.home_dir}/repos/{repo}" + + # Check if we're currently in a worktree - if so, append the worktree subdirectory + worktree_subdir = self._get_worktree_subdirectory() + if worktree_subdir: + return f"{base_path}/{worktree_subdir}" + + return base_path + + def _get_worktree_subdirectory(self) -> str | None: + """ + If the current directory is a worktree inside the mounted main repository, + returns the relative path from the main repo to the worktree. + + Returns: + Relative path like ".worktrees/branch-name" or None if not in a worktree + """ + cwd = Path.cwd() + git_dir = cwd / ".git" + + # Not a git worktree if .git is not a file + if not git_dir.is_file(): + return None + + try: + # Read the .git file to find the main repo + gitdir_content = git_dir.read_text().strip() + if not gitdir_content.startswith("gitdir: "): + return None + + # Extract path like: gitdir: /path/to/main/.git/worktrees/branch + worktree_git_path = gitdir_content[8:] # Remove "gitdir: " prefix + # Navigate up to find main repo: /path/to/main/.git/worktrees/branch -> /path/to/main + main_repo_path = Path(worktree_git_path).parent.parent.parent + + # Calculate relative path from main repo to current worktree directory + try: + relative_path = cwd.relative_to(main_repo_path) + return str(relative_path) + except ValueError: + # Worktree is outside the main repo - not supported + self.app.abort( + f"Git worktree at {cwd} is outside the main repository at {main_repo_path}.\n" + f"External worktrees are not currently supported. Please run dda commands from:\n" + f" - The main repository directory, or\n" + f" - A worktree inside the main repository (e.g., {main_repo_path}/.worktrees/branch-name)" + ) + except Exception: # noqa: BLE001 + # If anything fails, assume not a worktree + return None def _resolve_repository_path(self, repo_spec: str) -> Path: """