From 43fe479a41ce323b59339002da6b7defd00143ee Mon Sep 17 00:00:00 2001
From: Chris Krough <461869+ckrough@users.noreply.github.com>
Date: Sun, 21 Dec 2025 14:37:23 -0500
Subject: [PATCH 1/4] feat: add --attach flag to workspace create for existing
 branches

Allow creating a workspace that attaches to an existing branch without
creating a new branch. The workspace name matches the branch name.

Usage:
  as workspace create feature/auth --attach

Key changes:
- Add branch_exists() and worktree_add_existing() to git infrastructure
- Add sanitize_branch_name() to handle branches with slashes (feature/auth -> feature-auth)
- Add attach_worktree() for creating worktrees from existing branches
- Add attach_branch parameter to WorkspaceService.create()
- Add --attach/-a CLI flag to workspace create command

Closes the feature request for workspace and branch handling.
---
 src/agentspaces/cli/workspace.py              |  30 ++++-
 src/agentspaces/infrastructure/git.py         |  43 ++++++
 src/agentspaces/modules/workspace/service.py  |  31 +++--
 src/agentspaces/modules/workspace/worktree.py |  75 +++++++++++
 tests/unit/infrastructure/test_git.py         |  70 ++++++++++
 tests/unit/modules/workspace/test_service.py  |  94 +++++++++++++
 tests/unit/modules/workspace/test_worktree.py | 127 ++++++++++++++++++
 7 files changed, 455 insertions(+), 15 deletions(-)

