From 19f1cdedbb550eeb086220d06bf0cc6b5c280c28 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Thu, 19 Feb 2026 06:10:27 +0100 Subject: [PATCH] hotfix/github-feat-PR --- apps/backend/core/worktree.py | 6 + .../src/main/ipc-handlers/project-handlers.ts | 20 ++- .../ipc-handlers/task/worktree-handlers.ts | 4 +- tests/test_github_pr_regression.py | 130 ++++++++++++++++++ tests/test_gitlab_worktree.py | 112 +++++++++++++++ 5 files changed, 265 insertions(+), 7 deletions(-) diff --git a/apps/backend/core/worktree.py b/apps/backend/core/worktree.py index f8cbbfb9..55a3a79e 100644 --- a/apps/backend/core/worktree.py +++ b/apps/backend/core/worktree.py @@ -1211,6 +1211,9 @@ class WorktreeManager: ) target = target_branch or self.base_branch + # Strip remote prefix (e.g., "origin/feat/x" → "feat/x") since gh expects branch names only + if target.startswith("origin/"): + target = target[len("origin/") :] pr_title = title or f"auto-claude: {spec_name}" # Try AI-powered PR body from project's PR template, fall back to spec summary @@ -1381,6 +1384,9 @@ class WorktreeManager: ) target = target_branch or self.base_branch + # Strip remote prefix (e.g., "origin/feat/x" → "feat/x") since glab expects branch names only + if target.startswith("origin/"): + target = target[len("origin/") :] mr_title = title or f"auto-claude: {spec_name}" # Get MR body from spec.md if available diff --git a/apps/frontend/src/main/ipc-handlers/project-handlers.ts b/apps/frontend/src/main/ipc-handlers/project-handlers.ts index bde37c75..20c5403b 100644 --- a/apps/frontend/src/main/ipc-handlers/project-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/project-handlers.ts @@ -153,16 +153,24 @@ function getGitBranchesWithInfo(projectPath: string): GitBranchDetail[] { .map(b => b.trim()) // Remove HEAD pointer entries like "origin/HEAD" .filter(b => !b.endsWith('/HEAD')) - .map(name => ({ - name, - type: 'remote' as const, - displayName: name, - isCurrent: false - })); + .map(fullName => { + // Strip "origin/" prefix so branch names are clean for PR targets etc. + const name = fullName.replace(/^origin\//, ''); + return { + name, + type: 'remote' as const, + displayName: name, + isCurrent: false + }; + }); } catch { // Remote branches may not exist, continue with local only } + // Deduplicate: if a branch exists locally and remotely, keep only the local entry + const localNames = new Set(localBranches.map(b => b.name)); + remoteBranches = remoteBranches.filter(b => !localNames.has(b.name)); + // Combine and sort: local branches first, then remote branches, alphabetically within each group const allBranches = [...localBranches, ...remoteBranches]; diff --git a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts index 1319de8d..2eb2a3c9 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -1370,7 +1370,9 @@ function getTaskBaseBranch(specDir: string): string | undefined { if (metadata.baseBranch && metadata.baseBranch !== '__project_default__' && GIT_BRANCH_REGEX.test(metadata.baseBranch)) { - return metadata.baseBranch; + // Strip remote prefix if present (e.g., "origin/feat/x" → "feat/x") + const branch = metadata.baseBranch.replace(/^origin\//, ''); + return branch; } } } catch (e) { diff --git a/tests/test_github_pr_regression.py b/tests/test_github_pr_regression.py index 3f7e7081..ae42bf85 100644 --- a/tests/test_github_pr_regression.py +++ b/tests/test_github_pr_regression.py @@ -337,6 +337,136 @@ class TestGitHubCLIInvocation: assert result["success"] is True +class TestGitHubOriginPrefixStripping: + """Test that origin/ prefix is stripped from target_branch in create_pull_request.""" + + def test_origin_prefix_stripped_from_target_branch(self, tmp_path): + """Test that 'origin/develop' becomes 'develop' in --base argument to gh CLI.""" + # Setup + project_dir = tmp_path / "project" + project_dir.mkdir() + spec_dir = tmp_path / "spec" + spec_dir.mkdir() + + # Create .auto-claude directories + auto_claude_dir = project_dir / ".auto-claude" + auto_claude_dir.mkdir(exist_ok=True) + + # Create WorktreeManager + manager = WorktreeManager( + project_dir=project_dir, + base_branch="main", + ) + + # Mock get_worktree_info to return a valid WorktreeInfo + mock_worktree_info = WorktreeInfo( + path=spec_dir, + branch="auto-claude/001-test-spec", + spec_name="001-test-spec", + base_branch="main", + is_active=True, + ) + + # Mock subprocess result + mock_subprocess_result = MagicMock( + returncode=0, + stdout="https://github.com/user/repo/pull/123\n", + stderr="", + ) + + # Import the actual module to patch it directly + import core.worktree as worktree_module + + with ( + patch.object(manager, "get_worktree_info", return_value=mock_worktree_info), + patch.object( + worktree_module, "get_gh_executable", return_value="/usr/bin/gh" + ), + patch.object( + worktree_module.subprocess, "run", return_value=mock_subprocess_result + ) as mock_run, + patch.object(manager, "_extract_spec_summary", return_value="Test PR body"), + ): + result = manager.create_pull_request( + spec_name="001-test-spec", + target_branch="origin/develop", + title="Test PR Title", + draft=False, + ) + + # Verify gh CLI received "develop" (not "origin/develop") as --base + assert mock_run.called + call_args = mock_run.call_args[0][0] + base_idx = call_args.index("--base") + assert call_args[base_idx + 1] == "develop", ( + f"Expected 'develop' after --base, got '{call_args[base_idx + 1]}'" + ) + assert result["success"] is True + + def test_target_branch_without_origin_prefix_unchanged(self, tmp_path): + """Test that 'develop' (no prefix) is passed through unchanged to gh CLI.""" + # Setup + project_dir = tmp_path / "project" + project_dir.mkdir() + spec_dir = tmp_path / "spec" + spec_dir.mkdir() + + # Create .auto-claude directories + auto_claude_dir = project_dir / ".auto-claude" + auto_claude_dir.mkdir(exist_ok=True) + + # Create WorktreeManager + manager = WorktreeManager( + project_dir=project_dir, + base_branch="main", + ) + + # Mock get_worktree_info to return a valid WorktreeInfo + mock_worktree_info = WorktreeInfo( + path=spec_dir, + branch="auto-claude/001-test-spec", + spec_name="001-test-spec", + base_branch="main", + is_active=True, + ) + + # Mock subprocess result + mock_subprocess_result = MagicMock( + returncode=0, + stdout="https://github.com/user/repo/pull/123\n", + stderr="", + ) + + # Import the actual module to patch it directly + import core.worktree as worktree_module + + with ( + patch.object(manager, "get_worktree_info", return_value=mock_worktree_info), + patch.object( + worktree_module, "get_gh_executable", return_value="/usr/bin/gh" + ), + patch.object( + worktree_module.subprocess, "run", return_value=mock_subprocess_result + ) as mock_run, + patch.object(manager, "_extract_spec_summary", return_value="Test PR body"), + ): + result = manager.create_pull_request( + spec_name="001-test-spec", + target_branch="develop", + title="Test PR Title", + draft=False, + ) + + # Verify gh CLI received "develop" as --base + assert mock_run.called + call_args = mock_run.call_args[0][0] + base_idx = call_args.index("--base") + assert call_args[base_idx + 1] == "develop", ( + f"Expected 'develop' after --base, got '{call_args[base_idx + 1]}'" + ) + assert result["success"] is True + + class TestGitHubErrorHandling: """Test that GitHub error handling still works correctly.""" diff --git a/tests/test_gitlab_worktree.py b/tests/test_gitlab_worktree.py index cef32863..4d3764df 100644 --- a/tests/test_gitlab_worktree.py +++ b/tests/test_gitlab_worktree.py @@ -270,6 +270,118 @@ class TestCreateMergeRequest: assert result["pr_url"] == "https://gitlab.com/user/repo/-/merge_requests/44" +class TestGitLabOriginPrefixStripping: + """Test that origin/ prefix is stripped from target_branch in create_merge_request.""" + + def test_origin_prefix_stripped_from_target_branch( + self, worktree_manager, temp_project_dir + ): + """Test that 'origin/develop' becomes 'develop' in --target-branch argument to glab CLI.""" + import core.worktree as worktree_module + + spec_name = "test-feature" + + mock_worktree_info = WorktreeInfo( + path=temp_project_dir / ".auto-claude" / "worktrees" / "tasks" / spec_name, + branch=f"auto-claude/{spec_name}", + spec_name=spec_name, + base_branch="main", + is_active=True, + ) + + mock_subprocess_result = MagicMock( + returncode=0, + stdout="https://gitlab.com/user/repo/-/merge_requests/42\n", + stderr="", + ) + + with ( + patch.object( + worktree_manager, "get_worktree_info", return_value=mock_worktree_info + ), + patch.object( + worktree_module, + "get_glab_executable", + return_value="/usr/local/bin/glab", + ), + patch.object( + worktree_module.subprocess, "run", return_value=mock_subprocess_result + ) as mock_run, + patch.object( + worktree_manager, "_extract_spec_summary", return_value="Test MR body" + ), + ): + result = worktree_manager.create_merge_request( + spec_name=spec_name, + target_branch="origin/develop", + title="Test MR", + draft=False, + ) + + # Verify glab CLI received "develop" (not "origin/develop") as --target-branch + assert mock_run.called + call_args = mock_run.call_args[0][0] + target_idx = call_args.index("--target-branch") + assert call_args[target_idx + 1] == "develop", ( + f"Expected 'develop' after --target-branch, got '{call_args[target_idx + 1]}'" + ) + assert result["success"] is True + + def test_target_branch_without_origin_prefix_unchanged( + self, worktree_manager, temp_project_dir + ): + """Test that 'develop' (no prefix) is passed through unchanged to glab CLI.""" + import core.worktree as worktree_module + + spec_name = "test-feature" + + mock_worktree_info = WorktreeInfo( + path=temp_project_dir / ".auto-claude" / "worktrees" / "tasks" / spec_name, + branch=f"auto-claude/{spec_name}", + spec_name=spec_name, + base_branch="main", + is_active=True, + ) + + mock_subprocess_result = MagicMock( + returncode=0, + stdout="https://gitlab.com/user/repo/-/merge_requests/43\n", + stderr="", + ) + + with ( + patch.object( + worktree_manager, "get_worktree_info", return_value=mock_worktree_info + ), + patch.object( + worktree_module, + "get_glab_executable", + return_value="/usr/local/bin/glab", + ), + patch.object( + worktree_module.subprocess, "run", return_value=mock_subprocess_result + ) as mock_run, + patch.object( + worktree_manager, "_extract_spec_summary", return_value="Test MR body" + ), + ): + result = worktree_manager.create_merge_request( + spec_name=spec_name, + target_branch="develop", + title="Test MR", + draft=False, + ) + + # Verify glab CLI received "develop" as --target-branch + assert mock_run.called + call_args = mock_run.call_args[0][0] + target_idx = call_args.index("--target-branch") + assert call_args[target_idx + 1] == "develop", ( + f"Expected 'develop' after --target-branch, got '{call_args[target_idx + 1]}'" + ) + assert result["success"] is True + + class TestPushAndCreatePR: """Test push_and_create_pr method with provider detection."""