From ee0d0c6c59c211cbf55292c7ac9e9471fd9a31af Mon Sep 17 00:00:00 2001 From: Shirone Date: Wed, 14 Jan 2026 20:49:17 +0100 Subject: [PATCH] fix: merge worktree handler now uses correct branch name and path The merge handler previously hardcoded branch names as `feature/${featureId}` and worktree paths as `.worktrees/${featureId}`, which failed for auto-generated branches (e.g., `feature/v0.11.0rc-1768413895104-31pa`) and custom worktrees. Changes: - Server handler now accepts branchName and worktreePath directly from the UI - Added branch existence validation before attempting merge - Updated merge dialog with 2-step confirmation (type "merge" to confirm) - Removed feature branch naming restriction - any branch can now be merged - Updated API types and client to pass correct parameters Closes #408 Co-Authored-By: Claude Opus 4.5 --- .../src/routes/worktree/routes/merge.ts | 28 ++- apps/ui/src/components/views/board-view.tsx | 35 +++ .../dialogs/merge-worktree-dialog.tsx | 234 ++++++++++++++++++ .../components/worktree-actions-dropdown.tsx | 23 ++ .../components/worktree-tab.tsx | 3 + .../views/board-view/worktree-panel/types.ts | 1 + .../worktree-panel/worktree-panel.tsx | 5 + apps/ui/src/lib/electron.ts | 12 +- apps/ui/src/lib/http-api-client.ts | 8 +- apps/ui/src/types/electron.d.ts | 8 +- 10 files changed, 335 insertions(+), 22 deletions(-) create mode 100644 apps/ui/src/components/views/board-view/dialogs/merge-worktree-dialog.tsx diff --git a/apps/server/src/routes/worktree/routes/merge.ts b/apps/server/src/routes/worktree/routes/merge.ts index ab4e0c17..69f120b8 100644 --- a/apps/server/src/routes/worktree/routes/merge.ts +++ b/apps/server/src/routes/worktree/routes/merge.ts @@ -8,7 +8,6 @@ import type { Request, Response } from 'express'; import { exec } from 'child_process'; import { promisify } from 'util'; -import path from 'path'; import { getErrorMessage, logError } from '../common.js'; const execAsync = promisify(exec); @@ -16,28 +15,31 @@ const execAsync = promisify(exec); export function createMergeHandler() { return async (req: Request, res: Response): Promise => { try { - const { projectPath, featureId, options } = req.body as { + const { projectPath, branchName, worktreePath, options } = req.body as { projectPath: string; - featureId: string; + branchName: string; + worktreePath: string; options?: { squash?: boolean; message?: string }; }; - if (!projectPath || !featureId) { + if (!projectPath || !branchName || !worktreePath) { res.status(400).json({ success: false, - error: 'projectPath and featureId required', + error: 'projectPath, branchName, and worktreePath are required', }); return; } - const branchName = `feature/${featureId}`; - // Git worktrees are stored in project directory - const worktreePath = path.join(projectPath, '.worktrees', featureId); - - // Get current branch - const { stdout: currentBranch } = await execAsync('git rev-parse --abbrev-ref HEAD', { - cwd: projectPath, - }); + // Validate branch exists + try { + await execAsync(`git rev-parse --verify ${branchName}`, { cwd: projectPath }); + } catch { + res.status(400).json({ + success: false, + error: `Branch "${branchName}" does not exist`, + }); + return; + } // Merge the feature branch const mergeCmd = options?.squash diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index 632de914..2dd705b9 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -58,6 +58,7 @@ import { DeleteWorktreeDialog } from './board-view/dialogs/delete-worktree-dialo import { CommitWorktreeDialog } from './board-view/dialogs/commit-worktree-dialog'; import { CreatePRDialog } from './board-view/dialogs/create-pr-dialog'; import { CreateBranchDialog } from './board-view/dialogs/create-branch-dialog'; +import { MergeWorktreeDialog } from './board-view/dialogs/merge-worktree-dialog'; import { WorktreePanel } from './board-view/worktree-panel'; import type { PRInfo, WorktreeInfo } from './board-view/worktree-panel/types'; import { COLUMNS, getColumnsWithPipeline } from './board-view/constants'; @@ -148,6 +149,7 @@ export function BoardView() { const [showCommitWorktreeDialog, setShowCommitWorktreeDialog] = useState(false); const [showCreatePRDialog, setShowCreatePRDialog] = useState(false); const [showCreateBranchDialog, setShowCreateBranchDialog] = useState(false); + const [showMergeWorktreeDialog, setShowMergeWorktreeDialog] = useState(false); const [selectedWorktreeForAction, setSelectedWorktreeForAction] = useState<{ path: string; branch: string; @@ -1354,6 +1356,10 @@ export function BoardView() { }} onAddressPRComments={handleAddressPRComments} onResolveConflicts={handleResolveConflicts} + onMerge={(worktree) => { + setSelectedWorktreeForAction(worktree); + setShowMergeWorktreeDialog(true); + }} onRemovedWorktrees={handleRemovedWorktrees} runningFeatureIds={runningAutoTasks} branchCardCounts={branchCardCounts} @@ -1698,6 +1704,35 @@ export function BoardView() { }} /> + {/* Merge Worktree Dialog */} + f.branchName === selectedWorktreeForAction.branch).length + : 0 + } + onMerged={(mergedWorktree) => { + // Reset features that were assigned to the merged worktree (by branch) + hookFeatures.forEach((feature) => { + if (feature.branchName === mergedWorktree.branch) { + // Reset the feature's branch assignment - update both local state and persist + const updates = { + branchName: null as unknown as string | undefined, + }; + updateFeature(feature.id, updates); + persistFeatureUpdate(feature.id, updates); + } + }); + + setWorktreeRefreshKey((k) => k + 1); + setSelectedWorktreeForAction(null); + }} + /> + {/* Commit Worktree Dialog */} void; + projectPath: string; + worktree: WorktreeInfo | null; + onMerged: (mergedWorktree: WorktreeInfo) => void; + /** Number of features assigned to this worktree's branch */ + affectedFeatureCount?: number; +} + +type DialogStep = 'confirm' | 'verify'; + +export function MergeWorktreeDialog({ + open, + onOpenChange, + projectPath, + worktree, + onMerged, + affectedFeatureCount = 0, +}: MergeWorktreeDialogProps) { + const [isLoading, setIsLoading] = useState(false); + const [step, setStep] = useState('confirm'); + const [confirmText, setConfirmText] = useState(''); + + // Reset state when dialog opens + useEffect(() => { + if (open) { + setIsLoading(false); + setStep('confirm'); + setConfirmText(''); + } + }, [open]); + + const handleProceedToVerify = () => { + setStep('verify'); + }; + + const handleMerge = async () => { + if (!worktree) return; + + setIsLoading(true); + try { + const api = getElectronAPI(); + if (!api?.worktree?.mergeFeature) { + toast.error('Worktree API not available'); + return; + } + + // Pass branchName and worktreePath directly to the API + const result = await api.worktree.mergeFeature(projectPath, worktree.branch, worktree.path); + + if (result.success) { + toast.success('Branch merged to main', { + description: `Branch "${worktree.branch}" has been merged and cleaned up`, + }); + onMerged(worktree); + onOpenChange(false); + } else { + toast.error('Failed to merge branch', { + description: result.error, + }); + } + } catch (err) { + toast.error('Failed to merge branch', { + description: err instanceof Error ? err.message : 'Unknown error', + }); + } finally { + setIsLoading(false); + } + }; + + if (!worktree) return null; + + const confirmationWord = 'merge'; + const isConfirmValid = confirmText.toLowerCase() === confirmationWord; + + // First step: Show what will happen and ask for confirmation + if (step === 'confirm') { + return ( + + + + + + Merge to Main + + +
+ + Merge branch{' '} + {worktree.branch} into + main? + + +
+ This will: +
    +
  • Merge the branch into the main branch
    • +
  • Remove the worktree directory
    • +
  • Delete the branch
    • +