diff --git a/src/agentspaces/cli/workspace.py b/src/agentspaces/cli/workspace.py
index 63bf3b9..1cba004 100644
--- a/src/agentspaces/cli/workspace.py
+++ b/src/agentspaces/cli/workspace.py
@@ -43,8 +43,12 @@
 def create(
     branch: Annotated[
         str,
-        typer.Argument(help="Base branch to create workspace from"),
+        typer.Argument(help="Base branch (or target branch with --attach)"),
     ] = "HEAD",
+    attach: Annotated[
+        bool,
+        typer.Option("--attach", "-a", help="Attach to existing branch instead of creating new"),
+    ] = False,
     purpose: Annotated[
         str | None,
         typer.Option("--purpose", "-p", help="Purpose/description for this workspace"),
@@ -63,20 +67,32 @@ def create(
     Creates a git worktree with a unique name (e.g., eager-turing) and
     sets up a Python virtual environment using uv.
 
+    Use --attach to create a workspace for an existing branch without
+    creating a new branch. The workspace name will match the branch name.
+
     \b
     Examples:
         agentspaces workspace create                      # From current HEAD
         agentspaces workspace create main                 # From main branch
         agentspaces workspace create -p "Fix auth bug"   # With purpose
         agentspaces workspace create --no-venv            # Skip venv setup
+        agentspaces workspace create feature/auth --attach  # Attach to existing branch
     """
     try:
-        workspace = _service.create(
-            base_branch=branch,
-            purpose=purpose,
-            python_version=python_version,
-            setup_venv=not no_venv,
-        )
+        if attach:
+            workspace = _service.create(
+                attach_branch=branch,
+                purpose=purpose,
+                python_version=python_version,
+                setup_venv=not no_venv,
+            )
+        else:
+            workspace = _service.create(
+                base_branch=branch,
+                purpose=purpose,
+                python_version=python_version,
+                setup_venv=not no_venv,
+            )
     except WorkspaceError as e:
         print_error(str(e))
         raise typer.Exit(1) from e
diff --git a/src/agentspaces/infrastructure/git.py b/src/agentspaces/infrastructure/git.py
index 6aba8b1..6a79195 100644
--- a/src/agentspaces/infrastructure/git.py
+++ b/src/agentspaces/infrastructure/git.py
@@ -18,6 +18,7 @@
     "GitTimeoutError",
     "WorktreeInfo",
     "branch_delete",
+    "branch_exists",
     "get_current_branch",
     "get_main_git_dir",
     "get_repo_name",
@@ -26,6 +27,7 @@
     "is_git_repo",
     "is_in_worktree",
     "worktree_add",
+    "worktree_add_existing",
     "worktree_list",
     "worktree_remove",
 ]
@@ -210,6 +212,47 @@ def worktree_add(
     _run_git(["worktree", "add", "-b", branch, str(path), base], cwd=cwd)
 
 
+def worktree_add_existing(
+    path: Path,
+    branch: str,
+    *,
+    cwd: Path | None = None,
+) -> None:
+    """Create a new git worktree for an existing branch.
+
+    Unlike worktree_add, this does not create a new branch - it checks out
+    an existing branch into a new worktree location.
+
+    Args:
+        path: Path where the worktree will be created.
+        branch: Name of the existing branch to checkout.
+        cwd: Repository directory.
+
+    Raises:
+        GitError: If worktree creation fails or branch doesn't exist.
+    """
+    logger.info("worktree_add_existing", path=str(path), branch=branch)
+    _run_git(["worktree", "add", str(path), branch], cwd=cwd)
+
+
+def branch_exists(branch: str, *, cwd: Path | None = None) -> bool:
+    """Check if a local branch exists in the repository.
+
+    Args:
+        branch: Branch name to check.
+        cwd: Repository directory.
+
+    Returns:
+        True if the branch exists, False otherwise.
+    """
+    result = _run_git(
+        ["rev-parse", "--verify", f"refs/heads/{branch}"],
+        cwd=cwd,
+        check=False,
+    )
+    return result.returncode == 0
+
+
 def worktree_remove(
     path: Path, *, force: bool = False, cwd: Path | None = None
 ) -> None:
diff --git a/src/agentspaces/modules/workspace/service.py b/src/agentspaces/modules/workspace/service.py
index e3af7bb..e90fc20 100644
--- a/src/agentspaces/modules/workspace/service.py
+++ b/src/agentspaces/modules/workspace/service.py
@@ -101,6 +101,7 @@ def create(
         self,
         *,
         base_branch: str = "HEAD",
+        attach_branch: str | None = None,
         purpose: str | None = None,
         python_version: str | None = None,
         setup_venv: bool = True,
@@ -112,7 +113,9 @@ def create(
         persists workspace metadata, and generates a workspace-context skill.
 
         Args:
-            base_branch: Branch to create workspace from.
+            base_branch: Branch to create workspace from (ignored if attach_branch set).
+            attach_branch: Existing branch to attach to. When set, no new branch is
+                created and the workspace name matches the branch name.
             purpose: Description of workspace purpose.
             python_version: Python version for venv (auto-detected if not specified).
             setup_venv: Whether to create a virtual environment.
@@ -122,7 +125,7 @@ def create(
             WorkspaceInfo with details.
 
         Raises:
-            WorkspaceError: If creation fails.
+            WorkspaceError: If creation fails or attach_branch doesn't exist.
         """
         try:
             repo_root, project = worktree.get_repo_info(cwd)
@@ -133,17 +136,29 @@ def create(
             "workspace_create_start",
             project=project,
             base_branch=base_branch,
+            attach_branch=attach_branch,
             purpose=purpose,
             python_version=python_version,
         )
 
         try:
-            result = worktree.create_worktree(
-                project=project,
-                base_branch=base_branch,
-                repo_root=repo_root,
-                resolver=self._resolver,
-            )
+            if attach_branch is not None:
+                result = worktree.attach_worktree(
+                    project=project,
+                    branch=attach_branch,
+                    repo_root=repo_root,
+                    resolver=self._resolver,
+                )
+            else:
+                result = worktree.create_worktree(
+                    project=project,
+                    base_branch=base_branch,
+                    repo_root=repo_root,
+                    resolver=self._resolver,
+                )
+        except ValueError as e:
+            # attach_worktree raises ValueError for branch/workspace issues
+            raise WorkspaceError(str(e)) from e
         except git.GitError as e:
             logger.error("workspace_create_failed", error=e.stderr)
             raise WorkspaceError(f"Failed to create workspace: {e.stderr}") from e
diff --git a/src/agentspaces/modules/workspace/worktree.py b/src/agentspaces/modules/workspace/worktree.py
index 2282238..564811f 100644
--- a/src/agentspaces/modules/workspace/worktree.py
+++ b/src/agentspaces/modules/workspace/worktree.py
@@ -11,10 +11,12 @@
 
 __all__ = [
     "WorktreeCreateResult",
+    "attach_worktree",
     "create_worktree",
     "get_repo_info",
     "list_worktrees",
     "remove_worktree",
+    "sanitize_branch_name",
 ]
 
 
@@ -28,6 +30,79 @@ class WorktreeCreateResult:
     base_branch: str
 
 
+def sanitize_branch_name(branch: str) -> str:
+    """Convert a branch name to a valid workspace directory name.
+
+    Replaces slashes with hyphens to handle branches like 'feature/auth'.
+
+    Args:
+        branch: Git branch name.
+
+    Returns:
+        Sanitized name suitable for directory naming.
+    """
+    return branch.replace("/", "-")
+
+
+def attach_worktree(
+    project: str,
+    branch: str,
+    *,
+    repo_root: Path,
+    resolver: PathResolver | None = None,
+) -> WorktreeCreateResult:
+    """Attach to an existing git branch as a worktree.
+
+    Unlike create_worktree, this does not create a new branch - it creates
+    a worktree for an existing branch. The workspace name is derived from
+    the branch name.
+
+    Args:
+        project: Project/repository name.
+        branch: Existing branch to attach to.
+        repo_root: Path to the main repository.
+        resolver: Path resolver instance.
+
+    Returns:
+        WorktreeCreateResult with details.
+
+    Raises:
+        git.GitError: If worktree creation fails.
+        ValueError: If branch doesn't exist or workspace already exists.
+    """
+    resolver = resolver or PathResolver()
+    resolver.ensure_base()
+
+    # Verify branch exists
+    if not git.branch_exists(branch, cwd=repo_root):
+        raise ValueError(f"Branch does not exist: {branch}")
+
+    # Sanitize branch name for directory
+    workspace_name = sanitize_branch_name(branch)
+    workspace_path = resolver.workspace_dir(project, workspace_name)
+
+    # Check if workspace already exists
+    if workspace_path.exists():
+        raise ValueError(f"Workspace already exists: {workspace_name}")
+
+    # Create parent directory
+    workspace_path.parent.mkdir(parents=True, exist_ok=True)
+
+    # Create the worktree for existing branch (no -b flag)
+    git.worktree_add_existing(
+        path=workspace_path,
+        branch=branch,
+        cwd=repo_root,
+    )
+
+    return WorktreeCreateResult(
+        name=workspace_name,
+        path=workspace_path,
+        branch=branch,
+        base_branch=branch,  # For attach, base_branch equals branch
+    )
+
+
 def create_worktree(
     project: str,
     base_branch: str = "HEAD",
diff --git a/tests/unit/infrastructure/test_git.py b/tests/unit/infrastructure/test_git.py
index ff408ef..005e34f 100644
--- a/tests/unit/infrastructure/test_git.py
+++ b/tests/unit/infrastructure/test_git.py
@@ -201,6 +201,76 @@ def test_worktree_info_frozen(self) -> None:
             info.branch = "other"  # type: ignore[misc]
 
 
+class TestBranchExists:
+    """Tests for branch_exists function."""
+
+    def test_branch_exists_true(self, git_repo: Path) -> None:
+        """Should return True for existing branch."""
+        branch = git.get_current_branch(cwd=git_repo)
+        assert git.branch_exists(branch, cwd=git_repo) is True
+
+    def test_branch_exists_false(self, git_repo: Path) -> None:
+        """Should return False for non-existent branch."""
+        assert git.branch_exists("nonexistent-branch", cwd=git_repo) is False
+
+    def test_branch_exists_after_create(self, git_repo: Path, temp_dir: Path) -> None:
+        """Should return True for branch created via worktree."""
+        worktree_path = temp_dir / "worktree"
+        git.worktree_add(
+            path=worktree_path,
+            branch="new-test-branch",
+            base="HEAD",
+            cwd=git_repo,
+        )
+        assert git.branch_exists("new-test-branch", cwd=git_repo) is True
+
+        # Cleanup
+        git.worktree_remove(worktree_path, cwd=git_repo)
+
+
+class TestWorktreeAddExisting:
+    """Tests for worktree_add_existing function."""
+
+    def test_worktree_add_existing_success(self, git_repo: Path, temp_dir: Path) -> None:
+        """Should create worktree for existing branch."""
+        import subprocess
+
+        # Create a branch first (without a worktree)
+        subprocess.run(
+            ["git", "branch", "existing-branch"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        worktree_path = temp_dir / "existing-worktree"
+        git.worktree_add_existing(
+            path=worktree_path,
+            branch="existing-branch",
+            cwd=git_repo,
+        )
+
+        assert worktree_path.exists()
+        assert (worktree_path / "README.md").exists()
+
+        # Verify branch is checked out
+        worktrees = git.worktree_list(cwd=git_repo)
+        wt = next(w for w in worktrees if w.path.resolve() == worktree_path.resolve())
+        assert wt.branch == "existing-branch"
+
+    def test_worktree_add_existing_nonexistent_branch(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should raise GitError for non-existent branch."""
+        worktree_path = temp_dir / "nonexistent-worktree"
+
+        with pytest.raises(git.GitError):
+            git.worktree_add_existing(
+                path=worktree_path,
+                branch="nonexistent-branch",
+                cwd=git_repo,
+            )
+
+
 class TestIsDirty:
     """Tests for is_dirty function."""
 
diff --git a/tests/unit/modules/workspace/test_service.py b/tests/unit/modules/workspace/test_service.py
index b651020..36f11f8 100644
--- a/tests/unit/modules/workspace/test_service.py
+++ b/tests/unit/modules/workspace/test_service.py
@@ -179,6 +179,100 @@ def test_create_workspace_not_in_repo(self, temp_dir: Path) -> None:
             service.create(cwd=temp_dir)
 
 
+class TestWorkspaceServiceCreateAttach:
+    """Tests for WorkspaceService.create with attach_branch."""
+
+    def test_create_with_attach_branch(self, git_repo: Path, temp_dir: Path) -> None:
+        """Should create workspace attached to existing branch."""
+        import subprocess
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+        service = WorkspaceService(resolver=resolver)
+
+        # Create a branch first
+        subprocess.run(
+            ["git", "branch", "existing-branch"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = service.create(
+            attach_branch="existing-branch",
+            setup_venv=False,
+            cwd=git_repo,
+        )
+
+        assert result.name == "existing-branch"
+        assert result.branch == "existing-branch"
+        assert result.path.exists()
+
+    def test_create_attach_branch_with_slash(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should handle branch names with slashes."""
+        import subprocess
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+        service = WorkspaceService(resolver=resolver)
+
+        # Create a branch with slash
+        subprocess.run(
+            ["git", "branch", "feature/auth"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = service.create(
+            attach_branch="feature/auth",
+            setup_venv=False,
+            cwd=git_repo,
+        )
+
+        assert result.name == "feature-auth"  # Sanitized for directory
+        assert result.branch == "feature/auth"  # Original preserved
+        assert result.path.exists()
+
+    def test_create_attach_nonexistent_branch(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should raise WorkspaceError for non-existent branch."""
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+        service = WorkspaceService(resolver=resolver)
+
+        with pytest.raises(WorkspaceError, match="Branch does not exist"):
+            service.create(
+                attach_branch="nonexistent-branch",
+                setup_venv=False,
+                cwd=git_repo,
+            )
+
+    def test_create_attach_creates_metadata(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should create metadata for attached workspace."""
+        import subprocess
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+        service = WorkspaceService(resolver=resolver)
+
+        subprocess.run(
+            ["git", "branch", "attach-test-branch"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = service.create(
+            attach_branch="attach-test-branch",
+            purpose="Testing attach",
+            setup_venv=False,
+            cwd=git_repo,
+        )
+
+        metadata_dir = resolver.metadata_dir("test-repo", result.name)
+        assert metadata_dir.exists()
+        assert result.purpose == "Testing attach"
+
+
 class TestWorkspaceServiceList:
     """Tests for WorkspaceService.list method."""
 
diff --git a/tests/unit/modules/workspace/test_worktree.py b/tests/unit/modules/workspace/test_worktree.py
index 895fd53..87a4def 100644
--- a/tests/unit/modules/workspace/test_worktree.py
+++ b/tests/unit/modules/workspace/test_worktree.py
@@ -42,6 +42,133 @@ def test_create_result_is_frozen(self) -> None:
             result.name = "new-name"  # type: ignore[misc]
 
 
+class TestSanitizeBranchName:
+    """Tests for sanitize_branch_name function."""
+
+    def test_simple_branch_unchanged(self) -> None:
+        """Simple branch names should remain unchanged."""
+        assert worktree.sanitize_branch_name("main") == "main"
+        assert worktree.sanitize_branch_name("develop") == "develop"
+        assert worktree.sanitize_branch_name("my-branch") == "my-branch"
+
+    def test_branch_with_slash(self) -> None:
+        """Slashes should be replaced with hyphens."""
+        assert worktree.sanitize_branch_name("feature/auth") == "feature-auth"
+        assert worktree.sanitize_branch_name("fix/bug-123") == "fix-bug-123"
+
+    def test_multiple_slashes(self) -> None:
+        """Multiple slashes should all be replaced."""
+        assert worktree.sanitize_branch_name("feature/auth/login") == "feature-auth-login"
+        assert worktree.sanitize_branch_name("a/b/c/d") == "a-b-c-d"
+
+
+class TestAttachWorktree:
+    """Tests for attach_worktree function."""
+
+    def test_attach_worktree_success(self, git_repo: Path, temp_dir: Path) -> None:
+        """Should attach to an existing branch."""
+        import subprocess
+
+        import pytest
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+
+        # Create a branch first (without a worktree)
+        subprocess.run(
+            ["git", "branch", "existing-branch"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = worktree.attach_worktree(
+            project="test-repo",
+            branch="existing-branch",
+            repo_root=git_repo,
+            resolver=resolver,
+        )
+
+        assert result.name == "existing-branch"
+        assert result.branch == "existing-branch"
+        assert result.base_branch == "existing-branch"
+        assert result.path.exists()
+
+    def test_attach_worktree_with_slash_in_name(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should handle branch names with slashes."""
+        import subprocess
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+
+        # Create a branch with slash
+        subprocess.run(
+            ["git", "branch", "feature/auth"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = worktree.attach_worktree(
+            project="test-repo",
+            branch="feature/auth",
+            repo_root=git_repo,
+            resolver=resolver,
+        )
+
+        assert result.name == "feature-auth"  # Sanitized
+        assert result.branch == "feature/auth"  # Original preserved
+        assert result.path.exists()
+
+    def test_attach_worktree_nonexistent_branch(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should raise ValueError for non-existent branch."""
+        import pytest
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+
+        with pytest.raises(ValueError, match="Branch does not exist"):
+            worktree.attach_worktree(
+                project="test-repo",
+                branch="nonexistent-branch",
+                repo_root=git_repo,
+                resolver=resolver,
+            )
+
+    def test_attach_worktree_already_exists(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should raise ValueError if workspace already exists."""
+        import subprocess
+
+        import pytest
+
+        resolver = PathResolver(base=temp_dir / ".agentspaces")
+
+        # Create a branch
+        subprocess.run(
+            ["git", "branch", "test-branch"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        # Attach first time
+        worktree.attach_worktree(
+            project="test-repo",
+            branch="test-branch",
+            repo_root=git_repo,
+            resolver=resolver,
+        )
+
+        # Try to attach again - should fail
+        with pytest.raises(ValueError, match="Workspace already exists"):
+            worktree.attach_worktree(
+                project="test-repo",
+                branch="test-branch",
+                repo_root=git_repo,
+                resolver=resolver,
+            )
+
+
 class TestCreateWorktree:
     """Tests for create_worktree function."""
 

From 35f5567418a45666c61889164497bdb91ea551ab Mon Sep 17 00:00:00 2001
From: Chris Krough <461869+ckrough@users.noreply.github.com>
Date: Sun, 21 Dec 2025 14:45:34 -0500
Subject: [PATCH 2/4] fix: remove unused pytest import in test

---
 tests/unit/modules/workspace/test_worktree.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/unit/modules/workspace/test_worktree.py b/tests/unit/modules/workspace/test_worktree.py
index 87a4def..99b9dc7 100644
--- a/tests/unit/modules/workspace/test_worktree.py
+++ b/tests/unit/modules/workspace/test_worktree.py
@@ -69,8 +69,6 @@ def test_attach_worktree_success(self, git_repo: Path, temp_dir: Path) -> None:
         """Should attach to an existing branch."""
         import subprocess
 
-        import pytest
-
         resolver = PathResolver(base=temp_dir / ".agentspaces")
 
         # Create a branch first (without a worktree)

From 3748f9d33e7ad46becfad79cc387227cbd0360f9 Mon Sep 17 00:00:00 2001
From: Chris Krough <461869+ckrough@users.noreply.github.com>
Date: Sun, 21 Dec 2025 14:46:22 -0500
Subject: [PATCH 3/4] style: apply ruff formatting

---
 src/agentspaces/cli/workspace.py              | 4 +++-
 tests/unit/infrastructure/test_git.py         | 4 +++-
 tests/unit/modules/workspace/test_worktree.py | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/agentspaces/cli/workspace.py b/src/agentspaces/cli/workspace.py
index 1cba004..07f002a 100644
--- a/src/agentspaces/cli/workspace.py
+++ b/src/agentspaces/cli/workspace.py
@@ -47,7 +47,9 @@ def create(
     ] = "HEAD",
     attach: Annotated[
         bool,
-        typer.Option("--attach", "-a", help="Attach to existing branch instead of creating new"),
+        typer.Option(
+            "--attach", "-a", help="Attach to existing branch instead of creating new"
+        ),
     ] = False,
     purpose: Annotated[
         str | None,
diff --git a/tests/unit/infrastructure/test_git.py b/tests/unit/infrastructure/test_git.py
index 005e34f..90d297d 100644
--- a/tests/unit/infrastructure/test_git.py
+++ b/tests/unit/infrastructure/test_git.py
@@ -231,7 +231,9 @@ def test_branch_exists_after_create(self, git_repo: Path, temp_dir: Path) -> Non
 class TestWorktreeAddExisting:
     """Tests for worktree_add_existing function."""
 
-    def test_worktree_add_existing_success(self, git_repo: Path, temp_dir: Path) -> None:
+    def test_worktree_add_existing_success(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
         """Should create worktree for existing branch."""
         import subprocess
 
diff --git a/tests/unit/modules/workspace/test_worktree.py b/tests/unit/modules/workspace/test_worktree.py
index 99b9dc7..06a695e 100644
--- a/tests/unit/modules/workspace/test_worktree.py
+++ b/tests/unit/modules/workspace/test_worktree.py
@@ -58,7 +58,9 @@ def test_branch_with_slash(self) -> None:
 
     def test_multiple_slashes(self) -> None:
         """Multiple slashes should all be replaced."""
-        assert worktree.sanitize_branch_name("feature/auth/login") == "feature-auth-login"
+        assert (
+            worktree.sanitize_branch_name("feature/auth/login") == "feature-auth-login"
+        )
         assert worktree.sanitize_branch_name("a/b/c/d") == "a-b-c-d"
 
 

From ceced8aec0cbb0b4e1718b3110f52080879cb027 Mon Sep 17 00:00:00 2001
From: Chris Krough <461869+ckrough@users.noreply.github.com>
Date: Sun, 21 Dec 2025 16:01:18 -0500
Subject: [PATCH 4/4] fix: address PR review findings for --attach feature

- Replace contextlib.suppress(Exception) with explicit try/except and
  logging in service.py cleanup path
- Add logging for branch_delete() failures in git.py
- Add test for "branch already checked out" error scenario
- Add CLI integration tests for --attach flag using typer's CliRunner
---
 src/agentspaces/infrastructure/git.py        |   7 +
 src/agentspaces/modules/workspace/service.py |  10 +-
 tests/unit/cli/test_workspace.py             | 131 +++++++++++++++++++
 tests/unit/infrastructure/test_git.py        |  19 +++
 4 files changed, 165 insertions(+), 2 deletions(-)
 create mode 100644 tests/unit/cli/test_workspace.py

diff --git a/src/agentspaces/infrastructure/git.py b/src/agentspaces/infrastructure/git.py
index 6a79195..548368b 100644
--- a/src/agentspaces/infrastructure/git.py
+++ b/src/agentspaces/infrastructure/git.py
@@ -349,6 +349,13 @@ def branch_delete(branch: str, *, force: bool = False, cwd: Path | None = None)
     logger.info("branch_delete", branch=branch, force=force)
     flag = "-D" if force else "-d"
     result = _run_git(["branch", flag, branch], cwd=cwd, check=False)
+    if result.returncode != 0:
+        logger.warning(
+            "branch_delete_failed",
+            branch=branch,
+            returncode=result.returncode,
+            stderr=result.stderr.strip(),
+        )
     return result.returncode == 0
 
 
diff --git a/src/agentspaces/modules/workspace/service.py b/src/agentspaces/modules/workspace/service.py
index e90fc20..78fd1cc 100644
--- a/src/agentspaces/modules/workspace/service.py
+++ b/src/agentspaces/modules/workspace/service.py
@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import contextlib
 from dataclasses import dataclass
 from datetime import UTC, datetime
 from pathlib import Path  # noqa: TC003 - used at runtime in dataclass
@@ -204,7 +203,7 @@ def create(
         except Exception as e:
             # Metadata save is critical - attempt cleanup and fail
             logger.error("metadata_save_failed", error=str(e))
-            with contextlib.suppress(Exception):
+            try:
                 worktree.remove_worktree(
                     project=project,
                     name=result.name,
@@ -212,6 +211,13 @@ def create(
                     force=True,
                     resolver=self._resolver,
                 )
+            except Exception as cleanup_error:
+                logger.warning(
+                    "workspace_cleanup_failed",
+                    workspace=result.name,
+                    error=str(cleanup_error),
+                    original_error=str(e),
+                )
             raise WorkspaceError(f"Failed to save workspace metadata: {e}") from e
 
         # Generate workspace-context skill for agent discovery
diff --git a/tests/unit/cli/test_workspace.py b/tests/unit/cli/test_workspace.py
new file mode 100644
index 0000000..f3869f9
--- /dev/null
+++ b/tests/unit/cli/test_workspace.py
@@ -0,0 +1,131 @@
+"""Tests for workspace CLI commands."""
+
+from __future__ import annotations
+
+import subprocess
+from typing import TYPE_CHECKING
+from unittest.mock import patch
+
+import pytest
+from typer.testing import CliRunner
+
+from agentspaces.cli.workspace import app
+from agentspaces.infrastructure.paths import PathResolver
+from agentspaces.modules.workspace.service import WorkspaceService
+
+if TYPE_CHECKING:
+    from pathlib import Path
+
+runner = CliRunner()
+
+
+@pytest.fixture
+def isolated_env(git_repo: Path, temp_dir: Path, monkeypatch: pytest.MonkeyPatch):
+    """Set up an isolated environment for CLI tests.
+
+    Changes to the git_repo directory and provides a service with
+    a custom resolver pointing to temp_dir.
+    """
+    # Change to the git repo directory
+    monkeypatch.chdir(git_repo)
+
+    # Create a service with custom resolver
+    resolver = PathResolver(base=temp_dir / ".agentspaces")
+    service = WorkspaceService(resolver=resolver)
+
+    # Patch the module-level service
+    with patch("agentspaces.cli.workspace._service", service):
+        yield {
+            "git_repo": git_repo,
+            "temp_dir": temp_dir,
+            "resolver": resolver,
+            "service": service,
+        }
+
+
+class TestWorkspaceCreateAttach:
+    """Tests for workspace create --attach flag."""
+
+    def test_attach_to_existing_branch(self, isolated_env: dict) -> None:
+        """Should create workspace for existing branch with --attach."""
+        git_repo = isolated_env["git_repo"]
+
+        # Create a branch first
+        subprocess.run(
+            ["git", "branch", "feature-test"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = runner.invoke(
+            app,
+            ["create", "feature-test", "--attach", "--no-venv"],
+        )
+
+        assert result.exit_code == 0, f"Command failed: {result.output}"
+        assert "feature-test" in result.output
+
+    @pytest.mark.usefixtures("isolated_env")
+    def test_attach_to_nonexistent_branch_fails(self) -> None:
+        """Should fail when attaching to non-existent branch."""
+        result = runner.invoke(
+            app,
+            ["create", "nonexistent-branch", "--attach", "--no-venv"],
+        )
+
+        assert result.exit_code == 1
+        assert "does not exist" in result.output.lower()
+
+    def test_attach_with_slash_in_branch_name(self, isolated_env: dict) -> None:
+        """Should sanitize branch names with slashes."""
+        git_repo = isolated_env["git_repo"]
+
+        # Create a branch with slash
+        subprocess.run(
+            ["git", "branch", "feature/auth"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = runner.invoke(
+            app,
+            ["create", "feature/auth", "--attach", "--no-venv"],
+        )
+
+        assert result.exit_code == 0, f"Command failed: {result.output}"
+        # Workspace name should be sanitized (/ -> -)
+        assert "feature-auth" in result.output
+
+    def test_attach_short_flag(self, isolated_env: dict) -> None:
+        """Should accept -a as short form of --attach."""
+        git_repo = isolated_env["git_repo"]
+
+        subprocess.run(
+            ["git", "branch", "test-short-flag"],
+            cwd=git_repo,
+            check=True,
+        )
+
+        result = runner.invoke(
+            app,
+            ["create", "test-short-flag", "-a", "--no-venv"],
+        )
+
+        assert result.exit_code == 0, f"Command failed: {result.output}"
+        assert "test-short-flag" in result.output
+
+
+class TestWorkspaceCreateDefault:
+    """Tests for workspace create without --attach (default behavior)."""
+
+    @pytest.mark.usefixtures("isolated_env")
+    def test_create_generates_workspace_name(self) -> None:
+        """Should create workspace with generated name when not attaching."""
+        result = runner.invoke(
+            app,
+            ["create", "--no-venv"],
+        )
+
+        assert result.exit_code == 0, f"Command failed: {result.output}"
+        # Should show workspace created message
+        assert "workspace" in result.output.lower()
diff --git a/tests/unit/infrastructure/test_git.py b/tests/unit/infrastructure/test_git.py
index 90d297d..7480c6f 100644
--- a/tests/unit/infrastructure/test_git.py
+++ b/tests/unit/infrastructure/test_git.py
@@ -272,6 +272,25 @@ def test_worktree_add_existing_nonexistent_branch(
                 cwd=git_repo,
             )
 
+    def test_worktree_add_existing_branch_already_checked_out(
+        self, git_repo: Path, temp_dir: Path
+    ) -> None:
+        """Should raise GitError when branch is already checked out."""
+        # The main branch is already checked out in git_repo
+        current_branch = git.get_current_branch(cwd=git_repo)
+        worktree_path = temp_dir / "duplicate-worktree"
+
+        with pytest.raises(git.GitError) as excinfo:
+            git.worktree_add_existing(
+                path=worktree_path,
+                branch=current_branch,
+                cwd=git_repo,
+            )
+
+        # Git message varies by version: "already checked out" or "already used by worktree"
+        stderr = excinfo.value.stderr.lower()
+        assert "already" in stderr and ("checked out" in stderr or "worktree" in stderr)
+
 
 class TestIsDirty:
     """Tests for is_dirty function."""