+
+ + {worktree.hasChanges && ( +
+ + + This worktree has {worktree.changedFilesCount} uncommitted change(s). Please + commit or discard them before merging. + +
+ )} + + {affectedFeatureCount > 0 && ( +
+ + + {affectedFeatureCount} feature{affectedFeatureCount !== 1 ? 's' : ''}{' '} + {affectedFeatureCount !== 1 ? 'are' : 'is'} assigned to this branch and will + be unassigned after merge. + +
+ )} +
+ + + + + + + + + + ); + } + + // Second step: Type confirmation + return ( + + + + + + Confirm Merge + + +
+
+ + + This action cannot be undone. The branch{' '} + {worktree.branch} will be + permanently deleted after merging. + +
+ +
+ + setConfirmText(e.target.value)} + placeholder={confirmationWord} + disabled={isLoading} + className="font-mono" + autoComplete="off" + /> +
+
+ + + + + + + + + + ); +} diff --git a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx index 05597ada..459e2ce8 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx +++ b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx @@ -55,6 +55,7 @@ interface WorktreeActionsDropdownProps { onCreatePR: (worktree: WorktreeInfo) => void; onAddressPRComments: (worktree: WorktreeInfo, prInfo: PRInfo) => void; onResolveConflicts: (worktree: WorktreeInfo) => void; + onMerge: (worktree: WorktreeInfo) => void; onDeleteWorktree: (worktree: WorktreeInfo) => void; onStartDevServer: (worktree: WorktreeInfo) => void; onStopDevServer: (worktree: WorktreeInfo) => void; @@ -84,6 +85,7 @@ export function WorktreeActionsDropdown({ onCreatePR, onAddressPRComments, onResolveConflicts, + onMerge, onDeleteWorktree, onStartDevServer, onStopDevServer, @@ -231,6 +233,27 @@ export function WorktreeActionsDropdown({ {!canPerformGitOps && } + {!worktree.isMain && ( + + canPerformGitOps && onMerge(worktree)} + disabled={!canPerformGitOps} + className={cn( + 'text-xs text-green-600 focus:text-green-700', + !canPerformGitOps && 'opacity-50 cursor-not-allowed' + )} + > + + Merge to Main + {!canPerformGitOps && ( + + )} + + + )} {/* Open in editor - split button: click main area for default, chevron for other options */} {effectiveDefaultEditor && ( diff --git a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx index 994c81f2..5cb379d3 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx +++ b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx @@ -41,6 +41,7 @@ interface WorktreeTabProps { onCreatePR: (worktree: WorktreeInfo) => void; onAddressPRComments: (worktree: WorktreeInfo, prInfo: PRInfo) => void; onResolveConflicts: (worktree: WorktreeInfo) => void; + onMerge: (worktree: WorktreeInfo) => void; onDeleteWorktree: (worktree: WorktreeInfo) => void; onStartDevServer: (worktree: WorktreeInfo) => void; onStopDevServer: (worktree: WorktreeInfo) => void; @@ -84,6 +85,7 @@ export function WorktreeTab({ onCreatePR, onAddressPRComments, onResolveConflicts, + onMerge, onDeleteWorktree, onStartDevServer, onStopDevServer, @@ -344,6 +346,7 @@ export function WorktreeTab({ onCreatePR={onCreatePR} onAddressPRComments={onAddressPRComments} onResolveConflicts={onResolveConflicts} + onMerge={onMerge} onDeleteWorktree={onDeleteWorktree} onStartDevServer={onStartDevServer} onStopDevServer={onStopDevServer} diff --git a/apps/ui/src/components/views/board-view/worktree-panel/types.ts b/apps/ui/src/components/views/board-view/worktree-panel/types.ts index c6ecefde..d2040048 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/types.ts +++ b/apps/ui/src/components/views/board-view/worktree-panel/types.ts @@ -73,6 +73,7 @@ export interface WorktreePanelProps { onCreateBranch: (worktree: WorktreeInfo) => void; onAddressPRComments: (worktree: WorktreeInfo, prInfo: PRInfo) => void; onResolveConflicts: (worktree: WorktreeInfo) => void; + onMerge: (worktree: WorktreeInfo) => void; onRemovedWorktrees?: (removedWorktrees: Array<{ path: string; branch: string }>) => void; runningFeatureIds?: string[]; features?: FeatureInfo[]; diff --git a/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx b/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx index 35e70d51..2cc844f4 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx +++ b/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx @@ -30,6 +30,7 @@ export function WorktreePanel({ onCreateBranch, onAddressPRComments, onResolveConflicts, + onMerge, onRemovedWorktrees, runningFeatureIds = [], features = [], @@ -248,10 +249,12 @@ export function WorktreePanel({ onCreatePR={onCreatePR} onAddressPRComments={onAddressPRComments} onResolveConflicts={onResolveConflicts} + onMerge={onMerge} onDeleteWorktree={onDeleteWorktree} onStartDevServer={handleStartDevServer} onStopDevServer={handleStopDevServer} onOpenDevServerUrl={handleOpenDevServerUrl} + onViewDevServerLogs={handleViewDevServerLogs} onRunInitScript={handleRunInitScript} hasInitScript={hasInitScript} /> @@ -333,6 +336,7 @@ export function WorktreePanel({ onCreatePR={onCreatePR} onAddressPRComments={onAddressPRComments} onResolveConflicts={onResolveConflicts} + onMerge={onMerge} onDeleteWorktree={onDeleteWorktree} onStartDevServer={handleStartDevServer} onStopDevServer={handleStopDevServer} @@ -390,6 +394,7 @@ export function WorktreePanel({ onCreatePR={onCreatePR} onAddressPRComments={onAddressPRComments} onResolveConflicts={onResolveConflicts} + onMerge={onMerge} onDeleteWorktree={onDeleteWorktree} onStartDevServer={handleStartDevServer} onStopDevServer={handleStopDevServer} diff --git a/apps/ui/src/lib/electron.ts b/apps/ui/src/lib/electron.ts index 6f6c564c..8a88fc4a 100644 --- a/apps/ui/src/lib/electron.ts +++ b/apps/ui/src/lib/electron.ts @@ -1440,13 +1440,19 @@ function createMockSetupAPI(): SetupAPI { // Mock Worktree API implementation function createMockWorktreeAPI(): WorktreeAPI { return { - mergeFeature: async (projectPath: string, featureId: string, options?: object) => { + mergeFeature: async ( + projectPath: string, + branchName: string, + worktreePath: string, + options?: object + ) => { console.log('[Mock] Merging feature:', { projectPath, - featureId, + branchName, + worktreePath, options, }); - return { success: true, mergedBranch: `feature/${featureId}` }; + return { success: true, mergedBranch: branchName }; }, getInfo: async (projectPath: string, featureId: string) => { diff --git a/apps/ui/src/lib/http-api-client.ts b/apps/ui/src/lib/http-api-client.ts index f08e7620..8b2f97b1 100644 --- a/apps/ui/src/lib/http-api-client.ts +++ b/apps/ui/src/lib/http-api-client.ts @@ -1706,8 +1706,12 @@ export class HttpApiClient implements ElectronAPI { // Worktree API worktree: WorktreeAPI = { - mergeFeature: (projectPath: string, featureId: string, options?: object) => - this.post('/api/worktree/merge', { projectPath, featureId, options }), + mergeFeature: ( + projectPath: string, + branchName: string, + worktreePath: string, + options?: object + ) => this.post('/api/worktree/merge', { projectPath, branchName, worktreePath, options }), getInfo: (projectPath: string, featureId: string) => this.post('/api/worktree/info', { projectPath, featureId }), getStatus: (projectPath: string, featureId: string) => diff --git a/apps/ui/src/types/electron.d.ts b/apps/ui/src/types/electron.d.ts index 94a033f5..fbba4063 100644 --- a/apps/ui/src/types/electron.d.ts +++ b/apps/ui/src/types/electron.d.ts @@ -660,14 +660,14 @@ export interface FileDiffResult { } export interface WorktreeAPI { - // Merge feature worktree changes back to main branch + // Merge worktree branch into main and clean up mergeFeature: ( projectPath: string, - featureId: string, + branchName: string, + worktreePath: string, options?: { squash?: boolean; - commitMessage?: string; - squashMessage?: string; + message?: string; } ) => Promise<{ success: boolean;